New issue
Advanced search Search tips

Issue 837337 link

Starred by 3 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Use Pixel Aspect Ratio instead of Natural Size

Project Member Reported by sande...@chromium.org, Apr 26 2018

Issue description

Using aspect ratios throughout our stack can reduce rounding errors caused by repeatedly computing sizes in pixels.

In particular:
  - VideoDecoderConfig stores a |natural_size|, but it is only accurate if the encoded video's |visible_size| matches the configuration. Passing an aspect ratio solves the problem.
  - VideoFrame's |natural_size| is usually scaled for display. Passing an aspect ratio could make this more accurate (and remove the need to compute it in decoders).

One issue:
  - 1x1 stand-in frames still need an explicit |natural_size|, because otherwise they will appear to be very small. Some overall concept of scale is still required.

One extension:
  - H.264 decoders should use in-stream pixel aspect ratio information, but only if the VideoDecoderConfig does not contain an explicit pixel aspect ratio.
 
MP4: 'pasp' box contains pixel aspect ratio, and if present it is authoritative over the stream.

WebM: If present, DisplayWidth/DisplayHeight encode natural size. It is undocumented how this relates to in-stream metadata.

H.264: Pixel aspect ratio is optional in the VUI (SPS).

AOM: Each frame encodes the visible ('display') size and optionally a natural ('rendering') size as well.
Project Member

Comment 2 by bugdroid1@chromium.org, May 1 2018

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

commit af451f2b8adfb22eabf60b7232a3c050941c9261
Author: Dan Sanders <sandersd@chromium.org>
Date: Tue May 01 19:21:23 2018

[media] Use pixel aspect ratio to compute |natural_size| in all decoders.

This CL standardizes the |natural_size| computation in all decoder
implementations to use the decoded |visible_size| and the
VideoDecoderConfig's pixel aspect ratio.

Bug:  766657 , 837337
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I01a5bd59c353d6f07fd02c9fc3d3e64ab1a04434
Reviewed-on: https://chromium-review.googlesource.com/1026992
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555136}
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/base/video_decoder_config.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/base/video_decoder_config.h
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/base/video_util.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/base/video_util.h
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/base/video_util_unittest.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/cdm/cdm_adapter.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/cdm/cdm_adapter.h
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/filters/aom_video_decoder.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/filters/ffmpeg_video_decoder.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/gpu/android/media_codec_video_decoder.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/gpu/ipc/service/vda_video_decoder.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/gpu/ipc/service/vda_video_decoder_unittest.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/media/gpu/windows/d3d11_video_decoder_impl.cc
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/third_party/WebKit/LayoutTests/platform/linux/compositing/video-frame-size-change-expected.png
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/third_party/WebKit/LayoutTests/platform/linux/compositing/video-frame-size-change-expected.txt
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spv175/compositing/video-frame-size-change-expected.png
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spv175/compositing/video-frame-size-change-expected.txt
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/third_party/WebKit/LayoutTests/platform/mac/compositing/video-frame-size-change-expected.png
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/third_party/WebKit/LayoutTests/platform/mac/compositing/video-frame-size-change-expected.txt
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/third_party/WebKit/LayoutTests/platform/win/compositing/video-frame-size-change-expected.png
[modify] https://crrev.com/af451f2b8adfb22eabf60b7232a3c050941c9261/third_party/WebKit/LayoutTests/platform/win/compositing/video-frame-size-change-expected.txt

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 11

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

commit 8eed09b1ff9a59c6cc6e0386aa40dddffdcefe3a
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date: Tue Dec 11 02:10:43 2018

Use config specified aspect ratio in D3D11VideoDecoder

Other decoder implementations (VDA, ffmpeg, etc.) use the aspect ratio
from the config when setting natural size of video frames.  Duplicate
the logic from VDA to D3D11VideoDecoder.

Found due to failing Pixel_Video_MP4_FourColors_Aspect_4x3 test in CL to
enable D3D11VideoDecoder (and decode swap chain) on waterfall:
https://chromium-review.googlesource.com/c/chromium/src/+/1364341
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/win7_chromium_rel_ng/145995

Bug: 837337
Change-Id: I54a3928646f6d71dec26e9995efe4eb7bda458d7
Reviewed-on: https://chromium-review.googlesource.com/c/1368774
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615383}
[modify] https://crrev.com/8eed09b1ff9a59c6cc6e0386aa40dddffdcefe3a/media/gpu/windows/d3d11_video_decoder.cc
[modify] https://crrev.com/8eed09b1ff9a59c6cc6e0386aa40dddffdcefe3a/media/gpu/windows/d3d11_video_decoder.h

Sign in to add a comment