WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
213 stars 136 forks source link

Fix return of invalid playback position #1143

Closed filipe-norte-red closed 10 months ago

filipe-norte-red commented 1 year ago

After a seek, the gstreamer pipeline may not be able to return a valid position right away. The m_canFallBackToLastFinishedSeekPosition is reset in an async way when handleMessage() is called, so playbackPosition() might be called while we still not have a valid position from the pipeline.

calvaris commented 1 year ago

It would be nice if you can elaborate a bit more on how you're testing this. I'll try to write a test but I'd appreciate if you can give some more light on this.

calvaris commented 1 year ago

Actually there are already some nice tests upstream, such as LayoutTests/media/video-currentTime-set.html and LayoutTests/media/video-currentTime-set2.html and they seem to pass with flying colors.

filipe-norte-red commented 1 year ago

Hi @calvaris I wrote a few tests to check the current time under various conditions (e.g. seek while in play/pause state). Find them attached for reference. tests.zip

calvaris commented 1 year ago

@filipe-norte-red I got the tests and began to have a look at them but it lacks the lib/utils.js and the media (though I guess I could use any random one I have here)

filipe-norte-red commented 1 year ago

@calvaris , the tests use an internal test framework. The key part are the reportPass() / reportFail() methods which will update internal test results and cause the page with the test to be unloaded and next to be loaded (or test results be displayed). You should be able to get it to run by loading a new page in reportPass() / reportFail() which displays the argument passed to these methods (or log it to console). Any mp3 with 12 or more minutes duration should be ok to use.

filipe-norte-red commented 1 year ago

Hi @calvaris Did you have the chance to look further into this topic? Thanks

calvaris commented 1 year ago

I did, yes, but I am with some other task right now. I will be back at this as soon as possible.

filipe-norte-red commented 11 months ago

An additional corner case was identified where the cached position gets invalidated due the async nature of the invalidation process. Working on a fix and will update pull request accordingly

filipe-norte-red commented 11 months ago

Hi @calvaris I updated the pull request with a change in "MediaPlayerPrivateGStreamer::invalidateCachedPositionOnNextIteration()" do handle a corner case (described in the commit message).

I attached the test case for this scenario. You should see a "TEST PASS" / "TEST FAIL" message in the console log when you run the test. You need to replace the audio mp3 sample for "audioUrl" with one you have.

progressive_seek_audio_at_start.zip

calvaris commented 10 months ago

I added something that could fix this on commit 98827e77d1f9d263ec769632f59d9c27d6b19c08 . Please reopen if needed.