use the setup/teardown methods to save code. save code is good.

Original commit message from CVS:
use the setup/teardown methods to save code.  save code is good.

ChangeLog

@@ -1,3 +1,16 @@
+2005-08-21  Thomas Vander Stichele  <thomas at apestaart dot org>
+
+	* check/elements/volume.c: (setup_volume), (cleanup_volume),
+	(GST_START_TEST):
+	* check/elements/vorbisdec.c: (setup_vorbisdec),
+	(cleanup_vorbisdec), (GST_START_TEST), (vorbisdec_suite):
+	* ext/vorbis/vorbisdec.c: (vorbis_dec_src_query),
+	(vorbis_handle_identification_packet),
+	(vorbis_handle_comment_packet), (vorbis_handle_type_packet),
+	(vorbis_handle_header_packet), (vorbis_dec_push),
+	(vorbis_dec_chain):
+	  use the setup/teardown methods to save code.  save code is good.
+
 2005-08-20  Thomas Vander Stichele  <thomas at apestaart dot org>
 
 	* check/Makefile.am:

check/elements/volume.c

@@ -85,89 +85,22 @@ GstElement *
 setup_volume ()
 {
   GstElement *volume;
-  GstPad *srcpad, *sinkpad;
 
   GST_DEBUG ("setup_volume");
-
+  volume = gst_check_setup_element ("volume");
-  volume = gst_element_factory_make ("volume", "volume");
+  mysrcpad = gst_check_setup_src_pad (volume, &srctemplate, NULL);
-  fail_if (volume == NULL, "Could not create a volume");
+  mysinkpad = gst_check_setup_sink_pad (volume, &sinktemplate, NULL);
-
-  /* sending pad */
-  mysrcpad =
-      gst_pad_new_from_template (gst_static_pad_template_get (&srctemplate),
-      "src");
-  fail_if (mysrcpad == NULL, "Could not create a mysrcpad");
-  ASSERT_OBJECT_REFCOUNT (mysrcpad, "mysrcpad", 1);
-
-  sinkpad = gst_element_get_pad (volume, "sink");
-  fail_if (sinkpad == NULL, "Could not get source pad from volume");
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-  gst_pad_set_caps (mysrcpad, NULL);
-  fail_unless (gst_pad_link (mysrcpad, sinkpad) == GST_PAD_LINK_OK,
-      "Could not link source and volume sink pads");
-  gst_object_unref (sinkpad);   /* because we got it higher up */
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 1);
-
-  /* receiving pad */
-  mysinkpad =
-      gst_pad_new_from_template (gst_static_pad_template_get (&sinktemplate),
-      "sink");
-  fail_if (mysinkpad == NULL, "Could not create a mysinkpad");
-
-  srcpad = gst_element_get_pad (volume, "src");
-  fail_if (srcpad == NULL, "Could not get source pad from volume");
-  gst_pad_set_caps (mysinkpad, NULL);
-  gst_pad_set_chain_function (mysinkpad, chain_func);
-
-  fail_unless (gst_pad_link (srcpad, mysinkpad) == GST_PAD_LINK_OK,
-      "Could not link volume source and mysink pads");
-  gst_object_unref (srcpad);    /* because we got it higher up */
-  ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 1);
-
   return volume;
 }
 
 void
 cleanup_volume (GstElement * volume)
 {
-  GstPad *srcpad, *sinkpad;
-
   GST_DEBUG ("cleanup_volume");
 
-  fail_unless (gst_element_set_state (volume, GST_STATE_NULL) ==
+  gst_check_teardown_src_pad (volume);
-      GST_STATE_SUCCESS, "could not set to null");
+  gst_check_teardown_sink_pad (volume);
-  ASSERT_OBJECT_REFCOUNT (volume, "volume", 1);
+  gst_check_teardown_element (volume);
-
-  /* clean up floating src pad */
-  sinkpad = gst_element_get_pad (volume, "sink");
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-
-  gst_pad_unlink (mysrcpad, sinkpad);
-
-  /* pad refs held by both creator and this function (through _get) */
-  ASSERT_OBJECT_REFCOUNT (mysrcpad, "srcpad", 1);
-  gst_object_unref (mysrcpad);
-  mysrcpad = NULL;
-
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-  gst_object_unref (sinkpad);
-  /* one more ref is held by volume itself */
-
-  /* clean up floating sink pad */
-  srcpad = gst_element_get_pad (volume, "src");
-  gst_pad_unlink (srcpad, mysinkpad);
-
-  /* pad refs held by both creator and this function (through _get) */
-  ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 2);
-  gst_object_unref (srcpad);
-  /* one more ref is held by volume itself */
-
-  ASSERT_OBJECT_REFCOUNT (mysinkpad, "mysinkpad", 1);
-  gst_object_unref (mysinkpad);
-  mysinkpad = NULL;
-
-  ASSERT_OBJECT_REFCOUNT (volume, "volume", 1);
-  gst_object_unref (volume);
 }
 
 GST_START_TEST (test_unity)

check/elements/vorbisdec.c

@@ -45,6 +45,17 @@ guchar identification_header[30] = {
   0x01,                         /* framing_flag */
 };
 
+guchar comment_header[] = {
+  3,                            /* packet_type */
+  'v', 'o', 'r', 'b', 'i', 's',
+  2, 0, 0, 0,                   /* vendor_length */
+  'm', 'e',
+  1, 0, 0, 0,                   /* user_comment_list_length */
+  9, 0, 0, 0,                   /* length comment[0] */
+  'A', 'R', 'T', 'I', 'S', 'T', '=', 'm', 'e',
+  0x01,                         /* framing bit */
+};
+
 static GstStaticPadTemplate sinktemplate = GST_STATIC_PAD_TEMPLATE ("sink",
     GST_PAD_SINK,
     GST_PAD_ALWAYS,
@@ -54,15 +65,6 @@ static GstStaticPadTemplate srctemplate = GST_STATIC_PAD_TEMPLATE ("src",
     GST_PAD_ALWAYS,
     GST_STATIC_CAPS_ANY);
 
-GstFlowReturn
-chain_func (GstPad * pad, GstBuffer * buffer)
-{
-  GST_DEBUG ("chain_func: received buffer %p", buffer);
-  buffers = g_list_append (buffers, buffer);
-
-  return GST_FLOW_OK;
-}
-
 GstElement *
 setup_vorbisdec ()
 {
@@ -70,41 +72,9 @@ setup_vorbisdec ()
   GstPad *srcpad, *sinkpad;
 
   GST_DEBUG ("setup_vorbisdec");
-
+  vorbisdec = gst_check_setup_element ("vorbisdec");
-  vorbisdec = gst_element_factory_make ("vorbisdec", "vorbisdec");
+  mysrcpad = gst_check_setup_src_pad (vorbisdec, &srctemplate, NULL);
-  fail_if (vorbisdec == NULL, "Could not create a vorbisdec");
+  mysinkpad = gst_check_setup_sink_pad (vorbisdec, &sinktemplate, NULL);
-
-  /* sending pad */
-  mysrcpad =
-      gst_pad_new_from_template (gst_static_pad_template_get (&srctemplate),
-      "src");
-  fail_if (mysrcpad == NULL, "Could not create a mysrcpad");
-  ASSERT_OBJECT_REFCOUNT (mysrcpad, "mysrcpad", 1);
-
-  sinkpad = gst_element_get_pad (vorbisdec, "sink");
-  fail_if (sinkpad == NULL, "Could not get source pad from vorbisdec");
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-  gst_pad_set_caps (mysrcpad, NULL);
-  fail_unless (gst_pad_link (mysrcpad, sinkpad) == GST_PAD_LINK_OK,
-      "Could not link source and vorbisdec sink pads");
-  gst_object_unref (sinkpad);   /* because we got it higher up */
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 1);
-
-  /* receiving pad */
-  mysinkpad =
-      gst_pad_new_from_template (gst_static_pad_template_get (&sinktemplate),
-      "sink");
-  fail_if (mysinkpad == NULL, "Could not create a mysinkpad");
-
-  srcpad = gst_element_get_pad (vorbisdec, "src");
-  fail_if (srcpad == NULL, "Could not get source pad from vorbisdec");
-  gst_pad_set_caps (mysinkpad, NULL);
-  gst_pad_set_chain_function (mysinkpad, chain_func);
-
-  fail_unless (gst_pad_link (srcpad, mysinkpad) == GST_PAD_LINK_OK,
-      "Could not link vorbisdec source and mysink pads");
-  gst_object_unref (srcpad);    /* because we got it higher up */
-  ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 1);
 
   return vorbisdec;
 }
@@ -116,50 +86,57 @@ cleanup_vorbisdec (GstElement * vorbisdec)
 
   GST_DEBUG ("cleanup_vorbisdec");
 
