gst/id3demux/gstid3demux.c: Don't leak caps: make gst_id3demux_add_srcpad() not take ownership of the caps passed to ...

Original commit message from CVS:
* gst/id3demux/gstid3demux.c: (gst_id3demux_add_srcpad),
(gst_id3demux_sink_activate):
Don't leak caps: make gst_id3demux_add_srcpad() not take ownership of the
caps passed to it (previouslly one code path assumes it takes ownership
while another one assumes it doesn't).
* configure.ac:
* tests/files/Makefile.am:
* tests/files/id3-407349-1.tag:
* tests/files/id3-407349-2.tag:
Add directory where data for unit tests can be stored.
* tests/Makefile.am:
* tests/check/Makefile.am:
* tests/check/elements/.cvsignore:
* tests/check/elements/id3demux.c: (pad_added_cb), (error_cb),
(read_tags_from_file), (run_check_for_file),
(check_date_1977_06_23), (GST_START_TEST), (id3demux_suite):
Add unit test for id3demux, and in particular for bug #407349. Only
testing pull-mode for now; push mode doesn't work yet because the test
files are smaller than ID3_TYPE_FIND_MIN_SIZE.

ChangeLog

@@ -1,3 +1,27 @@
+2007-03-06  Tim-Philipp Müller  <tim at centricular dot net>
+
+	* gst/id3demux/gstid3demux.c: (gst_id3demux_add_srcpad),
+	(gst_id3demux_sink_activate):
+	  Don't leak caps: make gst_id3demux_add_srcpad() not take ownership of the
+	  caps passed to it (previouslly one code path assumes it takes ownership
+	  while another one assumes it doesn't).
+
+	* configure.ac:
+	* tests/files/Makefile.am:
+	* tests/files/id3-407349-1.tag:
+	* tests/files/id3-407349-2.tag:
+	  Add directory where data for unit tests can be stored.
+
+	* tests/Makefile.am:
+	* tests/check/Makefile.am:
+	* tests/check/elements/.cvsignore:
+	* tests/check/elements/id3demux.c: (pad_added_cb), (error_cb),
+	(read_tags_from_file), (run_check_for_file),
+	(check_date_1977_06_23), (GST_START_TEST), (id3demux_suite):
+	  Add unit test for id3demux, and in particular for bug #407349. Only
+	  testing pull-mode for now; push mode doesn't work yet because the test
+	  files are smaller than ID3_TYPE_FIND_MIN_SIZE.
+
 2007-03-06  Tim-Philipp Müller  <tim at centricular dot net>
 
 	* tests/check/Makefile.am:

configure.ac

@@ -907,9 +907,10 @@ sys/ximage/Makefile
 po/Makefile.in
 tests/Makefile
 tests/check/Makefile
-tests/icles/Makefile
 tests/examples/Makefile
 tests/examples/level/Makefile
+tests/files/Makefile
+tests/icles/Makefile
 gconf/Makefile
 gconf/gstreamer.schemas
 common/Makefile

gst/id3demux/gstid3demux.c

@@ -106,7 +106,7 @@ static GstFlowReturn gst_id3demux_src_getrange (GstPad * srcpad,
     guint64 offset, guint length, GstBuffer ** buffer);
 
 static gboolean gst_id3demux_add_srcpad (GstID3Demux * id3demux,
-    GstCaps * new_caps);
+    const GstCaps * new_caps);
 static gboolean gst_id3demux_remove_srcpad (GstID3Demux * id3demux);
 
 static gboolean gst_id3demux_srcpad_event (GstPad * pad, GstEvent * event);
@@ -239,12 +239,12 @@ gst_id3demux_dispose (GObject * object)
 }
 
 static gboolean
