From dadf2ec56c35e442577399d206b9569e0ea7a7b3 Mon Sep 17 00:00:00 2001 From: Edward Hervey Date: Thu, 11 Apr 2024 15:30:27 +0200 Subject: [PATCH] hlsdemux2: Fix handling of variant switching and playlist updates When updating playlists, we want to know whether the updated playlist is continuous with the previous one. That is : if we advance, will the next fragment need to have the DISCONT buffer set on it or not. If that happens (because we switched variants, or the playlist all of a sudden changed) we remember that there is a pending discont for the next fragment. That will be used and resetted the next time we get the fragment information. Previously this was only partially done. And it was racy because it was set directly on `GstAdaptiveDemux2Stream->discont` when a playlist was updated, instead of when the next fragment was prepared. Part-of: --- .../adaptivedemux2/hls/gsthlsdemux-stream.c | 68 +++++++++++-------- .../adaptivedemux2/hls/gsthlsdemux-stream.h | 5 ++ .../ext/adaptivedemux2/hls/gsthlsdemux.c | 3 +- .../ext/adaptivedemux2/hls/m3u8.c | 19 +++++- .../ext/adaptivedemux2/hls/m3u8.h | 3 +- 5 files changed, 66 insertions(+), 32 deletions(-) diff --git a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux-stream.c b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux-stream.c index 27a58ad1f3..1372d1c097 100644 --- a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux-stream.c +++ b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux-stream.c @@ -1291,36 +1291,47 @@ gst_hls_demux_stream_handle_playlist_update (GstHLSDemuxStream * stream, const gchar * new_playlist_uri, GstHLSMediaPlaylist * new_playlist) { GstHLSDemux *demux = GST_HLS_DEMUX_STREAM_GET_DEMUX (stream); + gboolean found_segment_discont = FALSE; /* Synchronize playlist with previous one. If we can't update the playlist * timing and inform the base class that we lost sync */ - if (stream->playlist - && !gst_hls_media_playlist_sync_to_playlist (new_playlist, - stream->playlist)) { - /* Failure to synchronize with the previous media playlist is only fatal for - * variant streams. */ - if (stream->is_variant) { - GST_DEBUG_OBJECT (stream, - "Could not synchronize new variant playlist with previous one !"); - goto lost_sync; - } + if (stream->playlist) { + if (!gst_hls_media_playlist_sync_to_playlist (new_playlist, + stream->playlist, &found_segment_discont)) { + /* Failure to synchronize with the previous media playlist is only fatal for + * variant streams. */ + if (stream->is_variant) { + GST_DEBUG_OBJECT (stream, + "Could not synchronize new variant playlist with previous one !"); + goto lost_sync; + } - /* For rendition streams, we can attempt synchronization against the - * variant playlist which is constantly updated */ - if (demux->main_stream->playlist - && !gst_hls_media_playlist_sync_to_playlist (new_playlist, - demux->main_stream->playlist)) { - GST_DEBUG_OBJECT (stream, - "Could not do fallback synchronization of rendition stream to variant stream"); - goto lost_sync; + /* For rendition streams, we can attempt synchronization against the + * variant playlist which is constantly updated */ + if (demux->main_stream->playlist + && !gst_hls_media_playlist_sync_to_playlist (new_playlist, + demux->main_stream->playlist, &found_segment_discont)) { + GST_DEBUG_OBJECT (stream, + "Could not do fallback synchronization of rendition stream to variant stream"); + goto lost_sync; + } } - } else if (!stream->is_variant && demux->main_stream->playlist) { - /* For initial rendition media playlist, attempt to synchronize the playlist - * against the variant stream. This is non-fatal if it fails. */ - GST_DEBUG_OBJECT (stream, - "Attempting to synchronize initial rendition stream with variant stream"); - gst_hls_media_playlist_sync_to_playlist (new_playlist, - demux->main_stream->playlist); + } else { + found_segment_discont = TRUE; + if (!stream->is_variant && demux->main_stream->playlist) { + /* For initial rendition media playlist, attempt to synchronize the playlist + * against the variant stream. This is non-fatal if it fails. */ + GST_DEBUG_OBJECT (stream, + "Attempting to synchronize initial rendition stream with variant stream"); + gst_hls_media_playlist_sync_to_playlist (new_playlist, + demux->main_stream->playlist, NULL); + } + } + + GST_DEBUG_OBJECT (stream, "Synchronized playlist. Update is discont : %d", + found_segment_discont); + if (found_segment_discont) { + stream->pending_discont = TRUE; } if (stream->current_segment) { @@ -1665,12 +1676,14 @@ gst_hls_demux_stream_update_fragment_info (GstAdaptiveDemux2Stream * stream) hlsdemux_stream->part_idx, GST_STIME_ARGS (part->stream_time)); discont = stream->discont; /* Use the segment discont flag only on the first partial segment */ - if (file->discont && hlsdemux_stream->part_idx == 0) + if ((hlsdemux_stream->pending_discont || file->discont) + && hlsdemux_stream->part_idx == 0) discont = TRUE; } else { GST_DEBUG_OBJECT (stream, "Current segment stream_time %" GST_STIME_FORMAT, GST_STIME_ARGS (file->stream_time)); - discont = file->discont || stream->discont; + discont = file->discont || stream->discont + || hlsdemux_stream->pending_discont; } gboolean need_header = GST_ADAPTIVE_DEMUX2_STREAM_NEED_HEADER (stream); @@ -1747,6 +1760,7 @@ gst_hls_demux_stream_update_fragment_info (GstAdaptiveDemux2Stream * stream) if (discont) stream->discont = TRUE; + hlsdemux_stream->pending_discont = FALSE; return ret; } diff --git a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux-stream.h b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux-stream.h index ce8c7dad0c..4096bb319a 100644 --- a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux-stream.h +++ b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux-stream.h @@ -155,6 +155,11 @@ struct _GstHLSDemuxStream GstClockTime presentation_offset; gboolean pdt_tag_sent; + + /* The next segment needs to have the discont flag set on it. This is set when + * a playlist update was detected as not being continuous/contiguous with the + * previous one. */ + gboolean pending_discont; }; GstFlowReturn diff --git a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux.c b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux.c index 8ffb95618b..33a30d80b5 100644 --- a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux.c +++ b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/gsthlsdemux.c @@ -1108,8 +1108,7 @@ gst_hls_demux_handle_variant_playlist_update (GstHLSDemux * demux, main_uri, "uri", G_TYPE_STRING, uri, "bitrate", G_TYPE_INT, new_bandwidth, NULL))); - /* Mark discont on the next packet after switching variant */ - GST_ADAPTIVE_DEMUX2_STREAM (demux->main_stream)->discont = TRUE; + GST_DEBUG_OBJECT (demux, "Changed variant"); } } diff --git a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/m3u8.c b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/m3u8.c index dbc6fc8939..7e027e4abb 100644 --- a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/m3u8.c +++ b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/m3u8.c @@ -2189,10 +2189,15 @@ gst_hls_media_playlist_get_starting_segment (GstHLSMediaPlaylist * self, * This should be used when a reference media segment couldn't be matched in the * playlist, but we still want to carry over the information from a reference * playlist to an updated one. This can happen with live playlists where the - * reference media segment is no longer present but the playlists intersect */ + * reference media segment is no longer present but the playlists intersect + * + * If the sync is sucessfull, discont will be set to TRUE if it was a perfect + * URI fragment match, else it will be FALSE (ex: match was done on PDT or + * SN/DSN). + **/ gboolean gst_hls_media_playlist_sync_to_playlist (GstHLSMediaPlaylist * playlist, - GstHLSMediaPlaylist * reference) + GstHLSMediaPlaylist * reference, gboolean * discont) { GstM3U8MediaSegment *res = NULL; GstM3U8MediaSegment *cand = NULL; @@ -2200,6 +2205,9 @@ gst_hls_media_playlist_sync_to_playlist (GstHLSMediaPlaylist * playlist, gboolean is_before; gboolean matched_pdt = FALSE; + if (discont) + *discont = FALSE; + g_return_val_if_fail (playlist && reference, FALSE); retry_without_dsn: @@ -2229,6 +2237,13 @@ retry_without_dsn: return FALSE; } + if (discont) { + /* If not a perfect match, mark as such */ + GST_DEBUG ("Checking match uri cand: %s", cand->uri); + GST_DEBUG ("Checking match uri res : %s", res->uri); + *discont = g_strcmp0 (res->uri, cand->uri) != 0; + } + /* Carry over reference stream time */ if (res->stream_time == GST_CLOCK_STIME_NONE) { GstClockTimeDiff stream_time_offset = 0; diff --git a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/m3u8.h b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/m3u8.h index 713ac6f0ca..1817a8fe9b 100644 --- a/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/m3u8.h +++ b/subprojects/gst-plugins-good/ext/adaptivedemux2/hls/m3u8.h @@ -302,7 +302,8 @@ gst_hls_media_playlist_sync_to_segment (GstHLSMediaPlaylist * m3u8, gboolean gst_hls_media_playlist_sync_to_playlist (GstHLSMediaPlaylist * m3u8, - GstHLSMediaPlaylist * reference); + GstHLSMediaPlaylist * reference, + gboolean *discont); gboolean gst_hls_media_playlist_has_next_fragment (GstHLSMediaPlaylist * m3u8,