Skip to content

Use new [T]::as_chunks method in CIEDE delta_e_row_avx2 - #313

Draft
FreezyLemon wants to merge 8 commits into
rust-av:masterfrom
FreezyLemon:post-v_frame-0.7-as_chunks
Draft

Use new [T]::as_chunks method in CIEDE delta_e_row_avx2#313
FreezyLemon wants to merge 8 commits into
rust-av:masterfrom
FreezyLemon:post-v_frame-0.7-as_chunks

Conversation

@FreezyLemon

Copy link
Copy Markdown
Contributor

Requires #311.

CIEDE2000 yuv420p8      time:   [1.0270 ms 1.0298 ms 1.0326 ms]
                        change: [−7.2170% −6.4565% −5.7976%] (p = 0.00 < 0.05)

CIEDE2000 yuv420p10     time:   [1.0189 ms 1.0242 ms 1.0316 ms]
                        change: [−6.1879% −5.7188% −5.1965%] (p = 0.00 < 0.05)

@FreezyLemon FreezyLemon mentioned this pull request Jul 2, 2026
.chroma_dimensions(width, height)
.expect("has chroma dimensions");
f.y_plane
.copy_from_u8_slice_with_stride(decoded.data(0), width * bytes)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm that this works properly with frames that have padding?

fn convert_u16(line: &[u8], index: usize) -> i32 {
let index = index * 2;
i32::cast_from(u16::cast_from(line[index + 1]) << 8 | u16::cast_from(line[index]))
i32::from(u16::from_ne_bytes([line[index + 1], line[index]]))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is backwards for what it should be on little endian. Can you test and confirm that HBD chroma copying is correct?

f.planes[0].copy_from_raw_u8(frame.get_y_plane(), width * bytes, bytes);
let (chroma_width, _) = chroma_sampling
.chroma_dimensions(width, height)
.expect("can subsample");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since chroma_dimensions returns None for monochrome input, we should handle that case to ensure we don't panic.

self.planes[1].can_compare(&other.planes[1])?;
self.planes[2].can_compare(&other.planes[2])?;
self.plane(0).can_compare(&other.plane(0))?;
self.plane(1).can_compare(&other.plane(1))?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should handle monochrome inputs by conditionally ignoring planes after the first.

@FreezyLemon

Copy link
Copy Markdown
Contributor Author

Sorry, this might not have been the best idea with all these PRs. The "real" upgrade to v_frame 0.7 is #311, the other ones depend on it to make sense. I'll mark them drafts for now. Thanks for the review though, I realize I didn't test this nearly enough and will do that first, and then react to your review comments.

@FreezyLemon
FreezyLemon marked this pull request as draft July 6, 2026 19:18
@shssoichiro

Copy link
Copy Markdown
Member

Some of these monochrome-related items may have been broken in main as well, but since they were spotted in review, we may as well fix them now. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants