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 731164 - hlsdemux: Attempt to reload variant playlist if refreshing playlist or downloading fragments failed
hlsdemux: Attempt to reload variant playlist if refreshing playlist or downlo...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Windows
: Normal normal
: 1.3.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-06-03 16:21 UTC by GstBlub
Modified: 2014-06-06 10:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.28 KB, patch)
2014-06-03 16:22 UTC, GstBlub
none Details | Review
Actual patch (13.47 KB, patch)
2014-06-03 16:25 UTC, GstBlub
reviewed Details | Review
Updated patch (13.67 KB, patch)
2014-06-04 15:58 UTC, GstBlub
committed Details | Review

Description GstBlub 2014-06-03 16:21:54 UTC
A HLS stream may invalidate playlists referenced by the master (variant) playlist, e.g. for load balancing or session validation purposes.  In that case, refreshing the playlist or downloading a fragment may fail.  If this happens, attempt to refresh the variant playlist, and if the referenced playlists can still be matched, update the URLs to these playlists.  All that may have changed is that these URLs now redirect to another host or path.
Comment 1 GstBlub 2014-06-03 16:22:28 UTC
Created attachment 277807 [details] [review]
Patch
Comment 2 GstBlub 2014-06-03 16:25:15 UTC
Created attachment 277810 [details] [review]
Actual patch
Comment 3 Sebastian Dröge (slomo) 2014-06-04 14:50:46 UTC
Review of attachment 277810 [details] [review]:

Thanks for the patch! Looks good except for some minor issues

::: ext/hls/m3u8.c
@@ +138,3 @@
+  dup->targetduration = self->targetduration;
+  dup->allowcache = self->allowcache;
+  dup->key = g_strdup (self->key);

You should probably copy the IV too (also check if you have all the members of the struct then)

@@ +843,3 @@
+      GST_ERROR
+          ("Cannot update variant playlist, unable to match all playlists");
+      goto out;

Why is that an error always? Maybe we played the low-bitrate variant and the high-bitrate variant was dropped?
Comment 4 GstBlub 2014-06-04 15:22:09 UTC
That's true, I guess we really only need the variant playlist that we are currently playing.  But even if that one went away, we could switch to an alternative variant.  I just don't really know how to safely switch over to another playlist, if we can't match.  Maybe this log message should be changed to a FIXME for future improvement?
Comment 5 Sebastian Dröge (slomo) 2014-06-04 15:32:41 UTC
ok :)
Comment 6 GstBlub 2014-06-04 15:58:47 UTC
Created attachment 277886 [details] [review]
Updated patch

This patch changes that error message to a FIXME.  The iv is maintained per fragment and does not need to be copied (actually there is no field for it in the _GstM3U8 structure).
Comment 7 Sebastian Dröge (slomo) 2014-06-04 16:02:56 UTC
I wonder why the key is per playlist though. It has exactly the same scope as the IV (all fragments following it until the next IV/key).
Comment 8 GstBlub 2014-06-04 16:24:25 UTC
Maybe they need to be cached somewhere in between EXT-X-KEY tags?  I guess the EXT-X-KEY doesn't have to be specified for each fragment, but it's just a guess from looking at the code.  I haven't looked at the spec in this regard.
Comment 9 Sebastian Dröge (slomo) 2014-06-04 16:25:49 UTC
Yes exactly. I don't know why we store them inside the playlist at all :)
Comment 10 Sebastian Dröge (slomo) 2014-06-06 10:04:38 UTC
commit 008edeadaef13f585eefba7ff449a25a68661275
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Fri Jun 6 13:04:04 2014 +0300

    hlsdemux: Fix compiler warnings

commit babd8969f29966e471c74f6411e6a15fec80f810
Author: Thomas Bluemel <tbluemel@control4.com>
Date:   Fri May 30 16:34:18 2014 -0600

    hlsdemux: Reload the variant playlist if refreshing a playlist or downloading a fragment fails
    
    This can happen if the playlists have moved due to the variant playlist
    now being redirected to another target. This currently only works as long
    as the referenced playlists don't change in relation to the variant
    playlist, and the new location is purely due to a new path triggered by a
    new redirection target of the variant playlist, or a new redirection
    target of the playlist itself.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=731164