Skip to content
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

yuv: Use more accurate BT.601 coefficients #12

Merged
merged 3 commits into from
Sep 24, 2021

Conversation

torokati44
Copy link
Member

In the spirit of preferring bite-sized PRs instead of comprehensive rewrites, I split this out of #9, as it is a relatively independent change.

Quoting #9 (comment) as explanation:

While adding tests, I also discovered some off-by-one errors in the results. These were caused by rounding more than necessary.

So I switched to a more precise formula to precompute the linear functions, with coefficients taken exactly from the BT.601 standard. (I'm much happier with these, instead of those 5 float literals just copied from a random piece of code somewhere.)

Now that all of this goes into a lookup table anyway, the cost of all of these additional computations shouldn't matter that much. (They are only done once, and only on the 256 values of a u8.)

@torokati44
Copy link
Member Author

Nope, this doesn't work, sorry.

@torokati44
Copy link
Member Author

Okay, I did end up making this work after all! Sorry for the spam... :|

@torokati44
Copy link
Member Author

This also matches exactly what the SWF specification says about the YUV to RGB conversion, for at least VP6 video.
Which just means that they did not make a mistake summarizing BT.601... :D

@kmeisthax
Copy link
Member

I merged in the other PR about nightly warnings, but we don't have the CI fixes from Ruffle itself in here, so I'm still going to need this to get rebased so the CI can pass.

@torokati44
Copy link
Member Author

Rebased, hopefully it'll pass.

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.

None yet

2 participants