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/4125>
This commit is contained in:
Edward Hervey 2023-03-07 11:40:42 +01:00 committed by GStreamer Marge Bot
parent 224030ff0c
commit b4cadf35bd

View file

@ -370,7 +370,7 @@ static void gst_uri_decode_bin3_dispose (GObject * obj);
static GstSourceHandler *new_source_handler (GstURIDecodeBin3 * uridecodebin, static GstSourceHandler *new_source_handler (GstURIDecodeBin3 * uridecodebin,
GstPlayItem * item, gboolean is_main); GstPlayItem * item, gboolean is_main);
static void free_source_handler (GstURIDecodeBin3 * uridecodebin, static void free_source_handler (GstURIDecodeBin3 * uridecodebin,
GstSourceHandler * item); GstSourceHandler * item, gboolean lock_state);
static void free_source_item (GstURIDecodeBin3 * uridecodebin, static void free_source_item (GstURIDecodeBin3 * uridecodebin,
GstSourceItem * item); 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 if (handler->is_main_source && handler->play_item->sub_item
&& !handler->play_item->sub_item->handler) { && !handler->play_item->sub_item->handler) {
GstStateChangeReturn ret; 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 = handler->play_item->sub_item->handler =
new_source_handler (uridecodebin, handler->play_item, FALSE); new_source_handler (uridecodebin, handler->play_item, FALSE);
ret = activate_source_item (handler->play_item->sub_item); ret = activate_source_item (handler->play_item->sub_item);
if (ret == GST_STATE_CHANGE_FAILURE) { 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; handler->play_item->sub_item->handler = NULL;
GST_STATE_UNLOCK (uridecodebin);
goto sub_item_activation_failed; goto sub_item_activation_failed;
} }
GST_STATE_UNLOCK (uridecodebin);
} }
return; 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 static void
free_source_handler (GstURIDecodeBin3 * uridecodebin, free_source_handler (GstURIDecodeBin3 * uridecodebin,
GstSourceHandler * handler) GstSourceHandler * handler, gboolean lock_state)
{ {
GST_LOG_OBJECT (uridecodebin, "source handler %p", handler); GST_LOG_OBJECT (uridecodebin, "source handler %p", handler);
if (handler->active) { if (handler->active) {
GList *iter; GList *iter;
GST_STATE_LOCK (uridecodebin); if (lock_state)
GST_STATE_LOCK (uridecodebin);
GST_LOG_OBJECT (uridecodebin, "Removing %" GST_PTR_FORMAT, GST_LOG_OBJECT (uridecodebin, "Removing %" GST_PTR_FORMAT,
handler->urisourcebin); handler->urisourcebin);
for (iter = handler->sourcepads; iter; iter = iter->next) { 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_element_set_state (handler->urisourcebin, GST_STATE_NULL);
gst_bin_remove ((GstBin *) uridecodebin, handler->urisourcebin); gst_bin_remove ((GstBin *) uridecodebin, handler->urisourcebin);
GST_STATE_UNLOCK (uridecodebin); if (lock_state)
GST_STATE_UNLOCK (uridecodebin);
g_list_free (handler->sourcepads); g_list_free (handler->sourcepads);
} }
if (handler->pending_buffering_msg) if (handler->pending_buffering_msg)
@ -1672,7 +1683,7 @@ free_source_item (GstURIDecodeBin3 * uridecodebin, GstSourceItem * item)
{ {
GST_LOG_OBJECT (uridecodebin, "source item %p", item); GST_LOG_OBJECT (uridecodebin, "source item %p", item);
if (item->handler) if (item->handler)
free_source_handler (uridecodebin, item->handler); free_source_handler (uridecodebin, item->handler, TRUE);
g_free (item->uri); g_free (item->uri);
g_free (item); g_free (item);
} }
@ -1899,12 +1910,16 @@ assign_handlers_to_item (GstURIDecodeBin3 * dec, GstPlayItem * item)
/* Create missing handlers */ /* Create missing handlers */
if (item->main_item->handler == NULL) { 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); item->main_item->handler = new_source_handler (dec, item, TRUE);
ret = activate_source_item (item->main_item); ret = activate_source_item (item->main_item);
if (ret == GST_STATE_CHANGE_FAILURE) { 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; item->main_item->handler = NULL;
} }
GST_STATE_UNLOCK (dec);
} }
return ret; return ret;