New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 798485 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Chrome , Mac
Pri: 3
Type: Bug

Blocked on:
issue libyuv:751


Participants' hotlists:
high-bit-depth


Sign in to add a comment

Use libyuv::Convert16To8Plane for high bit depth videos where HDR is not supported

Project Member Reported by mcasas@chromium.org, Jan 2 2018

Issue description

High bit depth videos (e.g. VP9 Profile 2 with 10bits per channel) are uploaded 
into textures in VideoResourceUpdater::CreateForSoftwarePlanes() [1] using a 
HalfFloatMaker [2] (when the target resource format is viz::LUMINANCE_F16 which
is very often the case since half_float_linear [3] textures are supported in
desktop core profile [4]).

If the final framebuffer is not HDR capable, there's no need to carry around
half floats and instead the 10bpc video (H010 [5]) can be downsized to I420,
and uploaded in the GpuMemoryBufferVideoFramePool, speeding up the rendering
(bc: less data to transfer), relieving the Impl thread, and also, in the case 
of Mac, actually working (crbug.com/792295).


[1] https://cs.chromium.org/chromium/src/cc/resources/video_resource_updater.cc?type=cs&q=VideoResourceUpdater::CreateForSoftwarePlanes&sq=package:chromium&l=308
[2] https://cs.chromium.org/chromium/src/cc/resources/video_resource_updater.cc?type=cs&q=VideoResourceUpdater::CreateForSoftwarePlanes&sq=package:chromium&l=533
[3] https://cs.chromium.org/chromium/src/cc/resources/resource_provider.cc?type=cs&sq=package:chromium&l=153
[4] https://cs.chromium.org/chromium/src/gpu/command_buffer/service/feature_info.cc?type=cs&l=1464
[5] https://cs.chromium.org/chromium/src/third_party/libyuv/include/libyuv/video_common.h?type=cs&q=libyuv+H010&sq=package:chromium&l=61
 
Doing tests with http://crosvideo.appspot.com/?codec=vp9_2&resolution=2176

WIP CL crev.com/c/843132 seems to improve the rendering indeed.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c5396fc07c9a8cf27bbd0186231345b02dda3ad3

commit c5396fc07c9a8cf27bbd0186231345b02dda3ad3
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Wed Jan 03 20:14:10 2018

VideoResourceUpdater: use libyuv functions for copy/copy+shift

This CL rewrites two parts of VideoResourceUpdater::CreateForSoftwarePlanes()
to take advantage of the faster libyuv library functions:
- libyuv::Convert16To8Plane() was landed as part of libyuv:751
and is way faster than the C counterpart [1]
- libyuv::CopyPlane() existed already and is also faster than
the C counterpart.

This CL is purely refactoring, functionality is covered by unit
and browser tests.

[1] https://bugs.chromium.org/p/libyuv/issues/detail?id=751#c11

Bug:  798485 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Iead82969b13917aabdaacee47799f6816a164171
Reviewed-on: https://chromium-review.googlesource.com/848055
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526781}
[modify] https://crrev.com/c5396fc07c9a8cf27bbd0186231345b02dda3ad3/cc/resources/video_resource_updater.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7861ba7dd744d28a79d9802e17b00d075efd341

commit c7861ba7dd744d28a79d9802e17b00d075efd341
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Thu Jan 04 03:17:45 2018

VideoResourceUpdater: cleanup CreateForSoftwarePlanes()

This CL cleans up a bit the casuistic around CreateForSoftwarePlanes()
handling of the different formats, leaving it a bit clearer IMHO:
- substitutes the big switch-case by a DCHECK() of supported formats
 and the explicit handling of LUMINANCE_F16 and R16_EXT.
- Uses early-continue in the for-plane_resources loop (less indenting).
- Simplifies the logic of |needs_conversion| (it is needed if:
 - the destination is LUMINANCE_F16
 - the destination is LUMINANCE_8 or RED_8 and either:
  - |bits_per_channel| > 8  or
  - the origin and destination strides differ.)
- Adds a few DCHECK()s.
- Extracts a few variables for readability.

The code should be equivalent, so it's covered by tests.

Bug:  798485 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I46b74cf4b6e82556777d65fa565de52c21891644
Reviewed-on: https://chromium-review.googlesource.com/848648
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526909}
[modify] https://crrev.com/c7861ba7dd744d28a79d9802e17b00d075efd341/cc/resources/video_resource_updater.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24f8e8b47270f67d1a4a43889ae77532b83e68da

commit 24f8e8b47270f67d1a4a43889ae77532b83e68da
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Jan 04 22:57:44 2018

Gfx: add BufferFormatToString helper function

