1304330 - BT.709 YUV to RGB of libyuv seems not correct

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Sotaro Ikeda [:sotaro]]

I faced this during Bug 1210357. libyuv::H420ToARGB() result seemed not correct.

The formula in the following seems not correct.
  https://dxr.mozilla.org/mozilla-central/source/media/libyuv/source/row_common.cc#1145

But libyuv master is still same.
  https://chromium.googlesource.com/libyuv/libyuv/+/master/source/row_common.cc#1145

Comment 1

[Sotaro Ikeda [:sotaro]]

The chromium source of the followings use different calculation than comment 0.
  https://chromium.googlesource.com/chromium/src.git/+/master/media/base/yuv_convert.cc#298
  https://chromium.googlesource.com/chromium/src.git/+/master/cc/output/gl_renderer.cc#2347

The following is referenced from the yuv_convert.cc.
  http://www.equasys.de/colorconversion.html

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Update YuvConstants of BT.709 YUV

Comment 3

[Sotaro Ikeda [:sotaro]]

attachment 8793261 [details] [diff] [review] mimics "BT.601 YUV to RGB reference".
  https://dxr.mozilla.org/mozilla-central/source/media/libyuv/source/row_common.cc#971

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8793261 [details] [diff] [review]
patch - Update YuvConstants of BT.709 YUV

Review of attachment 8793261 [details] [diff] [review]:
-----------------------------------------------------------------

Randal is probably a more appropriate reviewer than me.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8793261 [details] [diff] [review]
patch - Update YuvConstants of BT.709 YUV

Review of attachment 8793261 [details] [diff] [review]:
-----------------------------------------------------------------

Please file and issue and patch for upstream as well.  Thanks!

Comment 6

[Sotaro Ikeda [:sotaro]]

Opened the issues https://bugs.chromium.org/p/libyuv/issues/detail?id=639

Comment 7

[Frank Barchard]

The difference comes from full range vs constrained range.
Looking into the chromium media code (which I wrote), they choose to do constrained range BT709.  erg.  Can't we move forward to a good colorspace?!

Internally its implemented via a matrix, so you could define a custom matrix and call the low levels.

But I agree we should update the constants to match.
Your patch only updated the constants for YUV to RGB, but not RGB to YUV.  So the unittests fail.  I'll follow up in the libyuv bug report.

Comment 8

[Sotaro Ikeda [:sotaro]]

Thanks!

Comment 9

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Update YuvConstants of BT.709 YUV

Apply the comment in https://bugs.chromium.org/p/libyuv/issues/detail?id=639.

Comment 10

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Update YuvConstants of BT.709 YUV

Update a comment.

Comment 11

[Sotaro Ikeda [:sotaro]]

The problem was fixed in libyuv.
  https://chromium.googlesource.com/libyuv/libyuv/+/c11e9b7fb7c03f3480e43faa9a906aba48fec466%5E!/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c15a77349bc7
Update YuvConstants of BT.709 YUV r=jesup

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c15a77349bc7