-  fail_unless (gst_element_set_state (vorbisdec, GST_STATE_NULL) ==
+  gst_check_teardown_src_pad (vorbisdec);
-      GST_STATE_SUCCESS, "could not set to null");
+  gst_check_teardown_sink_pad (vorbisdec);
-  ASSERT_OBJECT_REFCOUNT (vorbisdec, "vorbisdec", 1);
+  gst_check_teardown_element (vorbisdec);
-
-  /* clean up floating src pad */
-  sinkpad = gst_element_get_pad (vorbisdec, "sink");
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-
-  gst_pad_unlink (mysrcpad, sinkpad);
-
-  /* pad refs held by both creator and this function (through _get) */
-  ASSERT_OBJECT_REFCOUNT (mysrcpad, "srcpad", 1);
-  gst_object_unref (mysrcpad);
-  mysrcpad = NULL;
-
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-  gst_object_unref (sinkpad);
-  /* one more ref is held by vorbisdec itself */
-
-  /* clean up floating sink pad */
-  srcpad = gst_element_get_pad (vorbisdec, "src");
-  gst_pad_unlink (srcpad, mysinkpad);
-
-  /* pad refs held by both creator and this function (through _get) */
-  ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 2);
-  gst_object_unref (srcpad);
-  /* one more ref is held by vorbisdec itself */
-
-  ASSERT_OBJECT_REFCOUNT (mysinkpad, "mysinkpad", 1);
-  gst_object_unref (mysinkpad);
-  mysinkpad = NULL;
-
-  ASSERT_OBJECT_REFCOUNT (vorbisdec, "vorbisdec", 1);
-  gst_object_unref (vorbisdec);
 }
 
+GST_START_TEST (test_wrong_channels_identification_header)
+{
+  GstElement *vorbisdec;
+  GstBuffer *inbuffer, *outbuffer;
+  GstBus *bus;
+  GstMessage *message;
+
+  vorbisdec = setup_vorbisdec ();
+  fail_unless (gst_element_set_state (vorbisdec,
+          GST_STATE_PLAYING) == GST_STATE_SUCCESS, "could not set to playing");
+  bus = gst_bus_new ();
+
+  inbuffer = gst_buffer_new_and_alloc (30);
+  memcpy (GST_BUFFER_DATA (inbuffer), identification_header, 30);
+  /* set the channel count to 7, which is not supported */
+  GST_BUFFER_DATA (inbuffer)[11] = 7;
+  ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
+  gst_buffer_ref (inbuffer);
+
+  gst_element_set_bus (vorbisdec, bus);
+  /* pushing gives away my reference ... */
+  fail_unless_equals_int (gst_pad_push (mysrcpad, inbuffer), GST_FLOW_ERROR);
+  /* ... and nothing ends up on the global buffer list */
+  ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
+  gst_buffer_unref (inbuffer);
+  fail_unless_equals_int (g_list_length (buffers), 0);
+
+  fail_if ((message = gst_bus_pop (bus)) == NULL);
+  fail_unless_message_error (message, STREAM, NOT_IMPLEMENTED);
+  gst_message_unref (message);
+  gst_element_set_bus (vorbisdec, NULL);
+
+  /* cleanup */
+  gst_object_unref (GST_OBJECT (bus));
+  cleanup_vorbisdec (vorbisdec);
+}
+
+GST_END_TEST;
+
+
 GST_START_TEST (test_empty_identification_header)
 {
   GstElement *vorbisdec;
   GstBuffer *inbuffer, *outbuffer;
   GstBus *bus;
   GstMessage *message;
-  GError *error;
-  gchar *debug;
 
   vorbisdec = setup_vorbisdec ();
   bus = gst_bus_new ();
@@ -189,21 +166,37 @@ GST_START_TEST (test_empty_identification_header)
 
 GST_END_TEST;
 
-
+/* FIXME: also tests comment header */
-GST_START_TEST (test_unity)
+GST_START_TEST (test_identification_header)
 {
   GstElement *vorbisdec;
   GstBuffer *inbuffer, *outbuffer;
-  gint16 in[2] = { 16384, -256 };
+  GstBus *bus;
+  GstMessage *message;
+  GstTagList *tag_list;
+  gchar *artist;
 
   vorbisdec = setup_vorbisdec ();
   fail_unless (gst_element_set_state (vorbisdec,
           GST_STATE_PLAYING) == GST_STATE_SUCCESS, "could not set to playing");
+  bus = gst_bus_new ();
 
   inbuffer = gst_buffer_new_and_alloc (30);
   memcpy (GST_BUFFER_DATA (inbuffer), identification_header, 30);
-  //FIXME: add a test for wrong channels, like so:
+  ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
-  //GST_BUFFER_DATA (inbuffer)[12] = 7;
+  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);
 
@@ -213,8 +206,20 @@ GST_START_TEST (test_unity)
   ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
   gst_buffer_unref (inbuffer);
   fail_unless (g_list_length (buffers) == 0);
+  /* there's a tag message waiting */
+  fail_if ((message = gst_bus_pop (bus)) == NULL);
+  gst_message_parse_tag (message, &tag_list);
+  fail_unless_equals_int (gst_tag_list_get_tag_size (tag_list, GST_TAG_ARTIST),
+      1);
+  fail_unless (gst_tag_list_get_string (tag_list, GST_TAG_ARTIST, &artist));
+  fail_unless_equals_string (artist, "me");
+  fail_unless_equals_int (gst_tag_list_get_tag_size (tag_list, "album"), 0);
+  gst_tag_list_free (tag_list);
+  gst_message_unref (message);
 
   /* cleanup */
+  gst_element_set_bus (vorbisdec, NULL);
+  gst_object_unref (GST_OBJECT (bus));
   cleanup_vorbisdec (vorbisdec);
 }
 