This CL adds a service function BufferFormatToString() to translate
from gfx::BufferFormat to a string, and uses it in a few places ISO
static_cast<>ing to int.

Bug:  798485 
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: I01093983b62fb9c8c495b521c890c4b45e4708a6
Reviewed-on: https://chromium-review.googlesource.com/847772
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527129}
[modify] https://crrev.com/24f8e8b47270f67d1a4a43889ae77532b83e68da/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.cc
[modify] https://crrev.com/24f8e8b47270f67d1a4a43889ae77532b83e68da/ui/gfx/buffer_format_util.cc
[modify] https://crrev.com/24f8e8b47270f67d1a4a43889ae77532b83e68da/ui/gfx/buffer_format_util.h
[modify] https://crrev.com/24f8e8b47270f67d1a4a43889ae77532b83e68da/ui/gl/gl_image_io_surface.mm

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/961bc0cc5e6e052ec9b51f1fcc5c94d090daf87a

commit 961bc0cc5e6e052ec9b51f1fcc5c94d090daf87a
Author: Adam Langley <agl@chromium.org>
Date: Fri Jan 05 06:07:30 2018

Dedup |BufferFormatToString| functions.

CL 847772 added a BufferFormatToString utility function which mirrors an
existing function. This also happens to break jumbo builds.

This change removes the original version and, instead, uses the newly
added function.

Bug:  798485 
Change-Id: I2226de043bf375bde05fc28b67f07182f0d0aa66
Reviewed-on: https://chromium-review.googlesource.com/851090
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527226}
[modify] https://crrev.com/961bc0cc5e6e052ec9b51f1fcc5c94d090daf87a/content/browser/gpu/gpu_internals_ui.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bfcafb4c8a10ef13b33b6633e6626919ce4a50af

commit bfcafb4c8a10ef13b33b6633e6626919ce4a50af
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Sun Jan 07 10:21:30 2018

Roll src/third_party/libyuv c67db60..263243a

263243a Add unittest for H010ToARGB conversion by Frank Barchard
a646585 I210ToARGB conversion from 10 bit YUV to RGB by Frank Barchard
ac088b4 Roll lss_revision 63f24c8221..e6527b0cd4 by Frank Barchard
911bfae Move DEPS to conditions. by Patrik Höglund
8db1c98 Update roll script to allow advanced url specs, update catapult repo. by Patrik Höglund
1e4600b Remove unused ARGBAttenuateRow_Any_SSE2 prototype by Frank Barchard
2ed2402 I420ToI010 for 8 to 10 bit YUV conversion. by Frank Barchard
140fc0a Remove LIBYUV_SSSE3_ONLY and ARGBSHUFFLEROW_SSE2 by Frank Barchard
768f103 Convert8To16 for better H010 support by Frank Barchard

Bug:  798485 ,  libyuv:751 
Change-Id: I841afc3a6861718082072244b8b7178253e7e997
Reviewed-on: https://chromium-review.googlesource.com/853236
Commit-Queue: Frank Barchard <fbarchard@chromium.org>
Reviewed-by: Frank Barchard <fbarchard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527534}
[modify] https://crrev.com/bfcafb4c8a10ef13b33b6633e6626919ce4a50af/DEPS

Blockedon: libyuv:751
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fcc38dfa593a94fe8b894e7da5b058cd41017585

commit fcc38dfa593a94fe8b894e7da5b058cd41017585
Author: Miguel Casas <mcasas@chromium.org>
Date: Mon Jan 08 20:46:21 2018

Roll src/third_party/libyuv 263243a..50f9e61

50f9e61 Add H010ToABGR, I010ToABGR and I010ToARGB functions by Frank Barchard
9d2cd6a H010ToAR30 optimized to 2 step conversion by Frank Barchard

Bug:  798485 ,  libyuv:751 
Change-Id: I03573262f062014b8e384b59fa76d87e6dfbfb37
Reviewed-on: https://chromium-review.googlesource.com/854819
Reviewed-by: Frank Barchard <fbarchard@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Commit-Queue: Frank Barchard <fbarchard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527745}
[modify] https://crrev.com/fcc38dfa593a94fe8b894e7da5b058cd41017585/DEPS

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0980968f7d046c9a2699b7b9d45bcac1c5a0ef3

commit b0980968f7d046c9a2699b7b9d45bcac1c5a0ef3
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Mon Jan 08 22:56:42 2018

PaintCanvasVideoRenderer: use 10bpc libyuv functions

This CL introduces libyuv::{I,H}010To{ARGB,ABGR}() to the
PaintCanvasVideoRenderer.

It also	simplifies the DownShiftHighbitVideoFrame() method
and extends the appropriate test case to use P9 and P12
I420 formats.

