Attaching test case.

Deleted: video_stutter_testcase.zip 4.6 MB

video_stutter_testcase.zip 4.6 MB Download

Some more information:

This issue doesn't happen when copying the frames to canvas or webgl every frame (using requestAnimationFrame).

It happens when using setInterval()/setTimeout() to match the video frame rate and avoid unneeded copies/uploads.

This might serve as a better test case for  Issue 807974 .

I would block  Issue 807974  on this one, but this is P2 and I'm not sure that it's warranted to raise it. We can duplicate it into the other issue if it turns out it's the same problem.

Thanks for filing the issue!

Able to reproduce the issue on reported chrome version 65.0.3325.146 and on the latest canary 67.0.3364.0 using Mac 10.13.1.
In the process of triaging the issue further, we checked one of M60(60.0.3072.0) builds, there the behaviour is different i.e., smooth play of animation isn't seen rather video is stuck several times while playing. Attaching the screen cast of the same.

@Reporter: Could you please check the screen cast from Chrome version 60.0.3072.0 attached here and let us know whether to consider this behaviour as good or bad?

Deleted: 819914.mp4 3.3 MB

	819914.mp4 3.3 MB View Download

I can't judge out of that screen recording as the frame rate is too low (~2fps?). Are you aware that Quicktime has a screen recorder built in? That's what I used for my recordings.

The ideal behavior would be the video playing at ~24fps without displaying previous frames randomly.

You can also use Firefox as reference. At least in my system it behaves correctly.

Thank you for providing more feedback. Adding the requester to the cc list.

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

I'll take a look.  Looks like the issue could be caused by defective synchronization between GPU command streams.

Removing blocker because  issue 807974  is fixed in 65.0.3325.146

M65 is already in stable, is it a M65 stable blocker for further roll out?

I think this issue is only on ToT and due to a long standing issue with the gpu memory buffer frame pool returning frames out of order. In the media-internals log you can see it printing that frames are decoded out of order, but that doesn't happen with GMB disabled.

Actually just saw this reproduces on M65, so it can't be my change then. The reproduction on canary can though.

More information:

This seems to only happen with webm. I can't reproduce when using mp4.

I've updated the testcase including a mp4 version of the video.

Deleted: video_stutter_testcase2.zip 5.1 MB

video_stutter_testcase2.zip 5.1 MB Download

mp4 is hardware decoded so won't hit the GMB pool, so that's more evidence this is due to my recent change 2068cef4dc94410597183df1ddc9b66eb08ef3f1, which inadvertently triggered a long standing concern with the pool where inflight copies may return out of order.

If this doesn't repro with --disable-gpu-memory-buffer-video-frames then the M65 part can be attributed to what junov@ fixed, and the M67+ issues attributed to my change (which dcastagna@ is fixing)

I've just tried with today's Canary (67.0.3365.0) and seems like the issue is gone 🎉

w/ the flag? Otherwise I wouldn't expect it to be fixed.

With and without the flag...

Hmm, I think you just got lucky; I can repro on ToT w/ it :) Check chrome://media-internals for "decoded frame out of order" warnings.

Strange... I can't test ToT, but Canary doesn't show any warnings.

Deleted: Screen Shot 2018-03-08 at 4.16.17 PM.png 73.8 KB

	Screen Shot 2018-03-08 at 4.16.17 PM.png 73.8 KB View Download

I've isolated the area of the code that has the bug to:
PaintCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture.

How I've come to this conclusion: in WebGLRenderingContextBase::TexImageHelperHTMLVideoElement, rendering works correctly if you comment out this block of code:

    if (video->CopyVideoTextureToPlatformTexture(
            ContextGL(), target, texture->Object(), internalformat, format,
            type, level, unpack_premultiply_alpha_, unpack_flip_y_,
            already_uploaded_id, frame_metadata_ptr)) {
      texture->UpdateLastUploadedFrame(frame_metadata);
      return;
    }

Without that code, the video to texture copy falls back to a presumably less optimal, but non-buggy code path.

Tracing inside that call led me to PaintCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture

I'm unfamiliar with this area: assigning to mcasas@

Thanks Justin for narrowing this down.

sunnyps@: does the cross-context synchronization look correct in that method?

https://cs.chromium.org/chromium/src/media/renderers/paint_canvas_video_renderer.cc?l=820