@@ -227,8 +232,9 @@ vorbisdec_suite (void)
   TCase *tc_chain = tcase_create ("general");
 
   suite_add_tcase (s, tc_chain);
-  tcase_add_test (tc_chain, test_unity);
   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);
 
   return s;
 }

common

@@ -1 +1 @@
-Subproject commit 4cc6f465857331531a09aff0a23dc0b133e7f669
+Subproject commit 2827052513b1aa41f4a2414c163cfd0f4790b43c

ext/vorbis/vorbisdec.c

@@ -299,7 +299,7 @@ vorbis_dec_src_query (GstPad * pad, GstQuery * query)
 
 error:
   {
-    GST_DEBUG ("error handling query");
+    GST_WARNING_OBJECT (dec, "error handling query");
     return res;
   }
 }
@@ -419,74 +419,11 @@ vorbis_dec_sink_event (GstPad * pad, GstEvent * event)
 }
 
 static GstFlowReturn
-vorbis_handle_comment_packet (GstVorbisDec * vd, ogg_packet * packet)
+vorbis_handle_identification_packet (GstVorbisDec * vd)
-{
-  guint bitrate = 0;
-  gchar *encoder = NULL;
-  GstMessage *message;
-  GstTagList *list;
-  GstBuffer *buf;
-
-  GST_DEBUG ("parsing comment packet");
-
-  buf = gst_buffer_new_and_alloc (packet->bytes);
-  GST_BUFFER_DATA (buf) = packet->packet;
-
-  list =
-      gst_tag_list_from_vorbiscomment_buffer (buf, (guint8 *) "\003vorbis", 7,
-      &encoder);
-
-  gst_buffer_unref (buf);
-
-  if (!list) {
-    GST_ERROR_OBJECT (vd, "couldn't decode comments");
-    list = gst_tag_list_new ();
-  }
-  if (encoder) {
-    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
-        GST_TAG_ENCODER, encoder, NULL);
-    g_free (encoder);
-  }
-  gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
-      GST_TAG_ENCODER_VERSION, vd->vi.version,
-      GST_TAG_AUDIO_CODEC, "Vorbis", NULL);
-  if (vd->vi.bitrate_nominal > 0) {
-    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
-        GST_TAG_NOMINAL_BITRATE, (guint) vd->vi.bitrate_nominal, NULL);
-    bitrate = vd->vi.bitrate_nominal;
-  }
-  if (vd->vi.bitrate_upper > 0) {
-    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
-        GST_TAG_MAXIMUM_BITRATE, (guint) vd->vi.bitrate_upper, NULL);
-    if (!bitrate)
-      bitrate = vd->vi.bitrate_upper;
-  }
-  if (vd->vi.bitrate_lower > 0) {
-    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
-        GST_TAG_MINIMUM_BITRATE, (guint) vd->vi.bitrate_lower, NULL);
-    if (!bitrate)
-      bitrate = vd->vi.bitrate_lower;
-  }
-  if (bitrate) {
-    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
-        GST_TAG_BITRATE, (guint) bitrate, NULL);
-  }
-
-  message = gst_message_new_tag ((GstObject *) vd, list);
-  gst_element_post_message (GST_ELEMENT (vd), message);
-
-  return GST_FLOW_OK;
-}
-
-static GstFlowReturn
-vorbis_handle_type_packet (GstVorbisDec * vd, ogg_packet * packet)
 {
   GstCaps *caps;
   const GstAudioChannelPosition *pos = NULL;
 
-  /* done */
-  vorbis_synthesis_init (&vd->vd, &vd->vi);
-  vorbis_block_init (&vd->vd, &vd->vb);
   caps = gst_caps_new_simple ("audio/x-raw-float",
       "rate", G_TYPE_INT, vd->vi.rate,
       "channels", G_TYPE_INT, vd->vi.channels,
@@ -549,8 +486,6 @@ vorbis_handle_type_packet (GstVorbisDec * vd, ogg_packet * packet)
   gst_pad_set_caps (vd->srcpad, caps);
   gst_caps_unref (caps);
 
-  vd->initialized = TRUE;
-
   return GST_FLOW_OK;
 
   /* ERROR */
@@ -563,12 +498,82 @@ channel_count_error:
   }
 }
 