NOTE: This CL needs libyuv I010ToARGB() and ..ToABGR() methods:
CQ-DEPEND=CL:852985

Bug:  798485 
Change-Id: I089fa6e8c7cf47e7819e1c77ca61561dbf65b173
Reviewed-on: https://chromium-review.googlesource.com/853392
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527796}
[modify] https://crrev.com/b0980968f7d046c9a2699b7b9d45bcac1c5a0ef3/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/b0980968f7d046c9a2699b7b9d45bcac1c5a0ef3/media/renderers/paint_canvas_video_renderer_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e101805e7f66d878389df053a4ce8a186e98d00

commit 5e101805e7f66d878389df053a4ce8a186e98d00
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue Jan 09 20:35:24 2018

GpuVideoAcceleratorFactories: add HDR indication and wire it

This CL adds a new method to GpuVideoAcceleratorFactories to
set HDR rendering enabled (or not), and pings it from
RenderThreadImpl on both creation and ScreenInfo update.

RenderThreadImpl, in turn, gets pinged by RenderWidget::Resize(),
which is actually pinged every time the Renderer sees a change
in parameters, e.g. a pure resize or a new Screen plugged in
causing the compositor to change some configuration.

This HDR rendering indication cannot be part of GpuInfo nor
gpu::Capabilities because it depends on the actual setting of
the screen and/or hot plug events.

This CL precedes crrev.com/c/843132, where a 16to8 bpc conversion
will happen during copy to GpuMemoryBuffers if HDR is not active
(Factories will test |is_hdr_rendering_enabled_|)

Bug:  798485 
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: I2d30119ed92c69bbbfca9487e72dda62c02d7c3b
Reviewed-on: https://chromium-review.googlesource.com/852912
Reviewed-by: Fredrik Hubinette <hubbe@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528093}
[modify] https://crrev.com/5e101805e7f66d878389df053a4ce8a186e98d00/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
[modify] https://crrev.com/5e101805e7f66d878389df053a4ce8a186e98d00/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.h
[modify] https://crrev.com/5e101805e7f66d878389df053a4ce8a186e98d00/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/5e101805e7f66d878389df053a4ce8a186e98d00/content/renderer/render_thread_impl.h
[modify] https://crrev.com/5e101805e7f66d878389df053a4ce8a186e98d00/content/renderer/render_widget.cc
[modify] https://crrev.com/5e101805e7f66d878389df053a4ce8a186e98d00/media/video/gpu_video_accelerator_factories.h
[modify] https://crrev.com/5e101805e7f66d878389df053a4ce8a186e98d00/media/video/mock_gpu_video_accelerator_factories.h

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/415e57f67dcd5ae52b7a62b4409a4176f938f64e

commit 415e57f67dcd5ae52b7a62b4409a4176f938f64e
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Jan 11 03:21:53 2018

GpuMemoryBufferVideoFramePool: support high bit depth formats

This CL teaches GpuMemoryBufferVideoFramePool to support 420
high bit depth formats, reducing their bit depth on upload
using libyuv::Convert16To8Plane(), which is faster than the
curent code path (we make HalfFloats on VideoResourceUpdater
on the compositor main thread).

CQ-DEPEND=CL:852912

Bug:  798485 
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: I865241b07c4c4c5d067f618f274f770b21ece811
Reviewed-on: https://chromium-review.googlesource.com/843132
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Fredrik Hubinette <hubbe@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528549}
[modify] https://crrev.com/415e57f67dcd5ae52b7a62b4409a4176f938f64e/content/browser/media/media_color_browsertest.cc
[modify] https://crrev.com/415e57f67dcd5ae52b7a62b4409a4176f938f64e/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
[modify] https://crrev.com/415e57f67dcd5ae52b7a62b4409a4176f938f64e/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.h
[modify] https://crrev.com/415e57f67dcd5ae52b7a62b4409a4176f938f64e/media/test/data/blackwhite.html
[modify] https://crrev.com/415e57f67dcd5ae52b7a62b4409a4176f938f64e/media/video/gpu_memory_buffer_video_frame_pool.cc
[modify] https://crrev.com/415e57f67dcd5ae52b7a62b4409a4176f938f64e/media/video/gpu_memory_buffer_video_frame_pool_unittest.cc
[modify] https://crrev.com/415e57f67dcd5ae52b7a62b4409a4176f938f64e/media/video/gpu_video_accelerator_factories.h
[modify] https://crrev.com/415e57f67dcd5ae52b7a62b4409a4176f938f64e/media/video/mock_gpu_video_accelerator_factories.h

Status: Fixed (was: Assigned)

Sign in to add a comment