We need to continue using the CopyTextureCHROMIUM code path for efficiency.

Any updates on this? It still repros for me on latest stable.

I thought I saw some fixes around this; do you see it on canary?

Yup, still repros for me on canary. I saw it on https://rawgit.com/mrdoob/three.js/dev/examples/webvr_video.html and in the testcase I outlined in https://bugs.chromium.org/p/chromium/issues/detail?id=814120

Hmm, I'm not able to reproduce out of order frames on Linux ToT. Are you seeing out of order frames or something else? On Mac?

I believe the issue is specific to Mac, the issue doesn't repro for me on Windows.

#23: The synchronization looks correct there.

Hmm don't have a mac today. I did find one odd thing with the uploader, but I don't think it was an error hit with this media, or at least it's not hit on linux and isn't something that should be platform specific...

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

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

commit 0a7793f3c3d53c48a3faee8af9970e3fccdedcec
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Fri Apr 27 01:38:22 2018

Don't rely on timestamps for frame equiavlency.

We added unique_id() a long time ago for checking if frames are the
same instead of relying on timestamps.

Though the linked issue doesn't seem to have problems with duplicate
timestamps, this should still be fixed and could be a source of error.

BUG= 819914 
TEST=none

Change-Id: Ia18ae947a09e48772973f94d89e03d7fa7f41e4d
Reviewed-on: https://chromium-review.googlesource.com/1031523
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554256}
[modify] https://crrev.com/0a7793f3c3d53c48a3faee8af9970e3fccdedcec/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/0a7793f3c3d53c48a3faee8af9970e3fccdedcec/media/renderers/paint_canvas_video_renderer.h

Tried Verifying the issue on chrome version 68.0.3415.0 using Mac 10.13.1 with the below mentioned steps.
1. Launched Chrome
2. Navigated to https://rawgit.com/mrdoob/three.js/dev/examples/webvr_video.html 
We observed the video is still glitchy. Attaching the screen cast for reference.

@Dale Curtis: Could you please have a look at the screen cast and let us know if anything missed from our end. It would be highly helpful if mentioned whether there are any fixes to be landed still. 

Thanks!

Deleted: 819914.mp4 4.7 MB

	819914.mp4 4.7 MB View Download

Yeah, my CL fixed potential cases of this, but I don't think any of the existing test cases actually triggered the path I fixed, so the overall issue is still expected to occur. It seems to be something mac specific. 

Sorry to ping you guys again, do you have an idea on when you could get to this? This is affecting all 360 videos on facebook. Let me know if there is anything I can do to help. 

Can I please ask that some investigation of this issue be done? Raising to P1 – this is a high priority embarrassing bug.

=>ccameron since it seems to be mac specific and not related to hardware decode.

