Bug 84858 – [GTK] media/video-seek-past-end-playing.html times out

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Philippe Normand]

Seems to happen only in 32-bit Release:

--- /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/media/video-seek-past-end-playing-expected.txt 
+++ /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/media/video-seek-past-end-playing-actual.txt 
@@ -1,3 +1,4 @@
+FAIL: Timed out waiting for notifyDone to be called
 Test that seeking video with 'loop' past it's end rewinds to the beginning and continues playback.
 
 EVENT(canplaythrough)
@@ -9,8 +10,3 @@
 EXPECTED (mediaElement.currentTime > '0') OK
 RUN(video.currentTime = 500)
 
-EXPECTED (video.paused == 'false') OK
-EXPECTED (mediaElement.currentTime > '0') OK
-
-END OF TEST
-

[Philippe Normand]

And flakily

[Dominik Röttsches (drott)]

It seems that the GStreamer backend is not correctly implemeting the http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#seeking behaviour, especially step 5. 
"If the new playback position is later than the end of the media resource, then let it be the end of the media resource instead."

Setting a new video.currentPosition that's larger than the length of the video actually makes the playback stop. No more timeupdate events are fired after such a seek position was set.

[Philippe Normand]

(In reply to comment #2)
> It seems that the GStreamer backend is not correctly implemeting the http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#seeking behaviour, especially step 5. 
> "If the new playback position is later than the end of the media resource, then let it be the end of the media resource instead."
> 

This step is handled in HTMLMediaElement::seek().

> Setting a new video.currentPosition that's larger than the length of the video actually makes the playback stop. No more timeupdate events are fired after such a seek position was set.

Could it be an issue with the duration() reporting?

[Dominik Röttsches (drott)]

(In reply to comment #3)
> (In reply to comment #2)
> > [...seeking behavior...]
> This step is handled in HTMLMediaElement::seek().

Yes, true, after looking a bit closer, I found that out.

> > Setting a new video.currentPosition that's larger than the length of the video actually makes the playback stop. No more timeupdate events are fired after such a seek position was set.

> Could it be an issue with the duration() reporting?

I actually have a patch - it seems that didEnd() does not need to explicitly pause the stream (at least for forward playback), then things go fine for at least this test case. I'll check reverse playback and run all media tests again for GTK.

Also, while looking at the code, I started to think we don't need to leave m_seeking state in GST_STATE_CHANGE_ASYNC case - only, after GST_STATE_CHANGE_SUCCESS or GST_STATE_CHANGE_FAILURE. What do you think?

[Dominik Röttsches (drott)]

Created attachment 140182 [details]
Proposed fix

[WebKit Review Bot]

Comment on attachment 140182 [details]
Proposed fix

Clearing flags on attachment: 140182

Committed r116328: <http://trac.webkit.org/changeset/116328>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.