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
This commit is contained in:
Jan Schmidt 2015-06-18 23:22:06 +10:00
parent d61e5393f1
commit 600bab0056
2 changed files with 84 additions and 28 deletions

View file

@ -903,9 +903,11 @@ type_found (GstElement * typefind, guint probability,
return; return;
} }
gst_element_set_locked_state (demux, TRUE);
gst_bin_add (GST_BIN_CAST (reader), demux); gst_bin_add (GST_BIN_CAST (reader), demux);
gst_element_link_pads (reader->typefind, "src", demux, NULL); gst_element_link_pads (reader->typefind, "src", demux, NULL);
gst_element_sync_state_with_parent (reader->demux); gst_element_sync_state_with_parent (reader->demux);
gst_element_set_locked_state (demux, FALSE);
/* Connect to demux signals */ /* Connect to demux signals */
g_signal_connect (demux, g_signal_connect (demux,
@ -1025,12 +1027,11 @@ gst_splitmux_part_reader_change_state (GstElement * element,
break; break;
} }
case GST_STATE_CHANGE_READY_TO_PAUSED:{ 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); SPLITMUX_PART_LOCK (reader);
reader->prep_state = PART_STATE_PREPARING_COLLECT_STREAMS; g_object_set (reader->src, "location", reader->path, NULL);
gst_splitmux_part_reader_set_flushing_locked (reader, FALSE);
reader->running = TRUE;
SPLITMUX_PART_UNLOCK (reader);
break; break;
} }
case GST_STATE_CHANGE_READY_TO_NULL: 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); 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; goto beach;
}
switch (transition) { switch (transition) {
case GST_STATE_CHANGE_READY_TO_PAUSED: case GST_STATE_CHANGE_READY_TO_PAUSED:
/* Sleep and wait until all streams have been collected, then do the seeks /* Sleep and wait until all streams have been collected, then do the seeks
* to measure the stream lengths */ * to measure the stream lengths. This took the part lock above already... */
SPLITMUX_PART_LOCK (reader); 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) { while (reader->prep_state == PART_STATE_PREPARING_COLLECT_STREAMS) {
GST_LOG_OBJECT (reader, "Waiting to collect all output streams"); GST_LOG_OBJECT (reader, "Waiting to collect all output streams");
SPLITMUX_PART_WAIT (reader); 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); gst_splitmux_part_reader_measure_streams (reader);
else if (reader->prep_state == PART_STATE_PREPARING_RESET_FOR_READY) } else if (reader->prep_state == PART_STATE_FAILED)
reader->prep_state = PART_STATE_READY;
else if (reader->prep_state == PART_STATE_FAILED)
ret = GST_STATE_CHANGE_FAILURE; ret = GST_STATE_CHANGE_FAILURE;
SPLITMUX_PART_UNLOCK (reader); SPLITMUX_PART_UNLOCK (reader);
break; 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)) { for (cur = g_list_first (reader->pads); cur != NULL; cur = g_list_next (cur)) {
GstSplitMuxPartPad *part_pad = SPLITMUX_PART_PAD_CAST (cur->data); GstSplitMuxPartPad *part_pad = SPLITMUX_PART_PAD_CAST (cur->data);
if (part_pad->target == target) { if (part_pad->target == target) {
result = (GstPad *) part_pad; result = (GstPad *) gst_object_ref (part_pad);
break; break;
} }
} }

View file

@ -518,9 +518,17 @@ gst_splitmux_pad_loop (GstPad * pad)
GstSplitMuxSrc *splitmux = (GstSplitMuxSrc *) gst_pad_get_parent (pad); GstSplitMuxSrc *splitmux = (GstSplitMuxSrc *) gst_pad_get_parent (pad);
GstDataQueueItem *item = NULL; GstDataQueueItem *item = NULL;
GstSplitMuxPartReader *reader = splitpad->reader; GstSplitMuxPartReader *reader = splitpad->reader;
GstPad *part_pad = splitpad->part_pad; GstPad *part_pad;
GstFlowReturn ret; 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 GST_LOG_OBJECT (splitpad, "Popping data queue item from %" GST_PTR_FORMAT
" pad %" GST_PTR_FORMAT, reader, part_pad); " pad %" GST_PTR_FORMAT, reader, part_pad);
ret = gst_splitmux_part_reader_pop (reader, part_pad, &item); 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); g_slice_free (GstDataQueueItem, item);
gst_object_unref (part_pad);
gst_object_unref (splitmux); gst_object_unref (splitmux);
return; return;
@ -561,11 +570,12 @@ error:
("Error reading part file %s", GST_STR_NULL (reader->path))); ("Error reading part file %s", GST_STR_NULL (reader->path)));
flushing: flushing:
gst_pad_pause_task (pad); gst_pad_pause_task (pad);
gst_object_unref (part_pad);
gst_object_unref (splitmux); gst_object_unref (splitmux);
return; return;
} }
static void static gboolean
gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part) gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part)
{ {
GList *cur; GList *cur;
@ -573,8 +583,9 @@ gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part)
GST_DEBUG_OBJECT (splitmux, "Activating part %d", part); GST_DEBUG_OBJECT (splitmux, "Activating part %d", part);
splitmux->cur_part = part; splitmux->cur_part = part;
gst_splitmux_part_reader_activate (splitmux->parts[part], if (!gst_splitmux_part_reader_activate (splitmux->parts[part],
&splitmux->play_segment); &splitmux->play_segment))
return FALSE;
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_LOCK (splitmux);
for (cur = g_list_first (splitmux->pads); 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); SplitMuxSrcPad *splitpad = (SplitMuxSrcPad *) (cur->data);
splitpad->cur_part = part; splitpad->cur_part = part;
splitpad->reader = splitmux->parts[splitpad->cur_part]; splitpad->reader = splitmux->parts[splitpad->cur_part];
if (splitpad->part_pad)
gst_object_unref (splitpad->part_pad);
splitpad->part_pad = splitpad->part_pad =
gst_splitmux_part_reader_lookup_pad (splitpad->reader, gst_splitmux_part_reader_lookup_pad (splitpad->reader,
(GstPad *) (splitpad)); (GstPad *) (splitpad));
@ -594,6 +607,8 @@ gst_splitmux_src_activate_part (GstSplitMuxSrc * splitmux, guint part)
(GstTaskFunction) gst_splitmux_pad_loop, splitpad, NULL); (GstTaskFunction) gst_splitmux_pad_loop, splitpad, NULL);
} }
SPLITMUX_SRC_PADS_UNLOCK (splitmux); SPLITMUX_SRC_PADS_UNLOCK (splitmux);
return TRUE;
} }
static gboolean static gboolean
@ -623,6 +638,7 @@ gst_splitmux_src_start (GstSplitMuxSrc * splitmux)
goto no_files; goto no_files;
SPLITMUX_SRC_LOCK (splitmux); SPLITMUX_SRC_LOCK (splitmux);
splitmux->pads_complete = FALSE;
splitmux->running = TRUE; splitmux->running = TRUE;
SPLITMUX_SRC_UNLOCK (splitmux); SPLITMUX_SRC_UNLOCK (splitmux);
@ -673,9 +689,9 @@ gst_splitmux_src_start (GstSplitMuxSrc * splitmux)
GST_INFO_OBJECT (splitmux, GST_INFO_OBJECT (splitmux,
"All parts prepared. Total duration %" GST_TIME_FORMAT "All parts prepared. Total duration %" GST_TIME_FORMAT
" Activating first part", GST_TIME_ARGS (total_duration)); " Activating first part", GST_TIME_ARGS (total_duration));
gst_splitmux_src_activate_part (splitmux, 0); ret = gst_splitmux_src_activate_part (splitmux, 0);
if (ret == FALSE)
ret = TRUE; goto failed_first_part;
done: done:
if (err != NULL) if (err != NULL)
g_error_free (err); g_error_free (err);
@ -699,6 +715,12 @@ failed_part:
("Failed to open any files for reading")); ("Failed to open any files for reading"));
goto done; goto done;
} }
failed_first_part:
{
GST_ELEMENT_ERROR (splitmux, RESOURCE, OPEN_READ, (NULL),
("Failed to activate first part for playback"));
goto done;
}
} }
static gboolean static gboolean
@ -706,7 +728,7 @@ gst_splitmux_src_stop (GstSplitMuxSrc * splitmux)
{ {
gboolean ret = TRUE; gboolean ret = TRUE;
guint i; guint i;
GList *cur; GList *cur, *pads_list;
SPLITMUX_SRC_LOCK (splitmux); SPLITMUX_SRC_LOCK (splitmux);
if (!splitmux->running) if (!splitmux->running)
@ -724,12 +746,16 @@ gst_splitmux_src_stop (GstSplitMuxSrc * splitmux)
} }
SPLITMUX_SRC_PADS_LOCK (splitmux); SPLITMUX_SRC_PADS_LOCK (splitmux);
for (cur = g_list_first (splitmux->pads); pads_list = splitmux->pads;
cur != NULL; cur = g_list_next (cur)) { 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); SplitMuxSrcPad *tmp = (SplitMuxSrcPad *) (cur->data);
gst_pad_stop_task (GST_PAD (tmp)); 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); g_free (splitmux->parts);
splitmux->parts = NULL; splitmux->parts = NULL;
@ -798,6 +824,7 @@ gst_splitmux_part_prepared (GstSplitMuxPartReader * reader,
{ {
gboolean need_no_more_pads; gboolean need_no_more_pads;
GST_LOG_OBJECT (splitmux, "Part %" GST_PTR_FORMAT " prepared", reader);
SPLITMUX_SRC_LOCK (splitmux); SPLITMUX_SRC_LOCK (splitmux);
need_no_more_pads = !splitmux->pads_complete; need_no_more_pads = !splitmux->pads_complete;
splitmux->pads_complete = TRUE; splitmux->pads_complete = TRUE;
@ -890,6 +917,8 @@ gst_splitmux_end_of_part (GstSplitMuxSrc * splitmux, SplitMuxSrcPad * splitpad)
" moving to part %d", splitpad, next_part); " moving to part %d", splitpad, next_part);
splitpad->cur_part = next_part; splitpad->cur_part = next_part;
splitpad->reader = splitmux->parts[splitpad->cur_part]; splitpad->reader = splitmux->parts[splitpad->cur_part];
if (splitpad->part_pad)
gst_object_unref (splitpad->part_pad);
splitpad->part_pad = splitpad->part_pad =
gst_splitmux_part_reader_lookup_pad (splitpad->reader, gst_splitmux_part_reader_lookup_pad (splitpad->reader,
(GstPad *) (splitpad)); (GstPad *) (splitpad));
@ -908,13 +937,19 @@ gst_splitmux_end_of_part (GstSplitMuxSrc * splitmux, SplitMuxSrcPad * splitpad)
GST_DEBUG_OBJECT (splitpad, GST_DEBUG_OBJECT (splitpad,
"First pad to change part. Activating part %d with seg %" "First pad to change part. Activating part %d with seg %"
GST_SEGMENT_FORMAT, next_part, &tmp); 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; res = TRUE;
} }
SPLITMUX_SRC_UNLOCK (splitmux); SPLITMUX_SRC_UNLOCK (splitmux);
return res; 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); G_DEFINE_TYPE (SplitMuxSrcPad, splitmux_src_pad, GST_TYPE_PAD);
@ -930,12 +965,28 @@ splitmux_src_pad_constructed (GObject * pad)
G_OBJECT_CLASS (splitmux_src_pad_parent_class)->constructed (pad); G_OBJECT_CLASS (splitmux_src_pad_parent_class)->constructed (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 static void
splitmux_src_pad_class_init (SplitMuxSrcPadClass * klass) splitmux_src_pad_class_init (SplitMuxSrcPadClass * klass)
{ {
GObjectClass *gobject_klass = (GObjectClass *) (klass); GObjectClass *gobject_klass = (GObjectClass *) (klass);
gobject_klass->constructed = splitmux_src_pad_constructed; gobject_klass->constructed = splitmux_src_pad_constructed;
gobject_klass->dispose = gst_splitmux_src_pad_dispose;
} }
static void static void
@ -1055,9 +1106,7 @@ splitmux_src_pad_event (GstPad * pad, GstObject * parent, GstEvent * event)
GST_TIME_FORMAT, GST_TIME_ARGS (position), GST_TIME_FORMAT, GST_TIME_ARGS (position),
i, GST_TIME_ARGS (position - part_start)); i, GST_TIME_ARGS (position - part_start));
gst_splitmux_src_activate_part (splitmux, i); ret = gst_splitmux_src_activate_part (splitmux, i);
ret = TRUE;
SPLITMUX_SRC_UNLOCK (splitmux); SPLITMUX_SRC_UNLOCK (splitmux);
} }
default: default: