kekyo / FlashCap

Independent video frame capture library on .NET/.NET Core and .NET Framework.
Apache License 2.0
203 stars 29 forks source link

Question about TranscodeFromUYVY method #107

Closed flat-eric147 closed 1 year ago

flat-eric147 commented 1 year ago

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 commented 1 year ago

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.

flat-eric147 commented 1 year ago

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 commented 1 year ago

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: https://github.com/kekyo/FlashCap/commit/be698dfb9db94cb6b1e5bfb47e6940ce0852fe19 Let me know if it's worth merging this modification.

kekyo commented 1 year ago

I will see later!

flat-eric147 commented 1 year ago

Thank you, please consider also a small fix on that https://github.com/kekyo/FlashCap/commit/761ce5d2fd5cf85aba6a08a989972dba2672c571

kekyo commented 1 year ago

Merged, I will release next version.

flat-eric147 commented 1 year ago

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 commented 1 year ago

@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?

flat-eric147 commented 1 year ago

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

kekyo commented 1 year ago

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

flat-eric147 commented 1 year ago

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