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 783401 - adaptivedemux: Clear "cancelled" on uridownloader before processing manifest
adaptivedemux: Clear "cancelled" on uridownloader before processing manifest
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.12.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-04 11:30 UTC by Seungha Yang
Modified: 2017-06-12 14:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
adaptivedemux: Clear "cancelled" on uridownloader before processing manifest (1.19 KB, patch)
2017-06-04 11:32 UTC, Seungha Yang
none Details | Review
adaptivedemux: Clear "cancelled" on uridownloader before processing manifest (1.63 KB, patch)
2017-06-04 23:12 UTC, Seungha Yang
committed Details | Review

Description Seungha Yang 2017-06-04 11:30:15 UTC
Previous commit let demux call gst_uri_downloader_cancel() on _demux_reset().
Since it's called when not only PAUSED_TO_READY, but also READY_TO_PAUSED,
this causes regression on hls streaming.
Comment 1 Seungha Yang 2017-06-04 11:32:49 UTC
Created attachment 353139 [details] [review]
adaptivedemux: Clear "cancelled" on uridownloader before processing manifest

This is to fix regression of bug#783028
currently any hls streams which includes variants and renditions streams are not playable.
Comment 2 Thiago Sousa Santos 2017-06-04 17:30:40 UTC
Review of attachment 353139 [details] [review]:

it seems more appropriate to reset it in the READY_TO_PAUSED transition. Bonus points for a comment in the code to state that subclasses use that outside of the manifest updating task.
Comment 3 Seungha Yang 2017-06-04 23:12:25 UTC
Created attachment 353155 [details] [review]
adaptivedemux: Clear "cancelled" on uridownloader before processing manifest

Previous commit let demux call gst_uri_downloader_cancel() on _demux_reset().
Note that, _demux_reset() called during PAUSED_TO_READY and READY_TO_PAUSED.
And, it will set "cancelled" on uridownloader which blocks the use of
uridownloader. The issue is that, subclass can use the uridownloader not only
live streaming for manifest update, but also for fetching another manifests
such as variant and rendition m3u8 of hls streaming. So to unblock it,
demux should clear "cancelled" before processing initial manifest.
Comment 4 Thiago Sousa Santos 2017-06-07 02:59:59 UTC
commit 87cb9fa49ede6923c929e61a0c263f584f94871e
Author: Seungha Yang <sh.yang@lge.com>
Date:   Sun Jun 4 20:23:36 2017 +0900

    adaptivedemux: Clear "cancelled" on uridownloader before processing manifest
    
    Previous commit let demux call gst_uri_downloader_cancel() on _demux_reset().
    Note that, _demux_reset() called during PAUSED_TO_READY and READY_TO_PAUSED.
    And, it will set "cancelled" on uridownloader which blocks the use of
    uridownloader. The issue is that, subclass can use the uridownloader not only
    live streaming for manifest update, but also for fetching another manifests
    such as variant and rendition m3u8 of hls streaming. So to unblock it,
    demux should clear "cancelled" before processing initial manifest.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=783401
Comment 5 Sebastian Dröge (slomo) 2017-06-07 07:05:17 UTC
Should this go into 1.12?
Comment 6 Seungha Yang 2017-06-07 08:04:51 UTC
(In reply to Sebastian Dröge (slomo) from comment #5)
> Should this go into 1.12?

Not yet, but need when following commit is cherry-picked into 1.12
5a693fd 2017-05-29 22:28 Thiago Santos adaptivedemux: do not erase data while updates-loop is running
Comment 7 Edward Hervey 2017-06-12 14:27:21 UTC
Both this commit and the related one are in 1.12 now