+static GstFlowReturn
+vorbis_handle_comment_packet (GstVorbisDec * vd, ogg_packet * packet)
+{
+  guint bitrate = 0;
+  gchar *encoder = NULL;
+  GstMessage *message;
+  GstTagList *list;
+  GstBuffer *buf;
+
+  GST_DEBUG_OBJECT (vd, "parsing comment packet");
+
+  buf = gst_buffer_new_and_alloc (packet->bytes);
+  GST_BUFFER_DATA (buf) = packet->packet;
+
+  list =
+      gst_tag_list_from_vorbiscomment_buffer (buf, (guint8 *) "\003vorbis", 7,
+      &encoder);
+
+  gst_buffer_unref (buf);
+
+  if (!list) {
+    GST_ERROR_OBJECT (vd, "couldn't decode comments");
+    list = gst_tag_list_new ();
+  }
+  if (encoder) {
+    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
+        GST_TAG_ENCODER, encoder, NULL);
+    g_free (encoder);
+  }
+  gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
+      GST_TAG_ENCODER_VERSION, vd->vi.version,
+      GST_TAG_AUDIO_CODEC, "Vorbis", NULL);
+  if (vd->vi.bitrate_nominal > 0) {
+    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
+        GST_TAG_NOMINAL_BITRATE, (guint) vd->vi.bitrate_nominal, NULL);
+    bitrate = vd->vi.bitrate_nominal;
+  }
+  if (vd->vi.bitrate_upper > 0) {
+    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
+        GST_TAG_MAXIMUM_BITRATE, (guint) vd->vi.bitrate_upper, NULL);
+    if (!bitrate)
+      bitrate = vd->vi.bitrate_upper;
+  }
+  if (vd->vi.bitrate_lower > 0) {
+    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
+        GST_TAG_MINIMUM_BITRATE, (guint) vd->vi.bitrate_lower, NULL);
+    if (!bitrate)
+      bitrate = vd->vi.bitrate_lower;
+  }
+  if (bitrate) {
+    gst_tag_list_add (list, GST_TAG_MERGE_REPLACE,
+        GST_TAG_BITRATE, (guint) bitrate, NULL);
+  }
+
+  message = gst_message_new_tag ((GstObject *) vd, list);
+  gst_element_post_message (GST_ELEMENT (vd), message);
+
+  return GST_FLOW_OK;
+}
+
+static GstFlowReturn
+vorbis_handle_type_packet (GstVorbisDec * vd)
+{
+  vorbis_synthesis_init (&vd->vd, &vd->vi);
+  vorbis_block_init (&vd->vd, &vd->vb);
+  vd->initialized = TRUE;
+
+  return GST_FLOW_OK;
+}
+
 static GstFlowReturn
 vorbis_handle_header_packet (GstVorbisDec * vd, ogg_packet * packet)
 {
   GstFlowReturn res;
 
-  GST_DEBUG ("parsing header packet");
+  GST_DEBUG_OBJECT (vd, "parsing header packet");
 
   /* Packetno = 0 if the first byte is exactly 0x01 */
   packet->b_o_s = (packet->packet[0] == 0x1) ? 1 : 0;
@@ -576,12 +581,17 @@ vorbis_handle_header_packet (GstVorbisDec * vd, ogg_packet * packet)
   if (vorbis_synthesis_headerin (&vd->vi, &vd->vc, packet))
     goto header_read_error;
 
+  /* FIXME: we should probably double-check if packet[0] is 1/3/5 for each
+   * of these */
   switch (packet->packetno) {
+    case 0:
+      res = vorbis_handle_identification_packet (vd);
+      break;
     case 1:
       res = vorbis_handle_comment_packet (vd, packet);
       break;
     case 2:
-      res = vorbis_handle_type_packet (vd, packet);
+      res = vorbis_handle_type_packet (vd);
       break;
     default:
       /* ignore */
@@ -645,8 +655,8 @@ vorbis_dec_push (GstVorbisDec * dec, GstBuffer * buf)
 
         GST_BUFFER_OFFSET (buffer) = outoffset;
         GST_BUFFER_TIMESTAMP (buffer) = outoffset * GST_SECOND / dec->vi.rate;;
-        GST_DEBUG_OBJECT (dec, "patch buffer %lld offset %lld", size,
+        GST_DEBUG_OBJECT (dec, "patch buffer %" G_GUINT64_FORMAT
-            outoffset);
+            " offset %" G_GUINT64_FORMAT, size, outoffset);
         size--;
       }
       for (walk = dec->queued; walk; walk = g_list_next (walk)) {
@@ -755,7 +765,7 @@ vorbis_dec_chain (GstPad * pad, GstBuffer * buffer)
 
   if (GST_BUFFER_SIZE (buffer) == 0) {
     gst_buffer_unref (buffer);
-    GST_ELEMENT_ERROR (vd, STREAM, DECODE, (NULL), ("Empty buffer received"));
+    GST_ELEMENT_ERROR (vd, STREAM, DECODE, (NULL), ("empty buffer received"));
     return GST_FLOW_ERROR;
   }
   /* make ogg_packet out of the buffer */
@@ -766,6 +776,8 @@ vorbis_dec_chain (GstPad * pad, GstBuffer * buffer)
   /*
    * FIXME. Is there anyway to know that this is the last packet and
    * set e_o_s??
+   * Yes there is, keep one packet at all times and only push out when
+   * you receive a new one.  Implement this.
    */
   packet.e_o_s = 0;
 

tests/check/elements/volume.c

@@ -85,89 +85,22 @@ GstElement *
 setup_volume ()
 {
   GstElement *volume;
-  GstPad *srcpad, *sinkpad;
 
   GST_DEBUG ("setup_volume");
-
+  volume = gst_check_setup_element ("volume");
-  volume = gst_element_factory_make ("volume", "volume");
+  mysrcpad = gst_check_setup_src_pad (volume, &srctemplate, NULL);
-  fail_if (volume == NULL, "Could not create a volume");
+  mysinkpad = gst_check_setup_sink_pad (volume, &sinktemplate, NULL);
-
-  /* sending pad */
-  mysrcpad =
-      gst_pad_new_from_template (gst_static_pad_template_get (&srctemplate),
-      "src");
-  fail_if (mysrcpad == NULL, "Could not create a mysrcpad");
-  ASSERT_OBJECT_REFCOUNT (mysrcpad, "mysrcpad", 1);
-
-  sinkpad = gst_element_get_pad (volume, "sink");
-  fail_if (sinkpad == NULL, "Could not get source pad from volume");
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-  gst_pad_set_caps (mysrcpad, NULL);
-  fail_unless (gst_pad_link (mysrcpad, sinkpad) == GST_PAD_LINK_OK,
-      "Could not link source and volume sink pads");
-  gst_object_unref (sinkpad);   /* because we got it higher up */
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 1);
-
-  /* receiving pad */
-  mysinkpad =
-      gst_pad_new_from_template (gst_static_pad_template_get (&sinktemplate),
-      "sink");
-  fail_if (mysinkpad == NULL, "Could not create a mysinkpad");
-
-  srcpad = gst_element_get_pad (volume, "src");
-  fail_if (srcpad == NULL, "Could not get source pad from volume");
-  gst_pad_set_caps (mysinkpad, NULL);
-  gst_pad_set_chain_function (mysinkpad, chain_func);
-
-  fail_unless (gst_pad_link (srcpad, mysinkpad) == GST_PAD_LINK_OK,
-      "Could not link volume source and mysink pads");
-  gst_object_unref (srcpad);    /* because we got it higher up */
-  ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 1);
-
   return volume;
 }
 
 void
 cleanup_volume (GstElement * volume)
 {
-  GstPad *srcpad, *sinkpad;
-
   GST_DEBUG ("cleanup_volume");
 
-  fail_unless (gst_element_set_state (volume, GST_STATE_NULL) ==
+  gst_check_teardown_src_pad (volume);
-      GST_STATE_SUCCESS, "could not set to null");
+  gst_check_teardown_sink_pad (volume);
-  ASSERT_OBJECT_REFCOUNT (volume, "volume", 1);
+  gst_check_teardown_element (volume);
-
-  /* clean up floating src pad */
-  sinkpad = gst_element_get_pad (volume, "sink");
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-
-  gst_pad_unlink (mysrcpad, sinkpad);
-
-  /* pad refs held by both creator and this function (through _get) */
-  ASSERT_OBJECT_REFCOUNT (mysrcpad, "srcpad", 1);
-  gst_object_unref (mysrcpad);
-  mysrcpad = NULL;
-
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-  gst_object_unref (sinkpad);
-  /* one more ref is held by volume itself */
-
-  /* clean up floating sink pad */
-  srcpad = gst_element_get_pad (volume, "src");
-  gst_pad_unlink (srcpad, mysinkpad);
-
-  /* pad refs held by both creator and this function (through _get) */
-  ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 2);
-  gst_object_unref (srcpad);
-  /* one more ref is held by volume itself */
-
-  ASSERT_OBJECT_REFCOUNT (mysinkpad, "mysinkpad", 1);
-  gst_object_unref (mysinkpad);
-  mysinkpad = NULL;
-
-  ASSERT_OBJECT_REFCOUNT (volume, "volume", 1);
-  gst_object_unref (volume);
 }
 
 GST_START_TEST (test_unity)