Couple of remarks:
1. This is not related to CoreAnimation compositing (this demo doesn't hit that path, and still repros when the path is forcibly disabled).
2. This is related to GpuMemoryBuffer video frames. This issue goes away when --disable-gpu-memory-buffer-video-frames is specified.

=>dcastagna

I'm checking with Canary 68.0.3437.2 and a Chromium dev build r561075
and the link provided [1] and I cannot repro any glitchy playback.

[1] https://rawgit.com/mrdoob/three.js/dev/examples/webvr_video.html 

Can this be a dup of  crbug.com/824389 ?

I tried on latest canary today (68.0.3439.0), and it still repros for me.(https://rawgit.com/mrdoob/three.js/dev/examples/webvr_video.html)

Hello, 
Any update on this issue? I am getting quite a lot of stuttering on latest canary too - and a lot of premium high quality 360 videos on facebook have been affected.
No such stutter for the same video on firefox (Attached as well).
Thank you for looking into this!

Deleted: chrome_canary_stutter_480p.mov 2.0 MB

	chrome_canary_stutter_480p.mov 2.0 MB View Download

Deleted: firefox_480p.mov 1.7 MB

	firefox_480p.mov 1.7 MB View Download

I cannot reproduce using Chrome Canary 69.0.3463.0 on MBP 2016 either. joohyun.lee@gmail.com, which device are you working on?

I could not repro either on Canary 69.0.3464.0 on my MBP running 10.13.4
not on a Chromium developer build r568035.

Hello,
I've seen this repro on a the macbook pro line (2016 and 2017) as well as the latest max'ed out mac pro trashcan. Again, as you can see in the video above, the same video on Firefox plays as it should.
What samples are you using to try and repro this?

I used https://rawgit.com/mrdoob/three.js/dev/examples/webvr_video.html given in the first post.

I've attached a video of my repro using https://rawgit.com/mrdoob/three.js/dev/examples/webvr_video.html on Chrome Canary 69.0.3464.0, MBP 2017 running High Sierra 10.13.4 

I think that this bug only repros when the GPU is sufficiently loaded. To test with high GPU load, I used Maxon's Cinebench software, and ran the OpenGL benchmark test, but in theory any method of overloading the GPU should give similar results. Ex. I can also repro if I view the three.js example while playing a 4k 60fps video at the same time.

Deleted: stutter_repro_720.mov 481 KB

	stutter_repro_720.mov 481 KB View Download

I did a bisect using bisect-builds. It looks like https://chromium.googlesource.com/chromium/src/+/60c45746804d93ccfdaead7ae92b2acd46391698 fixed the issue for me and I cannot repro beyond that CL. dalecurtis@ can you PTAL?

I can still reproduce on a MacBook Pro (Retina, 15-inch, Late 2013) with NVIDIA GPU running 10.13.4 (17E199), and Chrome 69.0.3451.0 (Official Build) canary (64-bit) with the following command line arguments:

--flag-switches-begin --enable-experimental-web-platform-features --memlog=manual --enable-features=OmniboxUIExperimentShowSuggestionFavicons,OmniboxUIExperimentVerticalMargin,WebAssembly --flag-switches-end

running:

https://rawgit.com/mrdoob/three.js/dev/examples/webvr_video.html 

with many (~30) other open tabs in the same window, though I don't think the GPU is highly loaded.

Sorry just got back from OOO and don't have bandwidth for this right now. That CL just changes when we upload to GMBs, not anything about the GMBs themselves.

Thanks dalecurtis@. Assigning back to dcastagna@ for re-triage then.

 Issue 814120  has been merged into this issue.

Issue 864825 has been merged into this issue.

Summary so far: this issue is triggered when the system is under heavy load, such as when decoding 1440p60 VP9 content (YouTube 360 video). I affects only textures coped from video elements to canvas, and only when the <video> element is not visible.

The root cause seems to be that GpuMemoryBufferVideoFramePool does not use a read fence to ensure that GPU operations have completed before the buffers are returned to the pool. Later frames can then be written over buffers that are still being used in the canvas context.

The issue surfaced when the buffer selection mechanism was changed from FIFO to allocation order. I will attempt to create a mitigation CL ASAP, and a correct fix will follow.

Rassigning to sandersd that is currently working on it.

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

commit 0fe0f82099b9b13567dcab374f95b39bde14f1e0
Author: Dan Sanders <sandersd@chromium.org>
Date: Wed Aug 08 19:42:08 2018

[media] Wait for release SyncToken in GpuMemoryBufferVideoFramePool

While this does not fix the complete set of synchronization issues, it does result in smooth playback on my Linux machine.

Bug:  819914 
Change-Id: I66f78e2d22d276c8dc2c67cb96626aab42ce0efe
Reviewed-on: https://chromium-review.googlesource.com/1162928
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581664}
[modify] https://crrev.com/0fe0f82099b9b13567dcab374f95b39bde14f1e0/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
[modify] https://crrev.com/0fe0f82099b9b13567dcab374f95b39bde14f1e0/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.h
[modify] https://crrev.com/0fe0f82099b9b13567dcab374f95b39bde14f1e0/media/video/gpu_memory_buffer_video_frame_pool.cc
[modify] https://crrev.com/0fe0f82099b9b13567dcab374f95b39bde14f1e0/media/video/gpu_memory_buffer_video_frame_pool_unittest.cc
[modify] https://crrev.com/0fe0f82099b9b13567dcab374f95b39bde14f1e0/media/video/gpu_video_accelerator_factories.h
[modify] https://crrev.com/0fe0f82099b9b13567dcab374f95b39bde14f1e0/media/video/mock_gpu_video_accelerator_factories.h

Requesting merge to M69. The fix is not complete but it does improve visual quality significantly.

This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

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

This is regressed since M65. Pls target fix for 70 as it is too late to take this  merge in for M69 which is going to stable soon.

The fix above is incorrect.  Waiting for the sync token only means that the commands have executed in the GPU process.  It doesn't mean that the GPU has finished execution.  One of my CLs which touches sync token code changed the timing just enough for WebglConformance_conformance2_textures_video_tex_2d_* tests to fail.

The correct fix would be to use a GL_COMMANDS_COMPLETED query, and check that before reusing the GMB.  There's a SignalQuery that's similar to SignalSyncToken.

CL: https://chromium-review.googlesource.com/c/chromium/src/+/1168155
Note: Latest revision has a glFinish and some CHECKs enabled.
Test failures (without glFinish): https://ci.chromium.org/p/chromium/builders/luci.chromium.try/mac_optional_gpu_tests_rel/9340 

Yes per the CL description, I think it was known that it was insufficient to  fix the problem -- just enough to paper over it for the time being.

dcastagna has a followup fix here https://chromium-review.googlesource.com/c/chromium/src/+/1170249 but he's OOO.

Who can pick up this work while dcastagna@ is OOO? We shouldn't leave things in a half-broken state for a long time.

Dan, can you steward this through if possible? Otherwise Daniele should be back sooner than his Monorail status suggests.

I'll try to get this done this week, but failing that I believe that dcastagna@ is back next week.

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

commit 9803c0eb2761892ad1e6826f80a1b3597d6cea67
Author: Dan Sanders <sandersd@chromium.org>
Date: Thu Sep 27 22:42:37 2018

[media] Fix video to canvas GPU-CPU race.

VideoFrames marked with VideoFrameMetadata::READ_LOCK_FENCES_ENABLED
must be retained until all reads have been completed. Such frames are
produced when software frames are uploaded to GpuMemoryBuffers. The
VideoResourceUpdater handles this, but PaintCanvasVideoRenderer
implemented only sync tokens.

As a result, WebGL reads of software-decoded frames can tear when the
storage is reused for later frames.

This CL adds a GL_COMMANDS_COMPLETED_CHROMIUM query after each potential
VideoFrame read in PaintCanvasVideoRenderer, and retains a reference to
the VideoFrame until it completes. This ensures that the memory is not
reused until after the reads complete in the GPU process.

Bug:  819914 
Change-Id: I3e6e5f83cff86beedbd76a406aa31c4eb84a0929
Reviewed-on: https://chromium-review.googlesource.com/1244835
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594900}
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/chrome/browser/android/download/video_frame_thumbnail_converter.cc
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/chrome/browser/devtools/devtools_eye_dropper.h
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/content/browser/devtools/devtools_video_consumer.cc
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/content/renderer/media/stream/webmediaplayer_ms.cc
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/content/renderer/media/stream/webmediaplayer_ms_compositor.cc
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/media/renderers/paint_canvas_video_renderer.h
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/media/renderers/paint_canvas_video_renderer_unittest.cc
[modify] https://crrev.com/9803c0eb2761892ad1e6826f80a1b3597d6cea67/media/renderers/video_resource_updater.cc

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

