Bug 77923 – [chromium] Allow retaining texture across frames in composited video playback and correctly handle lost context

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

[chromium] Allow retaining texture across frames in composited video playback and correctly handle lost context

[James Robinson]

Created attachment 125748 [details]
Patch

[James Robinson]

This improves the composited video playback a good bit. It depends on http://trac.webkit.org/changeset/106840, in case we want to merge this.

Ami - we avoid regressing lost context case for other types of content by adding layout tests like LayoutTests/platform/chromium/compositing/lost-compositor-context*. I'd like to do the same for html5 video but am not sure how to make a reliable layout test involving video playback, do you have any hints or suggestions or pointers to similar tests that I could copy? I'd like to make sure we draw at least one frame, lost the compositor context, then draw at least one more frame from the video.

[James Robinson]

Actually, depends on http://trac.webkit.org/changeset/106891 as well since I'm using ManagedTexture::setTextureManager()

[Ami Fischman]

(In reply to comment #2)
> This improves the composited video playback a good bit.

OOC, how do you observe this?

> Ami - we avoid regressing lost context case for other types of content by adding layout tests like LayoutTests/platform/chromium/compositing/lost-compositor-context*.

Oooh, nice.

> I'd like to do the same for html5 video but am not sure how to make a reliable layout test involving video playback, do you have any hints or suggestions or pointers to similar tests that I could copy? I'd like to make sure we draw at least one frame, lost the compositor context, then draw at least one more frame from the video.

http://trac.webkit.org/browser/trunk/LayoutTests/media/ is where most video-related layouttests live.  
I think you could have a reliable test by seeking a video that hasn't been play()'d yet to a known spot, losing the context, and seeking to another spot, asserting the expected currentTime when the latter 'seeked' event fires.

[Ami Fischman]

Also, the patch lgtm, fwiw.  Thanks!

[Vangelis Kokkevis]

Comment on attachment 125748 [details]
Patch

Looks great!

[Kenneth Russell]

Comment on attachment 125748 [details]
Patch

rs=me based on Vangelis's review.

[James Robinson]

Committed r106992: <http://trac.webkit.org/changeset/106992>

[James Robinson]

(In reply to comment #4)
> (In reply to comment #2)
> > This improves the composited video playback a good bit.
> 
> OOC, how do you observe this?

I should clarify - it improves the behavior I see in tracing (there aren't any new textures created) but I haven't measured any framerate improvement or anything like that.

> 
> > Ami - we avoid regressing lost context case for other types of content by adding layout tests like LayoutTests/platform/chromium/compositing/lost-compositor-context*.
> 
> Oooh, nice.
> 
> > I'd like to do the same for html5 video but am not sure how to make a reliable layout test involving video playback, do you have any hints or suggestions or pointers to similar tests that I could copy? I'd like to make sure we draw at least one frame, lost the compositor context, then draw at least one more frame from the video.
> 
> http://trac.webkit.org/browser/trunk/LayoutTests/media/ is where most video-related layouttests live.  
> I think you could have a reliable test by seeking a video that hasn't been play()'d yet to a known spot, losing the context, and seeking to another spot, asserting the expected currentTime when the latter 'seeked' event fires.

Added a layout test here: https://bugs.webkit.org/show_bug.cgi?id=78060 that I believe would have caught this regression