Bug 33015 - [GTK] Failing media/video-seek-past-end-playing.html
: [GTK] Failing media/video-seek-past-end-playing.html
Status: RESOLVED FIXED
: WebKit
WebKit Gtk
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
: Gtk
:
:
  Show dependency treegraph
 
Reported: 2009-12-29 05:03 PST by
Modified: 2010-01-14 04:12 PST (History)


Attachments
listen on timeupdate event instead of waiting a fixed amount of time, which is prone to test flackyness (4.92 KB, patch)
2010-01-12 05:32 PST, Philippe Normand
eric.carlson: review+
Review Patch | Details | Formatted Diff | Diff
Removed the last timer of this test (2.51 KB, patch)
2010-01-13 00:15 PST, Philippe Normand
no flags Review Patch | Details | Formatted Diff | Diff
Removed the last timer of this test (3.01 KB, patch)
2010-01-14 03:56 PST, Philippe Normand
xan.lopez: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-12-29 05:03:44 PST
This test runs on and off locally, but seems to always fail in the release bot.
------- Comment #1 From 2009-12-29 05:05:51 PST -------
Skipped in r52616.
------- Comment #2 From 2010-01-11 03:28:39 PST -------
The seek back to beginning is done and the pipeline is in PLAYING but haven't had time to play any data :/
------- Comment #3 From 2010-01-12 03:19:41 PST -------
That test is really flacky in our case because it expects that currentTime has been updated 200ms after play() was called. But the pipeline can take more than 200ms to preroll and start playing for real...

So I suggest to remove that 200ms timeout from the test and listen on timeupdate instead. The second time it is fired we are sure that playback really started and that currentTime is > 0. What do you think?

I can see the exact same issue in media/audio-mpeg-supported.html.

Moreover I found out that our player performs un-needed seeks:

- seek(0): un-needed when currentTime is 0 already
- setRate(1): un-needed at beginning of playback as it is already the default in playbin.
------- Comment #4 From 2010-01-12 03:59:40 PST -------
(In reply to comment #3)
> That test is really flacky in our case because it expects that currentTime has
> been updated 200ms after play() was called. But the pipeline can take more than
> 200ms to preroll and start playing for real...
> 
> So I suggest to remove that 200ms timeout from the test and listen on
> timeupdate instead. The second time it is fired we are sure that playback
> really started and that currentTime is > 0. What do you think?

Sounds like a plan to me. =)

> I can see the exact same issue in media/audio-mpeg-supported.html.

Aha, well, I'm all for removing timeouts, and making the tests state-based.

> Moreover I found out that our player performs un-needed seeks:
> 
> - seek(0): un-needed when currentTime is 0 already
> - setRate(1): un-needed at beginning of playback as it is already the default
> in playbin.

Make patches =D.
------- Comment #5 From 2010-01-12 05:32:39 PST -------
Created an attachment (id=46364) [details]
listen on timeupdate event instead of waiting a fixed amount of time, which is prone to test flackyness
------- Comment #6 From 2010-01-12 08:45:20 PST -------
(From update of attachment 46364 [details])
In both tests set timeupdateEventCount to 0 before ending the test. This isn't necessary and should be removed. A comment about why the test ends after two timpupdate events would be nice.

r=me with these minor changes.

Extra points if you cleanup video-seek-past-end-playing.html to make it a reasonable html file instead of the ugly fragment it is now :-)
------- Comment #7 From 2010-01-12 10:34:52 PST -------
Added the comments, made the test an html document and landed as r53147. Thanks!
------- Comment #8 From 2010-01-12 11:35:30 PST -------
The last timer of the test needs to be killed, the test still fails on the release bot.
------- Comment #9 From 2010-01-13 00:15:04 PST -------
Created an attachment (id=46430) [details]
Removed the last timer of this test
------- Comment #10 From 2010-01-14 03:51:52 PST -------
(From update of attachment 46364 [details])
This one was landed.
------- Comment #11 From 2010-01-14 03:56:49 PST -------
Created an attachment (id=46554) [details]
Removed the last timer of this test
------- Comment #12 From 2010-01-14 03:58:56 PST -------
(From update of attachment 46554 [details])
LGTM.
------- Comment #13 From 2010-01-14 04:12:22 PST -------
Landed as r53258, Thanks!