Bug 84858

Summary: [GTK] media/video-seek-past-end-playing.html times out
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: WebKitGTKAssignee: Dominik Röttsches (drott) <d-r>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, d-r, eric.carlson, feature-media-reviews, gustavo, menard, mrobinson, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed fix none

Description Philippe Normand 2012-04-25 06:43:43 PDT
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
-
Comment 1 Philippe Normand 2012-04-25 06:43:55 PDT
And flakily
Comment 2 Dominik Röttsches (drott) 2012-05-02 08:45:07 PDT
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.
Comment 3 Philippe Normand 2012-05-03 08:53:50 PDT
(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?
Comment 4 Dominik Röttsches (drott) 2012-05-04 00:28:50 PDT
(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?
Comment 5 Dominik Röttsches (drott) 2012-05-04 02:14:39 PDT
Created attachment 140182 [details]
Proposed fix
Comment 6 WebKit Review Bot 2012-05-07 11:31:31 PDT
Comment on attachment 140182 [details]
Proposed fix

Clearing flags on attachment: 140182

Committed r116328: <http://trac.webkit.org/changeset/116328>
Comment 7 WebKit Review Bot 2012-05-07 11:31:36 PDT
All reviewed patches have been landed.  Closing bug.