| Summary: | [GTK] Failing media/video-seek-past-end-playing.html | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gustavo Noronha (kov) <gustavo> | ||||||||
| Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | pnormand | ||||||||
| Priority: | P2 | Keywords: | Gtk | ||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | OS X 10.5 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Gustavo Noronha (kov)
2009-12-29 05:03:44 PST
The seek back to beginning is done and the pipeline is in PLAYING but haven't had time to play any data :/ 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. (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. Created attachment 46364 [details]
listen on timeupdate event instead of waiting a fixed amount of time, which is prone to test flackyness
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 :-)
Added the comments, made the test an html document and landed as r53147. Thanks! The last timer of the test needs to be killed, the test still fails on the release bot. Created attachment 46430 [details]
Removed the last timer of this test
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.
Created attachment 46554 [details]
Removed the last timer of this test
Comment on attachment 46554 [details]
Removed the last timer of this test
LGTM.
Landed as r53258, Thanks! |