metadata: cleanup, fix the test, add comments

First do not build the plugin, if we have none of the backend, as it won't work
then. Fix the miniobject_unref error in the test. Sprinkle a first handful of
debug logs into the element code.

configure.ac

@@ -718,9 +718,9 @@ AG_GST_CHECK_FEATURE(METADATA, [METADATA muxer and demuxer], metadata, [
   fi
   if test x$HAVE_METADATA = xno; then
     AC_MSG_WARN(no of metadata libraries (exif, iptc or xmp) found)
+  else
+    HAVE_METADATA="yes"
   fi
-  dnl metadata plugin will parse whole chunks anyway
-  HAVE_METADATA="yes"
   AC_SUBST(METADATA_CFLAGS)
   AC_SUBST(METADATA_LIBS)
 ])

ext/metadata/metadata.c

@@ -248,28 +248,34 @@ metadata_parse (MetaData * meta_data, const guint8 * buf,
 
   switch (meta_data->img_type) {
     case IMG_JPEG:
-      if (G_LIKELY (meta_data->options & META_OPT_DEMUX))
+      if (G_LIKELY (meta_data->options & META_OPT_DEMUX)) {
+        GST_DEBUG ("parsing jpeg");
         ret =
             metadataparse_jpeg_parse (&meta_data->format_data.jpeg_parse,
             (guint8 *) buf, &buf_size, meta_data->offset_orig, &next_start,
             next_size);
-      else
+      } else {
+        GST_DEBUG ("formatting jpeg");
         ret =
             metadatamux_jpeg_parse (&meta_data->format_data.jpeg_mux,
             (guint8 *) buf, &buf_size, meta_data->offset_orig, &next_start,
             next_size);
+      }
       break;
     case IMG_PNG:
-      if (G_LIKELY (meta_data->options & META_OPT_DEMUX))
+      if (G_LIKELY (meta_data->options & META_OPT_DEMUX)) {
+        GST_DEBUG ("parsing png");
         ret =
             metadataparse_png_parse (&meta_data->format_data.png_parse,
             (guint8 *) buf, &buf_size, meta_data->offset_orig, &next_start,
             next_size);
-      else
+      } else {
+        GST_DEBUG ("formatting png");
         ret =
             metadatamux_png_parse (&meta_data->format_data.png_mux,
             (guint8 *) buf, &buf_size, meta_data->offset_orig, &next_start,
             next_size);
+      }
       break;
     default:
       /* unexpected */
@@ -286,6 +292,7 @@ done:
   if (ret == META_PARSING_DONE) {
     meta_data->state = STATE_DONE;
   }
+  GST_DEBUG ("parsing/formatting done : %d", ret);
 
   return ret;
 }

ext/metadata/metadatamuxjpeg.c

@@ -211,6 +211,7 @@ metadatamux_jpeg_parse (JpegMuxData * jpeg_data, guint8 * buf,
   if (jpeg_data->state == JPEG_MUX_NULL) {
 
     if (*bufsize < 2) {
+      GST_INFO ("need more data");
       *next_size = (buf - *next_start) + 2;
       ret = META_PARSING_NEED_MORE_DATA;
       goto done;
@@ -220,6 +221,7 @@ metadatamux_jpeg_parse (JpegMuxData * jpeg_data, guint8 * buf,
     mark[1] = READ (buf, *bufsize);
 
     if (mark[0] != 0xFF || mark[1] != 0xD8) {
+      GST_INFO ("missing marker");
       ret = META_PARSING_ERROR;
       goto done;
     }
@@ -231,6 +233,7 @@ metadatamux_jpeg_parse (JpegMuxData * jpeg_data, guint8 * buf,
   while (ret == META_PARSING_DONE) {
     switch (jpeg_data->state) {
       case JPEG_MUX_READING:
+        GST_DEBUG ("start reading");
         ret =
             metadatamux_jpeg_reading (jpeg_data, &buf, bufsize,
             offset, step_buf, next_start, next_size);
@@ -239,6 +242,7 @@ metadatamux_jpeg_parse (JpegMuxData * jpeg_data, guint8 * buf,
         goto done;
         break;
       default:
+        GST_INFO ("invalid parser state");
         ret = META_PARSING_ERROR;
         break;
     }
@@ -379,6 +383,7 @@ metadatamux_jpeg_reading (JpegMuxData * jpeg_data, guint8 ** buf,
   *next_start = *buf;
 
   if (*bufsize < 2) {
+    GST_INFO ("need more data");
     *next_size = (*buf - *next_start) + 2;
     ret = META_PARSING_NEED_MORE_DATA;
     goto done;
@@ -396,6 +401,7 @@ metadatamux_jpeg_reading (JpegMuxData * jpeg_data, guint8 ** buf,
 
       if (chunk_size >= 16) {
         if (*bufsize < 5) {
+          GST_INFO ("need more data");
           *next_size = (*buf - *next_start) + 5;
           ret = META_PARSING_NEED_MORE_DATA;
           goto done;
@@ -406,11 +412,13 @@ metadatamux_jpeg_reading (JpegMuxData * jpeg_data, guint8 ** buf,
         }
       } else {
         /* FIXME: should we check if the first chunk is EXIF? */
+        GST_INFO ("chunk size too small %u", chunk_size);
       }
 
     }
 
     if (!jfif_found) {
+      GST_INFO ("no jfif found");
       ret = META_PARSING_ERROR;
       goto done;
     }
@@ -456,7 +464,7 @@ metadatamux_jpeg_reading (JpegMuxData * jpeg_data, guint8 ** buf,
     ret = META_PARSING_DONE;
 
   } else {
-    /* invalid JPEG chunk */
+    GST_INFO ("invalid JPEG chunk");
     ret = META_PARSING_ERROR;
   }
 

ext/metadata/metadataparsejpeg.c

@@ -245,6 +245,7 @@ metadataparse_jpeg_parse (JpegParseData * jpeg_data, guint8 * buf,
        type to be sure it is a JPEG */
 
     if (*bufsize < 2) {
+      GST_INFO ("need more data");
       *next_size = (buf - *next_start) + 2;
       ret = META_PARSING_NEED_MORE_DATA;
       goto done;
@@ -254,6 +255,7 @@ metadataparse_jpeg_parse (JpegParseData * jpeg_data, guint8 * buf,
     mark[1] = READ (buf, *bufsize);
 
     if (mark[0] != 0xFF || mark[1] != 0xD8) {
+      GST_INFO ("missing marker");
       ret = META_PARSING_ERROR;
       goto done;
     }
@@ -265,21 +267,25 @@ metadataparse_jpeg_parse (JpegParseData * jpeg_data, guint8 * buf,
   while (ret == META_PARSING_DONE) {
     switch (jpeg_data->state) {
       case JPEG_PARSE_READING:
+        GST_DEBUG ("start reading");
         ret =
             metadataparse_jpeg_reading (jpeg_data, &buf, bufsize,
             offset, step_buf, next_start, next_size);
         break;
       case JPEG_PARSE_JUMPING:
+        GST_DEBUG ("jump");
         ret =
             metadataparse_jpeg_jump (jpeg_data, &buf, bufsize, next_start,
             next_size);
         break;
       case JPEG_PARSE_EXIF:
+        GST_DEBUG ("parse exif");
         ret =
             metadataparse_jpeg_exif (jpeg_data, &buf, bufsize, next_start,
             next_size);
         break;
       case JPEG_PARSE_IPTC:
+        GST_DEBUG ("parse iptc");
 #ifdef HAVE_IPTC
         ret =
             metadataparse_jpeg_iptc (jpeg_data, &buf, bufsize, next_start,
@@ -287,6 +293,7 @@ metadataparse_jpeg_parse (JpegParseData * jpeg_data, guint8 * buf,
 #endif
         break;
       case JPEG_PARSE_XMP:
+        GST_DEBUG ("parse xmp");
         ret =
             metadataparse_jpeg_xmp (jpeg_data, &buf, bufsize, next_start,
             next_size);
@@ -295,6 +302,7 @@ metadataparse_jpeg_parse (JpegParseData * jpeg_data, guint8 * buf,
         goto done;
         break;
       default:
+        GST_INFO ("invalid parser state");
         ret = META_PARSING_ERROR;
         break;
     }

tests/check/pipelines/metadata.c

@@ -38,7 +38,6 @@ bus_handler (GstBus * bus, GstMessage * message, gpointer data)
 
       gst_message_parse_error (message, &gerror, &debug);
       gst_object_default_error (GST_MESSAGE_SRC (message), gerror, debug);
-      gst_message_unref (message);
       g_error_free (gerror);
       g_free (debug);
       g_main_loop_quit (loop);
@@ -81,12 +80,18 @@ test_tags (const gchar * tag_str)
   gint i, j, n_recv, n_sent;
   const gchar *name_sent, *name_recv;
   const GValue *value_sent, *value_recv;
-  gboolean found;
+  gboolean found, ok;
   gint comparison;
-
   GstElement *videotestsrc, *jpegenc, *metadatamux, *metadatademux, *fakesink;
   GstTagSetter *setter;
 
+  GST_DEBUG ("testing tags : %s", tag_str);
+
+  if (received_tags) {
+    gst_tag_list_free (received_tags);
+    received_tags = NULL;
+  }
+
   pipeline = gst_pipeline_new ("pipeline");
   fail_unless (pipeline != NULL);
 
@@ -94,7 +99,7 @@ test_tags (const gchar * tag_str)
   fail_unless (videotestsrc != NULL);
   g_object_set (G_OBJECT (videotestsrc), "num-buffers", 1, NULL);
 
-  jpegenc = gst_element_factory_make ("jpegenc", "enc");
+  jpegenc = gst_element_factory_make ("q", "enc");
   if (jpegenc == NULL) {
     g_print ("Cannot test - jpegenc not available\n");
     return;
@@ -113,8 +118,9 @@ test_tags (const gchar * tag_str)
   gst_bin_add_many (GST_BIN (pipeline), videotestsrc, jpegenc, metadatamux,
       metadatademux, fakesink, NULL);
 
-  fail_unless (gst_element_link_many (videotestsrc, jpegenc, metadatamux,
+  ok = gst_element_link_many (videotestsrc, jpegenc, metadatamux, metadatademux,
-          metadatademux, fakesink, NULL));
+      fakesink, NULL);
+  fail_unless (ok == TRUE);
 
   loop = g_main_loop_new (NULL, TRUE);
   fail_unless (loop != NULL);
@@ -129,11 +135,14 @@ test_tags (const gchar * tag_str)
   setter = GST_TAG_SETTER (metadatamux);
   fail_unless (setter != NULL);
   sent_tags = gst_structure_from_string (tag_str, NULL);
+  fail_unless (sent_tags != NULL);
   gst_tag_setter_merge_tags (setter, sent_tags, GST_TAG_MERGE_REPLACE);
 
   gst_element_set_state (pipeline, GST_STATE_PLAYING);
   g_main_loop_run (loop);
 
+  GST_DEBUG ("mainloop done : %p", received_tags);
+
   /* verify tags */
   fail_unless (received_tags != NULL);
   n_recv = gst_structure_n_fields (received_tags);