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

Issue 827032 link

Starred by 1 user

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

video_VideoDecodeAccelerator: need to update thumbnail MD5 due to picture format change

Project Member Reported by hiroh@chromium.org, Mar 29 2018

Issue description

Because the picture format was changed (crrev.com/c/950007), the binary of resulted picture was changed.
Thumbnail test in video_decode_accelerator_unittest started to fail, which checks the validity of decoded picture by md5sum.
It is needed to update md5sum values for a new format like this change crrev.com/c/742830.

A new md5sum can be known looking the logs of video_VideoDecodeAccelerator.
https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/testDetails?milestone=67&daysBack=30&testName=video_VideoDecodeAccelerator

Or should we revert the format change CL? (crrev.com/c/950007)
 

Comment 1 by hiroh@chromium.org, Mar 29 2018

Description: Show this description

Comment 2 by hiroh@chromium.org, Mar 29 2018

Summary: video_VideoDecodeAccelerator: need to update thumbnail MD5 due to picture format change (was: video_VideoDecodeAccelerator: need to update thumbnail MD5 due to )
Project Member

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

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

commit 4059c7455517ccf2f980620ad41362d2e9db622e
Author: Kristian H. Kristensen <hoegsberg@chromium.org>
Date: Mon Apr 02 02:02:15 2018

media/test: Update video thumbnail MD5 sums

On ChromeOS, CL:950007 changed the decoder output format to NV12
instead of RGB. This means that the color space conversion now happens
when the test suite textures from the NV12 EGLImage, which accounts
for small differences in the conversion.

BUG= 827032 
TEST=video_VideoDecodeAccelerator

Change-Id: Ic3f22736dae0632756a62e4b9226da30cc561e68
Reviewed-on: https://chromium-review.googlesource.com/989024
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Commit-Queue: Pawel Osciak <posciak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547401}
[modify] https://crrev.com/4059c7455517ccf2f980620ad41362d2e9db622e/media/test/data/test-25fps.h264.md5
[modify] https://crrev.com/4059c7455517ccf2f980620ad41362d2e9db622e/media/test/data/test-25fps.vp8.md5
[modify] https://crrev.com/4059c7455517ccf2f980620ad41362d2e9db622e/media/test/data/test-25fps.vp9.md5

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 5 2018

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

commit 84e7725ab45e955b3c2411419c7b7ca1acc8b67d
Author: Kristian H. Kristensen <hoegsberg@chromium.org>
Date: Thu Apr 05 13:42:22 2018

media/test: Update video thumbnail MD5 sums

CL:989024 updated the MD5 sums for the video_VideoDecodeAccelerator
test, but thanks to the magic of copy and paste, the VP8 sum ended up
being the same as the VP9 sum. Fix that.

BUG= 827032 
TEST=video_VideoDecodeAccelerator

Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>

TBR=dalecurtis@chromium.org,posciak@chromium.org

Change-Id: If55a5371c45ae01103f80f4007df8e7338570142
Reviewed-on: https://chromium-review.googlesource.com/994068
Commit-Queue: Ricky Liang <jcliang@chromium.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548410}
[modify] https://crrev.com/84e7725ab45e955b3c2411419c7b7ca1acc8b67d/media/test/data/test-25fps.vp8.md5

Comment 5 by hiroh@chromium.org, Apr 9 2018

Not all the devices passed in thumbnail test case.

Comment 6 by hiroh@chromium.org, Apr 9 2018

Sorry, perhaps #c5 is false. There is another issue to crash VDA unittest on ToT.
Let's wait and see for some time.
I spot checked many of the errors for 10563.0.0 and they were all crashes. Let wait for https://chromium-review.googlesource.com/1003076 to land in an image and see if there are still mismatched MD5 sums left.

Comment 8 by hiroh@chromium.org, Apr 16 2018

Update: video_VideoDecodeAccelerator passed on some of intel devices thanks to the above MD5 updates.

The failure are
* vp9_2 on KabyLake.
* h264 IvyBridge and BayTrail
What's the status of this? What's left to do?

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

This would be fixed by https://chromium-review.googlesource.com/c/chromium/src/+/1023615.
But I don't get CR+2 yet.
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 25 2018

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

commit 9a093ef21658f421b38b2cdeff90d6fd777f7b60
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Wed Apr 25 03:02:03 2018

media/test/data: Update video thumbnail MD5 sum

On ChromeOS, CL:950007 changed the decoder output format to NV12
instead of RGB. So we need to update MD5.
This updates md5 for h264 on ivybridge and rambi platforms.

TBR=hoegsberg@chromium.org
TBR=mcasas@chromium.org

BUG= chromium:827032 
TEST=video_VideoDecodeAccelerator on link

Change-Id: I3a117ada2038c41077ab116a8c72ba2f4c96df08
Reviewed-on: https://chromium-review.googlesource.com/1023615
Reviewed-by: Kristian H. Kristensen <hoegsberg@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553432}
[modify] https://crrev.com/9a093ef21658f421b38b2cdeff90d6fd777f7b60/media/test/data/test-25fps.h264.md5

Comment 12 by hiroh@chromium.org, Apr 25 2018

Labels: Merge-Request-67
TPM, may I ask you to approve merging #11 to M67?