tests/check/elements/vorbisdec.c

@@ -45,6 +45,17 @@ guchar identification_header[30] = {
   0x01,                         /* framing_flag */
 };
 
+guchar comment_header[] = {
+  3,                            /* packet_type */
+  'v', 'o', 'r', 'b', 'i', 's',
+  2, 0, 0, 0,                   /* vendor_length */
+  'm', 'e',
+  1, 0, 0, 0,                   /* user_comment_list_length */
+  9, 0, 0, 0,                   /* length comment[0] */
+  'A', 'R', 'T', 'I', 'S', 'T', '=', 'm', 'e',
+  0x01,                         /* framing bit */
+};
+
 static GstStaticPadTemplate sinktemplate = GST_STATIC_PAD_TEMPLATE ("sink",
     GST_PAD_SINK,
     GST_PAD_ALWAYS,
@@ -54,15 +65,6 @@ static GstStaticPadTemplate srctemplate = GST_STATIC_PAD_TEMPLATE ("src",
     GST_PAD_ALWAYS,
     GST_STATIC_CAPS_ANY);
 
-GstFlowReturn
-chain_func (GstPad * pad, GstBuffer * buffer)
-{
-  GST_DEBUG ("chain_func: received buffer %p", buffer);
-  buffers = g_list_append (buffers, buffer);
-
-  return GST_FLOW_OK;
-}
-
 GstElement *
 setup_vorbisdec ()
 {
@@ -70,41 +72,9 @@ setup_vorbisdec ()
   GstPad *srcpad, *sinkpad;
 
   GST_DEBUG ("setup_vorbisdec");
-
+  vorbisdec = gst_check_setup_element ("vorbisdec");
-  vorbisdec = gst_element_factory_make ("vorbisdec", "vorbisdec");
+  mysrcpad = gst_check_setup_src_pad (vorbisdec, &srctemplate, NULL);
-  fail_if (vorbisdec == NULL, "Could not create a vorbisdec");
+  mysinkpad = gst_check_setup_sink_pad (vorbisdec, &sinktemplate, NULL);
-
-  /* sending pad */
-  mysrcpad =
-      gst_pad_new_from_template (gst_static_pad_template_get (&srctemplate),
-      "src");
-  fail_if (mysrcpad == NULL, "Could not create a mysrcpad");
-  ASSERT_OBJECT_REFCOUNT (mysrcpad, "mysrcpad", 1);
-
-  sinkpad = gst_element_get_pad (vorbisdec, "sink");
-  fail_if (sinkpad == NULL, "Could not get source pad from vorbisdec");
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-  gst_pad_set_caps (mysrcpad, NULL);
-  fail_unless (gst_pad_link (mysrcpad, sinkpad) == GST_PAD_LINK_OK,
-      "Could not link source and vorbisdec sink pads");
-  gst_object_unref (sinkpad);   /* because we got it higher up */
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 1);
-
-  /* receiving pad */
-  mysinkpad =
-      gst_pad_new_from_template (gst_static_pad_template_get (&sinktemplate),
-      "sink");
-  fail_if (mysinkpad == NULL, "Could not create a mysinkpad");
-
-  srcpad = gst_element_get_pad (vorbisdec, "src");
-  fail_if (srcpad == NULL, "Could not get source pad from vorbisdec");
-  gst_pad_set_caps (mysinkpad, NULL);
-  gst_pad_set_chain_function (mysinkpad, chain_func);
-
-  fail_unless (gst_pad_link (srcpad, mysinkpad) == GST_PAD_LINK_OK,
-      "Could not link vorbisdec source and mysink pads");
-  gst_object_unref (srcpad);    /* because we got it higher up */
-  ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 1);
 
   return vorbisdec;
 }
