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

Issue 795424 link

Starred by 1 user
Status: Assigned
Owner:
mcasas@chromium.org
Cc:
dcasta...@chromium.org
m...@chromium.org
█ hubbe@chromium.org
dalecur...@chromium.org
mcasas@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Teach VideoFrame to use bit depth ISO carrying that info in VideoPixelFormat

Project Member Reported by mcasas@chromium.org, Dec 15 2017 
VideoPixelFormat [1] amalgamates a few things but in particular
it's now bundling bit depth and planarity/buffer formats, i.e.

  PIXEL_FORMAT_YUV420P9 = 16,
  PIXEL_FORMAT_YUV420P10 = 17,
  PIXEL_FORMAT_YUV422P9 = 18,
  PIXEL_FORMAT_YUV422P10 = 19,
  PIXEL_FORMAT_YUV444P9 = 20,
  PIXEL_FORMAT_YUV444P10 = 21,
  PIXEL_FORMAT_YUV420P12 = 22,
  PIXEL_FORMAT_YUV422P12 = 23,
  PIXEL_FORMAT_YUV444P12 = 24,

are equivalent to
  PIXEL_FORMAT_I420 and
  PIXEL_FORMAT_I422 / PIXEL_FORMAT_YV16 (these are synonyms)
  PIXEL_FORMAT_YV24

and then we have PIXEL_FORMAT_Y16 which is PIXEL_FORMAT_Y8
with a different bit depth.

--> Consider deprecating the bit-depth versions and instead 
carrying bit depth in the VideoFrame. Note that passing bit 
depth to the VF methods might touch a bazillion places, we'll 
see.


[1] https://cs.chromium.org/chromium/src/media/base/video_types.h?type=cs&sq=package:chromium&l=19

Comment 1 by mcasas@chromium.org, Dec 15 2017

Blocking: 778093
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 16 2017 

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

commit e46874e8ad1f05913ff89a0e1d360b2962f056a9
Author: Miguel Casas <mcasas@chromium.org>
Date: Sat Dec 16 01:21:14 2017

VideoPixelFormat: remove unused PIXEL_FORMAT_I422

This CL removes the superfluous PIXEL_FORMAT_I422, synonym of the
older PIXEL_FORMAT_YV16.

This format was landed in crrev.com/2571163002 with a comment that
"Apart from how the buffer is allocated the format is basically
idential to I422 (a.k.a YU16).", but I can't see any difference
from the code, i.e. both PIXEL_FORMAT_YV16 and PIXEL_FORMAT_I422
are treated the same everywhere.

Note that usually we should not roll back VideoPixelFormat's
PIXEL_FORMAT_MAX but PIXEL_FORMAT_I422 was never added to enums.xml,
so UMA knows nothing about it [1], and we can just pretend it was
never there and was never logged [2] (it'd be logged as some hash
name)

[1] https://cs.chromium.org/chromium/src/tools/metrics/histograms/enums.xml?type=cs&q=histograms+enums.xml+y16&sq=package:chromium&l=43271
[2] https://uma.googleplex.com/timeline_v2?sid=deaff528e0268c7d0fdaa405130a075f

TBR=reveman@chromium.org for the autochange in video_resource_updater.cc

Bug: 795424
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;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: I9e5d52d8013bc4f6d237c8274ceb63327d78fa30
Reviewed-on: https://chromium-review.googlesource.com/830769
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524549}
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/cc/resources/video_resource_updater.cc
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/base/video_frame.cc
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/base/video_frame_unittest.cc
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/base/video_types.cc
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/base/video_types.h
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/filters/aom_video_decoder.cc
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/gpu/v4l2/v4l2_device.cc
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/remoting/proto_enum_utils.cc
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/remoting/rpc.proto
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/e46874e8ad1f05913ff89a0e1d360b2962f056a9/media/video/gpu_memory_buffer_video_frame_pool.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 18 2017 

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

commit 134e466415cf125f27905d1ca8646bdaf265496a
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Mon Dec 18 20:28:23 2017

VideoFrame::BitsPerChannel() doesn't need the parameter

This CL removes the parameter |format| of VideoFrame::BitsPerChannel()
since it's a member function and can read it from |format_|. Also
renames it to BitDepth() in preparation to just reading from an
upcoming member |bit_depth_| and makes it const and return size_t.

This CL is a cleanup related to the bug below.

TBR=reveman@chromium.org for the mechanical change in
cc/resources/video_resource_updater.cc

Bug: 795424
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I5a89b490ef1e6a0880c639ee142266c8573ae77a
Reviewed-on: https://chromium-review.googlesource.com/830930
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524786}
[modify] https://crrev.com/134e466415cf125f27905d1ca8646bdaf265496a/cc/resources/video_resource_updater.cc
[modify] https://crrev.com/134e466415cf125f27905d1ca8646bdaf265496a/media/base/video_frame.cc
[modify] https://crrev.com/134e466415cf125f27905d1ca8646bdaf265496a/media/base/video_frame.h
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 18 2017 

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