commit 78edec2847cc58e74a27bf67f09dae2da176a059
Author: Dan Sanders <sandersd@chromium.org>
Date: Fri Sep 28 01:00:47 2018

[media] Fix video frame copy when context is lost.

If a GPU channel cannot be established, a WebMediaPlayerImpl can be
constructed without a |context_provider_|. Handle that case correctly
in CopyVideoTextureToPlatformTexture().

Bug:  819914 
Change-Id: I620b602ee0c092cc31273b4f513133bf8af7e1d0
Reviewed-on: https://chromium-review.googlesource.com/1250137
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Fredrik Hubinette <hubbe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594939}
[modify] https://crrev.com/78edec2847cc58e74a27bf67f09dae2da176a059/media/blink/webmediaplayer_impl.cc

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

commit b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3
Author: Jamie Madill <jmadill@chromium.org>
Date: Fri Sep 28 11:25:41 2018

Revert "[media] Fix video to canvas GPU-CPU race."

This reverts commit 9803c0eb2761892ad1e6826f80a1b3597d6cea67.

Reason for revert: WebGL 2 video test failures on Win/Mac.
Blocking the ANGLE CQ.

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20FYI%20Release%20%28NVIDIA%29/2542

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20FYI%20Release%20%28Intel%29/6848

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20FYI%20Release%20%28NVIDIA%29/2571

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20FYI%20Release%20%28AMD%29/2795

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20FYI%20Retina%20Release%20%28AMD%29/8042