@@ -116,50 +86,57 @@ cleanup_vorbisdec (GstElement * vorbisdec)
 
   GST_DEBUG ("cleanup_vorbisdec");
 
-  fail_unless (gst_element_set_state (vorbisdec, GST_STATE_NULL) ==
+  gst_check_teardown_src_pad (vorbisdec);
-      GST_STATE_SUCCESS, "could not set to null");
+  gst_check_teardown_sink_pad (vorbisdec);
-  ASSERT_OBJECT_REFCOUNT (vorbisdec, "vorbisdec", 1);
+  gst_check_teardown_element (vorbisdec);
-
-  /* clean up floating src pad */
-  sinkpad = gst_element_get_pad (vorbisdec, "sink");
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-
-  gst_pad_unlink (mysrcpad, sinkpad);
-
-  /* pad refs held by both creator and this function (through _get) */
-  ASSERT_OBJECT_REFCOUNT (mysrcpad, "srcpad", 1);
-  gst_object_unref (mysrcpad);
-  mysrcpad = NULL;
-
-  ASSERT_OBJECT_REFCOUNT (sinkpad, "sinkpad", 2);
-  gst_object_unref (sinkpad);
-  /* one more ref is held by vorbisdec itself */
-
-  /* clean up floating sink pad */
-  srcpad = gst_element_get_pad (vorbisdec, "src");
-  gst_pad_unlink (srcpad, mysinkpad);
-
-  /* pad refs held by both creator and this function (through _get) */
-  ASSERT_OBJECT_REFCOUNT (srcpad, "srcpad", 2);
-  gst_object_unref (srcpad);
-  /* one more ref is held by vorbisdec itself */
-
-  ASSERT_OBJECT_REFCOUNT (mysinkpad, "mysinkpad", 1);
-  gst_object_unref (mysinkpad);
-  mysinkpad = NULL;
-
-  ASSERT_OBJECT_REFCOUNT (vorbisdec, "vorbisdec", 1);
-  gst_object_unref (vorbisdec);
 }
 
