GNOME Bugzilla – Bug 775665
hlsdemux: problems with broken iHeartRadio streams
Last modified: 2017-01-18 11:38:19 UTC
Created attachment 341430 [details] Small player to repro issue When playing the following iHeartRadio stream: https://c5.prod.playlists.ihrhls.com/1469/playlist.m3u8 the following problems occur: - Start up time is a few seconds even on a fast connection - Playback stutters and buffers very often - Playback stops often - After a certain point, playback discontinues indefinitely See attached file for code reference. This was tested on a Raspberry Pi 3 device using Raspbian (Debian) OS using alsa drivers. Other audio-only HLS streams were tested (e.g. Groove music) and they seemed to work fine. This looks specific to iHeartRadio streams.
It works by using "playbin3" to play given url. gst-launch-1.0 playbin3 uri=https://c5.prod.playlists.ihrhls.com/1469/playlist.m3u8
This commit makes it work in playbin / decodebin: https://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=991758c3d68a0a7131df4e6244fe3ce29f2b9f7f And this fixes another problem with it: https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=ee647ca4be8b5beecada6ebc130df5d376a1ad23 However the stream is broken in general and not according to the spec. Usage of the media sequence (ids) is inconsistent, and the spec recommends in this case to even halt playback. Which is what happens with this stream after a while in Safari on an iPad. We can probably work around this, but this stream should really be fixed.
Created attachment 342974 [details] [review] hlsdemux: Detect and work around media sequence number inconsistencies And also handle live streams without any media sequence numbers at all. Partially works around brokenness with iHeartRadio streams, which have a) inconsistent media sequence numbers, that just jump back and forth all the time and wrap around at 24 or something b) broken CDN caching or similar, which causes older versions of the playlist to be fetched sometimes. This works well now unless the situation happens, that a playlist is fetched that has *only* fragments that are before the previous playlist. Which unfortunately happens with the iHearRadio streams (meaning: playlist is 30s shifted into the past!).
These streams are indeed impressively broken, and they don't play in any software I have here. Does anybody have contacts there so they can fix their stuff? Is any software known that can actually play these streams for a while?
And even when played like this, without running into the remaining problem, it sounds like fragments are not audible contiguous: the audio just jumps forwards or backwards by a few seconds on a segment boundary sometimes.
Did we ever see part of those spec breakages in other streams? It seems like we add quite some code to handle something that is broken beyond repair. Is it worth adding more complexity in this case? It would be better to try and contact the maintainers of the service.
Review of attachment 342974 [details] [review]: ::: ext/hls/m3u8.c @@ +684,3 @@ + g_list_free (previous_files); + previous_files = NULL; + } I'd move this to a separate function to avoid making the _update() function longer. @@ +711,3 @@ + } else { + mediasequence = file->sequence; + } This could be a separate patch. Does the spec mention what to do in this case?
(In reply to Thiago Sousa Santos from comment #7) > > This could be a separate patch. Does the spec mention what to do in this > case? Yes, halt playback like with all media sequence ID inconsistencies.
For the original issue with the iHeartRadio streams. The problem is application-side here as Jan noticed. Just try playing the stream in a browser and look at how it requests the m3u8 playlist: it adds lots of query parameters, including sessions IDs and other stuff that somehow comes from the Javascript. If you pass that full URL with the query parameters to GStreamer, the stream plays just fine without any code changes at all. The media sequence ids are continuous, playlists are always updated and never go back in time, there's no buffering, no anything. Also note that there are RTMP and Icecast variants of the streams available, which might or might not work better without the query parameters. For the attached patch. I agree that it adds a lot of complexity and the code is also not very nice. Maybe we should not merge it then :) However it implements two parts: 1) detecting of inconsistent media sequence IDs (the first part) and ignoring them then, and 2) handling media sequence IDs correctly for live playlists that don't specify them inside the playlist (it's not mandatory). 1) is something that the spec suggests, detecting these scenarios and then just stopping playback. 2) is something we need to implement if we want to support such (valid) streams.
I guess for 2 the extra work to also ignore broken media sequence and try to play is minimal, right? I'd still prefer to abort as the spec suggests, using some meaningful error. But, as this was already implemented, feel free to merge it too if you see some use to it. Aside my minor suggested refactoring, it looks good to me.
(In reply to Thiago Sousa Santos from comment #10) > I guess for 2 the extra work to also ignore broken media sequence and try to > play is minimal, right? The "if (have_mediasequence && !self->broken_media_sequence) {}" is for 1), the "if (!have_mediasequence || self->broken_media_sequence) {}" is for 2). Or what did you mean?
I just meant that for 2, officially according to the spec, you wouldn't need the 'broken_media_sequence' because we would have aborted earlier. My question is: shouldn't we just should abort when 'broken_media_sequence' happens or we have a good reason to support those streams? Widely available kind of broken?
I think we should error out, just like apple does.
(In reply to Thiago Sousa Santos from comment #12) > I just meant that for 2, officially according to the spec, you wouldn't need > the 'broken_media_sequence' because we would have aborted earlier. That's correct, yes > My question is: shouldn't we just should abort when 'broken_media_sequence' > happens or we have a good reason to support those streams? We could, yes. The only difference in the patch would be the removal of the new boolean in the struct and that the first "if" just errors out instead of falling back to the second "if" > Widely available kind of broken? I'm not aware of any
If that's the consensus, I can update the patch accordingly (and move it to a new function too).
Yes, no? :)
I agree with Tim. Let's do it.
commit 075ceffd9b21339b50130a5207a0df4ce9f1f5a7 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Jan 5 19:10:52 2017 +0200 hlsdemux: Detect media sequence number inconsistencies and fail Without failing, we would play back random parts of the stream which is arguably a worse user experience, and failing is also recommended by the spec here. And also handle live streams without any media sequence numbers at all properly, that is, make sure the sequence numbers are increasing instead of starting again at 0 every time. https://bugzilla.gnome.org/show_bug.cgi?id=775665