I believe the root cause is as follows (see also comment #23 in issue 696130):

VideoCaptureDeviceClient spins up the hardware jpeg decoder asynchronously. While waiting for the decoder to be created and initialized, it performs decoding of incoming frame in software (using libyuv). If the asynchronous initialization takes longer than the duration of the video (which seems to have only 25 frames), the test run will finish before the initialization callback is made, and as a result no UMA histogram entry is created.

I have seen this same thing happen in a integration test I have worked on recently (not yet landed). In order to make the test deterministic, we would have to keep decoding video frames (maybe in a loop) until the initialization callback has arrived (or abort with a timeout). We can also have VideoCaptureDeviceClient send an event when the decoder has finished initialization. This is what I did for the integration test. However, we may not want to surface this event all the way up to JavaScript, so maybe a loop playback plus timeout would be sufficient for this test.

From the code, it seems file_video_capture_device.cc is already looping the file. No?

https://cs.chromium.org/chromium/src/media/capture/video/file_video_capture_device.cc?type=cs&l=202

https://cs.chromium.org/chromium/src/media/capture/video/file_video_capture_device.cc?type=cs&l=262

Daniel. Can you take a look?


This test is still flaky from wmatrix.

Not checked yet.

Any update?

Not checked yet.

Some experiment results:
1. elm-release/R58-9334.11.0: ~15% fail rate
2. Self-built Chrome with latest code on top of elm-release/R58-9334.11.0: 100% pass (110 times)
3. elm-release/R59-9395.0.0: ~15% fail rate
2. Self-built Chrome with latest code on top of elm-release/R59-9395.0.0: 100% pass (110 times)

Daniel. Please check about comment #1 first. I remember we are looping the fake file so it's not only 25 frames. I could be wrong. 

The failure happens when the jpeg_decode_accelerator_supported value is found to be false at this line.
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.cc?q=VideoCaptureGpuJpegDecoder::fini+package:%5Echromium$&l=240

It seems to be caused by that GpuProcessHost::OnInitialized() and GpuProcessHost::OnChannelEstablished() are executed out of order; OnChannelEstablished() sends out "gpu_info_" with default values before OnInitialized() has the chance to update it.
https://chromium.googlesource.com/chromium/src/+/6aafa7de713ba655b50e35b9b0d1a2d9f5078333/content/browser/gpu/gpu_process_host.cc#809
https://chromium.googlesource.com/chromium/src/+/6aafa7de713ba655b50e35b9b0d1a2d9f5078333/content/browser/gpu/gpu_process_host.cc#839

This is traced on version R59-9395.0.0. There are many changes to the corresponding code in the past week. I think we need GPU team check whether the "gpu_info_" is handled properly in the new version.

Deleted: ui.20170328-195559.FAIL 12.4 KB

ui.20170328-195559.FAIL 12.4 KB Download

Deleted: ui.20170328-202137.PASS 10.5 KB

ui.20170328-202137.PASS 10.5 KB Download

It looks like the regression started since around m58 9290.0.0-9294.0.0. Before 9290.0.0, the test was all green. M57 was all green.

Chromium CLs between 9290 and 9294.
https://chromium.googlesource.com/chromium/src/+log/58.0.3011.0..58.0.3015.0?pretty=fuller&n=10000

Not sure if this is related. https://codereview.chromium.org/2696473002

+sadrul

Hi Sadrul. We are debugging an issue. JPEG hardware decoder is used to decode the camera frames. Browser process first creates a gpu channel. Then it checks GpuInfo and see if HW decoder is supported. GpuInfo is passed to browser by the initialization callback -- GpuProcessHost::DidInitialize. The problem is sometimes when gpu channel established callback (GpuProcessHost::OnChannelEstablished()) is called, gpu initialization callback is not called yet. So we get an empty GpuInfo. This is strange because I thought initialization callback would arrive before channel established callback. This issue might be related to this patch --
 https://codereview.chromium.org/2696473002, which changed the timing between initialization and creating a channel. Do you think it's related? Any suggestion to debug this further?


https://codereview.chromium.org/2696473002:
gpu: Use mojom API for creating gpu channel.

chrome-gpu and chrome-browser already talk over the mojom API for some
things (e.g. initialization). Update these to use the mojom API for
creating a gpu channel too.

Can you try on ToT? I made some changes earlier this week that may affect this (in a positive way).

I'm running tests on ToT. It has passed 70 rounds of tests and looks promising. I'll run it overnight.
Can you indicate to us which changes are related to this issue?

https://codereview.chromium.org/2777973002 is the CL that made more changes for initialization. So that's probably the one affecting this.

It fails at 199th round.

Thanks. It looks like ToT is much butter. We'll know the autotest result after Chrome uprev in ChromeOS. It failed in 199th round so Daniel will add the logs and reproduce again.

Sadrul. Can you explain how it's guaranteed GpuProcessHost::DidInitialize is called before GpuProcessHost::OnChannelEstablished?

video_ChromeRTCHWDecodeUsed is mostly green after chrome uprevs. So https://codereview.chromium.org/2777973002 should have fixed it. There are two other failures though. One is probably http://crbug.com/702930.

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

commit e9e10eb142375a55ba6f58d4a60ebad99b07e48d
Author: sadrul <sadrul@chromium.org>
Date: Tue Mar 28 01:06:03 2017

gpu: Replace GpuHostMsg_Initialized with mojom api.

Remove GpuHostMsg_Initialized, and instead hook up the corresponding
DidInitialize() and DidFailInitialize() mojom api.

BUG= 643746 

Review-Url: https://codereview.chromium.org/2777973002
Cr-Commit-Position: refs/heads/master@{#459961}

[modify] https://crrev.com/e9e10eb142375a55ba6f58d4a60ebad99b07e48d/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/e9e10eb142375a55ba6f58d4a60ebad99b07e48d/content/browser/gpu/gpu_process_host.h
[modify] https://crrev.com/e9e10eb142375a55ba6f58d4a60ebad99b07e48d/content/common/gpu_host_messages.h
[modify] https://crrev.com/e9e10eb142375a55ba6f58d4a60ebad99b07e48d/content/gpu/gpu_child_thread.cc

Please add applicable OSs.  Thanks!

The root cause is probably https://chromium.googlesource.com/chromium/src.git/+/550e264fded00563fd094dfdf527b5073924432b , maybe we can revert that one on the branch instead?

Consider the merge approved whichever route you feel is best (new fix or revert). 

More information about Antoine's commnet: before https://codereview.chromium.org/2696473002, IPCs were properly ordered, but after that patch, one IPC goes over chrome IPC, the other one over mojo, so order is no longer guaranteed. https://codereview.chromium.org/2777973002
makes the other IPC go over mojo too, so order is restored. But it probably need more than just this one patch, and reversion may be more desirable.

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

sadrul@, please review https://codereview.chromium.org/2798363003/.

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

commit d93c1602a934af474125c49432617d4768cb45c6
Author: hywu <hywu@google.com>
Date: Fri Apr 14 13:35:22 2017

Revert "gpu: Use mojom API for creating gpu channel."

This reverts commit 550e264fded00563fd094dfdf527b5073924432b.
The commit changes the creating gpu channel IPC to go over mojo while
initialization still over Chrome IPC, and thus the IPCs may be out of
order. This results in flaky video_ChromeRTCHWDecodeUsed test.

BUG= 697660 
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2798363003
Cr-Commit-Position: refs/branch-heads/3029@{#713}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/d93c1602a934af474125c49432617d4768cb45c6/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/d93c1602a934af474125c49432617d4768cb45c6/content/browser/gpu/gpu_process_host.h
[modify] https://crrev.com/d93c1602a934af474125c49432617d4768cb45c6/content/common/gpu_host_messages.h
[modify] https://crrev.com/d93c1602a934af474125c49432617d4768cb45c6/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/d93c1602a934af474125c49432617d4768cb45c6/content/gpu/gpu_child_thread.h
[modify] https://crrev.com/d93c1602a934af474125c49432617d4768cb45c6/services/ui/gpu/interfaces/gpu_service.mojom

on R58, video_ChromeRTCHWDecodeUsed becomes mostly green after R58-9334.48.0.

commit 70570221a22b979931a3efac719d18e2e25fba30
Author: Hung-yu Wu <hywu@chromium.org>
Date: Tue Apr 18 05:50:22 2017

Revert "Move the flaky video_ChromeRTCHWDecodeUsed from bvt-cq to bvt-perbuild"

This reverts commit acb63b0af8c42a7df766b87aed40b1378bf084ce.

Reason for revert: wmatrix has video_ChromeRTCHWDecodeUsed almost all green after Chrome uprevs. https://codereview.chromium.org/2777973002 should have fixed it.

Original change's description:
> Move the flaky video_ChromeRTCHWDecodeUsed from bvt-cq to bvt-perbuild
>
> This test has been very flaky lately causing multiple CQ, informational, and PFQ
> builders failures lately.
>
> BUG= chromium:696727 
> TEST=none
>
> Change-Id: I09ae6d02ba598d26ab630eb8ef4acec5912a861d
> Reviewed-on: https://chromium-review.googlesource.com/447908
> Reviewed-by: Aviv Keshet <akeshet@chromium.org>
> Reviewed-by: Ilja H. Friedel <ihf@chromium.org>
> Tested-by: Ahmed Fakhry <afakhry@chromium.org>
>

TBR=kinaba@chromium.org,ihf@chromium.org,posciak@chromium.org,wuchengli@chromium.org,akeshet@chromium.org,afakhry@google.com,afakhry@chromium.org
BUG= chromium:696727 

Change-Id: I36e9f577509842bc42d73aae704be42efa67a523
Reviewed-on: https://chromium-review.googlesource.com/468486
Commit-Ready: Hung-yu Wu <hywu@chromium.org>
Tested-by: Hung-yu Wu <hywu@chromium.org>
Reviewed-by: Wu-cheng Li <wuchengli@chromium.org>

[modify] https://crrev.com/70570221a22b979931a3efac719d18e2e25fba30/client/site_tests/video_ChromeRTCHWDecodeUsed/control.y4m
[modify] https://crrev.com/70570221a22b979931a3efac719d18e2e25fba30/client/site_tests/video_ChromeRTCHWDecodeUsed/control.mjpeg