After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 773159 - hlsdemux: Errors and timeouts with scrubbing and fast forward/reverse
hlsdemux: Errors and timeouts with scrubbing and fast forward/reverse
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.12.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-10-18 13:38 UTC by Edward Hervey
Modified: 2017-07-13 09:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
adaptivedemux: Handle prepared streams on seeks (3.08 KB, patch)
2017-05-09 09:45 UTC, Edward Hervey
committed Details | Review
adaptivedemux: small refactor to avoid repeated code (5.76 KB, patch)
2017-05-14 03:13 UTC, Thiago Sousa Santos
committed Details | Review
adaptivedemux: switch immediately to next bitrate on flushing seeks (1.24 KB, patch)
2017-05-14 03:13 UTC, Thiago Sousa Santos
none Details | Review

Description Edward Hervey 2016-10-18 13:38:53 UTC
This bug is to track various errors/timeouts that happen with hlsdemux in the following scenarios:

* validate.hls.playback.reverse_playback
* validate.hls.playback.scrub_forward_seeking
* validate.hls.playback.fast_forward

Happens with both playbin and playbin3
Comment 1 Edward Hervey 2017-05-09 09:45:34 UTC
Created attachment 351415 [details] [review]
adaptivedemux: Handle prepared streams on seeks

This is a race that was exposed by the {hls|dash}.scrub_forward_seeking
validate test.

The "race" is that a subclass might want to change format, causing
a new stream to be created (but not exposed/switched yet) and put on the
prepared_streams list. That stream will have values (including pending
segment) from the pre-seek state.

Before the stream is exposed/switched, a new seek comes in and the stream
values get updated ... but the ones that will be changed don't get updated
causing them to push out wrong segments once they are exposed.
Comment 2 Thiago Sousa Santos 2017-05-13 19:30:08 UTC
Review of attachment 351415 [details] [review]:

Looks good. Would you mind putting that repeated code into a function?
Comment 3 Thiago Sousa Santos 2017-05-14 03:13:29 UTC
For flushing seeks, it also seems that it won't switch to the next representation until the end of the fragment after the seek, right?

Not that this blocks your work, just wondering if we should/could also fix that by immediately going to the next representation by exposing the prepared_streams using gst_adaptive_demux_start_tasks(demux, TRUE) in the end if there are new streams to expose.
Comment 4 Thiago Sousa Santos 2017-05-14 03:13:47 UTC
Created attachment 351798 [details] [review]
adaptivedemux: small refactor to avoid repeated code

Move segment event update to a function
Comment 5 Thiago Sousa Santos 2017-05-14 03:13:51 UTC
Created attachment 351799 [details] [review]
adaptivedemux: switch immediately to next bitrate on flushing seeks

if there are new streams to be exposed after a flushing seek, do
it right when restarting to avoid maintaining the previous resolution
for another fragment.
Comment 6 Thiago Sousa Santos 2017-05-14 03:14:51 UTC
Something like the above ^

Didn't see any regressions on validate but also the issue you fixed was very hard for me to reproduce so please verify that your fix still works.
Comment 7 Edward Hervey 2017-07-13 05:45:57 UTC
commit 33afa6bd18110b3db9ffde9d473c6af3d2d05936
Author: Thiago Santos <thiagossantos@gmail.com>
Date:   Sat May 13 15:17:57 2017 -0700

    adaptivedemux: small refactor to avoid repeated code
    
    Move segment event update to a function
    
    https://bugzilla.gnome.org/show_bug.cgi?id=773159

commit 1df725e5cdc53fbe9d7f2845e5418abea5673080
Author: Edward Hervey <edward@centricular.com>
Date:   Tue May 9 11:41:49 2017 +0200

    adaptivedemux: Handle prepared streams on seeks
    
    This is a race that was exposed by the {hls|dash}.scrub_forward_seeking
    validate test.
    
    The "race" is that a subclass might want to change format, causing
    a new stream to be created (but not exposed/switched yet) and put on the
    prepared_streams list. That stream will have values (including pending
    segment) from the pre-seek state.
    
    Before the stream is exposed/switched, a new seek comes in and the stream
    values get updated ... but the ones that will be changed don't get updated
    causing them to push out wrong segments once they are exposed.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=773159
Comment 8 Edward Hervey 2017-07-13 05:47:08 UTC
Not 100% certain on that last patch
Comment 9 Edward Hervey 2017-07-13 09:19:40 UTC
Pushed to 1.12 also