Also reverts "[media] Fix video frame copy when context is lost."

Bug:  890281 

Original change's description:
> [media] Fix video to canvas GPU-CPU race.
>
> VideoFrames marked with VideoFrameMetadata::READ_LOCK_FENCES_ENABLED
> must be retained until all reads have been completed. Such frames are
> produced when software frames are uploaded to GpuMemoryBuffers. The
> VideoResourceUpdater handles this, but PaintCanvasVideoRenderer
> implemented only sync tokens.
>
> As a result, WebGL reads of software-decoded frames can tear when the
> storage is reused for later frames.
>
> This CL adds a GL_COMMANDS_COMPLETED_CHROMIUM query after each potential
> VideoFrame read in PaintCanvasVideoRenderer, and retains a reference to
> the VideoFrame until it completes. This ensures that the memory is not
> reused until after the reads complete in the GPU process.
>
> Bug:  819914 
> Change-Id: I3e6e5f83cff86beedbd76a406aa31c4eb84a0929
> Reviewed-on: https://chromium-review.googlesource.com/1244835
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Commit-Queue: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#594900}

TBR=pfeldman@chromium.org,tedchoc@chromium.org,sandersd@chromium.org,dcastagna@chromium.org,sunnyps@chromium.org,piman@chromium.org

Change-Id: Ib21a4879a9ca6fc4b84f6f2c2f1171c794db54b7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  819914 
Reviewed-on: https://chromium-review.googlesource.com/1251321
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595058}
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/chrome/browser/android/download/video_frame_thumbnail_converter.cc
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/chrome/browser/devtools/devtools_eye_dropper.h
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/content/browser/devtools/devtools_video_consumer.cc
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/content/renderer/media/stream/webmediaplayer_ms.cc
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/content/renderer/media/stream/webmediaplayer_ms_compositor.cc
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/media/renderers/paint_canvas_video_renderer.h
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/media/renderers/paint_canvas_video_renderer_unittest.cc
[modify] https://crrev.com/b39e1696cad4b7b661a91fe12e4fcc0aef45a9d3/media/renderers/video_resource_updater.cc

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

commit 1c700bea035df3f3fa803758624540e88e61b83b
Author: Dan Sanders <sandersd@chromium.org>
Date: Tue Oct 02 01:09:57 2018

[media] Reland: Fix video to canvas GPU-CPU race.

VideoFrames marked with VideoFrameMetadata::READ_LOCK_FENCES_ENABLED
must be retained until all reads have been completed. Such frames are
produced when software frames are uploaded to GpuMemoryBuffers. The
VideoResourceUpdater handles this, but PaintCanvasVideoRenderer
implemented only sync tokens.

As a result, WebGL reads of software-decoded frames can tear when the
storage is reused for later frames.

This CL adds a GL_COMMANDS_COMPLETED_CHROMIUM query after each potential
VideoFrame read in PaintCanvasVideoRenderer, and retains a reference to
the VideoFrame until it completes. This ensures that the memory is not
reused until after the reads complete in the GPU process.

TBR=pfeldman@chromium.org

Bug:  819914 
Change-Id: Id181a476b78e2799bdbff149c9655427ae552743
Reviewed-on: https://chromium-review.googlesource.com/1252962
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595676}
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/chrome/browser/android/download/download_media_parser.cc
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/chrome/browser/devtools/devtools_eye_dropper.h
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/content/browser/devtools/devtools_video_consumer.cc
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/content/renderer/media/stream/webmediaplayer_ms.cc
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/content/renderer/media/stream/webmediaplayer_ms_compositor.cc
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/media/renderers/paint_canvas_video_renderer.h
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/media/renderers/paint_canvas_video_renderer_unittest.cc
[modify] https://crrev.com/1c700bea035df3f3fa803758624540e88e61b83b/media/renderers/video_resource_updater.cc

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

commit 3423a87985eb81c61bd67195956e7e1d93607b02
Author: Corentin Wallez <cwallez@chromium.org>
Date: Tue Oct 02 13:01:06 2018