commit 3efa83fb46b13d4f6509c67eb6d677825b4b0c5e
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Mon Dec 18 21:35:01 2017

VideoPixelFormat: s/PIXEL_FORMAT_YV16/PIXEL_FORMAT_I422/

This CL mechanically renames PIXEL_FORMAT_YV1 to PIXEL_FORMAT_I422,
because what we mean is the latter. It's also a follow up to
Dale's comments [1, 2].

Also updated a few comments. FTR, the UMA has a few entries [3].

[1] https://chromium-review.googlesource.com/c/chromium/src/+/830769/3/media/base/video_types.h#24
[2] https://chromium-review.googlesource.com/c/chromium/src/+/830769#message-2bf7d04e0fe95d581973157f9800d0359089ad3d
[3] https://uma.googleplex.com/timeline_v2?sid=316acb54ccd2cb7501704bd4240f0ab8

Bug: 795424,  784627 

TBR=ccameron@chromium.org and nick@chromium.org for mechanical renames in
cc/ and components/, and content/, respectively.

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;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: I0e001a19e969c934717cffd8868bf8b0d09ef738
Reviewed-on: https://chromium-review.googlesource.com/830727
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524806}
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/cc/resources/video_resource_updater.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/cc/resources/video_resource_updater_unittest.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/components/viz/service/display/renderer_pixeltest.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/content/renderer/pepper/video_decoder_shim.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/base/mac/video_frame_mac_unittests.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/base/video_frame.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/base/video_frame_unittest.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/base/video_types.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/base/video_types.h
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/cast/test/utility/barcode.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/ffmpeg/ffmpeg_common.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/filters/aom_video_decoder.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/filters/ffmpeg_video_decoder.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/gpu/v4l2/v4l2_device.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/remoting/proto_enum_utils.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/remoting/rpc.proto
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/media/video/gpu_memory_buffer_video_frame_pool.cc
[modify] https://crrev.com/3efa83fb46b13d4f6509c67eb6d677825b4b0c5e/tools/metrics/histograms/enums.xml
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 20 2017 

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

commit 16690101187bb176291702e6255daf2a13651a06
Author: Miguel Casas <mcasas@chromium.org>
Date: Wed Dec 20 00:53:06 2017

VideoPixelFormat: s/PIXEL_FORMAT_YV24/PIXEL_FORMAT_I444/

This CL renames PIXEL_FORMAT_YV24 --> PIXEL_FORMAT_I444 since
we mean the latter rather than the former, as can be seen by
the comment in VideoPixelFormat: "24bpp YUV planar...".

It was a mechanical change:
sed -i -e 's/PIXEL_FORMAT_YV24/PIXEL_FORMAT_I444/' `grep -rn PIXEL_FORMAT_YV24 media/ cc/ components/viz/ tools/metrics/histograms/ content chrome | cut -d: -f1 | uniq`

TBR=piman@chromium.org for the automatic renaming in
cc/resources, components/viz/service and content/renderer.

Bug: 795424,  784627 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;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: I0b60fcab72b8d8d42d6ec33f70f62aaac3594f88
Reviewed-on: https://chromium-review.googlesource.com/834289
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525200}
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/cc/resources/video_resource_updater.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/components/viz/service/display/renderer_pixeltest.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/content/renderer/media/webrtc/media_stream_remote_video_source.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/content/renderer/pepper/video_decoder_shim.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/base/mac/video_frame_mac_unittests.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/base/video_frame.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/base/video_frame_unittest.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/base/video_types.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/base/video_types.h
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/ffmpeg/ffmpeg_common.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/filters/aom_video_decoder.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/filters/ffmpeg_video_decoder.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/filters/vpx_video_decoder.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/filters/vpx_video_decoder_fuzzertest.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/remoting/proto_enum_utils.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/remoting/rpc.proto
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/test/pipeline_integration_test.cc
[modify] https://crrev.com/16690101187bb176291702e6255daf2a13651a06/media/video/gpu_memory_buffer_video_frame_pool.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 20 2017 

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

commit 0a3fd92a1ae022e729ef1f522314586909db0f2c
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Wed Dec 20 19:33:28 2017

VideoPixelFormat: remove unused PIXEL_FORMAT_Y8

This CL removes PIXEL_FORMAT_Y8 because it's unused (from the code
and this is partially confirmed by the UMA [1]). I'm not aware of
any downstream project using this pixel enumeration entry.

