flvdemux: don't emit pad-added until caps are ready

In other words, gst_pad_get_current_caps should never return NULL
in a pad-added callback from the demuxer.

Added tests for the two special cases with AAC and H.264 where this
would happen every time.

https://bugzilla.gnome.org/show_bug.cgi?id=763780

diff --git a/gst/flv/gstflvdemux.c b/gst/flv/gstflvdemux.c
index 40edb99..d45902e 100644 (file)
--- a/gst/flv/gstflvdemux.c
+++ b/gst/flv/gstflvdemux.c
@@ -1043,9 +1043,42 @@ gst_flv_demux_parse_tag_audio (GstFlvDemux * demux, GstBuffer * buffer)
       "%d bits width, codec tag %u (flags %02X)", channels, rate, width,
       codec_tag, flags);
 
+  if (codec_tag == 10) {
+    guint8 aac_packet_type = GST_READ_UINT8 (data + 8);
+
+    switch (aac_packet_type) {
+      case 0:
+      {
+        /* AudioSpecificConfig data */
+        GST_LOG_OBJECT (demux, "got an AAC codec data packet");
+        if (demux->audio_codec_data) {
+          gst_buffer_unref (demux->audio_codec_data);
+        }
+        demux->audio_codec_data = gst_buffer_copy_region (buffer, GST_BUFFER_COPY_MEMORY,
+            7 + codec_data, demux->tag_data_size - codec_data);
+
+        /* Use that buffer data in the caps */
+        if (demux->audio_pad)
+          gst_flv_demux_audio_negotiate (demux, codec_tag, rate, channels, width);
+        goto beach;
+      }
+      case 1:
+        if (!demux->audio_codec_data) {
+          GST_ERROR_OBJECT (demux, "got AAC audio packet before codec data");
+          ret = GST_FLOW_OK;
+          goto beach;
+        }
+        /* AAC raw packet */
+        GST_LOG_OBJECT (demux, "got a raw AAC audio packet");
+        break;
+      default:
+        GST_WARNING_OBJECT (demux, "invalid AAC packet type %u",
+            aac_packet_type);
+    }
+  }
+
   /* If we don't have our audio pad created, then create it. */
   if (G_UNLIKELY (!demux->audio_pad)) {
-
     demux->audio_pad =
         gst_pad_new_from_template (gst_element_class_get_pad_template
         (GST_ELEMENT_GET_CLASS (demux), "audio"), "audio");
@@ -1131,38 +1164,6 @@ gst_flv_demux_parse_tag_audio (GstFlvDemux * demux, GstBuffer * buffer)
   outbuf = gst_buffer_copy_region (buffer, GST_BUFFER_COPY_MEMORY,
       7 + codec_data, demux->tag_data_size - codec_data);
 
-  if (demux->audio_codec_tag == 10) {
-    guint8 aac_packet_type = GST_READ_UINT8 (data + 8);
-
-    switch (aac_packet_type) {
-      case 0:
-      {
-        /* AudioSpecificConfig data */
-        GST_LOG_OBJECT (demux, "got an AAC codec data packet");
-        if (demux->audio_codec_data) {
-          gst_buffer_unref (demux->audio_codec_data);
-        }
-        demux->audio_codec_data = outbuf;
-        /* Use that buffer data in the caps */
-        gst_flv_demux_audio_negotiate (demux, codec_tag, rate, channels, width);
-        goto beach;
-      }
-      case 1:
-        if (!demux->audio_codec_data) {
-          GST_ERROR_OBJECT (demux, "got AAC audio packet before codec data");
-          ret = GST_FLOW_OK;
-          gst_buffer_unref (outbuf);
-          goto beach;
-        }
-        /* AAC raw packet */
-        GST_LOG_OBJECT (demux, "got a raw AAC audio packet");
-        break;
-      default:
-        GST_WARNING_OBJECT (demux, "invalid AAC packet type %u",
-            aac_packet_type);
-    }
-  }
-
   /* detect (and deem to be resyncs)  large pts gaps */
   if (gst_flv_demux_update_resync (demux, pts, demux->audio_need_discont,
           &demux->last_audio_pts, &demux->audio_time_offset)) {
@@ -1457,6 +1458,40 @@ gst_flv_demux_parse_tag_video (GstFlvDemux * demux, GstBuffer * buffer)
   GST_LOG_OBJECT (demux, "video tag with codec tag %u, keyframe (%d) "
       "(flags %02X)", codec_tag, keyframe, flags);
 
+  if (codec_tag == 7) {
+    guint8 avc_packet_type = GST_READ_UINT8 (data + 8);
+
+    switch (avc_packet_type) {
+      case 0:
+      {
+        /* AVCDecoderConfigurationRecord data */
+        GST_LOG_OBJECT (demux, "got an H.264 codec data packet");
+        if (demux->video_codec_data) {
+          gst_buffer_unref (demux->video_codec_data);
+        }
+        demux->video_codec_data = gst_buffer_copy_region (buffer,
+            GST_BUFFER_COPY_MEMORY, 7 + codec_data,
+            demux->tag_data_size - codec_data);;
+        /* Use that buffer data in the caps */
+        if (demux->video_pad)
+          gst_flv_demux_video_negotiate (demux, codec_tag);
+        goto beach;
+      }
+      case 1:
+        /* H.264 NALU packet */
+        if (!demux->video_codec_data) {
+          GST_ERROR_OBJECT (demux, "got H.264 video packet before codec data");
+          ret = GST_FLOW_OK;
+          goto beach;
+        }
+        GST_LOG_OBJECT (demux, "got a H.264 NALU video packet");
+        break;
+      default:
+        GST_WARNING_OBJECT (demux, "invalid video packet type %u",
+            avc_packet_type);
+    }
+  }
+
   /* If we don't have our video pad created, then create it. */
   if (G_UNLIKELY (!demux->video_pad)) {
     demux->video_pad =
@@ -1548,38 +1583,6 @@ gst_flv_demux_parse_tag_video (GstFlvDemux * demux, GstBuffer * buffer)
   outbuf = gst_buffer_copy_region (buffer, GST_BUFFER_COPY_MEMORY,
       7 + codec_data, demux->tag_data_size - codec_data);
 
-  if (demux->video_codec_tag == 7) {
-    guint8 avc_packet_type = GST_READ_UINT8 (data + 8);
-
-    switch (avc_packet_type) {
-      case 0:
-      {
-        /* AVCDecoderConfigurationRecord data */
-        GST_LOG_OBJECT (demux, "got an H.264 codec data packet");
-        if (demux->video_codec_data) {
-          gst_buffer_unref (demux->video_codec_data);
-        }
-        demux->video_codec_data = outbuf;
-        /* Use that buffer data in the caps */
-        gst_flv_demux_video_negotiate (demux, codec_tag);
-        goto beach;
-      }
-      case 1:
-        /* H.264 NALU packet */
-        if (!demux->video_codec_data) {
-          GST_ERROR_OBJECT (demux, "got H.264 video packet before codec data");
-          ret = GST_FLOW_OK;
-          gst_buffer_unref (outbuf);
-          goto beach;
-        }
-        GST_LOG_OBJECT (demux, "got a H.264 NALU video packet");
-        break;
-      default:
-        GST_WARNING_OBJECT (demux, "invalid video packet type %u",
-            avc_packet_type);
-    }
-  }
-
   /* detect (and deem to be resyncs)  large dts gaps */
   if (gst_flv_demux_update_resync (demux, dts, demux->video_need_discont,
           &demux->last_video_dts, &demux->video_time_offset)) {

diff --git a/tests/check/elements/flvdemux.c b/tests/check/elements/flvdemux.c
index 4ca5092..8f834ed 100644 (file)
--- a/tests/check/elements/flvdemux.c
+++ b/tests/check/elements/flvdemux.c
@@ -1,6 +1,7 @@
 /* GStreamer unit tests for flvdemux
  *
  * Copyright (C) 2009 Tim-Philipp Müller  <tim centricular net>
+ * Copyright (C) 2016 Havard Graff        <havard@pexip.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -184,7 +185,12 @@ create_buffer (guint8 * data, gsize size)
 static void
 flvdemux_pad_added (GstElement * flvdemux, GstPad * srcpad, GstHarness * h)
 {
+  GstCaps *caps;
   (void) flvdemux;
+
+  caps = gst_pad_get_current_caps (srcpad);
+  fail_unless (caps != NULL);
+  gst_caps_unref (caps);
   gst_harness_add_element_src_pad (h, srcpad);
 }
 
@@ -318,6 +324,210 @@ GST_START_TEST (test_speex)
 
 GST_END_TEST;
 
+
+GST_START_TEST (test_aac)
+{
+  guint8 flv_header[] = {
+    0x46, 0x4c, 0x56, 0x01, 0x04, 0x00, 0x00, 0x00,
+    0x09, 0x00, 0x00, 0x00, 0x00,
+    0x12, 0x00, 0x00, 0x2d, /* script tag */
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
+    0x00, 0x0a, 0x6f, 0x6e, 0x4d, 0x65, 0x74, 0x61,
+    0x44, 0x61, 0x74, 0x61, 0x03, 0x00, 0x06, 0x53,
+    0x65, 0x72, 0x76, 0x65, 0x72, 0x02, 0x00, 0x11,
+    0x50, 0x65, 0x78, 0x69, 0x70, 0x20, 0x52, 0x54,
+    0x4d, 0x50, 0x20, 0x53, 0x65, 0x72, 0x76, 0x65,
+    0x72, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x38,
+  };
+
+  guint8 aac_header[] = {
+    0x08, 0x00, 0x00, 0x04, /* audio tag */
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xaf,
+    0x00, 0x13, 0x10, 0x00, 0x00, 0x00, 0x0f,
+  };
+
+  guint8 aac_buffer[] = {
+    0x08, 0x00, 0x01, 0x57, /* audio tag */
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xaf,
+    0x01, 0x21, 0x21, 0x45, 0x00, 0x14, 0x50, 0x01,
+    0x46, 0xf0, 0x4d, 0xfb, 0x01, 0x3c, 0x08, 0x40,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x07, 0x0e, 0x00, 0x0d, 0xff, 0xe2, 0x14,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4, 0xb4,
+    0xb4, 0xb4, 0xb4, 0xbb, 0xc6, 0x84, 0x29, 0x69,
+    0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69,
+    0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69,
+    0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69,
+    0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69,
+    0x69, 0x69, 0x69, 0x69, 0x69, 0x78, 0x00, 0x00,
+    0x01, 0x62
+  };
+
+  GstHarness *h = gst_harness_new_with_padnames ("flvdemux", "sink", NULL);
+  gst_harness_set_src_caps_str (h, "video/x-flv");
+
+  g_signal_connect (h->element, "pad-added",
+      G_CALLBACK (flvdemux_pad_added), h);
+
+  gst_harness_push (h, create_buffer (flv_header, sizeof (flv_header)));
+  gst_harness_push (h, create_buffer (aac_header, sizeof (aac_header)));
+  gst_harness_push (h, create_buffer (aac_buffer, sizeof (aac_buffer)));
+
+  {
+    GstCaps *caps;
+    const GstStructure *s;
+    gint mpegversion;
+    gboolean framed;
+    const gchar *stream_format;
+    gint rate;
+    gint channels;
+    const GValue *codec_data;
+    const GstBuffer *buf;
+
+    caps = gst_pad_get_current_caps (h->sinkpad);
+    s = gst_caps_get_structure (caps, 0);
+
+    fail_unless (gst_structure_has_name (s, "audio/mpeg"));
+
+    gst_structure_get_int (s, "mpegversion", &mpegversion);
+    fail_unless_equals_int (4, mpegversion);
+
+    gst_structure_get_boolean (s, "framed", &framed);
+    fail_unless (framed == TRUE);
+
+    stream_format = gst_structure_get_string (s, "stream-format");
+    fail_unless_equals_string ("raw", stream_format);
+
+    gst_structure_get_int (s, "rate", &rate);
+    fail_unless_equals_int (24000, rate);
+
+    gst_structure_get_int (s, "channels", &channels);
+    fail_unless_equals_int (2, channels);
+
+    codec_data = gst_structure_get_value (s, "codec_data");
+    fail_unless (codec_data != NULL);
+    fail_unless (G_VALUE_HOLDS (codec_data, GST_TYPE_BUFFER));
+    buf = gst_value_get_buffer (codec_data);
+
+    gst_caps_unref (caps);
+  }
+
+  /* we should have gotten one encoded buffer */
+  fail_unless_equals_int (1, gst_harness_buffers_in_queue (h));
+
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
+
+GST_START_TEST (test_h264)
+{
+  guint8 flv_header[] = {
+    0x46, 0x4c, 0x56, 0x01, 0x01, 0x00, 0x00, 0x00,
+    0x09, 0x00, 0x00, 0x00, 0x00,
+    0x12, 0x00, 0x00, 0x2d, /* script tag */
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
+    0x00, 0x0a, 0x6f, 0x6e, 0x4d, 0x65, 0x74, 0x61,
+    0x44, 0x61, 0x74, 0x61, 0x03, 0x00, 0x06, 0x53,
+    0x65, 0x72, 0x76, 0x65, 0x72, 0x02, 0x00, 0x11,
+    0x50, 0x65, 0x78, 0x69, 0x70, 0x20, 0x52, 0x54,
+    0x4d, 0x50, 0x20, 0x53, 0x65, 0x72, 0x76, 0x65,
+    0x72, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x38,
+  };
+
+  guint8 h264_packet0[] = {
+    0x09, 0x00, 0x00, 0x1e, /* video tag */
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x17,
+    0x00, 0x00, 0x00, 0x00, 0x01, 0x42, 0xc0, 0x1e,
+    0xff, 0xe1, 0x00, 0x0a, 0x67, 0x42, 0xc0, 0x1e,
+    0x95, 0xa0, 0x28, 0x0b, 0xde, 0x54, 0x01, 0x00,
+    0x04, 0x68, 0xce, 0x3c, 0x80, 0x00, 0x00, 0x00,
+    0x29
+  };
+
+  guint8 h264_packet1[] = {
+    0x09, 0x00, 0x00, 0x1b, /* video tag */
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x27,
+    0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a,
+    0x67, 0x42, 0xc0, 0x1e, 0x95, 0xa0, 0x28, 0x0b,
+    0xde, 0x54, 0x00, 0x00, 0x00, 0x04, 0x68, 0xce,
+    0x3c, 0x80, 0x00, 0x00, 0x00, 0x26
+  };
+
+  GstHarness *h = gst_harness_new_with_padnames ("flvdemux", "sink", NULL);
+  gst_harness_set_src_caps_str (h, "video/x-flv");
+
+  g_signal_connect (h->element, "pad-added",
+      G_CALLBACK (flvdemux_pad_added), h);
+
+  gst_harness_push (h, create_buffer (flv_header, sizeof (flv_header)));
+  gst_harness_push (h, create_buffer (h264_packet0, sizeof (h264_packet0)));
+  gst_harness_push (h, create_buffer (h264_packet1, sizeof (h264_packet1)));
+
+  {
+    GstCaps *caps;
+    const GstStructure *s;
+    const gchar *stream_format;
+    const GValue *codec_data;
+    const GstBuffer *buf;
+
+    caps = gst_pad_get_current_caps (h->sinkpad);
+    s = gst_caps_get_structure (caps, 0);
+
+    fail_unless (gst_structure_has_name (s, "video/x-h264"));
+
+    stream_format = gst_structure_get_string (s, "stream-format");
+    fail_unless_equals_string ("avc", stream_format);
+
+    codec_data = gst_structure_get_value (s, "codec_data");
+    fail_unless (codec_data != NULL);
+    fail_unless (G_VALUE_HOLDS (codec_data, GST_TYPE_BUFFER));
+    buf = gst_value_get_buffer (codec_data);
+
+    gst_caps_unref (caps);
+  }
+
+  /* we should have gotten one encoded buffer */
+  fail_unless_equals_int (1, gst_harness_buffers_in_queue (h));
+
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
+
+
 static Suite *
 flvdemux_suite (void)
 {
@@ -329,6 +539,8 @@ flvdemux_suite (void)
   tcase_add_test (tc_chain, test_reuse_pull);
 
   tcase_add_test (tc_chain, test_speex);
+  tcase_add_test (tc_chain, test_aac);
+  tcase_add_test (tc_chain, test_h264);
 
   return s;
 }