Thanks
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 25 2018

Labels: merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/91cc06c65d25acf7dbb8ab5f749631afb9e3d8d6

commit 91cc06c65d25acf7dbb8ab5f749631afb9e3d8d6
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Wed Apr 25 19:42:04 2018

media/test/data: Update video thumbnail MD5 sum

On ChromeOS, CL:950007 changed the decoder output format to NV12
instead of RGB. So we need to update MD5.
This updates md5 for h264 on ivybridge and rambi platforms.

TBR=hoegsberg@chromium.org
TBR=mcasas@chromium.org

BUG= chromium:827032 
TEST=video_VideoDecodeAccelerator on link

Change-Id: I3a117ada2038c41077ab116a8c72ba2f4c96df08
Reviewed-on: https://chromium-review.googlesource.com/1023615
Reviewed-by: Kristian H. Kristensen <hoegsberg@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#553432}(cherry picked from commit 9a093ef21658f421b38b2cdeff90d6fd777f7b60)
Reviewed-on: https://chromium-review.googlesource.com/1027212
Cr-Commit-Position: refs/branch-heads/3396@{#302}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/91cc06c65d25acf7dbb8ab5f749631afb9e3d8d6/media/test/data/test-25fps.h264.md5

Project Member

Comment 14 by sheriffbot@chromium.org, Apr 26 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 15 by hiroh@chromium.org, Apr 26 2018

Labels: -Hotlist-Merge-Approved -Merge-Approved-67
Status: Fixed (was: Assigned)
Status: Started (was: Fixed)
I wrongly deleted the md5sum required for intel platform in the CL #11.
Need to reupdate. The CL is https://chromium-review.googlesource.com/c/chromium/src/+/1037044
Project Member

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

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

commit 40443c69032869b89d2e97ce08b56ebc5c2f96d5
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Tue May 01 20:10:52 2018

media/test/data: Update video thumbnail MD5 sum

CL:950007 wrongly deletes md5sum for intel platforms
Reupdate to add the md5sum.

BUG= chromium:827032 
TEST=video_VideoDecodeAccelerator on eve

Change-Id: I2d8ee3b7517f997721741d4b532596c2fbf0641e
Reviewed-on: https://chromium-review.googlesource.com/1037044
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555162}
[modify] https://crrev.com/40443c69032869b89d2e97ce08b56ebc5c2f96d5/media/test/data/test-25fps.h264.md5

Labels: -merge-merged-3396 Merge-Request-67
M68 Looks green on all the devices.
Let us merge to M67.

TPM, could you approve the CL in #c17?
Cherry-picked CL is created and here.
https://chromium-review.googlesource.com/c/chromium/src/+/1043845
Project Member

Comment 19 by sheriffbot@chromium.org, May 4 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: kbleicher@chromium.org
kbleicher@, could you approve https://chromium-review.googlesource.com/c/chromium/src/+/1043845?
Better for someone on the team with context to approve CLs.

For the merge request:  Can you identify the CLs impacted and give background on testing on each since there is more than one?  Thanks
video_VideoDecodeAccelerator generates thumbnails each of which is a snapshot of decoded picture and checks its validity by finiding md5sum is known one.
In M67, some md5sum is missed and then video_VideoDecodeAccelerator fails because the generated md5sum is unknown one.
The CL just adds md5sum to the known list. Will never break the test.

ping
Labels: M67
Is the merge request for https://chromium-review.googlesource.com/c/chromium/src/+/1043845 only?

Assume this was thoroughly tested across more than one board?  Is the scope all intel-based boards?  

Impact if we don't merge?

Comment 26 by hiroh@chromium.org, May 11 2018

>Is the merge request for https://chromium-review.googlesource.com/c/chromium/src/+/1043845 only?

Yes.

> Assume this was thoroughly tested across more than one board?  Is the scope all intel-based boards?
> Impact if we don't merge?

The file will be used on any platform, but the change is to append MD5sum used for intel-based boards.
Unless this is merged, video_VideoDecodeAccelerator in M67 continuously fails on intel-based boards.
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 Chrome OS.

Project Member

Comment 28 by bugdroid1@chromium.org, May 14 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97ab4ed6e6907cf0dc1e8687d2db5a910c6e6d0a

commit 97ab4ed6e6907cf0dc1e8687d2db5a910c6e6d0a
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Mon May 14 02:40:19 2018

media/test/data: Update video thumbnail MD5 sum

CL:1023615 wrongly deletes md5sum for intel platforms
Reupdate to add the md5sum.

BUG= chromium:827032 
TEST=video_VideoDecodeAccelerator on eve

Change-Id: I2d8ee3b7517f997721741d4b532596c2fbf0641e
Reviewed-on: https://chromium-review.googlesource.com/1037044
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555162}(cherry picked from commit 40443c69032869b89d2e97ce08b56ebc5c2f96d5)
Reviewed-on: https://chromium-review.googlesource.com/1043845
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#582}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/97ab4ed6e6907cf0dc1e8687d2db5a910c6e6d0a/media/test/data/test-25fps.h264.md5

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

Labels: -Hotlist-Merge-Review -M67 -merge-merged-3396
Status: Verified (was: Started)

Sign in to add a comment