FrameSinkVideoCapturer pipeline needs to handle color space |
|||||||
Issue descriptionIn the initial implementation of viz::FrameSinkVideoCapturerImpl, viz::GLRendererCopier, viz::GLHelper[Scaling], and in FSVC's downstream clients; there are TODOs sprinkled throughout the code to revisit issues related to incorrect (or naive) handling of color space considerations. This bug tracks the resolution of all of that, with the end goal of having color accuracy testing in web_contents_video_capture_device_browsertests.cc. This is also retated to bug 758057 and bug 754986 and likely several others. Also note that resolution of this is likely to mean color space is being handled correctly in single-snapshot CopyOutputRequests as well.
,
May 18 2018
,
Jun 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d35ae16c1b4b3f2b6262b75cd271c79f3267b452 commit d35ae16c1b4b3f2b6262b75cd271c79f3267b452 Author: Yuri Wiitala <miu@chromium.org> Date: Sat Jun 09 04:30:56 2018 Add gfx::ColorSpace(const SkColorSpace&) constructor. gfx::ColorSpace can convert to a SkColorSpace, but there was no simple way to convert the other way around. This change addresses that need. Bug: 758057, 810131 Change-Id: Id6bb1b079f17cc632ab04cddd162dc94ee50a287 Reviewed-on: https://chromium-review.googlesource.com/1090234 Commit-Queue: Yuri Wiitala <miu@chromium.org> Reviewed-by: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#565859} [modify] https://crrev.com/d35ae16c1b4b3f2b6262b75cd271c79f3267b452/ui/gfx/color_space.cc [modify] https://crrev.com/d35ae16c1b4b3f2b6262b75cd271c79f3267b452/ui/gfx/color_space.h [modify] https://crrev.com/d35ae16c1b4b3f2b6262b75cd271c79f3267b452/ui/gfx/color_space_unittest.cc
,
Aug 14
Is this fixed?
,
Aug 15
,
Aug 15
,
Sep 4
,
Oct 16
Was this fixed by the commit on Jun 9 or is there more that needs to be done?
,
Oct 17
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8bac8fb474b3b5d5b054350a1c2e28f5e78e6ecf commit 8bac8fb474b3b5d5b054350a1c2e28f5e78e6ecf Author: Yuri Wiitala <miu@chromium.org> Date: Tue Oct 30 05:36:58 2018 GLScaler: Add gamma-aware scaling, for improved color accuracy. Adds the implementation to support gamma-aware scaling: It uses half- floats to increase the precision of the color values, and adds an "import stage" to linearize the color values before any texture sampling or scaling occurs. Added a pixel test to prove the gamma-aware versus non-gamma-aware configuration of GLScaler produces the expected results. Bug: 870036, 810131 Change-Id: Ie68121add789169dbda0a578f03a257d089d4f6c Reviewed-on: https://chromium-review.googlesource.com/c/1297540 Commit-Queue: Yuri Wiitala <miu@chromium.org> Reviewed-by: Xiangjun Zhang <xjz@chromium.org> Cr-Commit-Position: refs/heads/master@{#603797} [modify] https://crrev.com/8bac8fb474b3b5d5b054350a1c2e28f5e78e6ecf/components/viz/common/DEPS [modify] https://crrev.com/8bac8fb474b3b5d5b054350a1c2e28f5e78e6ecf/components/viz/common/gl_scaler.cc [modify] https://crrev.com/8bac8fb474b3b5d5b054350a1c2e28f5e78e6ecf/components/viz/common/gl_scaler_pixeltest.cc [modify] https://crrev.com/8bac8fb474b3b5d5b054350a1c2e28f5e78e6ecf/components/viz/common/gl_scaler_test_util.cc [modify] https://crrev.com/8bac8fb474b3b5d5b054350a1c2e28f5e78e6ecf/components/viz/common/gl_scaler_test_util.h [add] https://crrev.com/8bac8fb474b3b5d5b054350a1c2e28f5e78e6ecf/components/viz/test/data/rasp-grayator-half.png [add] https://crrev.com/8bac8fb474b3b5d5b054350a1c2e28f5e78e6ecf/components/viz/test/data/rasp-grayator.png
,
Nov 8
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8bb8b5625a427f8953c18d829af14aae824e525a commit 8bb8b5625a427f8953c18d829af14aae824e525a Author: Yuri Wiitala <miu@chromium.org> Date: Thu Nov 15 00:10:07 2018 Add GLI420Converter: Uses GLScaler to produce I420 video images. This is the replacement for GLHelper::I420Converter. It uses the new GLScaler to efficiently scale and re-format RGBA textures into I420 image planes. In a soon-upcoming CL, GLRendererCopier will be switched over to use this. Includes unit and pixel testing. Bug: 870036, 810131 Change-Id: I465eb933e7eadeb205676211432cced361a3739b Reviewed-on: https://chromium-review.googlesource.com/c/1330830 Reviewed-by: Ria Jiang <riajiang@chromium.org> Commit-Queue: Yuri Wiitala <miu@chromium.org> Cr-Commit-Position: refs/heads/master@{#608182} [modify] https://crrev.com/8bb8b5625a427f8953c18d829af14aae824e525a/components/viz/common/BUILD.gn [add] https://crrev.com/8bb8b5625a427f8953c18d829af14aae824e525a/components/viz/common/gl_i420_converter.cc [add] https://crrev.com/8bb8b5625a427f8953c18d829af14aae824e525a/components/viz/common/gl_i420_converter.h [add] https://crrev.com/8bb8b5625a427f8953c18d829af14aae824e525a/components/viz/common/gl_i420_converter_pixeltest.cc [add] https://crrev.com/8bb8b5625a427f8953c18d829af14aae824e525a/components/viz/common/gl_i420_converter_unittest.cc [modify] https://crrev.com/8bb8b5625a427f8953c18d829af14aae824e525a/components/viz/common/gl_scaler.cc [modify] https://crrev.com/8bb8b5625a427f8953c18d829af14aae824e525a/components/viz/common/gl_scaler.h [modify] https://crrev.com/8bb8b5625a427f8953c18d829af14aae824e525a/components/viz/common/gl_scaler_unittest.cc
,
Nov 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/82751409267d5f4bfc718aebb728ae42f04e2ba6 commit 82751409267d5f4bfc718aebb728ae42f04e2ba6 Author: Yuri Wiitala <miu@chromium.org> Date: Tue Nov 27 06:16:51 2018 GLRendererCopier: Replace use of GLHelper with GLScaler/GLI420Converter. Switches-over GLRendererCopier to use the new GLScaler and GLI420Converter when scaling and/or format-converting images for CopyOutputRequests. This change also addresses: 1. Simpler handling of cached GL resources (used across copy requests having the same requester). Rather than separate methods for each of the possible cached resources (with some duplicated logic), there is now one "ReusableThings" struct to hold them all (and is passed around on the stack). 2. Simplified the I420 code around handling of "packed textures" and "aligned rects". 3. Simplified the detection/execution of whether to byte-swizzle on the CPU (after readback) or GPU (before readback). Simplified the RGBA bitmap copy-from-pixel-buffer code (w.r.t. upside-down row-by-row copy and byte-swizzling). Testing: Confirmed existing unit and pixel tests are thoroughly-testing all possible code paths in GLRendererCopier. Manually confirmed tab and desktop capture (incl. CrOS), and screenshots work. Performance improvement: A local run of performance_browser_tests, on a machine with a high-end nVidia GPU, has shown a ~20% improvement in capture latency (i.e., the interval of time from when copy request was issued until an SkBitmap result is ready in system memory). http://chromeperf.appspot.com/ will reveal how other platform configurations are affected. Bug: 870036, 810131 , 781986 ,758057 Binary-Size: Increase is temporary (linking in new impl, future CLs will switch-over other clients to stop using old impl). Change-Id: Ibcb16c1382ea0cffb00bc7605e0594b1fe28356a Reviewed-on: https://chromium-review.googlesource.com/c/1343247 Commit-Queue: Yuri Wiitala <miu@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Ria Jiang <riajiang@chromium.org> Cr-Commit-Position: refs/heads/master@{#611027} [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/chrome/browser/extensions/api/cast_streaming/cast_streaming_apitest.cc [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/common/gl_scaler.cc [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/common/gl_scaler_pixeltest.cc [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/copy_output_scaling_pixeltest.cc [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/gl_renderer_copier.cc [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/gl_renderer_copier.h [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/gl_renderer_copier_pixeltest.cc [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/gl_renderer_copier_unittest.cc [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/content/browser/media/capture/fake_video_capture_stack.cc [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/content/browser/media/capture/frame_test_util.h [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/content/browser/media/capture/web_contents_video_capture_device_browsertest.cc [modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/content/browser/renderer_host/render_widget_host_view_browsertest.cc
,
Nov 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40d67e07e3dd7e4fc134ba57c7c580582a4ab13b commit 40d67e07e3dd7e4fc134ba57c7c580582a4ab13b Author: Yuri Wiitala <miu@chromium.org> Date: Wed Nov 28 16:14:09 2018 Plumb-thru color space in skia/ext/image_operations. Copies all the source ImageInfo to the SkBitmap result. Previously, the color space info was being dropped; probably because this code was written before Skia had a concept of color space. The new code uses a simpler API that should be more future-proof. Unit tests were modified to check that the color space is being plumbed-through. Bug: 810131 Change-Id: Ibf6dd685731c6fa1f98b1812a70110cf07405e2f Reviewed-on: https://chromium-review.googlesource.com/c/1352487 Reviewed-by: Mike Klein <mtklein@chromium.org> Commit-Queue: Yuri Wiitala <miu@chromium.org> Cr-Commit-Position: refs/heads/master@{#611720} [modify] https://crrev.com/40d67e07e3dd7e4fc134ba57c7c580582a4ab13b/skia/ext/image_operations.cc [modify] https://crrev.com/40d67e07e3dd7e4fc134ba57c7c580582a4ab13b/skia/ext/image_operations_unittest.cc
,
Dec 17
Due to a typo, the following commit was not recorded on this bug: commit ed44fc45d0c3b3675efec4c550d28c5de1569c9d Author: Yuri Wiitala <miu@chromium.org> Date: Fri Dec 14 19:14:24 2018 Complete the screen capture color space plumbing.
,
Dec 17
With the last code change, this issue is now fixed. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by m...@chromium.org
, May 18 2018