adaptivedemux2: Fix manifest access during seeking query

Avoid a deadlock if a downstream seeking query happens while the scheduler
thread is holding the manifest lock (for example during a seek back to live).

Instead, do a more elaborate fix where the external calls that need access to a
'manifest' access a copy that's updated during a manually triggered manifest
update callback.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3883>
This commit is contained in:
Jan Schmidt 2022-12-30 23:37:23 +11:00 committed by GStreamer Marge Bot
parent 5334007a0b
commit 3d0e8aa07e
5 changed files with 89 additions and 11 deletions

View file

@ -83,6 +83,8 @@ struct _GstAdaptiveDemuxPrivate
/* Callback / timer id for the next manifest update */ /* Callback / timer id for the next manifest update */
guint manifest_updates_cb; guint manifest_updates_cb;
gboolean manifest_updates_enabled;
gboolean need_manual_manifest_update;
/* Count of failed manifest updates */ /* Count of failed manifest updates */
gint update_failed_count; gint update_failed_count;

View file

@ -1964,7 +1964,7 @@ gst_adaptive_demux_seek_to_input_period (GstAdaptiveDemux * demux)
} }
} }
/* must be called with manifest_lock taken */ /* must be called with scheduler lock taken */
gboolean gboolean
gst_adaptive_demux_get_live_seek_range (GstAdaptiveDemux * demux, gst_adaptive_demux_get_live_seek_range (GstAdaptiveDemux * demux,
gint64 * range_start, gint64 * range_stop) gint64 * range_start, gint64 * range_stop)
@ -1978,7 +1978,7 @@ gst_adaptive_demux_get_live_seek_range (GstAdaptiveDemux * demux,
return klass->get_live_seek_range (demux, range_start, range_stop); return klass->get_live_seek_range (demux, range_start, range_stop);
} }
/* must be called with manifest_lock taken */ /* must be called from scheduler task */
gboolean gboolean
gst_adaptive_demux2_stream_in_live_seek_range (GstAdaptiveDemux * demux, gst_adaptive_demux2_stream_in_live_seek_range (GstAdaptiveDemux * demux,
GstAdaptiveDemux2Stream * stream) GstAdaptiveDemux2Stream * stream)
@ -2684,6 +2684,7 @@ gst_adaptive_demux_src_query (GstPad * pad, GstObject * parent,
if (can_seek) { if (can_seek) {
if (gst_adaptive_demux_is_live (demux)) { if (gst_adaptive_demux_is_live (demux)) {
ret = gst_adaptive_demux_get_live_seek_range (demux, &start, &stop); ret = gst_adaptive_demux_get_live_seek_range (demux, &start, &stop);
if (!ret) { if (!ret) {
GST_MANIFEST_UNLOCK (demux); GST_MANIFEST_UNLOCK (demux);
GST_INFO_OBJECT (demux, "can't answer seeking query"); GST_INFO_OBJECT (demux, "can't answer seeking query");
@ -2797,6 +2798,7 @@ static void
gst_adaptive_demux_stop_manifest_update_task (GstAdaptiveDemux * demux) gst_adaptive_demux_stop_manifest_update_task (GstAdaptiveDemux * demux)
{ {
GST_DEBUG_OBJECT (demux, "requesting stop of the manifest update task"); GST_DEBUG_OBJECT (demux, "requesting stop of the manifest update task");
demux->priv->manifest_updates_enabled = FALSE;
if (demux->priv->manifest_updates_cb != 0) { if (demux->priv->manifest_updates_cb != 0) {
gst_adaptive_demux_loop_cancel_call (demux->priv->scheduler_task, gst_adaptive_demux_loop_cancel_call (demux->priv->scheduler_task,
demux->priv->manifest_updates_cb); demux->priv->manifest_updates_cb);
@ -2811,6 +2813,12 @@ static void
gst_adaptive_demux_start_manifest_update_task (GstAdaptiveDemux * demux) gst_adaptive_demux_start_manifest_update_task (GstAdaptiveDemux * demux)
{ {
GstAdaptiveDemuxClass *demux_class = GST_ADAPTIVE_DEMUX_GET_CLASS (demux); GstAdaptiveDemuxClass *demux_class = GST_ADAPTIVE_DEMUX_GET_CLASS (demux);
demux->priv->manifest_updates_enabled = TRUE;
if (demux->priv->need_manual_manifest_update) {
gst_adaptive_demux2_manual_manifest_update (demux);
demux->priv->need_manual_manifest_update = FALSE;
}
if (gst_adaptive_demux_is_live (demux)) { if (gst_adaptive_demux_is_live (demux)) {
/* Task to periodically update the manifest */ /* Task to periodically update the manifest */
@ -3756,6 +3764,35 @@ gst_adaptive_demux_update_manifest (GstAdaptiveDemux * demux)
return ret; return ret;
} }
static gboolean
gst_adaptive_demux2_manual_manifest_update_cb (GstAdaptiveDemux * demux)
{
GST_MANIFEST_LOCK (demux);
gst_adaptive_demux_update_manifest (demux);
GST_MANIFEST_UNLOCK (demux);
return G_SOURCE_REMOVE;
}
/* called by a subclass that needs a callback to 'update_manifest'
* done with with MANIFEST_LOCK held */
void
gst_adaptive_demux2_manual_manifest_update (GstAdaptiveDemux * demux)
{
if (demux->priv->manifest_updates_cb != 0)
return; /* Callback already pending */
if (!demux->priv->manifest_updates_enabled) {
GST_LOG_OBJECT (demux, "Marking manual manifest update pending");
demux->priv->need_manual_manifest_update = TRUE;
return;
}
demux->priv->manifest_updates_cb =
gst_adaptive_demux_loop_call (demux->priv->scheduler_task,
(GSourceFunc) gst_adaptive_demux2_manual_manifest_update_cb, demux, NULL);
}
/* must be called with manifest_lock taken */ /* must be called with manifest_lock taken */
gboolean gboolean
gst_adaptive_demux_has_next_period (GstAdaptiveDemux * demux) gst_adaptive_demux_has_next_period (GstAdaptiveDemux * demux)

View file

@ -476,6 +476,7 @@ GstCaps * gst_codec_utils_caps_from_iso_rfc6831 (gchar * codec);
gdouble gst_adaptive_demux_play_rate (GstAdaptiveDemux *demux); gdouble gst_adaptive_demux_play_rate (GstAdaptiveDemux *demux);
void gst_adaptive_demux2_manual_manifest_update (GstAdaptiveDemux * demux);
GstAdaptiveDemuxLoop *gst_adaptive_demux_get_loop (GstAdaptiveDemux *demux); GstAdaptiveDemuxLoop *gst_adaptive_demux_get_loop (GstAdaptiveDemux *demux);
G_END_DECLS G_END_DECLS

View file

@ -93,6 +93,7 @@ static gboolean gst_hls_demux_is_live (GstAdaptiveDemux * demux);
static GstClockTime gst_hls_demux_get_duration (GstAdaptiveDemux * demux); static GstClockTime gst_hls_demux_get_duration (GstAdaptiveDemux * demux);
static gboolean gst_hls_demux_process_initial_manifest (GstAdaptiveDemux * static gboolean gst_hls_demux_process_initial_manifest (GstAdaptiveDemux *
demux, GstBuffer * buf); demux, GstBuffer * buf);
static GstFlowReturn gst_hls_demux_update_manifest (GstAdaptiveDemux * demux);
static void gst_hls_prune_time_mappings (GstHLSDemux * demux); static void gst_hls_prune_time_mappings (GstHLSDemux * demux);
@ -173,7 +174,8 @@ hlsdemux_requires_periodical_playlist_update_default (GstAdaptiveDemux *
demux G_GNUC_UNUSED) demux G_GNUC_UNUSED)
{ {
/* We don't need the base class to update our manifest periodically, the /* We don't need the base class to update our manifest periodically, the
* playlist loader for the main stream will do that */ * playlist loader for the main stream will do that and trigger
* an update manual */
return FALSE; return FALSE;
} }
@ -223,6 +225,7 @@ gst_hls_demux2_class_init (GstHLSDemux2Class * klass)
adaptivedemux_class->process_manifest = adaptivedemux_class->process_manifest =
gst_hls_demux_process_initial_manifest; gst_hls_demux_process_initial_manifest;
adaptivedemux_class->reset = gst_hls_demux_reset; adaptivedemux_class->reset = gst_hls_demux_reset;
adaptivedemux_class->update_manifest = gst_hls_demux_update_manifest;
adaptivedemux_class->seek = gst_hls_demux_seek; adaptivedemux_class->seek = gst_hls_demux_seek;
} }
@ -775,6 +778,10 @@ gst_hls_demux_process_initial_manifest (GstAdaptiveDemux * demux,
} }
} }
/* Make sure the external manifest copy of the main playlist
* is available to the baseclass at the start */
gst_hls_demux_update_manifest (demux);
return TRUE; return TRUE;
} }
@ -784,21 +791,21 @@ gst_hls_demux_get_duration (GstAdaptiveDemux * demux)
GstHLSDemux *hlsdemux = GST_HLS_DEMUX_CAST (demux); GstHLSDemux *hlsdemux = GST_HLS_DEMUX_CAST (demux);
GstClockTime duration = GST_CLOCK_TIME_NONE; GstClockTime duration = GST_CLOCK_TIME_NONE;
if (hlsdemux->main_stream) if (hlsdemux->main_playlist)
duration = duration = gst_hls_media_playlist_get_duration (hlsdemux->main_playlist);
gst_hls_media_playlist_get_duration (hlsdemux->main_stream->playlist);
return duration; return duration;
} }
/* Called from base class with the MANIFEST_LOCK held */
static gboolean static gboolean
gst_hls_demux_is_live (GstAdaptiveDemux * demux) gst_hls_demux_is_live (GstAdaptiveDemux * demux)
{ {
GstHLSDemux *hlsdemux = GST_HLS_DEMUX_CAST (demux); GstHLSDemux *hlsdemux = GST_HLS_DEMUX_CAST (demux);
gboolean is_live = FALSE; gboolean is_live = FALSE;
if (hlsdemux->main_stream && hlsdemux->main_stream->playlist) if (hlsdemux->main_playlist)
is_live = gst_hls_media_playlist_is_live (hlsdemux->main_stream->playlist); is_live = gst_hls_media_playlist_is_live (hlsdemux->main_playlist);
return is_live; return is_live;
} }
@ -1044,6 +1051,26 @@ gst_hls_update_time_mappings (GstHLSDemux * demux,
} }
} }
/* Called by the base class with the manifest lock held */
static GstFlowReturn
gst_hls_demux_update_manifest (GstAdaptiveDemux * demux)
{
GstHLSDemux *hlsdemux = GST_HLS_DEMUX_CAST (demux);
/* Take a copy of the main variant playlist for base class
* calls that need access from outside the scheduler task,
* holding the MANIFEST_LOCK */
if (hlsdemux->main_stream && hlsdemux->main_stream->playlist) {
if (hlsdemux->main_playlist)
gst_hls_media_playlist_unref (hlsdemux->main_playlist);
hlsdemux->main_playlist =
gst_hls_media_playlist_ref (hlsdemux->main_stream->playlist);
return GST_FLOW_OK;
}
return GST_ADAPTIVE_DEMUX_FLOW_BUSY;
}
void void
gst_hls_demux_handle_variant_playlist_update (GstHLSDemux * demux, gst_hls_demux_handle_variant_playlist_update (GstHLSDemux * demux,
const gchar * playlist_uri, GstHLSMediaPlaylist * playlist) const gchar * playlist_uri, GstHLSMediaPlaylist * playlist)
@ -1099,6 +1126,10 @@ gst_hls_demux_handle_variant_playlist_update (GstHLSDemux * demux,
* be based. */ * be based. */
gst_hls_update_time_mappings (demux, playlist); gst_hls_update_time_mappings (demux, playlist);
gst_hls_media_playlist_dump (playlist); gst_hls_media_playlist_dump (playlist);
/* Get the base class to call the update_manifest() vfunc with the MANIFEST_LOCK()
* held */
gst_adaptive_demux2_manual_manifest_update (GST_ADAPTIVE_DEMUX (demux));
} }
void void
@ -1242,6 +1273,10 @@ gst_hls_demux_reset (GstAdaptiveDemux * ademux)
gst_hls_master_playlist_unref (demux->master); gst_hls_master_playlist_unref (demux->master);
demux->master = NULL; demux->master = NULL;
} }
if (demux->main_playlist) {
gst_hls_media_playlist_unref (demux->main_playlist);
demux->main_playlist = NULL;
}
if (demux->current_variant != NULL) { if (demux->current_variant != NULL) {
gst_hls_variant_stream_unref (demux->current_variant); gst_hls_variant_stream_unref (demux->current_variant);
demux->current_variant = NULL; demux->current_variant = NULL;
@ -1325,10 +1360,10 @@ gst_hls_demux_get_live_seek_range (GstAdaptiveDemux * demux, gint64 * start,
GstHLSDemux *hlsdemux = GST_HLS_DEMUX_CAST (demux); GstHLSDemux *hlsdemux = GST_HLS_DEMUX_CAST (demux);
gboolean ret = FALSE; gboolean ret = FALSE;
if (hlsdemux->main_stream && hlsdemux->main_stream->playlist) { if (hlsdemux->main_playlist) {
ret = ret =
gst_hls_media_playlist_get_seek_range (hlsdemux->main_stream->playlist, gst_hls_media_playlist_get_seek_range (hlsdemux->main_playlist,
hlsdemux->main_stream->llhls_enabled, start, stop); hlsdemux->llhls_enabled, start, stop);
} }
return ret; return ret;

View file

@ -92,6 +92,9 @@ struct _GstHLSDemux2
* created at demuxer start based on the input multivariant playlist */ * created at demuxer start based on the input multivariant playlist */
GstHLSMasterPlaylist *master; GstHLSMasterPlaylist *master;
/* A ref to the main playlist, for access from external threads */
GstHLSMediaPlaylist *main_playlist;
GstHLSVariantStream *current_variant; GstHLSVariantStream *current_variant;
/* The variant we're switching to (currently being loaded by the playlist loader) */ /* The variant we're switching to (currently being loaded by the playlist loader) */
GstHLSVariantStream *pending_variant; GstHLSVariantStream *pending_variant;