-gst_id3demux_add_srcpad (GstID3Demux * id3demux, GstCaps * new_caps)
+gst_id3demux_add_srcpad (GstID3Demux * id3demux, const GstCaps * new_caps)
 {
   if (id3demux->src_caps == NULL ||
       !gst_caps_is_equal (new_caps, id3demux->src_caps)) {
 
-    gst_caps_replace (&(id3demux->src_caps), new_caps);
+    gst_caps_replace (&(id3demux->src_caps), (GstCaps *) new_caps);
     if (id3demux->srcpad != NULL) {
       GST_DEBUG_OBJECT (id3demux, "Changing src pad caps to %" GST_PTR_FORMAT,
           id3demux->src_caps);
@@ -252,8 +252,7 @@ gst_id3demux_add_srcpad (GstID3Demux * id3demux, GstCaps * new_caps)
       gst_pad_set_caps (id3demux->srcpad, id3demux->src_caps);
     }
   } else {
-    /* Caps never changed */
-    gst_caps_unref (new_caps);
+    /* caps didn't change */
   }
 
   if (id3demux->srcpad == NULL) {
@@ -909,7 +908,6 @@ gst_id3demux_sink_activate (GstPad * sinkpad)
   id3demux->state = GST_ID3DEMUX_STREAMING;
 
   /* 6 Add the srcpad for output now we know caps. */
-  /* add_srcpad takes ownership of the caps */
   if (!gst_id3demux_add_srcpad (id3demux, caps)) {
     GST_DEBUG_OBJECT (id3demux, "Could not add source pad");
     goto done_activate;
@@ -925,6 +923,9 @@ gst_id3demux_sink_activate (GstPad * sinkpad)
 
 done_activate:
 
+  if (caps)
+    gst_caps_unref (caps);
+
   return ret;
 }
 

tests/Makefile.am

@@ -10,7 +10,7 @@ else
 SUBDIRS_ICLES =
 endif
 
-SUBDIRS = $(SUBDIRS_CHECK) $(SUBDIRS_ICLES) examples
+SUBDIRS = $(SUBDIRS_CHECK) $(SUBDIRS_ICLES) examples files
 
-DIST_SUBDIRS = check icles examples
+DIST_SUBDIRS = check icles examples files
 

tests/check/Makefile.am

@@ -1,6 +1,7 @@
 include $(top_srcdir)/common/check.mak
 
 CHECK_REGISTRY = $(top_builddir)/tests/check/test-registry.xml
+TEST_FILES_DIRECTORY = $(top_srcdir)/tests/files
 
 REGISTRY_ENVIRONMENT = \
 	GST_REGISTRY=$(CHECK_REGISTRY)
@@ -9,6 +10,7 @@ TESTS_ENVIRONMENT = \
 	$(REGISTRY_ENVIRONMENT)					\
 	GST_PLUGIN_SYSTEM_PATH=					\
 	GST_PLUGIN_PATH=$(top_builddir)/gst:$(top_builddir)/ext:$(top_builddir)/sys:$(GSTPB_PLUGINS_DIR):$(GST_PLUGINS_DIR)			\
+	GST_TEST_FILES_PATH=$(TEST_FILES_DIRECTORY) \
 	STATE_IGNORE_ELEMENTS="aasink cacasink autovideosink gconfvideosink"
 
 # ths core dumps of some machines have PIDs appended
@@ -39,6 +41,7 @@ check_PROGRAMS = \
 	elements/audioinvert \
 	elements/audioamplify \
 	elements/avimux \
+	elements/id3demux \
 	elements/level \
 	elements/matroskamux \
 	elements/icydemux \

tests/check/elements/.gitignore

@@ -7,6 +7,7 @@ cmmldec
 cmmlenc
 icydemux
 avimux
+id3demux
 id3v2mux
 apev2mux
 audiopanorama

tests/check/elements/id3demux.c

@@ -0,0 +1,191 @@
+/* GStreamer unit tests for id3demux
+ *
+ * Copyright (C) 2007 Tim-Philipp Müller  <tim centricular net>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+#include <gst/check/gstcheck.h>
+
+#include <gst/gst.h>
+
+typedef void (CheckTagsFunc) (const GstTagList * tags, const gchar * file);
+
+static void
+pad_added_cb (GstElement * id3demux, GstPad * pad, GstBin * pipeline)
+{
+  GstElement *sink;
+  GstPad *sinkpad;
+
+  sink = gst_bin_get_by_name (pipeline, "fakesink");
+  fail_unless (gst_element_link (id3demux, sink));
+  gst_object_unref (sink);
+
+  gst_element_set_state (sink, GST_STATE_PAUSED);
+}
+
+static GstBusSyncReply
+error_cb (GstBus * bus, GstMessage * msg, gpointer user_data)
+{
+  if (GST_MESSAGE_TYPE (msg) == GST_MESSAGE_ERROR) {
+    const gchar *file = (const gchar *) user_data;
+    GError *err = NULL;
+    gchar *dbg = NULL;
+
+    gst_message_parse_error (msg, &err, &dbg);
+    g_error ("ERROR for %s: %s\n%s\n", file, err->message, dbg);
+  }
+
+  return GST_BUS_PASS;
+}
+
+static GstTagList *
+read_tags_from_file (const gchar * file, gboolean push_mode)
+{
+  GstStateChangeReturn state_ret;
+  GstTagList *tags = NULL;
+  GstMessage *msg;
+  GstElement *src, *sep, *sink, *id3demux, *pipeline;
+  GstBus *bus;
+  const gchar *dir;
+  gchar *path;
+
+  pipeline = gst_pipeline_new ("pipeline");
+  fail_unless (pipeline != NULL, "Failed to create pipeline!");
+
+  bus = gst_element_get_bus (pipeline);
+
+  /* kids, don't use a sync handler for this at home, really; we do because
+   * we just want to abort and nothing else */
+  gst_bus_set_sync_handler (bus, error_cb, (gpointer) file);
+
+  src = gst_element_factory_make ("filesrc", "filesrc");
+  fail_unless (src != NULL, "Failed to create 'filesrc' element!");
+
+  if (push_mode) {
+    sep = gst_element_factory_make ("queue", "queue");
+    fail_unless (sep != NULL, "Failed to create 'queue' element");
+  } else {
+    sep = gst_element_factory_make ("identity", "identity");
+    fail_unless (sep != NULL, "Failed to create 'identity' element");
+  }
+
+  id3demux = gst_element_factory_make ("id3demux", "id3demux");
+  fail_unless (id3demux != NULL, "Failed to create 'id3demux' element!");
+
+  sink = gst_element_factory_make ("fakesink", "fakesink");
+  fail_unless (sink != NULL, "Failed to create 'fakesink' element!");
+
+  gst_bin_add_many (GST_BIN (pipeline), src, sep, id3demux, sink, NULL);
+
+  fail_unless (gst_element_link (src, sep));
+  fail_unless (gst_element_link (sep, id3demux));
+
+  /* can't link id3demux and sink yet, do that later */
+  g_signal_connect (id3demux, "pad-added", G_CALLBACK (pad_added_cb), pipeline);
+
+  dir = g_getenv ("GST_TEST_FILES_PATH");
+  fail_unless (dir != NULL, "GST_TEST_FILES_PATH environment variable not set");
+
+  path = g_build_filename (dir, file, NULL);
+  GST_LOG ("reading file '%s'", path);
+  g_object_set (src, "location", path, NULL);
+
+  state_ret = gst_element_set_state (pipeline, GST_STATE_PAUSED);
+  fail_unless (state_ret != GST_STATE_CHANGE_FAILURE);
+
+  if (state_ret == GST_STATE_CHANGE_ASYNC) {
+    GST_LOG ("waiting for pipeline to reach PAUSED state");
+    state_ret = gst_element_get_state (pipeline, NULL, NULL, -1);
+    fail_unless_equals_int (state_ret, GST_STATE_CHANGE_SUCCESS);
+  }
+
+  GST_LOG ("PAUSED, let's retrieve our tags");
+
+  msg = gst_bus_poll (bus, GST_MESSAGE_TAG, -1);
+  fail_unless (msg != NULL, "Expected TAG message on bus! (%s)", file);
+
+  gst_message_parse_tag (msg, &tags);
+  fail_unless (tags != NULL, "TAG message did not contain taglist! (%s)", file);
+
+  gst_message_unref (msg);
+  gst_object_unref (bus);
+
+  fail_unless_equals_int (gst_element_set_state (pipeline, GST_STATE_NULL),
+      GST_STATE_CHANGE_SUCCESS);
+  gst_object_unref (pipeline);
+
+  g_free (path);
+
+  GST_INFO ("%s: tags = %" GST_PTR_FORMAT, file, tags);
+  return tags;
+}
+
+static void
+run_check_for_file (const gchar * filename, CheckTagsFunc * check_func)
+{
+  GstTagList *tags;
+
+  /* first, pull-based */
+  tags = read_tags_from_file (filename, FALSE);
+  fail_unless (tags != NULL, "Failed to extract tags from '%s'", filename);
+  check_func (tags, filename);
+  gst_tag_list_free (tags);
+
+  /* FIXME: need to fix id3demux for short content in push mode */
+#if 0
+  /* now try push-based */
+  tags = read_tags_from_file (filename, TRUE);
+  fail_unless (tags != NULL, "Failed to extract tags from '%s'", filename);
+  check_func (tags, filename);
+  gst_tag_list_free (tags);
+#endif
+}
+
+static void
+check_date_1977_06_23 (const GstTagList * tags, const gchar * file)
+{
+  GDate *date = NULL;
+
+  gst_tag_list_get_date (tags, GST_TAG_DATE, &date);
+  fail_unless (date != NULL, "Tags from %s should contain a GST_TAG_DATE tag");
+  fail_unless_equals_int (g_date_get_year (date), 1977);
+  fail_unless_equals_int (g_date_get_month (date), 6);
+  fail_unless_equals_int (g_date_get_day (date), 23);
+  g_date_free (date);
+}
+
+GST_START_TEST (test_tdat_tyer)
+{
+  run_check_for_file ("id3-407349-1.tag", check_date_1977_06_23);
+  run_check_for_file ("id3-407349-2.tag", check_date_1977_06_23);
+}
+
+GST_END_TEST;
+
+static Suite *
+id3demux_suite (void)
+{
+  Suite *s = suite_create ("id3demux");
+  TCase *tc_chain = tcase_create ("general");
+
+  suite_add_tcase (s, tc_chain);
+  tcase_add_test (tc_chain, test_tdat_tyer);
+
+  return s;
+}
+
+GST_CHECK_MAIN (id3demux)

tests/files/Makefile.am

@@ -0,0 +1,5 @@
+
+EXTRA_DIST = \
+	id3-407349-1.tag \
+	id3-407349-2.tag
+