From 600bab0056a8771ae9cc82a85713528601e7122f Mon Sep 17 00:00:00 2001 From: Jan Schmidt Date: Thu, 18 Jun 2015 23:22:06 +1000 Subject: [PATCH] splitmuxsrc: Fix startup and shutdown races. Fix 2 startup races when things happen too quickly, and 1 at shutdown by holding a ref to the pads in use until the loop functions exit. Handle errors activating file parts and publish them on the bus. https://bugzilla.gnome.org/show_bug.cgi?id=750747 --- gst/multifile/gstsplitmuxpartreader.c | 33 +++++++++------ gst/multifile/gstsplitmuxsrc.c | 79 ++++++++++++++++++++++++++++------- 2 files changed, 84 insertions(+), 28 deletions(-) diff --git a/gst/multifile/gstsplitmuxpartreader.c b/gst/multifile/gstsplitmuxpartreader.c index 6e92c71..54bd6af 100644 --- a/gst/multifile/gstsplitmuxpartreader.c +++ b/gst/multifile/gstsplitmuxpartreader.c @@ -903,9 +903,11 @@ type_found (GstElement * typefind, guint probability, return; } + gst_element_set_locked_state (demux, TRUE); gst_bin_add (GST_BIN_CAST (reader), demux); gst_element_link_pads (reader->typefind, "src", demux, NULL); gst_element_sync_state_with_parent (reader->demux); + gst_element_set_locked_state (demux, FALSE); /* Connect to demux signals */ g_signal_connect (demux, @@ -1025,12 +1027,11 @@ gst_splitmux_part_reader_change_state (GstElement * element, break; } case GST_STATE_CHANGE_READY_TO_PAUSED:{ - g_object_set (reader->src, "location", reader->path, NULL); + /* Hold the splitmux part lock until after the + * parent state change function has finished + * changing the states of things */ SPLITMUX_PART_LOCK (reader); - reader->prep_state = PART_STATE_PREPARING_COLLECT_STREAMS; - gst_splitmux_part_reader_set_flushing_locked (reader, FALSE); - reader->running = TRUE; - SPLITMUX_PART_UNLOCK (reader); + g_object_set (reader->src, "location", reader->path, NULL); break; } case GST_STATE_CHANGE_READY_TO_NULL: @@ -1053,25 +1054,31 @@ gst_splitmux_part_reader_change_state (GstElement * element, } ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition); - if (ret == GST_STATE_CHANGE_FAILURE) + if (ret == GST_STATE_CHANGE_FAILURE) { + if (transition == GST_STATE_CHANGE_READY_TO_PAUSED) { + /* Make sure to release the lock we took above */ + SPLITMUX_PART_UNLOCK (reader); + } goto beach; + } switch (transition) { case GST_STATE_CHANGE_READY_TO_PAUSED: /* Sleep and wait until all streams have been collected, then do the seeks - * to measure the stream lengths */ - SPLITMUX_PART_LOCK (reader); + * to measure the stream lengths. This took the part lock above already... */ + reader->prep_state = PART_STATE_PREPARING_COLLECT_STREAMS; + gst_splitmux_part_reader_set_flushing_locked (reader, FALSE); + reader->running = TRUE; while (reader->prep_state == PART_STATE_PREPARING_COLLECT_STREAMS) { GST_LOG_OBJECT (reader, "Waiting to collect all output streams"); SPLITMUX_PART_WAIT (reader); } - if (reader->prep_state == PART_STATE_PREPARING_MEASURE_STREAMS) + if (reader->prep_state == PART_STATE_PREPARING_MEASURE_STREAMS || + reader->prep_state == PART_STATE_PREPARING_RESET_FOR_READY) { gst_splitmux_part_reader_measure_streams (reader); - else if (reader->prep_state == PART_STATE_PREPARING_RESET_FOR_READY) - reader->prep_state = PART_STATE_READY; - else if (reader->prep_state == PART_STATE_FAILED) + } else if (reader->prep_state == PART_STATE_FAILED) ret = GST_STATE_CHANGE_FAILURE; SPLITMUX_PART_UNLOCK (reader); break; @@ -1256,7 +1263,7 @@ gst_splitmux_part_reader_lookup_pad (GstSplitMuxPartReader * reader, for (cur = g_list_first (reader->pads); cur != NULL; cur = g_list_next (cur)) { GstSplitMuxPartPad *part_pad = SPLITMUX_PART_PAD_CAST (cur->data); if (part_pad->target == target) { - result = (GstPad *) part_pad; + result = (GstPad *) gst_object_ref (part_pad); break; } } diff --git a/gst/multifile/gstsplitmuxsrc.c b/gst/multifile/gstsplitmuxsrc.c index fec3963..b827f43 100644 --- a/gst/multifile/gstsplitmuxsrc.c +++ b/gst/multifile/gstsplitmuxsrc.c @@ -518,9 +518,17 @@ gst_splitmux_pad_loop (GstPad * pad) GstSplitMuxSrc *splitmux = (GstSplitMuxSrc *) gst_pad_get_parent (pad); GstDataQueueItem *item = NULL; GstSplitMuxPartReader *reader = splitpad->reader; - GstPad *part_pad = splitpad->part_pad; + GstPad *part_pad; GstFlowReturn ret; + GST_OBJECT_LOCK (splitpad); + if (splitpad->part_pad == NULL) { + GST_OBJECT_UNLOCK (splitpad); + return; + } + part_pad = gst_object_ref (splitpad->part_pad); + GST_OBJECT_UNLOCK (splitpad); + GST_LOG_OBJECT (splitpad, "Popping data queue item from %" GST_PTR_FORMAT " pad %" GST_PTR_FORMAT, reader, part_pad); ret = gst_splitmux_part_reader_pop (reader, part_pad, &item); @@ -552,6 +560,7 @@ gst_splitmux_pad_loop (GstPad * pad) } g_slice_free (GstDataQueueItem, item); + gst_object_unref (part_pad); gst_object_unref (splitmux); return; @@ -561,11 +570,12 @@ error: ("Error reading part file %s", GST_STR_NULL (reader->path))); flushing: gst_pad_pause_task (pad); + gst_object_unref (part_pad); gst_object_unref (splitmux); return; } -static void +static gboolean gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part) { GList *cur; @@ -573,8 +583,9 @@ gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part) GST_DEBUG_OBJECT (splitmux, "Activating part %d", part); splitmux->cur_part = part; - gst_splitmux_part_reader_activate (splitmux->parts[part], - &splitmux->play_segment); + if (!gst_splitmux_part_reader_activate (splitmux->parts[part], + &splitmux->play_segment)) + return FALSE; SPLITMUX_SRC_PADS_LOCK (splitmux); for (cur = g_list_first (splitmux->pads); @@ -582,6 +593,8 @@ gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part) SplitMuxSrcPad *splitpad = (SplitMuxSrcPad *) (cur->data); splitpad->cur_part = part; splitpad->reader = splitmux->parts[splitpad->cur_part]; + if (splitpad->part_pad) + gst_object_unref (splitpad->part_pad); splitpad->part_pad = gst_splitmux_part_reader_lookup_pad (splitpad->reader, (GstPad *) (splitpad)); @@ -594,6 +607,8 @@ gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part) (GstTaskFunction) gst_splitmux_pad_loop, splitpad, NULL); } SPLITMUX_SRC_PADS_UNLOCK (splitmux); + + return TRUE; } static gboolean @@ -623,6 +638,7 @@ gst_splitmux_src_start (GstSplitMuxSrc * splitmux) goto no_files; SPLITMUX_SRC_LOCK (splitmux); + splitmux->pads_complete = FALSE; splitmux->running = TRUE; SPLITMUX_SRC_UNLOCK (splitmux); @@ -673,9 +689,9 @@ gst_splitmux_src_start (GstSplitMuxSrc * splitmux) GST_INFO_OBJECT (splitmux, "All parts prepared. Total duration %" GST_TIME_FORMAT " Activating first part", GST_TIME_ARGS (total_duration)); - gst_splitmux_src_activate_part (splitmux, 0); - - ret = TRUE; + ret = gst_splitmux_src_activate_part (splitmux, 0); + if (ret == FALSE) + goto failed_first_part; done: if (err != NULL) g_error_free (err); @@ -699,6 +715,12 @@ failed_part: ("Failed to open any files for reading")); goto done; } +failed_first_part: + { + GST_ELEMENT_ERROR (splitmux, RESOURCE, OPEN_READ, (NULL), + ("Failed to activate first part for playback")); + goto done; + } } static gboolean @@ -706,7 +728,7 @@ gst_splitmux_src_stop (GstSplitMuxSrc * splitmux) { gboolean ret = TRUE; guint i; - GList *cur; + GList *cur, *pads_list; SPLITMUX_SRC_LOCK (splitmux); if (!splitmux->running) @@ -724,12 +746,16 @@ gst_splitmux_src_stop (GstSplitMuxSrc * splitmux) } SPLITMUX_SRC_PADS_LOCK (splitmux); - for (cur = g_list_first (splitmux->pads); - cur != NULL; cur = g_list_next (cur)) { + pads_list = splitmux->pads; + splitmux->pads = NULL; + SPLITMUX_SRC_PADS_UNLOCK (splitmux); + + for (cur = g_list_first (pads_list); cur != NULL; cur = g_list_next (cur)) { SplitMuxSrcPad *tmp = (SplitMuxSrcPad *) (cur->data); gst_pad_stop_task (GST_PAD (tmp)); + gst_element_remove_pad (GST_ELEMENT (splitmux), GST_PAD (tmp)); } - SPLITMUX_SRC_PADS_UNLOCK (splitmux); + g_list_free (pads_list); g_free (splitmux->parts); splitmux->parts = NULL; @@ -798,6 +824,7 @@ gst_splitmux_part_prepared (GstSplitMuxPartReader * reader, { gboolean need_no_more_pads; + GST_LOG_OBJECT (splitmux, "Part %" GST_PTR_FORMAT " prepared", reader); SPLITMUX_SRC_LOCK (splitmux); need_no_more_pads = !splitmux->pads_complete; splitmux->pads_complete = TRUE; @@ -890,6 +917,8 @@ gst_splitmux_end_of_part (GstSplitMuxSrc * splitmux, SplitMuxSrcPad * splitpad) " moving to part %d", splitpad, next_part); splitpad->cur_part = next_part; splitpad->reader = splitmux->parts[splitpad->cur_part]; + if (splitpad->part_pad) + gst_object_unref (splitpad->part_pad); splitpad->part_pad = gst_splitmux_part_reader_lookup_pad (splitpad->reader, (GstPad *) (splitpad)); @@ -908,13 +937,19 @@ gst_splitmux_end_of_part (GstSplitMuxSrc * splitmux, SplitMuxSrcPad * splitpad) GST_DEBUG_OBJECT (splitpad, "First pad to change part. Activating part %d with seg %" GST_SEGMENT_FORMAT, next_part, &tmp); - gst_splitmux_part_reader_activate (splitpad->reader, &tmp); + if (!gst_splitmux_part_reader_activate (splitpad->reader, &tmp)) + goto error; } res = TRUE; } SPLITMUX_SRC_UNLOCK (splitmux); return res; +error: + SPLITMUX_SRC_UNLOCK (splitmux); + GST_ELEMENT_ERROR (splitmux, RESOURCE, READ, (NULL), + ("Failed to activate part %d", splitmux->cur_part)); + return FALSE; } G_DEFINE_TYPE (SplitMuxSrcPad, splitmux_src_pad, GST_TYPE_PAD); @@ -931,11 +966,27 @@ splitmux_src_pad_constructed (GObject * pad) } static void +gst_splitmux_src_pad_dispose (GObject * object) +{ + SplitMuxSrcPad *pad = (SplitMuxSrcPad *) (object); + + GST_OBJECT_LOCK (pad); + if (pad->part_pad) { + gst_object_unref (pad->part_pad); + pad->part_pad = NULL; + } + GST_OBJECT_UNLOCK (pad); + + G_OBJECT_CLASS (splitmux_src_pad_parent_class)->dispose (object); +} + +static void splitmux_src_pad_class_init (SplitMuxSrcPadClass * klass) { GObjectClass *gobject_klass = (GObjectClass *) (klass); gobject_klass->constructed = splitmux_src_pad_constructed; + gobject_klass->dispose = gst_splitmux_src_pad_dispose; } static void @@ -1055,9 +1106,7 @@ splitmux_src_pad_event (GstPad * pad, GstObject * parent, GstEvent * event) GST_TIME_FORMAT, GST_TIME_ARGS (position), i, GST_TIME_ARGS (position - part_start)); - gst_splitmux_src_activate_part (splitmux, i); - - ret = TRUE; + ret = gst_splitmux_src_activate_part (splitmux, i); SPLITMUX_SRC_UNLOCK (splitmux); } default: -- 2.7.4