diff --git a/ChangeLog b/ChangeLog index 2ef1667d6b..69e141e2ca 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,27 @@ +2007-03-06 Tim-Philipp Müller + + * 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 * tests/check/Makefile.am: diff --git a/configure.ac b/configure.ac index 620a63826c..d642ece632 100644 --- a/configure.ac +++ b/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 diff --git a/gst/id3demux/gstid3demux.c b/gst/id3demux/gstid3demux.c index 469514f9d0..886264f6e0 100644 --- a/gst/id3demux/gstid3demux.c +++ b/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; } diff --git a/tests/Makefile.am b/tests/Makefile.am index a0a361141c..5b098b94ee 100644 --- a/tests/Makefile.am +++ b/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 diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am index 1890ab888b..9fd70f3ac2 100644 --- a/tests/check/Makefile.am +++ b/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 \ diff --git a/tests/check/elements/.gitignore b/tests/check/elements/.gitignore index 4766f219fd..6befc6c246 100644 --- a/tests/check/elements/.gitignore +++ b/tests/check/elements/.gitignore @@ -7,6 +7,7 @@ cmmldec cmmlenc icydemux avimux +id3demux id3v2mux apev2mux audiopanorama diff --git a/tests/check/elements/id3demux.c b/tests/check/elements/id3demux.c new file mode 100644 index 0000000000..125cc5245f --- /dev/null +++ b/tests/check/elements/id3demux.c @@ -0,0 +1,191 @@ +/* GStreamer unit tests for id3demux + * + * Copyright (C) 2007 Tim-Philipp Müller + * + * 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 + +#include + +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) diff --git a/tests/files/Makefile.am b/tests/files/Makefile.am new file mode 100644 index 0000000000..c065d00778 --- /dev/null +++ b/tests/files/Makefile.am @@ -0,0 +1,5 @@ + +EXTRA_DIST = \ + id3-407349-1.tag \ + id3-407349-2.tag + diff --git a/tests/files/id3-407349-1.tag b/tests/files/id3-407349-1.tag new file mode 100644 index 0000000000..47a27634af Binary files /dev/null and b/tests/files/id3-407349-1.tag differ diff --git a/tests/files/id3-407349-2.tag b/tests/files/id3-407349-2.tag new file mode 100644 index 0000000000..d44a9a1d03 Binary files /dev/null and b/tests/files/id3-407349-2.tag differ