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

Issue 845076 link

Starred by 10 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Decoded frames are corrupted on most intel platforms

Project Member Reported by hiroh@chromium.org, May 21 2018

Issue description

Decoded frames are corrupted on most intel platforms since 10695.0.0.
video_VideoDecodeAccelerator (VDA unittest) also fails because of a generated thumbnails validity check. As I attach the resulted thumbanils on caroline, they are obviously bad thumbnails.

The Culprit CL is https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/996648.

Reproduce way:
(A) Play a video in crosvideo.appspot.com, or
(B) VDA unittest (thumbnail test case).
 
test-25fps.h264.bad_thumbnails.png
231 KB View Download

Comment 1 by hiroh@chromium.org, May 21 2018

Because the CL broke video decoding on most intel platforms, it is so severe that reverting the CL is reasonable.
Let me revert the CL now.
Cc: y...@chromium.org jcliang@chromium.org shenghao@chromium.org ajha@chromium.org wuchengli@chromium.org brajkumar@chromium.org
 Issue 845066  has been merged into this issue.
IS that for buffers allocated by chrome?

Comment 4 by hiroh@chromium.org, May 21 2018

> IS that for buffers allocated by chrome?
Yes. The above ones are Chrome pure playback and VDA unittest. They are pure Chrome, not ARC++ at all.
Cc: vsu...@chromium.org
 Issue 845286  has been merged into this issue.
Issue reproduced with video captured through Chrome OS Camera app. Attached gallery view of camera app.
Screenshot 2018-05-21 at 17.11.00.png
1021 KB View Download
Cc: keiichiw@chromium.org
Project Member

Comment 8 by bugdroid1@chromium.org, May 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/minigbm/+/2b682fde2d2fe12b95b434a47466bc69aa08b6bd

commit 2b682fde2d2fe12b95b434a47466bc69aa08b6bd
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Tue May 22 08:06:31 2018

Revert "i915: Add GBM_BO_USE_HW_VIDEO_DECODER use flag"

This reverts commit 1bd7b04a3ae68c0314bdee06c559093de9e5a304.

Reason for revert: Breaks video decoding on most intel platforms, e.g., caroline and samus

Original change's description:
> i915: Add GBM_BO_USE_HW_VIDEO_DECODER use flag
> 
> This flag is used to indicate that the platform video decoder will be
> writing into this buffer and that it should be allocated
> accordingly. On Intel, this means that we have to allocate y-tiled
> NV12 for libva to be able to decode to the buffer.
> 
> We force gralloc NV12 allocations to be linear for now, since ARC++
> doesn't properly pass modifiers to ChromeOS.
> 
> BUG=822346
> TEST=test_that graphics_Gbm
> 
> Change-Id: I840c30d22355d26816df718b49717407e2e4620f
> Reviewed-on: https://chromium-review.googlesource.com/996648
> Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
> Tested-by: Kristian H. Kristensen <hoegsberg@chromium.org>
> Reviewed-by: Kristian H. Kristensen <hoegsberg@chromium.org>

Bug: 822346,  845076 
Change-Id: I7681ddb66e4789951e840821993fc5562a55d1af
Reviewed-on: https://chromium-review.googlesource.com/1066032
Tested-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Kuo Jen Wei <inker@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>

[modify] https://crrev.com/2b682fde2d2fe12b95b434a47466bc69aa08b6bd/i915.c
[modify] https://crrev.com/2b682fde2d2fe12b95b434a47466bc69aa08b6bd/drv.h
[modify] https://crrev.com/2b682fde2d2fe12b95b434a47466bc69aa08b6bd/cros_gralloc/cros_gralloc_driver.cc
[modify] https://crrev.com/2b682fde2d2fe12b95b434a47466bc69aa08b6bd/gbm_helpers.c
[modify] https://crrev.com/2b682fde2d2fe12b95b434a47466bc69aa08b6bd/gbm.h

Labels: ReleaseBlock-Dev M-68
Able to reproduce the issue on Dev RC 68.0.3436.0/10703.0.0 Reks, Daisy and Candy. Hence, marking it as dev blocker.

Thanks!
Unable to reproduce the issue on 68.0.3437.0/10704.0.0 - Candy, Reks
Status: Verified (was: Assigned)
Comment 10 sounds like verified.
 Issue 845513  has been merged into this issue.
I can reproduce this on caroline easily. youtube doesn't trigger it but regular hmtl5 video tags do reproduce the problem. Thanks for reverting, I'm trying figure out why this breaks on SKL but not KBL.
Here's the fix:

