Bug 33015 – [GTK] Failing media/video-seek-past-end-playing.html

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

[Gustavo Noronha (kov)]

This test runs on and off locally, but seems to always fail in the release bot.

[Gustavo Noronha (kov)]

Skipped in r52616.

[Philippe Normand]

The seek back to beginning is done and the pipeline is in PLAYING but haven't had time to play any data :/

[Philippe Normand]

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.

[Gustavo Noronha (kov)]

(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.

[Philippe Normand]

Created attachment 46364 [details]
listen on timeupdate event instead of waiting a fixed amount of time, which is prone to test flackyness

[Eric Carlson]

Comment on attachment 46364 [details]
listen on timeupdate event instead of waiting a fixed amount of time, which is prone to test flackyness

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 :-)

[Philippe Normand]

Added the comments, made the test an html document and landed as r53147. Thanks!

[Philippe Normand]

The last timer of the test needs to be killed, the test still fails on the release bot.

[Philippe Normand]

Created attachment 46430 [details]
Removed the last timer of this test

[Philippe Normand]

Comment on attachment 46364 [details]
listen on timeupdate event instead of waiting a fixed amount of time, which is prone to test flackyness

This one was landed.

[Philippe Normand]

Created attachment 46554 [details]
Removed the last timer of this test

[Xan Lopez]

Comment on attachment 46554 [details]
Removed the last timer of this test

LGTM.

[Philippe Normand]

Landed as r53258, Thanks!