diff --git a/ChangeLog b/ChangeLog index f8907b3a17..16b26ed094 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2007-06-27 Tim-Philipp Müller + + * ext/vorbis/vorbisdec.c: (vorbis_dec_decode_buffer): + Skip empty buffers, but not empty header buffers. That way the original + vorbisdec unit test still passes (#451145); also, take into account + that those empty packets might carry a granulepos. + + * tests/check/Makefile.am: + * tests/check/elements/vorbisdec.c: + (_create_codebook_header_buffer), (_create_audio_buffer), + (GST_START_TEST), (vorbisdec_suite): + Add unit test that sends an empty packet. + 2007-06-27 Wim Taymans * ext/vorbis/vorbisdec.c: (vorbis_dec_decode_buffer): diff --git a/ext/vorbis/vorbisdec.c b/ext/vorbis/vorbisdec.c index fa11617854..1eba3ccfd0 100644 --- a/ext/vorbis/vorbisdec.c +++ b/ext/vorbis/vorbisdec.c @@ -1127,8 +1127,13 @@ vorbis_dec_decode_buffer (GstVorbisDec * vd, GstBuffer * buffer) */ packet.e_o_s = 0; - if (G_UNLIKELY (packet.bytes < 1)) - goto wrong_size; + /* error out on empty header packets, but just skip empty data packets */ + if (G_UNLIKELY (packet.bytes == 0)) { + if (vd->initialized) + goto empty_buffer; + else + goto empty_header; + } GST_DEBUG_OBJECT (vd, "vorbis granule: %" G_GINT64_FORMAT, (gint64) packet.granulepos); @@ -1147,14 +1152,22 @@ vorbis_dec_decode_buffer (GstVorbisDec * vd, GstBuffer * buffer) done: return result; - /* ERRORS */ -wrong_size: +empty_buffer: { - /* don't error out here, just ignore the buffer, it's invalid for vorbis but - * not fatal. */ - GST_ELEMENT_WARNING (vd, STREAM, DECODE, (NULL), - ("empty buffer received, ignoring")); + /* don't error out here, just ignore the buffer, it's invalid for vorbis + * but not fatal. */ + GST_WARNING_OBJECT (vd, "empty buffer received, ignoring"); + if (packet.granulepos != -1) + vd->granulepos = packet.granulepos; result = GST_FLOW_OK; + goto done; + } + +/* ERRORS */ +empty_header: + { + GST_ELEMENT_ERROR (vd, STREAM, DECODE, (NULL), ("empty header received")); + result = GST_FLOW_ERROR; vd->discont = TRUE; goto done; } diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am index 6d2b4636d2..5de12992bf 100644 --- a/tests/check/Makefile.am +++ b/tests/check/Makefile.am @@ -205,6 +205,16 @@ elements_volume_CFLAGS = \ $(GST_BASE_CFLAGS) \ $(AM_CFLAGS) +elements_vorbisdec_LDADD = \ + $(LDADD) \ + $(VORBIS_LIBS) + +elements_vorbisdec_CFLAGS = \ + $(GST_PLUGINS_BASE_CFLAGS) \ + $(AM_CFLAGS) \ + $(VORBIS_CFLAGS) \ + $(CFLAGS) + elements_vorbistag_LDADD = \ $(LDADD) \ $(VORBIS_LIBS) diff --git a/tests/check/elements/vorbisdec.c b/tests/check/elements/vorbisdec.c index 101a8f095b..fa412ec9ba 100644 --- a/tests/check/elements/vorbisdec.c +++ b/tests/check/elements/vorbisdec.c @@ -24,6 +24,9 @@ #include +#include +#include + /* For ease of programming we use globals to keep refs for our floating * src and sink pads we create; otherwise we always have to do get_pad, * get_peer, and then remove references in every test function */ @@ -230,7 +233,137 @@ GST_START_TEST (test_identification_header) GST_END_TEST; -Suite * +vorbis_comment vc; +vorbis_dsp_state vd; +vorbis_info vi; +vorbis_block vb; + +static GstBuffer * +_create_codebook_header_buffer (void) +{ + GstBuffer *buffer; + ogg_packet header; + ogg_packet header_comm; + ogg_packet header_code; + + vorbis_info_init (&vi); + vorbis_encode_setup_vbr (&vi, 1, 44000, 0.5); + vorbis_encode_setup_init (&vi); + vorbis_analysis_init (&vd, &vi); + vorbis_block_init (&vd, &vb); + vorbis_comment_init (&vc); + vorbis_analysis_headerout (&vd, &vc, &header, &header_comm, &header_code); + + buffer = gst_buffer_new_and_alloc (header_code.bytes); + memcpy (GST_BUFFER_DATA (buffer), header_code.packet, header_code.bytes); + + return buffer; +} + +static GstBuffer * +_create_audio_buffer (void) +{ + GstBuffer *buffer; + ogg_packet packet; + float **vorbis_buffer; + gint i; + + vorbis_buffer = vorbis_analysis_buffer (&vd, 44100); + for (i = 0; i < 44100 * 1; ++i) + vorbis_buffer[0][i] = 0.0; + vorbis_analysis_wrote (&vd, 44100); + vorbis_analysis_blockout (&vd, &vb); + vorbis_analysis (&vb, NULL); + vorbis_bitrate_addblock (&vb); + vorbis_bitrate_flushpacket (&vd, &packet); + buffer = gst_buffer_new_and_alloc (packet.bytes); + memcpy (GST_BUFFER_DATA (buffer), packet.packet, packet.bytes); + + vorbis_comment_clear (&vc); + vorbis_block_clear (&vb); + vorbis_dsp_clear (&vd); + vorbis_info_clear (&vi); + + return buffer; +} + +GST_START_TEST (test_empty_vorbis_packet) +{ + GstElement *vorbisdec; + GstBuffer *inbuffer; + GstMessage *message; + GstBus *bus; + + vorbisdec = setup_vorbisdec (); + fail_unless_equals_int (gst_element_set_state (vorbisdec, GST_STATE_PLAYING), + GST_STATE_CHANGE_SUCCESS); + + bus = gst_bus_new (); + + inbuffer = gst_buffer_new_and_alloc (30); + memcpy (GST_BUFFER_DATA (inbuffer), identification_header, 30); + ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1); + gst_buffer_ref (inbuffer); + + gst_element_set_bus (vorbisdec, bus); + + /* pushing gives away my reference ... */ + fail_unless (gst_pad_push (mysrcpad, inbuffer) == GST_FLOW_OK); + /* ... and nothing ends up on the global buffer list */ + ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1); + gst_buffer_unref (inbuffer); + fail_unless (g_list_length (buffers) == 0); + fail_if ((message = gst_bus_pop (bus)) != NULL); + + inbuffer = gst_buffer_new_and_alloc (sizeof (comment_header)); + memcpy (GST_BUFFER_DATA (inbuffer), comment_header, sizeof (comment_header)); + ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1); + gst_buffer_ref (inbuffer); + + /* pushing gives away my reference ... */ + fail_unless (gst_pad_push (mysrcpad, inbuffer) == GST_FLOW_OK); + /* ... and nothing ends up on the global buffer list */ + ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1); + gst_buffer_unref (inbuffer); + fail_unless (g_list_length (buffers) == 0); + + /* send minimal codebook header and audio packers */ + inbuffer = _create_codebook_header_buffer (); + fail_unless_equals_int (gst_pad_push (mysrcpad, inbuffer), GST_FLOW_OK); + + /* now send an empty vorbis packet, which should just be skipped */ + inbuffer = gst_buffer_new_and_alloc (0); + gst_buffer_ref (inbuffer); + fail_unless_equals_int (gst_pad_push (mysrcpad, inbuffer), GST_FLOW_OK); + ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1); + gst_buffer_unref (inbuffer); + fail_unless (g_list_length (buffers) == 0); + + /* create and push an encoded audio packet */ + inbuffer = _create_audio_buffer (); + fail_unless_equals_int (gst_pad_push (mysrcpad, inbuffer), GST_FLOW_OK); + + /* now send another empty vorbis packet, which should just be skipped */ + inbuffer = gst_buffer_new_and_alloc (0); + gst_buffer_ref (inbuffer); + fail_unless_equals_int (gst_pad_push (mysrcpad, inbuffer), GST_FLOW_OK); + ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1); + gst_buffer_unref (inbuffer); + + /* make sure there's no error on the bus */ + message = gst_bus_poll (bus, GST_MESSAGE_ERROR, 0); + fail_if (message != NULL); + + /* cleanup */ + gst_bus_set_flushing (bus, TRUE); + gst_element_set_bus (vorbisdec, NULL); + gst_object_unref (GST_OBJECT (bus)); + cleanup_vorbisdec (vorbisdec); +} + +GST_END_TEST; + +static Suite * vorbisdec_suite (void) { Suite *s = suite_create ("vorbisdec"); @@ -240,6 +373,7 @@ vorbisdec_suite (void) tcase_add_test (tc_chain, test_empty_identification_header); tcase_add_test (tc_chain, test_wrong_channels_identification_header); tcase_add_test (tc_chain, test_identification_header); + tcase_add_test (tc_chain, test_empty_vorbis_packet); return s; }