Archaeology: PIXEL_FORMAT_Y8 was landed in crrev.com/2113243003
together with PIXEL_FORMAT_Y16, which is used for Depth capture.

[1] https://uma.googleplex.com/timeline_v2?sid=6f456e7a675431867acab57847fc9f7e

TBR=ccameron@chromium.org for the automatic removal in video_resource_updater.cc

Bug: 795424
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I02a96b0ef20e5b72315dfdc81aff2b303da29b33
Reviewed-on: https://chromium-review.googlesource.com/831086
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525408}
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/cc/resources/video_resource_updater.cc
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/media/base/video_frame.cc
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/media/base/video_frame_unittest.cc
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/media/base/video_types.cc
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/media/base/video_types.h
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/media/remoting/proto_enum_utils.cc
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/media/remoting/rpc.proto
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/media/video/gpu_memory_buffer_video_frame_pool.cc
[modify] https://crrev.com/0a3fd92a1ae022e729ef1f522314586909db0f2c/tools/metrics/histograms/enums.xml
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 21 2017 

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

commit 006301baa4d770675e7c814d96c8a8b01facbb7a
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Dec 21 16:34:04 2017

VideoPixelFormat: s/PIXEL_FORMAT_YV12A/PIXEL_FORMAT_I420A/

This CL renames PIXEL_FORMAT_YV12A to PIXEL_FORMAT_I420A since
we mean the latter rather than the former, as can be seen by
the comment in VideoPixelFormat: "20bpp YUVA planar... 2x2 UV,".

(After this and crrev.com/c/834289, the only dangling incorrect
usage is VideoPixelFormat in YV12 when we mean I420; this might
need more nuanced substitutions).

The autoawesome scriptskills were:
sed -i -e 's/PIXEL_FORMAT_YV12A/PIXEL_FORMAT_I420A/' `grep -rn PIXEL_FORMAT_YV12A media/ cc/ components/viz/ tools/metrics/histograms/ content chrome | cut -d: -f1 | uniq`

TBR=jam@chromium.org for the 3 mechanical renames in cc/ chrome/
and components/

Bug: 795424,  784627 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;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: I4e842775894a5c088e44fc88388e1e5bf3b4cbc1
Reviewed-on: https://chromium-review.googlesource.com/834292
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525711}
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/cc/resources/video_resource_updater.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/chrome/browser/extensions/api/cast_streaming/cast_streaming_apitest.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/chrome/renderer/media/cast_rtp_stream.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/components/viz/service/display/renderer_pixeltest.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/image_capture/image_capture_frame_grabber.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media/media_stream_video_renderer_sink_unittest.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media/webmediaplayer_ms_compositor.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media/webmediaplayer_ms_unittest.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media/webrtc/media_stream_remote_video_source.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media/webrtc/webrtc_video_capturer_adapter.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media/webrtc/webrtc_video_frame_adapter.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media_capture_from_element/canvas_capture_handler.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media_capture_from_element/canvas_capture_handler_unittest.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/media_recorder/vpx_encoder.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/pepper/pepper_media_stream_video_track_host.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/pepper/pepper_video_source_host.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/content/renderer/pepper/video_decoder_shim.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/base/mac/video_frame_mac_unittests.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/base/video_frame.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/base/video_frame_pool_unittest.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/base/video_frame_unittest.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/base/video_types.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/base/video_types.h
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/base/video_util.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/ffmpeg/ffmpeg_common.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/filters/vpx_video_decoder.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/filters/vpx_video_decoder_fuzzertest.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/formats/webm/webm_video_client.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/remoting/proto_enum_utils.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/remoting/rpc.proto
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/renderers/video_renderer_impl_unittest.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/test/pipeline_integration_test.cc
[modify] https://crrev.com/006301baa4d770675e7c814d96c8a8b01facbb7a/media/video/gpu_memory_buffer_video_frame_pool.cc
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/+/9e776602c9c59a762c6c214b205c8268d452472b

commit 9e776602c9c59a762c6c214b205c8268d452472b
Author: Miguel Casas <mcasas@chromium.org>
Date: Mon Jan 08 18:26:08 2018

VideoPixelFormat: Use PIXEL_FORMAT_I420 ISO YV12 where needed

This CL replaces the uses of PIXEL_FORMAT_YV12 where we mean
PIXEL_FORMAT_I420, most notably in the sw video decoders
{FFmpeg,Vpx}VideoDecoder, but also when it comes to rendering,
since we should use YUV instead of YVU.

For coherency, this CL also changes many VideoFrame factory
methods' calls to use I420.

Note that some _YV12 uses are legit, see the bug comment [1];
essentially those are present when we translate from/to other
API yv12 formats. These uses are left untouched.

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=784627#c8

