gst/icydemux/gsticydemux.*: When we merge/collect multiple incoming buffers for typef...

Original commit message from CVS:
* gst/icydemux/gsticydemux.c: (gst_icydemux_reset),
(gst_icydemux_typefind_or_forward):
* gst/icydemux/gsticydemux.h:
When we merge/collect multiple incoming buffers for typefinding
purposes, keep an initial 0 offset on the first outgoing buffer
as well (otherwise id3demux won't work right). Fixes #345449.
Also Make buffer metadata writable before setting buffer caps.
* tests/check/elements/icydemux.c: (typefind_succeed),
(cleanup_icydemux), (push_data), (GST_START_TEST),
(icydemux_suite):
Small test case for the above.

diff --git a/ChangeLog b/ChangeLog
index 2e5a8b6..731d079 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2006-09-14  Tim-Philipp Müller  <tim at centricular dot net>
+
+       * gst/icydemux/gsticydemux.c: (gst_icydemux_reset),
+       (gst_icydemux_typefind_or_forward):
+       * gst/icydemux/gsticydemux.h:
+         When we merge/collect multiple incoming buffers for typefinding
+         purposes, keep an initial 0 offset on the first outgoing buffer
+         as well (otherwise id3demux won't work right). Fixes #345449.
+         Also Make buffer metadata writable before setting buffer caps.
+
+       * tests/check/elements/icydemux.c: (typefind_succeed),
+       (cleanup_icydemux), (push_data), (GST_START_TEST),
+       (icydemux_suite):
+         Small test case for the above.
+
 2006-09-13  Stefan Kost  <ensonic@users.sf.net>
 
        * gst/avi/gstavidemux.c: (gst_avi_demux_peek_chunk),

diff --git a/common b/common
index d287125..a8c15b7 160000 (submodule)
--- a/common
+++ b/common
@@ -1 +1 @@
-Subproject commit d287125f93da692bc25d53b0b7b0e2f90424a212
+Subproject commit a8c15b7a2c75fc2bd83850cb17cb05a1ee84ecaf

diff --git a/gst/icydemux/gsticydemux.c b/gst/icydemux/gsticydemux.c
index 1518b26..f2ba6e3 100644 (file)
--- a/gst/icydemux/gsticydemux.c
+++ b/gst/icydemux/gsticydemux.c
@@ -177,10 +177,9 @@ gst_icydemux_reset (GstICYDemux * icydemux)
     icydemux->meta_adapter = NULL;
   }
 
-  if (icydemux->typefind_adapter) {
-    gst_adapter_clear (icydemux->typefind_adapter);
-    g_object_unref (icydemux->typefind_adapter);
-    icydemux->typefind_adapter = NULL;
+  if (icydemux->typefind_buf) {
+    gst_buffer_unref (icydemux->typefind_buf);
+    icydemux->typefind_buf = NULL;
   }
 }
 
@@ -427,49 +426,38 @@ static GstFlowReturn
 gst_icydemux_typefind_or_forward (GstICYDemux * icydemux, GstBuffer * buf)
 {
   if (icydemux->typefinding) {
-    GstBuffer *typefind_buf;
-    const guint8 *data;
+    GstBuffer *tf_buf;
     GstCaps *caps;
-    int size;
     guint prob;
 
-    if (!icydemux->typefind_adapter)
-      icydemux->typefind_adapter = gst_adapter_new ();
-
-    gst_adapter_push (icydemux->typefind_adapter, buf);
-
-    size = gst_adapter_available (icydemux->typefind_adapter);
-    typefind_buf = gst_buffer_new ();
-
-    data = gst_adapter_peek (icydemux->typefind_adapter, size);
-
-    GST_BUFFER_DATA (typefind_buf) = (guint8 *) data;
-    GST_BUFFER_SIZE (typefind_buf) = size;
+    if (icydemux->typefind_buf) {
+      icydemux->typefind_buf = gst_buffer_join (icydemux->typefind_buf, buf);
+    } else {
+      icydemux->typefind_buf = buf;
+    }
 
     caps = gst_type_find_helper_for_buffer (GST_OBJECT (icydemux),
-        typefind_buf, &prob);
+        icydemux->typefind_buf, &prob);
 
     if (caps == NULL) {
-      if (size < ICY_TYPE_FIND_MAX_SIZE) {
-        gst_buffer_unref (typefind_buf);
+      if (GST_BUFFER_SIZE (icydemux->typefind_buf) < ICY_TYPE_FIND_MAX_SIZE) {
         /* Just break for more data */
         return GST_FLOW_OK;
       }
 
       /* We failed typefind */
-      GST_ELEMENT_ERROR (icydemux, CORE, CAPS,
-          ("Could not determine the mime type of the file"),
+      GST_ELEMENT_ERROR (icydemux, STREAM, TYPE_NOT_FOUND, (NULL),
           ("No caps found for contents within an ICY stream"));
-      gst_buffer_unref (typefind_buf);
-      gst_adapter_clear (icydemux->typefind_adapter);
+      gst_buffer_unref (icydemux->typefind_buf);
+      icydemux->typefind_buf = NULL;
       return GST_FLOW_ERROR;
     }
 
     if (!gst_icydemux_add_srcpad (icydemux, caps)) {
       GST_DEBUG_OBJECT (icydemux, "Failed to add srcpad");
       gst_caps_unref (caps);
-      gst_buffer_unref (typefind_buf);
-      gst_adapter_clear (icydemux->typefind_adapter);
+      gst_buffer_unref (icydemux->typefind_buf);
+      icydemux->typefind_buf = NULL;
       return GST_FLOW_ERROR;
     }
     gst_caps_unref (caps);
@@ -487,21 +475,23 @@ gst_icydemux_typefind_or_forward (GstICYDemux * icydemux, GstBuffer * buf)
      * to get that forwarded. */
     icydemux->typefinding = FALSE;
 
-    data = gst_adapter_take (icydemux->typefind_adapter, size);
-    GST_BUFFER_DATA (typefind_buf) = (guint8 *) data;
-    GST_BUFFER_MALLOCDATA (typefind_buf) = (guint8 *) data;
-
-    return gst_icydemux_typefind_or_forward (icydemux, typefind_buf);
+    tf_buf = icydemux->typefind_buf;
+    icydemux->typefind_buf = NULL;
+    return gst_icydemux_typefind_or_forward (icydemux, tf_buf);
   } else {
     if (G_UNLIKELY (icydemux->srcpad == NULL)) {
       gst_buffer_unref (buf);
       return GST_FLOW_ERROR;
     }
 
+    buf = gst_buffer_make_metadata_writable (buf);
     gst_buffer_set_caps (buf, icydemux->src_caps);
 
-    /* Most things don't care, and it's a pain to track */
-    GST_BUFFER_OFFSET (buf) = GST_BUFFER_OFFSET_NONE;
+    /* Most things don't care, and it's a pain to track (we should preserve a
+     * 0 offset on the first buffer though if it's there, for id3demux etc.) */
+    if (GST_BUFFER_OFFSET (buf) != 0) {
+      GST_BUFFER_OFFSET (buf) = GST_BUFFER_OFFSET_NONE;
+    }
 
     return gst_pad_push (icydemux->srcpad, buf);
   }

diff --git a/gst/icydemux/gsticydemux.h b/gst/icydemux/gsticydemux.h
index aea8882..3e676d1 100644 (file)
--- a/gst/icydemux/gsticydemux.h
+++ b/gst/icydemux/gsticydemux.h
@@ -69,7 +69,7 @@ struct _GstICYDemux
 
   GstAdapter *meta_adapter;
 
-  GstAdapter *typefind_adapter;
+  GstBuffer *typefind_buf;
 };
 
 struct _GstICYDemuxClass 

diff --git a/tests/check/elements/icydemux.c b/tests/check/elements/icydemux.c
index db67bea..10504e4 100644 (file)
--- a/tests/check/elements/icydemux.c
+++ b/tests/check/elements/icydemux.c
@@ -54,18 +54,22 @@ static GstStaticPadTemplate sinktemplate = GST_STATIC_PAD_TEMPLATE ("sink",
     GST_STATIC_CAPS (SINK_CAPS)
     );
 
-GstElement *icydemux;
-GstBus *bus;
+static GstElement *icydemux;
+static GstBus *bus;
 GstPad *srcpad, *sinkpad;
 
 static GstStaticCaps typefind_caps =
 GST_STATIC_CAPS ("application/octet-stream");
 
+static gboolean fake_typefind_caps;     /* FALSE */
+
 static void
 typefind_succeed (GstTypeFind * tf, gpointer private)
 {
-  gst_type_find_suggest (tf, GST_TYPE_FIND_MAXIMUM,
-      gst_static_caps_get (&typefind_caps));
+  if (fake_typefind_caps) {
+    gst_type_find_suggest (tf, GST_TYPE_FIND_MAXIMUM,
+        gst_static_caps_get (&typefind_caps));
+  }
 }
 
 static gboolean
@@ -124,14 +128,19 @@ cleanup_icydemux (void)
 {
   gst_bus_set_flushing (bus, TRUE);
   gst_object_unref (bus);
+  bus = NULL;
 
   gst_check_teardown_src_pad (icydemux);
   gst_check_teardown_sink_pad (icydemux);
   gst_check_teardown_element (icydemux);
+
+  srcpad = NULL;
+  sinkpad = NULL;
+  icydemux = NULL;
 }
 
 static void
-push_data (guint8 * data, int len, GstCaps * caps)
+push_data (const guint8 * data, int len, GstCaps * caps, gint64 offset)
 {
   GstFlowReturn res;
   GstBuffer *buffer = gst_buffer_new_and_alloc (len);
@@ -139,6 +148,8 @@ push_data (guint8 * data, int len, GstCaps * caps)
   memcpy (GST_BUFFER_DATA (buffer), data, len);
   gst_buffer_set_caps (buffer, caps);
 
+  GST_BUFFER_OFFSET (buffer) = offset;
+
   res = gst_pad_push (srcpad, buffer);
 
   fail_unless (res == GST_FLOW_OK, "Failed pushing buffer: %d", res);
@@ -152,11 +163,13 @@ GST_START_TEST (test_demux)
   const gchar *tag;
   GstCaps *caps;
 
+  fake_typefind_caps = TRUE;
+
   caps = gst_caps_from_string (ICYCAPS);
 
   create_icydemux ();
 
-  push_data ((guint8 *) ICY_DATA, sizeof (ICY_DATA), caps);
+  push_data ((guint8 *) ICY_DATA, sizeof (ICY_DATA), caps, -1);
 
   message = gst_bus_poll (bus, GST_MESSAGE_TAG, -1);
   fail_unless (message != NULL);
@@ -177,18 +190,64 @@ GST_START_TEST (test_demux)
   gst_caps_unref (caps);
 
   cleanup_icydemux ();
+
+  fake_typefind_caps = FALSE;
+}
+
+GST_END_TEST;
+
+/* run this test first before the custom typefind function is set up */
+GST_START_TEST (test_first_buf_offset_when_merged_for_typefinding)
+{
+  const guint8 buf1[] = { 'M' };
+  const guint8 buf2[] = { 'P', '+', 0xff, 0xfb, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+  };
+  GstCaps *icy_caps;
+  GstPad *icy_srcpad;
+
+  fake_typefind_caps = FALSE;
+
+  create_icydemux ();
+
+  icy_caps = gst_caps_from_string (ICYCAPS);
+
+  push_data (buf1, G_N_ELEMENTS (buf1), icy_caps, 0);
+
+  /* one byte isn't really enough for typefinding, can't have a srcpad yet */
+  fail_unless (gst_element_get_pad (icydemux, "src") == NULL);
+
+  push_data (buf2, G_N_ELEMENTS (buf2), icy_caps, -1);
+
+  /* should have been enough to create a audio/x-musepack source pad .. */
+  icy_srcpad = gst_element_get_pad (icydemux, "src");
+  fail_unless (icy_srcpad != NULL);
+  gst_object_unref (icy_srcpad);
+
+  fail_unless (g_list_length (buffers) > 0);
+
+  /* first buffer should have offset 0 even after it was merged with 2nd buf */
+  fail_unless (GST_BUFFER_OFFSET (GST_BUFFER_CAST (buffers->data)) == 0);
+
+  /* first buffer should have caps set */
+  fail_unless (GST_BUFFER_CAPS (GST_BUFFER_CAST (buffers->data)) != NULL);
+
+  gst_caps_unref (icy_caps);
+
+  cleanup_icydemux ();
 }
 
 GST_END_TEST;
 
-Suite *
-icydemux_suite ()
+static Suite *
+icydemux_suite (void)
 {
   Suite *s = suite_create ("icydemux");
   TCase *tc_chain = tcase_create ("general");
 
   suite_add_tcase (s, tc_chain);
   tcase_add_test (tc_chain, test_demux);
+  tcase_add_test (tc_chain, test_first_buf_offset_when_merged_for_typefinding);
 
   return s;
 }