+GST_START_TEST (test_wrong_channels_identification_header)
+{
+  GstElement *vorbisdec;
+  GstBuffer *inbuffer, *outbuffer;
+  GstBus *bus;
+  GstMessage *message;
+
+  vorbisdec = setup_vorbisdec ();
+  fail_unless (gst_element_set_state (vorbisdec,
+          GST_STATE_PLAYING) == GST_STATE_SUCCESS, "could not set to playing");
+  bus = gst_bus_new ();
+
+  inbuffer = gst_buffer_new_and_alloc (30);
+  memcpy (GST_BUFFER_DATA (inbuffer), identification_header, 30);
+  /* set the channel count to 7, which is not supported */
+  GST_BUFFER_DATA (inbuffer)[11] = 7;
+  ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
+  gst_buffer_ref (inbuffer);
+
+  gst_element_set_bus (vorbisdec, bus);
+  /* pushing gives away my reference ... */
+  fail_unless_equals_int (gst_pad_push (mysrcpad, inbuffer), GST_FLOW_ERROR);
+  /* ... and nothing ends up on the global buffer list */
+  ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
+  gst_buffer_unref (inbuffer);
+  fail_unless_equals_int (g_list_length (buffers), 0);
+
+  fail_if ((message = gst_bus_pop (bus)) == NULL);
+  fail_unless_message_error (message, STREAM, NOT_IMPLEMENTED);
+  gst_message_unref (message);
+  gst_element_set_bus (vorbisdec, NULL);
+
+  /* cleanup */
+  gst_object_unref (GST_OBJECT (bus));
+  cleanup_vorbisdec (vorbisdec);
+}
+
+GST_END_TEST;
+
+
 GST_START_TEST (test_empty_identification_header)
 {
   GstElement *vorbisdec;
   GstBuffer *inbuffer, *outbuffer;
   GstBus *bus;
   GstMessage *message;
-  GError *error;
-  gchar *debug;
 
   vorbisdec = setup_vorbisdec ();
   bus = gst_bus_new ();
@@ -189,21 +166,37 @@ GST_START_TEST (test_empty_identification_header)
 
 GST_END_TEST;
 
-
+/* FIXME: also tests comment header */
-GST_START_TEST (test_unity)
+GST_START_TEST (test_identification_header)
 {
   GstElement *vorbisdec;
   GstBuffer *inbuffer, *outbuffer;
-  gint16 in[2] = { 16384, -256 };
+  GstBus *bus;
+  GstMessage *message;
+  GstTagList *tag_list;
+  gchar *artist;
 
   vorbisdec = setup_vorbisdec ();
   fail_unless (gst_element_set_state (vorbisdec,
           GST_STATE_PLAYING) == GST_STATE_SUCCESS, "could not set to playing");
+  bus = gst_bus_new ();
 
   inbuffer = gst_buffer_new_and_alloc (30);
   memcpy (GST_BUFFER_DATA (inbuffer), identification_header, 30);
-  //FIXME: add a test for wrong channels, like so:
+  ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
-  //GST_BUFFER_DATA (inbuffer)[12] = 7;
+  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);
 
@@ -213,8 +206,20 @@ GST_START_TEST (test_unity)
   ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
   gst_buffer_unref (inbuffer);
   fail_unless (g_list_length (buffers) == 0);
+  /* there's a tag message waiting */
+  fail_if ((message = gst_bus_pop (bus)) == NULL);
+  gst_message_parse_tag (message, &tag_list);
+  fail_unless_equals_int (gst_tag_list_get_tag_size (tag_list, GST_TAG_ARTIST),
+      1);
+  fail_unless (gst_tag_list_get_string (tag_list, GST_TAG_ARTIST, &artist));
+  fail_unless_equals_string (artist, "me");
+  fail_unless_equals_int (gst_tag_list_get_tag_size (tag_list, "album"), 0);
+  gst_tag_list_free (tag_list);
+  gst_message_unref (message);
 
   /* cleanup */
+  gst_element_set_bus (vorbisdec, NULL);
+  gst_object_unref (GST_OBJECT (bus));
   cleanup_vorbisdec (vorbisdec);
 }
 
@@ -227,8 +232,9 @@ vorbisdec_suite (void)
   TCase *tc_chain = tcase_create ("general");
 
   suite_add_tcase (s, tc_chain);
-  tcase_add_test (tc_chain, test_unity);
   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);
 
   return s;
 }