Revert "[media] Reland: Fix video to canvas GPU-CPU race."

This reverts commit 1c700bea035df3f3fa803758624540e88e61b83b.

Reason for revert: Causes flakiness on the WebGL2 video tests on Mac. See crbug.com/891249

Original change's description:
> [media] Reland: Fix video to canvas GPU-CPU race.
> 
> VideoFrames marked with VideoFrameMetadata::READ_LOCK_FENCES_ENABLED
> must be retained until all reads have been completed. Such frames are
> produced when software frames are uploaded to GpuMemoryBuffers. The
> VideoResourceUpdater handles this, but PaintCanvasVideoRenderer
> implemented only sync tokens.
> 
> As a result, WebGL reads of software-decoded frames can tear when the
> storage is reused for later frames.
> 
> This CL adds a GL_COMMANDS_COMPLETED_CHROMIUM query after each potential
> VideoFrame read in PaintCanvasVideoRenderer, and retains a reference to
> the VideoFrame until it completes. This ensures that the memory is not
> reused until after the reads complete in the GPU process.
> 
> TBR=pfeldman@chromium.org
> 
> Bug:  819914 
> Change-Id: Id181a476b78e2799bdbff149c9655427ae552743
> Reviewed-on: https://chromium-review.googlesource.com/1252962
> Commit-Queue: Dan Sanders <sandersd@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#595676}

TBR=pfeldman@chromium.org,tedchoc@chromium.org,sandersd@chromium.org,sunnyps@chromium.org,piman@chromium.org

Change-Id: I5a1c66dc55ef5d4a5980704ef97dadea7ab41051
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  819914 
Reviewed-on: https://chromium-review.googlesource.com/1256372
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595808}
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/chrome/browser/android/download/download_media_parser.cc
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/chrome/browser/devtools/devtools_eye_dropper.h
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/content/browser/devtools/devtools_video_consumer.cc
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/content/renderer/media/stream/webmediaplayer_ms.cc
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/content/renderer/media/stream/webmediaplayer_ms_compositor.cc
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/media/renderers/paint_canvas_video_renderer.h
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/media/renderers/paint_canvas_video_renderer_unittest.cc
[modify] https://crrev.com/3423a87985eb81c61bd67195956e7e1d93607b02/media/renderers/video_resource_updater.cc

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

commit 930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8
Author: Dan Sanders <sandersd@chromium.org>
Date: Wed Oct 03 00:45:08 2018

[media] Reland: Fix video to canvas GPU-CPU race.

VideoFrames marked with VideoFrameMetadata::READ_LOCK_FENCES_ENABLED
must be retained until all reads have been completed. Such frames are
produced when software frames are uploaded to GpuMemoryBuffers. The
VideoResourceUpdater handles this, but PaintCanvasVideoRenderer
implemented only sync tokens.

As a result, WebGL reads of software-decoded frames can tear when the
storage is reused for later frames.

This CL adds a GL_COMMANDS_COMPLETED_CHROMIUM query after each potential
VideoFrame read in PaintCanvasVideoRenderer, and retains a reference to
the VideoFrame until it completes. This ensures that the memory is not
reused until after the reads complete in the GPU process.

TBR=tedchoc@chromium.org,pfeldman@chromium.org

Bug:  819914 
Cq-Include-Trybots: luci.chromium.try:mac_optional_gpu_tests_rel
Change-Id: Iec05d3bca69e68ae3d3b231db84caa9bed24d777
Reviewed-on: https://chromium-review.googlesource.com/c/1258212
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596072}
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/chrome/browser/android/download/download_media_parser.cc
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/chrome/browser/devtools/devtools_eye_dropper.h
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/content/browser/devtools/devtools_video_consumer.cc
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/content/renderer/media/stream/webmediaplayer_ms.cc
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/content/renderer/media/stream/webmediaplayer_ms_compositor.cc
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/media/renderers/paint_canvas_video_renderer.cc
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/media/renderers/paint_canvas_video_renderer.h
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/media/renderers/paint_canvas_video_renderer_unittest.cc
[modify] https://crrev.com/930cc1d5f861d51d4544f5ea60e73c33d8f2cdc8/media/renderers/video_resource_updater.cc