Skip to content

Commit

Permalink
qtmux: properly support initial caps nego failure
Browse files Browse the repository at this point in the history
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: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/732>
  • Loading branch information
ystreet committed Sep 28, 2020
1 parent b27dc54 commit e81ce6f
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 10 deletions.
30 changes: 20 additions & 10 deletions gst/isomp4/gstqtmux.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions gst/isomp4/gstqtmux.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ struct _GstQTMuxPad
guint64 raw_audio_adapter_offset;
GstClockTime raw_audio_adapter_pts;
GstFlowReturn flow_status;

GstCaps *configured_caps;
};

struct _GstQTMuxPadClass
Expand Down
49 changes: 49 additions & 0 deletions tests/check/elements/qtmux.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <glib/gstdio.h>

#include <gst/check/gstcheck.h>
#include <gst/check/gstharness.h>
#include <gst/pbutils/encoding-profile.h>

/* For ease of programming we use globals to keep refs for our floating
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit e81ce6f

Please sign in to comment.