Skip to content

YUV->RGBA conversion: Special case the edge pixels, do the middle without index clamping #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

torokati44
Copy link

@torokati44 torokati44 commented Jan 20, 2021

This PR splits the color space conversion of the edge pixels and "the rest", to allow fewer operations on the inside pixels that are the straightforward case.

The effects of each included commit (plus two excluded ones) on the runtime of a particular command are:

image

These numbers are the output of time (in seconds) running the following command (after compilation is done), with each sample averaged from three runs. The error bars are one standard deviation long in both directions.

time cargo run --package=exporter --release -- ../../Downloads/z0r-de_4145.swf --frames 1000

I also commented out the actual saving of the frames into files, so the effect on rendering itself is more directly measurable.

While the "utility functions" commit regresses a little bit, doing it is almost a necessity for the one after it, which is the one providing the significant gains.

Overall, these changes sped up the rendering by about 25%.

I also made two more experiments (independently) that I then discarded because they both regressed slightly:
The first one was doing the bilinear interpolation differently: on f32 numbers, in two steps (the usual way, in a rotated H-shape).
The second one was simply omitting the .min() and .max() calls from clamp(), relying on the saturating property of the f32 to u8 cast instead.

I don't know if this is starting to stretch the "code simplicity/cleanliness" vs. "runtime performance" trade-off a little bit too far, but at least there is still no unsafe anywhere... :)

@kmeisthax kmeisthax merged commit 2589356 into kmeisthax:video-h263 Jan 21, 2021
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