diff options
author | Edward Hervey <edward@centricular.com> | 2023-03-07 11:40:42 +0100 |
---|---|---|
committer | Tim-Philipp Müller <tim@centricular.com> | 2023-05-12 14:53:06 +0100 |
commit | 0956b94184f2e0fad61a17620faac6259ff0da88 (patch) | |
tree | 81db9d36c3ebb535c23babb92ea7182905035342 | |
parent | c95b7b8e7aa80276c0604dd3dd03eeda634f42b9 (diff) | |
download | gstreamer-0956b94184f2e0fad61a17620faac6259ff0da88.tar.gz |
uridecodebin3: Ensure atomic urisourcebin state change
When dynamically adding and synchronizing the state of urisourcebin, we need to
ensure that no-one else attempts to change the state in case of failures
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1803
Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4620>
-rw-r--r-- | subprojects/gst-plugins-base/gst/playback/gsturidecodebin3.c | 29 |
1 files changed, 22 insertions, 7 deletions
diff --git a/subprojects/gst-plugins-base/gst/playback/gsturidecodebin3.c b/subprojects/gst-plugins-base/gst/playback/gsturidecodebin3.c index 8f7181c493..6d21a22d0b 100644 --- a/subprojects/gst-plugins-base/gst/playback/gsturidecodebin3.c +++ b/subprojects/gst-plugins-base/gst/playback/gsturidecodebin3.c @@ -370,7 +370,7 @@ static void gst_uri_decode_bin3_dispose (GObject * obj); static GstSourceHandler *new_source_handler (GstURIDecodeBin3 * uridecodebin, GstPlayItem * item, gboolean is_main); static void free_source_handler (GstURIDecodeBin3 * uridecodebin, - GstSourceHandler * item); + GstSourceHandler * item, gboolean lock_state); static void free_source_item (GstURIDecodeBin3 * uridecodebin, GstSourceItem * item); @@ -976,14 +976,21 @@ link_src_pad_to_db3 (GstURIDecodeBin3 * uridecodebin, GstSourcePad * spad) if (handler->is_main_source && handler->play_item->sub_item && !handler->play_item->sub_item->handler) { GstStateChangeReturn ret; + + /* The state lock is taken to ensure we can atomically change the + * urisourcebin back to NULL in case of failures */ + GST_STATE_LOCK (uridecodebin); handler->play_item->sub_item->handler = new_source_handler (uridecodebin, handler->play_item, FALSE); ret = activate_source_item (handler->play_item->sub_item); if (ret == GST_STATE_CHANGE_FAILURE) { - free_source_handler (uridecodebin, handler->play_item->sub_item->handler); + free_source_handler (uridecodebin, handler->play_item->sub_item->handler, + FALSE); handler->play_item->sub_item->handler = NULL; + GST_STATE_UNLOCK (uridecodebin); goto sub_item_activation_failed; } + GST_STATE_UNLOCK (uridecodebin); } return; @@ -1631,14 +1638,17 @@ gst_uri_decode_bin3_get_property (GObject * object, guint prop_id, } } +/* lock_state: TRUE if the STATE LOCK should be taken. Set to FALSE if the + * caller already has taken it */ static void free_source_handler (GstURIDecodeBin3 * uridecodebin, - GstSourceHandler * handler) + GstSourceHandler * handler, gboolean lock_state) { GST_LOG_OBJECT (uridecodebin, "source handler %p", handler); if (handler->active) { GList *iter; - GST_STATE_LOCK (uridecodebin); + if (lock_state) + GST_STATE_LOCK (uridecodebin); GST_LOG_OBJECT (uridecodebin, "Removing %" GST_PTR_FORMAT, handler->urisourcebin); for (iter = handler->sourcepads; iter; iter = iter->next) { @@ -1648,7 +1658,8 @@ free_source_handler (GstURIDecodeBin3 * uridecodebin, } gst_element_set_state (handler->urisourcebin, GST_STATE_NULL); gst_bin_remove ((GstBin *) uridecodebin, handler->urisourcebin); - GST_STATE_UNLOCK (uridecodebin); + if (lock_state) + GST_STATE_UNLOCK (uridecodebin); g_list_free (handler->sourcepads); } if (handler->pending_buffering_msg) @@ -1672,7 +1683,7 @@ free_source_item (GstURIDecodeBin3 * uridecodebin, GstSourceItem * item) { GST_LOG_OBJECT (uridecodebin, "source item %p", item); if (item->handler) - free_source_handler (uridecodebin, item->handler); + free_source_handler (uridecodebin, item->handler, TRUE); g_free (item->uri); g_slice_free (GstSourceItem, item); } @@ -1899,12 +1910,16 @@ assign_handlers_to_item (GstURIDecodeBin3 * dec, GstPlayItem * item) /* Create missing handlers */ if (item->main_item->handler == NULL) { + /* The state lock is taken to ensure we can atomically change the + * urisourcebin back to NULL in case of failures */ + GST_STATE_LOCK (dec); item->main_item->handler = new_source_handler (dec, item, TRUE); ret = activate_source_item (item->main_item); if (ret == GST_STATE_CHANGE_FAILURE) { - free_source_handler (dec, item->main_item->handler); + free_source_handler (dec, item->main_item->handler, FALSE); item->main_item->handler = NULL; } + GST_STATE_UNLOCK (dec); } return ret; |