From e81ce6f2d722095b25d98a33de2d9431849b750c Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Fri, 31 Jul 2020 16:47:37 +1000 Subject: [PATCH] qtmux: properly support initial caps nego failure Scenario: - gap event causes h264parse to push made up caps that may fail checks inside qtmux (e.g missing codec_data). - the caps event has already been marked as received and is sticky on the sink pad - gst_qt_mux_pad_can_renegotiate() will retrieve the failed caps event using gst_pad_get_current_caps() and reject the correct updated caps with codec_data. - Failure! Keep track of the configured caps ourselves instead of relying on the sticky event on the pad. Part-of: --- gst/isomp4/gstqtmux.c | 30 ++++++++++++++++++--------- gst/isomp4/gstqtmux.h | 2 ++ tests/check/elements/qtmux.c | 49 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/gst/isomp4/gstqtmux.c b/gst/isomp4/gstqtmux.c index c2f1474..15eb3f2 100644 --- a/gst/isomp4/gstqtmux.c +++ b/gst/isomp4/gstqtmux.c @@ -660,6 +660,8 @@ gst_qt_mux_pad_reset (GstQTMuxPad * qtpad) gst_buffer_replace (&qtpad->last_buf, NULL); + gst_caps_replace (&qtpad->configured_caps, NULL); + if (qtpad->tags) { gst_tag_list_unref (qtpad->tags); qtpad->tags = NULL; @@ -5569,7 +5571,8 @@ find_best_pad (GstQTMux * qtmux) gboolean push_stored = FALSE; GST_OBJECT_LOCK (qtmux); - if (GST_ELEMENT (qtmux)->sinkpads->next || qtmux->force_chunks) { + if ((GST_ELEMENT (qtmux)->sinkpads && GST_ELEMENT (qtmux)->sinkpads->next) + || qtmux->force_chunks) { /* Only switch pads if we have more than one, otherwise * we can just put everything into a single chunk and save * a few bytes of offsets. @@ -5734,30 +5737,30 @@ gst_qtmux_caps_is_subset_full (GstQTMux * qtmux, GstCaps * subset, static gboolean gst_qt_mux_can_renegotiate (GstQTMux * qtmux, GstPad * pad, GstCaps * caps) { - GstCaps *current_caps; + GstQTMuxPad *qtmuxpad = GST_QT_MUX_PAD_CAST (pad); /* does not go well to renegotiate stream mid-way, unless * the old caps are a subset of the new one (this means upstream * added more info to the caps, as both should be 'fixed' caps) */ - current_caps = gst_pad_get_current_caps (pad); - if (!current_caps) + if (!qtmuxpad->configured_caps) { + GST_DEBUG_OBJECT (qtmux, "pad %s accepted caps %" GST_PTR_FORMAT, + GST_PAD_NAME (pad), caps); return TRUE; + } g_assert (caps != NULL); - if (!gst_qtmux_caps_is_subset_full (qtmux, current_caps, caps)) { - gst_caps_unref (current_caps); + if (!gst_qtmux_caps_is_subset_full (qtmux, qtmuxpad->configured_caps, caps)) { GST_WARNING_OBJECT (qtmux, - "pad %s refused renegotiation to %" GST_PTR_FORMAT, - GST_PAD_NAME (pad), caps); + "pad %s refused renegotiation to %" GST_PTR_FORMAT " from %" + GST_PTR_FORMAT, GST_PAD_NAME (pad), caps, qtmuxpad->configured_caps); return FALSE; } GST_DEBUG_OBJECT (qtmux, "pad %s accepted renegotiation to %" GST_PTR_FORMAT " from %" - GST_PTR_FORMAT, GST_PAD_NAME (pad), caps, current_caps); - gst_caps_unref (current_caps); + GST_PTR_FORMAT, GST_PAD_NAME (pad), caps, qtmuxpad->configured_caps); return TRUE; } @@ -6827,6 +6830,10 @@ gst_qt_mux_sink_event (GstAggregator * agg, GstAggregatorPad * agg_pad, g_assert (qtmux_pad->set_caps); ret = qtmux_pad->set_caps (qtmux_pad, caps); + + if (ret) + gst_caps_replace (&qtmux_pad->configured_caps, caps); + gst_event_unref (event); event = NULL; break; @@ -6893,9 +6900,12 @@ static void gst_qt_mux_release_pad (GstElement * element, GstPad * pad) { GstQTMux *mux = GST_QT_MUX_CAST (element); + GstQTMuxPad *muxpad = GST_QT_MUX_PAD_CAST (pad); GST_DEBUG_OBJECT (element, "Releasing %s:%s", GST_DEBUG_PAD_NAME (pad)); + gst_qt_mux_pad_reset (muxpad); + gst_element_remove_pad (element, pad); if (mux->current_pad && GST_PAD (mux->current_pad) == pad) { diff --git a/gst/isomp4/gstqtmux.h b/gst/isomp4/gstqtmux.h index 3778632..f403885 100644 --- a/gst/isomp4/gstqtmux.h +++ b/gst/isomp4/gstqtmux.h @@ -177,6 +177,8 @@ struct _GstQTMuxPad guint64 raw_audio_adapter_offset; GstClockTime raw_audio_adapter_pts; GstFlowReturn flow_status; + + GstCaps *configured_caps; }; struct _GstQTMuxPadClass diff --git a/tests/check/elements/qtmux.c b/tests/check/elements/qtmux.c index 77c2af9..e613d2b 100644 --- a/tests/check/elements/qtmux.c +++ b/tests/check/elements/qtmux.c @@ -27,6 +27,7 @@ #include #include +#include #include /* For ease of programming we use globals to keep refs for our floating @@ -1806,6 +1807,52 @@ GST_START_TEST (test_muxing_initial_gap) GST_END_TEST; +GST_START_TEST (test_caps_renego) +{ + GstHarness *h; + GstBuffer *buf; + GstCaps *caps; + GstSegment segment; + GstPad *pad; + + h = gst_harness_new_with_padnames ("qtmux", "video_0", "src"); + /* appease the harness */ + pad = gst_element_get_static_pad (h->element, "video_0"); + + fail_unless (gst_harness_push_event (h, + gst_event_new_stream_start ("random"))); + + /* caps event with not enough information, should probably fail but + * currently only does from aggregate() */ + caps = gst_caps_from_string + ("video/x-h264, stream-format=(string)avc, alignment=(string)au, width=(int)16, height=(int)16"); + fail_unless (gst_harness_push_event (h, gst_event_new_caps (caps))); + gst_caps_unref (caps); + /* subsequent caps event with enough information should succeed */ + caps = gst_caps_from_string + ("video/x-h264, width=(int)800, height=(int)600, " + "framerate=(fraction)1/1, stream-format=(string)avc, codec_data=(buffer)0000," + " alignment=(string)au, level=(int)2, profile=(string)high"); + fail_unless (gst_harness_push_event (h, gst_event_new_caps (caps))); + gst_caps_unref (caps); + gst_segment_init (&segment, GST_FORMAT_TIME); + fail_unless (gst_harness_push_event (h, gst_event_new_segment (&segment))); + + buf = create_buffer (0 * GST_SECOND, 0, GST_SECOND, 4096); + fail_unless_equals_int (GST_FLOW_OK, gst_harness_push (h, buf)); + + fail_unless (gst_harness_push_event (h, gst_event_new_eos ())); + + buf = gst_harness_pull (h); + fail_unless (buf != NULL); + gst_buffer_unref (buf); + + gst_harness_teardown (h); + gst_object_unref (pad); +} + +GST_END_TEST; + static Suite * qtmux_suite (void) { @@ -1852,6 +1899,8 @@ qtmux_suite (void) tcase_add_test (tc_chain, test_muxing_dts_outside_segment); tcase_add_test (tc_chain, test_muxing_initial_gap); + tcase_add_test (tc_chain, test_caps_renego); + return s; } -- 2.7.4