bt709 is interpreted differently in different browsers |
|||||||||||||||||||
Issue descriptionConsider the attached gradient video. The output colors on an sRGB monitor are as follows: HW Decode Composited by CoreAnimation (same as Safari) 0.00 0.11 0.23 0.34 0.44 0.54 0.64 0.73 0.82 0.91 1.00 SW Decode Composited by CoreAnimation 0.00 0.16 0.26 0.36 0.45 0.55 0.64 0.73 0.82 0.91 1.00 HW Decode Composited by OpenGL 0.00 0.10 0.20 0.31 0.40 0.50 0.60 0.70 0.80 0.90 1.00 Notice that, depending on the path, we end up with different results. Problem 1 : Compositing by CoreAnimation versus OpenGL: This is different because we have a special case for handling bt709 to sRGB color transform here: https://cs.chromium.org/chromium/src/ui/gfx/color_transform.cc?rcl=a3c45f3749fe0fab81aea4391518fd2bd9f090dc&l=814 That color transform is used when compositing with OpenGL, but not when compositing with CoreAnimation. When compositing with CoreAnimation, the transfer function that we assign for bt709 (see below) is used to convert encoded-to-linear. Problem 2 : HW Decode / Safari vs SW Decode This is because, when we do HW decode, we use Apple's VideoToolkit library. This, when it encounters bt709 content, uses a curve that is T(x) = x^1.961 to convert encoded-to-linear. This can be verified by extracting the IOSurfaceColorSpace CFDataRef from the IOSurfaces produced by the decoder, and finding the attached vt.icc profile data. Chrome, meanwhile, uses a curve that is { (0.91 * x + 0.09) ^ 2.22 : x >= 0.08 T(x) = { { 0.22 * x : x < 0.08 to convert encoded-to-linear. We should produce the same results for all of these paths.
,
Sep 8 2017
rec709/smpte 1886 presentation is open to some interpretation, but gamma 2.2 / sRGB is both close to ideal and offers a lot of opportunity for optimization, making it the best choice for how we should present rec709 content. Unfortunately it might not be possible or efficient to match that exactly in all cases, in which case we should just try to get as close as possible to that ideal.
,
Sep 8 2017
> Chrome, meanwhile, uses a curve that is
> { (0.91 * x + 0.09) ^ 2.22 : x >= 0.08
> T(x) = {
> { 0.22 * x : x < 0.08
> to convert encoded-to-linear.
When are we actually using this formula?
It may be in the code, but we should never actually use it.
If something is bypassing the "special case" in color_transform.cc will lead to content not looking right, so we need to fix that.
,
Sep 8 2017
Capturing ffmpeg source code (color_utils.cc) for posterity here:
double avpriv_get_gamma_from_trc(enum AVColorTransferCharacteristic trc)
{
double gamma;
switch (trc) {
case AVCOL_TRC_BT709:
case AVCOL_TRC_SMPTE170M:
case AVCOL_TRC_SMPTE240M:
case AVCOL_TRC_BT1361_ECG:
case AVCOL_TRC_BT2020_10:
case AVCOL_TRC_BT2020_12:
/* these share a segmented TRC, but gamma 1.961 is a close
approximation, and also more correct for decoding content */
gamma = 1.961;
break;
,
Sep 8 2017
,
Sep 12 2017
WRT #8, we are actually using this formula, but apparently we never should be. We're going to see what happens on Windows here before proceeding (currently I'm away from Windows machines, but I'll get the data soon). My guess here is that we're going to #ifdef Mac to use 1.961 as the transfer function (the horror) and then #ifdef everything else to match the behavior on the platform (probably sRGB trfn or 2.2 or 2.4).
,
Sep 12 2017
I think we need some pretty compelling reasons to use 1.961 on Mac and sRGB everywhere else. We really want chrome to behave the same everywhere, so if we're not going to do that, we'll need a well written, clearly argued document explaining why I think.
,
Sep 19 2017
Did you have a chance to try it on Windows? Let me know if there is anything I can help.
,
Sep 19 2017
,
Sep 19 2017
Yes, I tested this on Windows and got the following for Edge and FireFox: Safari, and Chrome-Mac-HW-Decode-CoreAnimation 0.00, 0.11, 0.23, 0.34, 0.44, 0.54, 0.64, 0.73, 0.82, 0.91, 1.00 Chrome-Mac-HW-Decode-CoreAnimation 0.00, 0.16, 0.26, 0.36, 0.45, 0.55, 0.64, 0.73, 0.82, 0.91, 1.00 Chrome-Mac-GLRenderer-to-sRGB, Edge, Mac-Firefox, Win-Firefox 0.00, 0.10, 0.20, 0.31, 0.40, 0.50, 0.60, 0.70, 0.80, 0.90, 1.00 I've attached a graph of the response curves for the video above. I'm pretty content to use the sRGB transfer function for bt709, put a grumpy comment in the code, and call it a day.
,
Sep 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/41d66154969ea403326bdd7b96b959ff6ad7fb77 commit 41d66154969ea403326bdd7b96b959ff6ad7fb77 Author: Christopher Cameron <ccameron@chromium.org> Date: Mon Sep 25 21:52:48 2017 Always use sRGB transfer function for bt709 Previous behavior was: * If the destination transfer function is sRGB, use the sRGB transfer function * Otherwise use the transfer function from the bt709 spec This new behavior will more closely match other platforms and user studies. Bug: 763260 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Iaf49c0c8a5a20e53248854b6a7ecad72d685436d Reviewed-on: https://chromium-review.googlesource.com/673165 Commit-Queue: ccameron chromium <ccameron@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Reviewed-by: Fredrik Hubinette <hubbe@chromium.org> Cr-Commit-Position: refs/heads/master@{#504175} [modify] https://crrev.com/41d66154969ea403326bdd7b96b959ff6ad7fb77/content/test/gpu/gpu_tests/pixel_expectations.py [modify] https://crrev.com/41d66154969ea403326bdd7b96b959ff6ad7fb77/content/test/gpu/gpu_tests/pixel_test_pages.py [modify] https://crrev.com/41d66154969ea403326bdd7b96b959ff6ad7fb77/ui/gfx/color_space.cc [modify] https://crrev.com/41d66154969ea403326bdd7b96b959ff6ad7fb77/ui/gfx/color_transform.cc [modify] https://crrev.com/41d66154969ea403326bdd7b96b959ff6ad7fb77/ui/gfx/color_transform_unittest.cc
,
Sep 25 2017
Merge requesting for 62 -- this is a very low risk bugfix (change some math).
,
Sep 26 2017
Confirmed with ccameron@, merges are safe and tested in Canary. Approving merge to M62. branch:3202
,
Sep 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e6575a5abcfc49bfcac977ab038c51866ca7a324 commit e6575a5abcfc49bfcac977ab038c51866ca7a324 Author: Christopher Cameron <ccameron@chromium.org> Date: Fri Sep 29 05:15:57 2017 Always use sRGB transfer function for bt709 Previous behavior was: * If the destination transfer function is sRGB, use the sRGB transfer function * Otherwise use the transfer function from the bt709 spec This new behavior will more closely match other platforms and user studies. TBR=ccameron@chromium.org (cherry picked from commit 41d66154969ea403326bdd7b96b959ff6ad7fb77) Bug: 763260 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Iaf49c0c8a5a20e53248854b6a7ecad72d685436d Reviewed-on: https://chromium-review.googlesource.com/673165 Commit-Queue: ccameron chromium <ccameron@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Reviewed-by: Fredrik Hubinette <hubbe@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#504175} Reviewed-on: https://chromium-review.googlesource.com/691125 Reviewed-by: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#501} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/e6575a5abcfc49bfcac977ab038c51866ca7a324/content/test/gpu/gpu_tests/pixel_expectations.py [modify] https://crrev.com/e6575a5abcfc49bfcac977ab038c51866ca7a324/content/test/gpu/gpu_tests/pixel_test_pages.py [modify] https://crrev.com/e6575a5abcfc49bfcac977ab038c51866ca7a324/ui/gfx/color_space.cc [modify] https://crrev.com/e6575a5abcfc49bfcac977ab038c51866ca7a324/ui/gfx/color_transform.cc [modify] https://crrev.com/e6575a5abcfc49bfcac977ab038c51866ca7a324/ui/gfx/color_transform_unittest.cc
,
Sep 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c2bc0bc530974f5b039c105ca392fa585d4cc34 commit 9c2bc0bc530974f5b039c105ca392fa585d4cc34 Author: Christopher Cameron <ccameron@chromium.org> Date: Fri Sep 29 05:22:07 2017 Update MP4 video pixel test expectation This needs to be updated due to bt709 interpretation change in crbug.com/763260 TBR=ccameron@chromium.org (cherry picked from commit dec4b78a6b54fd15ac764c44c4194a64658413a0) Bug: 763260 (merge from 768608) Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I9321869000d10f79f82f9dd4ca778e6517ed7ef7 Reviewed-on: https://chromium-review.googlesource.com/683078 Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: ccameron chromium <ccameron@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#504264} Reviewed-on: https://chromium-review.googlesource.com/691519 Reviewed-by: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#502} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/9c2bc0bc530974f5b039c105ca392fa585d4cc34/content/test/gpu/gpu_tests/pixel_expectations.py [modify] https://crrev.com/9c2bc0bc530974f5b039c105ca392fa585d4cc34/content/test/gpu/gpu_tests/pixel_test_pages.py
,
Oct 2 2017
,
Oct 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07cd8b41e92d4a37c1fa17638bef4213d8ae8380 commit 07cd8b41e92d4a37c1fa17638bef4213d8ae8380 Author: Kenneth Russell <kbr@chromium.org> Date: Mon Oct 02 23:17:04 2017 Remove failure expectations for video-related pixel tests. BUG= 763260 TBR=ccameron@chromium.org Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Ic487862b1da9406c2d38947b9f9826631c18c63d Reviewed-on: https://chromium-review.googlesource.com/692997 Commit-Queue: Kenneth Russell <kbr@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#505842} [modify] https://crrev.com/07cd8b41e92d4a37c1fa17638bef4213d8ae8380/content/test/gpu/gpu_tests/pixel_expectations.py |
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by ccameron@chromium.org
, Sep 8 2017