https://chromium-review.googlesource.com/c/chromium/src/+/1070995

What's the best way to run video_VideoDecodeAccelerator locally?
Oh, I guess that test_that IP video_VideoDecodeAccelerator, I just needed to wait a little longer.
Thanks for looking into this!

The video_decode_accelerator_unittest binary can also be run manually on DUT (parameters documented here: https://cs.chromium.org/chromium/src/media/gpu/video_decode_accelerator_unittest.cc?l=109, test streams and md5 sums are in https://cs.chromium.org/chromium/src/media/test/data/).

Happy to help if you'd have any additional questions about this. Thanks!

Comment 17 by hiroh@chromium.org, May 24 2018

video_VideoDecodeAccelerator runs VDA unittest. It will use VDA unittest based on Chrome contained on ChromeOS by default. The binary is located under /usr/local/autotest.
Instead of running video_VideoDecodeAccelerator, you would rather do in the following simpler way.
In cros chrome-sdk,
(1) ninja -C out_${SDK_BOARD}/Release -j2000 -l 100 video_decode_accelerator_unittest
(2) scp out_${SDK_BOARD}/Release/video_decode_accelerator_unittest ${DUT_IP}:/tmp
(3) scp media/test/data/test-25fps* ${DUT_IP}:/tmp
In DUT:/tmp,
$ stop ui
$ ./video_decode_accelerator_unittest --test_video_data=test-25fps.h264:320:240:250:258:35:150:1 --ozone-platform=gbm
$ ./video_decode_accelerator_unittest --test_video_data=test-25fps.vp8:320:240:250:250:35:150:11 --ozone-platform=gbm
$ ./video_decode_accelerator_unittest --test_video_data=test-25fps.vp9:320:240:250:250:35:150:12 --ozone-platform=gbm

Please also confirm to play a video on crosvideo.appspot.com.

Thanks.


hoegsberg@, if you need more information on running the VDA unittest manually, the following document may help a bit:

https://docs.google.com/document/d/1asty6gacbKzo_QSfXXGEMaKRtIA5MhyA_TKvmh33wtU/
Project Member

Comment 19 by bugdroid1@chromium.org, May 24 2018

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

commit d50daf01476b7a84b8e30515c8d33ac97d2acd7b
Author: Kristian H. Kristensen <hoegsberg@chromium.org>
Date: Thu May 24 03:45:25 2018

ozone/gbm_buffer: Always set format_modifier_

We only set the format_modifier_ field if the GBM_BO_USE_SCANOUT flags
was set. On platforms where we can't scanout NV12, we fall back to
allocating GbmBuffers without the scanout flag and consequently end up
assuming those are all linear. Except libva, which picks up the tiling
from the kernel bo and decodes with y-tiling enabled.

We need to always and unconditionally store the format modifier, it's
as much part of the buffer meta data as the pixel format. Move
assignment to initializers.

Bug:  845076 
Test: crosvideo.appspot.com plays correctly on pre-KBL Intel chromebooks
Change-Id: I74c435807947afb60ea4417ace39751e213792bc
Reviewed-on: https://chromium-review.googlesource.com/1070995
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561380}
[modify] https://crrev.com/d50daf01476b7a84b8e30515c8d33ac97d2acd7b/ui/ozone/platform/drm/gpu/gbm_buffer.cc

acourbot@, hiroh@, thanks that was very helpful. 
Project Member

Comment 22 by bugdroid1@chromium.org, May 25 2018

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

commit 96e8525ced558445035c9e780b8c33aeb9d8587d
Author: Kristian H. Kristensen <hoegsberg@chromium.org>
Date: Fri May 25 22:02:52 2018

vda_unittest: Use SCANOUT_VDA_WRITE when allocating on ChromeOS

We need to allocate the native pixmap for the unittest using the
SCANOUT_VDA_WRITE usage flag like we do in Chrome so we get the
right tiling.

Bug:  845076 
Test: video_decode_accelerator_unittest passes
Change-Id: I877bf94e6f7f90c14973be76c1f53d841aed4305
Reviewed-on: https://chromium-review.googlesource.com/1072638
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Kristian H. Kristensen <hoegsberg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562031}
[modify] https://crrev.com/96e8525ced558445035c9e780b8c33aeb9d8587d/media/gpu/test/video_decode_accelerator_unittest_helpers.cc

Sign in to add a comment