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

Question about TranscodeFromUYVY method #107

Closed
flat-eric147 opened this issue Nov 4, 2023 · 11 comments · Fixed by #115 or #117
Closed

Question about TranscodeFromUYVY method #107

flat-eric147 opened this issue Nov 4, 2023 · 11 comments · Fixed by #115 or #117
Assignees
Labels
enhancement New feature or request

Comments

@flat-eric147
Copy link

The method TranscodeFromUYVY in BitmapTranscoder.cs:22 contains a YUV to RGB conversion.
As everything in the method is converted to integer arithmetic it is not obvious to me which color space
conversion is in use. There are two main standards BT.709 (usually used for HD formats) and BT.601 (usually used for SD formats).
Can anybody tell me which standard is used here? It would be cool to provide a parameter to the method or make different methods for the various color spaces, also in view of 4k/UHD which is coming with yet another standard (BT.2020).

Thank you!

@kekyo
Copy link
Owner

kekyo commented Nov 5, 2023

That is an excellent insight!
Indeed, when doing a YUV conversion, the color space conversion matrix is affected.

As shown on the source code, this is implemented based on the MS Article. The reason is that specific instructions were given to use realistic integer arithmetic (I wanted to avoid floating point arithmetic for speed).

Since I do not have a deep understanding of color space, I adopted my decision about the validity of these calculations in comparison to the miscellaneous other blog posts on the topic.

The actual calculations are exactly as the formulas here. That is:

R = clip(( 298 * C           + 409 * E + 128) >> 8)
G = clip(( 298 * C - 100 * D - 208 * E + 128) >> 8)
B = clip(( 298 * C + 516 * D           + 128) >> 8)

and although not clearly shown, I believe this is BT.709 based.

Returning to FlashCap, I think the idea of giving the format assumed in the YUV transcoding as an argument is a good one, so I will adopt it. However, the only known format type is BT.709, so if you know of other formats that could be added (e.g., BT.601, BT.2020, etc.), please help me out.

@kekyo kekyo self-assigned this Nov 5, 2023
@kekyo kekyo added the enhancement New feature or request label Nov 5, 2023
@flat-eric147
Copy link
Author

Thank you for the information and all the references. From what I could see it "felt" like BT.709, which nowadays is ok in most cases I would guess. I will try to have a closer look and try to help you out :)

@flat-eric147
Copy link
Author

Hi, I had a closer look at the code, and it looked like the current conversion is using the standard BT.601.

I pushed a change to my forked repository adding all possible conversion standards:
be698df
Let me know if it's worth merging this modification.

@kekyo
Copy link
Owner

kekyo commented Nov 9, 2023

I will see later!

@flat-eric147
Copy link
Author

Thank you, please consider also a small fix on that 761ce5d

@kekyo kekyo linked a pull request Nov 11, 2023 that will close this issue
@kekyo
Copy link
Owner

kekyo commented Nov 14, 2023

Merged, I will release next version.

@kekyo kekyo closed this as completed Nov 14, 2023
@flat-eric147
Copy link
Author

Thank you! The only thing I see is missing is the patch I attached manually. If you prefer I can re-fork from here and make a merge request.

@kekyo
Copy link
Owner

kekyo commented Nov 14, 2023

@flat-eric147 Ouch! I forgot it... Sorry will be released 1.7.0 package shorty.
So, it will be merged next release. Please fork latest develop branch and apply it?

@kekyo kekyo reopened this Nov 14, 2023
@flat-eric147
Copy link
Author

create pull request and while reviewing the code spotted a bug as well.

@kekyo
Copy link
Owner

kekyo commented Nov 15, 2023

Thanks! If no other problems are found, I will release it as 1.0.8.

@flat-eric147
Copy link
Author

ok, it looks good to me. Thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants