From 8dd42666e34d3d90eacfcb7d9f164de821d954ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alicia=20Boya=20Garc=C3=ADa?= Date: Thu, 9 Jan 2020 18:43:02 +0000 Subject: [PATCH] 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. --- gst/isomp4/qtdemux.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/gst/isomp4/qtdemux.c b/gst/isomp4/qtdemux.c index aa0fa6a58c..ff2345837b 100644 --- a/gst/isomp4/qtdemux.c +++ b/gst/isomp4/qtdemux.c @@ -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);