qtdemux: Fix race on pad reconnection

Elements emitting frames through several srcpads should use a
flow combiner to aggregate the chain returns and therefore only return
GST_FLOW_NOT_LINKED to upstream when all the downstream pads have
received GST_FLOW_NOT_LINKED.

In addition to that, in order to handle pads being relinked downstream,
the flow combiner should be reset in response to RECONFIGURE events.
This ensures that a both srcpads process a chain operation before a
GST_FLOW_NOT_LINKED can be propagated upstream (which would usually stop
the pipeline).

Otherwise, in a configuration with two srcpads, only one linked at a
time, after the relink the element could chain data through the now
unlinked pad and the flow combiner would resolve as GST_FLOW_NOT_LINKED
(stopping the pipeline) just because the now linked pad has not been
chained yet to update the flow combiner.

This patch adds handling of RECONFIGURE events to qtdemux. Also, since
this event handling causes the flow combiner to be used from a thread
other than the qtdemux streaming thread, usages of the flow combiner
has been guarded by the object lock.
This commit is contained in:
Alicia Boya García 2020-01-09 18:43:02 +00:00 committed by Thibault Saunier
parent 8445685a21
commit 8dd42666e3

View file

@ -1614,7 +1614,9 @@ gst_qtdemux_perform_seek (GstQTDemux * qtdemux, GstSegment * segment,
}
/* and set all streams to the final position */
GST_OBJECT_LOCK (qtdemux);
gst_flow_combiner_reset (qtdemux->flowcombiner);
GST_OBJECT_UNLOCK (qtdemux);
qtdemux->segment_seqnum = seqnum;
for (i = 0; i < QTDEMUX_N_STREAMS (qtdemux); i++) {
QtDemuxStream *stream = QTDEMUX_NTH_STREAM (qtdemux, i);
@ -1780,6 +1782,12 @@ gst_qtdemux_handle_src_event (GstPad * pad, GstObject * parent,
GstQTDemux *qtdemux = GST_QTDEMUX (parent);
switch (GST_EVENT_TYPE (event)) {
case GST_EVENT_RECONFIGURE:
GST_OBJECT_LOCK (qtdemux);
gst_flow_combiner_reset (qtdemux->flowcombiner);
GST_OBJECT_UNLOCK (qtdemux);
res = gst_pad_event_default (pad, parent, event);
break;
case GST_EVENT_SEEK:
{
#ifndef GST_DISABLE_GST_DEBUG
@ -2738,7 +2746,9 @@ gst_qtdemux_stream_unref (QtDemuxStream * stream)
if (stream->pad) {
GstQTDemux *demux = stream->demux;
gst_element_remove_pad (GST_ELEMENT_CAST (demux), stream->pad);
GST_OBJECT_LOCK (demux);
gst_flow_combiner_remove_pad (demux->flowcombiner, stream->pad);
GST_OBJECT_UNLOCK (demux);
}
g_free (stream->stream_id);
g_free (stream);
@ -6533,7 +6543,9 @@ gst_qtdemux_loop_state_movie (GstQTDemux * qtdemux)
}
/* combine flows */
GST_OBJECT_LOCK (qtdemux);
ret = gst_qtdemux_combine_flows (qtdemux, stream, ret);
GST_OBJECT_UNLOCK (qtdemux);
/* ignore unlinked, we will not push on the pad anymore and we will EOS when
* we have no more data for the pad to push */
if (ret == GST_FLOW_EOS)
@ -7563,7 +7575,9 @@ gst_qtdemux_process_adapter (GstQTDemux * demux, gboolean force)
}
/* combine flows */
GST_OBJECT_LOCK (demux);
ret = gst_qtdemux_combine_flows (demux, stream, ret);
GST_OBJECT_UNLOCK (demux);
} else {
/* skip this data, stream is EOS */
gst_adapter_flush (demux->adapter, demux->neededbytes);
@ -8953,7 +8967,9 @@ gst_qtdemux_add_stream (GstQTDemux * qtdemux,
GST_DEBUG_OBJECT (qtdemux, "adding pad %s %p to qtdemux %p",
GST_OBJECT_NAME (stream->pad), stream->pad, qtdemux);
gst_element_add_pad (GST_ELEMENT_CAST (qtdemux), stream->pad);
GST_OBJECT_LOCK (qtdemux);
gst_flow_combiner_add_pad (qtdemux->flowcombiner, stream->pad);
GST_OBJECT_UNLOCK (qtdemux);
if (stream->stream_tags)
gst_tag_list_unref (stream->stream_tags);