From 75ecc060b1cf06b23d1369891765298630988bb6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Tim-Philipp=20M=C3=BCller?= Date: Thu, 17 Aug 2006 10:00:00 +0000 Subject: [PATCH] gst-libs/gst/cdda/gstcddabasesrc.c: Make buffer durations add up (duration should be next_ts-ts for perfect streams).... Original commit message from CVS: * gst-libs/gst/cdda/gstcddabasesrc.c: (gst_cdda_base_src_create): Make buffer durations add up (duration should be next_ts-ts for perfect streams). Fixes CD ripping to Ogg/Vorbis with vorbisenc from CVS. * tests/check/libs/cddabasesrc.c: (gst_cd_foo_src_close), (test_buffer_timestamps), (cddabasesrc_suite): Add unit test for the above. * tests/check/Makefile.am: Don't know why cddabasesrc test was in VALGRIND_TO_FIX, remove to see what happens. --- ChangeLog | 15 +++++++++ gst-libs/gst/cdda/gstcddabasesrc.c | 22 +++++++++---- tests/check/Makefile.am | 1 - tests/check/libs/cddabasesrc.c | 65 ++++++++++++++++++++++++++++++++++---- 4 files changed, 89 insertions(+), 14 deletions(-) diff --git a/ChangeLog b/ChangeLog index 40fcf3b..8ee24d9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2006-08-17 Tim-Philipp Müller + + * gst-libs/gst/cdda/gstcddabasesrc.c: (gst_cdda_base_src_create): + Make buffer durations add up (duration should be next_ts-ts for + perfect streams). Fixes CD ripping to Ogg/Vorbis with vorbisenc + from CVS. + + * tests/check/libs/cddabasesrc.c: (gst_cd_foo_src_close), + (test_buffer_timestamps), (cddabasesrc_suite): + Add unit test for the above. + + * tests/check/Makefile.am: + Don't know why cddabasesrc test was in VALGRIND_TO_FIX, remove + to see what happens. + 2006-08-16 Wim Taymans * ext/alsa/gstalsasink.c: (gst_alsasink_set_property), diff --git a/gst-libs/gst/cdda/gstcddabasesrc.c b/gst-libs/gst/cdda/gstcddabasesrc.c index 5cd582b..bbf6f49 100644 --- a/gst-libs/gst/cdda/gstcddabasesrc.c +++ b/gst-libs/gst/cdda/gstcddabasesrc.c @@ -1496,8 +1496,8 @@ gst_cdda_base_src_create (GstPushSrc * pushsrc, GstBuffer ** buffer) GstBuffer *buf; GstFormat format; gboolean eos; - gint64 position; - gint64 duration; + gint64 position = GST_CLOCK_TIME_NONE; + gint64 duration = GST_CLOCK_TIME_NONE; g_assert (klass->read_sector != NULL); @@ -1551,13 +1551,21 @@ gst_cdda_base_src_create (GstPushSrc * pushsrc, GstBuffer ** buffer) } format = GST_FORMAT_TIME; - if (!gst_pad_query_position (GST_BASE_SRC_PAD (src), &format, &position)) { - position = GST_CLOCK_TIME_NONE; + if (gst_pad_query_position (GST_BASE_SRC_PAD (src), &format, &position)) { + gint64 next_ts = 0; + + ++src->cur_sector; + if (gst_pad_query_position (GST_BASE_SRC_PAD (src), &format, &next_ts)) { + duration = next_ts - position; + } + --src->cur_sector; } - /* 4 bytes per sample, 44100 samples per second */ - duration = gst_util_uint64_scale_int (GST_BUFFER_SIZE (buf) >> 2, GST_SECOND, - 44100); + /* fallback duration: 4 bytes per sample, 44100 samples per second */ + if (duration == GST_CLOCK_TIME_NONE) { + duration = gst_util_uint64_scale_int (GST_BUFFER_SIZE (buf) >> 2, + GST_SECOND, 44100); + } GST_BUFFER_TIMESTAMP (buf) = position; GST_BUFFER_DURATION (buf) = duration; diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am index 213f62f..a9e0f15 100644 --- a/tests/check/Makefile.am +++ b/tests/check/Makefile.am @@ -66,7 +66,6 @@ VALGRIND_TO_FIX = \ elements/alsa \ elements/audioresample \ generic/states \ - libs/cddabasesrc \ libs/video # these tests don't even pass diff --git a/tests/check/libs/cddabasesrc.c b/tests/check/libs/cddabasesrc.c index c614449..0556c99 100644 --- a/tests/check/libs/cddabasesrc.c +++ b/tests/check/libs/cddabasesrc.c @@ -20,6 +20,12 @@ * Boston, MA 02111-1307, USA. */ +/* TODO: + * - test different modes (when seeking to tracks in track mode, buffer + * timestamps should start from 0, when seeking to tracks in disc mode, + * buffer timestamps should increment, etc.) + */ + #ifdef HAVE_CONFIG_H #include "config.h" #endif @@ -27,6 +33,7 @@ #include #include +#include #include #include @@ -250,8 +257,6 @@ gst_cd_foo_src_close (GstCddaBaseSrc * cddabasesrc) g_assert (g_str_equal (cddabasesrc->mb_discid, src->cur_test->musicbrainz_discid)); } - - ++src->cur_disc; } static GstBuffer * @@ -280,9 +285,9 @@ GST_PLUGIN_DEFINE_STATIC (GST_VERSION_MAJOR, GST_VERSION_MINOR, "cdfoosrc", "Read audio from CD", - plugin_init, VERSION, "LGPL", GST_PACKAGE_NAME, GST_PACKAGE_ORIGIN) + plugin_init, VERSION, "LGPL", GST_PACKAGE_NAME, GST_PACKAGE_ORIGIN); - GST_START_TEST (test_discid_calculations) +GST_START_TEST (test_discid_calculations) { GstElement *foosrc; gint i; @@ -290,7 +295,8 @@ GST_PLUGIN_DEFINE_STATIC (GST_VERSION_MAJOR, foosrc = gst_element_factory_make ("cdfoosrc", "cdfoosrc"); for (i = 0; i < G_N_ELEMENTS (test_discs); ++i) { - /* g_print ("Testing test disc layout %u ...\n", i); */ + GST_LOG ("Testing disc layout %u ...\n", i); + GST_CD_FOO_SRC (foosrc)->cur_disc = i; gst_element_set_state (foosrc, GST_STATE_PLAYING); gst_element_get_state (foosrc, NULL, NULL, -1); gst_element_set_state (foosrc, GST_STATE_NULL); @@ -301,7 +307,53 @@ GST_PLUGIN_DEFINE_STATIC (GST_VERSION_MAJOR, GST_END_TEST; -Suite * +GST_START_TEST (test_buffer_timestamps) +{ + GstElement *foosrc, *pipeline, *fakesink; + GstClockTime prev_ts, prev_duration, ts; + GstPad *sinkpad; + gint i; + + pipeline = gst_pipeline_new ("pipeline"); + foosrc = gst_element_factory_make ("cdfoosrc", "cdfoosrc"); + fakesink = gst_element_factory_make ("fakesink", "fakesink"); + gst_bin_add_many (GST_BIN (pipeline), foosrc, fakesink, NULL); + fail_unless (gst_element_link (foosrc, fakesink)); + sinkpad = gst_element_get_pad (fakesink, "sink"); + + GST_CD_FOO_SRC (foosrc)->cur_disc = 0; + + gst_buffer_straw_start_pipeline (pipeline, sinkpad); + + for (i = 0; i < 100; ++i) { + GstBuffer *buf; + + buf = gst_buffer_straw_get_buffer (pipeline, sinkpad); + GST_LOG ("buffer, ts=%" GST_TIME_FORMAT ", dur=%" GST_TIME_FORMAT, + GST_TIME_ARGS (GST_BUFFER_TIMESTAMP (buf)), + GST_TIME_ARGS (GST_BUFFER_DURATION (buf))); + ts = GST_BUFFER_TIMESTAMP (buf); + fail_unless (GST_CLOCK_TIME_IS_VALID (ts)); + fail_unless (GST_BUFFER_DURATION_IS_VALID (buf)); + if (i > 0) { + fail_unless (GST_CLOCK_TIME_IS_VALID (prev_ts)); + fail_unless (GST_CLOCK_TIME_IS_VALID (prev_duration)); + fail_unless ((prev_ts + prev_duration) == ts); + } + prev_ts = ts; + prev_duration = GST_BUFFER_DURATION (buf); + gst_buffer_unref (buf); + } + + gst_buffer_straw_stop_pipeline (pipeline, sinkpad); + + gst_object_unref (pipeline); + gst_object_unref (sinkpad); +} + +GST_END_TEST; + +static Suite * cddabasesrc_suite (void) { Suite *s = suite_create ("cddabasesrc"); @@ -309,6 +361,7 @@ cddabasesrc_suite (void) suite_add_tcase (s, tc_chain); tcase_add_test (tc_chain, test_discid_calculations); + tcase_add_test (tc_chain, test_buffer_timestamps); return s; } -- 2.7.4