TBR=kenrb@chromium.org for the trivial s/YV12/I420/ in
media/mojo/interfaces/video_decoder_config_struct_traits_unittest.cc

Bug: 795424,  784627 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;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: Ie677c7a71a88a194aad2823c0c994445f25e213c
Reviewed-on: https://chromium-review.googlesource.com/840703
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527677}
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/cc/layers/video_frame_provider_client_impl_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/cc/layers/video_layer_impl_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/components/viz/common/yuv_readback_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/components/viz/service/display/renderer_pixeltest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/browser/media/capture/cursor_renderer_aura_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/browser/media/capture/cursor_renderer_mac_unittest.mm
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/browser/renderer_host/render_widget_host_view_browsertest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/browser/renderer_host/render_widget_host_view_frame_subscriber.h
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/renderer/image_capture/image_capture_frame_grabber.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/renderer/media/pepper_to_video_track_adapter.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/renderer/media/webmediaplayer_ms_compositor.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/renderer/media/webmediaplayer_ms_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/renderer/media/webrtc/media_stream_remote_video_source.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/renderer/media/webrtc/webrtc_video_capturer_adapter.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/renderer/media/webrtc/webrtc_video_frame_adapter.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/content/renderer/pepper/video_decoder_shim.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/base/fake_demuxer_stream.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/base/null_video_sink_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/base/test_helpers.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/base/video_decoder_config_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/base/video_frame.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/base/video_frame_pool_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/base/video_frame_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/base/video_util_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/blink/video_decode_stats_reporter_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/blink/video_frame_compositor_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/capture/content/android/screen_capture_machine_android.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/cast/receiver/video_decoder_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/cast/sender/vp8_quantizer_parser_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/cast/test/fake_media_source.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/cdm/simple_cdm_allocator.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/cdm/simple_cdm_allocator_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/ffmpeg/ffmpeg_common.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/filters/ffmpeg_demuxer_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/filters/ffmpeg_video_decoder.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/filters/ffmpeg_video_decoder_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/filters/source_buffer_state_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/filters/video_renderer_algorithm_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/filters/vpx_video_decoder.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/filters/vpx_video_decoder_fuzzertest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/formats/mp2t/es_parser_h264.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/formats/mp4/mp4_stream_parser.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/formats/webm/webm_video_client.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/formats/webm/webm_video_client_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/mojo/common/mojo_shared_buffer_video_frame_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/mojo/interfaces/video_decoder_config_struct_traits_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/renderers/paint_canvas_video_renderer_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/renderers/video_renderer_impl_unittest.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/test/pipeline_integration_test.cc
[modify] https://crrev.com/9e776602c9c59a762c6c214b205c8268d452472b/media/video/gpu_memory_buffer_video_frame_pool_unittest.cc

Comment 9 by mcasas@chromium.org, Mar 2 2018

Blocking: -778093

Comment 10 by hubbe@chromium.org, Apr 24 2018 

I think we should consolidate all the P9/P10/P12 formats into one 16-bit format, but I don't think we should consolidate them into the 8-bit formats since the layout is different.

Comment 11 by mcasas@chromium.org, Apr 27 2018 

I mulled over this for a while, now we have

  PIXEL_FORMAT_YUV420P9 
  PIXEL_FORMAT_YUV420P10
  PIXEL_FORMAT_YUV420P12

etc, which I don't like because they mix the subsampling format, memory layout 
and the bit depth, from which we derive part of the memory layout...  
OTOH, ffmpeg uses this taxonomy (AV_PIX_FMT_YUV444P9LE etc).


The alternative started in crrev.com/c/828121 has as a goal to extract 
completely the bit depth out of the pixel format (and remove the P9, P10 etc
variants), but has the minus that VideoFrame static functions e.g. 
AllocationSize(), PlaneSize(), and its creation methods CreateFrame()
etc, they all have to get a |bit_depth| parameter. This solution also makes 
harder to enforce (DCHECK()s) that clients will pass correct pixel_formats 
+ bit_depths to VideoFrame.


The solution proposed in [1] is a hybrid, i.e. add PIXEL_FORMAT_I420_16
I422_16 etc and also add the |bit_depth| format to VideoFrame. This solution
makes it unnecessary to have a |bit_depth| parameter to the VideoFrame static 
functions (e.g. AllocationSize(), PlaneSize() etc), but still is needed in
the Createxxx() methods. libvpx has something like this (VPX_IMG_FMT_I42016 
etc). I feel only lukewarm towards this solution because it creates more 
complexity than what it takes away, because we add another dimension without 
tackling the one this Issue tries to remove.


People in cc please have your say !  :-)


[1] https://chromium-review.googlesource.com/c/chromium/src/+/828121/24/cc/resources/video_resource_updater_unittest.cc#167
►

Sign in to add a comment