[tracker/tracker-0.6] Fixes NB#100123, Image:Date is empty for some of the images

[Martyn James Russell <mr src gnome org>]

commit bb0a4c25b89eb7b468566c9c6cc935a090ca755a
Author: Martyn Russell <martyn imendio com>
Date:   Thu May 7 11:13:17 2009 +0100

    Fixes NB#100123, Image:Date is empty for some of the images
    
    Always fallback to mtime for Image:Date metadata
---
 src/tracker-extract/tracker-extract-gstreamer.c |   78 ++++++++++++-----------
 src/tracker-extract/tracker-extract-jpeg.c      |   55 +++++++++-------
 src/tracker-extract/tracker-extract-png.c       |   69 +++++++++++---------
 src/tracker-extract/tracker-extract-tiff.c      |   14 +++--
 4 files changed, 119 insertions(+), 97 deletions(-)

diff --git a/src/tracker-extract/tracker-extract-gstreamer.c b/src/tracker-extract/tracker-extract-gstreamer.c
index c537ce4..90a498c 100644
--- a/src/tracker-extract/tracker-extract-gstreamer.c
+++ b/src/tracker-extract/tracker-extract-gstreamer.c
@@ -33,6 +33,7 @@
 #include <gst/tag/tag.h>
 
 #include <libtracker-common/tracker-type-utils.h>
+#include <libtracker-common/tracker-file-utils.h>
 
 #include "tracker-main.h"
 #include "tracker-extract-albumart.h"
@@ -239,8 +240,9 @@ extract_metadata (MetadataExtractor *extractor,
 		  GHashTable        *metadata)
 {
 	gchar *value;
-	g_return_if_fail (extractor);
-	g_return_if_fail (metadata);
+
+	g_return_if_fail (extractor != NULL);
+	g_return_if_fail (metadata != NULL);
 
 	if (extractor->tagcache) {
 		/* General */
@@ -348,7 +350,6 @@ extract_metadata (MetadataExtractor *extractor,
 					     "Audio:Genre");
 		}
 	}
-
 }
 
 static void
@@ -389,10 +390,9 @@ dbin_dpad_cb (GstElement* e, GstPad* pad, gboolean cont, gpointer data)
 	gst_object_unref (fsinkpad);
 }
 
-
-
 static void
-add_stream_tags_tagreadbin_for_element (MetadataExtractor *extractor, GstElement *elem)
+add_stream_tags_tagreadbin_for_element (MetadataExtractor *extractor, 
+					GstElement        *elem)
 {
 	GstStructure      *s         = NULL;
 	GstCaps	          *caps      = NULL;
@@ -405,10 +405,9 @@ add_stream_tags_tagreadbin_for_element (MetadataExtractor *extractor, GstElement
 	while (!done) {
 		switch (gst_iterator_next (iter, &item)) {
 		case GST_ITERATOR_OK:
-			
 			if ((caps = GST_PAD_CAPS (item))) {
 				s = gst_caps_get_structure (caps, 0);
-
+				
 				if (s) {
 					if (g_strrstr (gst_structure_get_name (s), "audio")) {
 						if ( ( (extractor->audio_channels != -1) &&
@@ -685,27 +684,27 @@ tracker_extract_gstreamer (const gchar *uri,
 	
 	extractor->pipeline = gst_element_factory_make ("pipeline", NULL);
 	if (!extractor->pipeline) {
-		g_error ("Failed to create pipeline");
-		return;
+		g_critical ("Failed to create GStreamer pipeline");
+		goto fail;
 	}
-	extractor->filesrc  = gst_element_factory_make ("filesrc",  NULL);
+	extractor->filesrc = gst_element_factory_make ("filesrc", NULL);
 	if (!extractor->filesrc) {
-		g_error ("Failed to create filesrc");
-		return;
+		g_critical ("Failed to create GStreamer filesrc");
+		goto fail;
 	}
 	if (use_cache) {
-		extractor->cache    = gst_element_factory_make ("cache",    NULL);
+		extractor->cache = gst_element_factory_make ("cache", NULL);
 		if (!extractor->cache) {
-			g_error ("Failed to create cache");
-			return;
+			g_critical ("Failed to create GStreamer cache");
+			goto fail;
 		}
 	}
 
 	if (use_dbin) {
 		extractor->bin = gst_element_factory_make ("decodebin2", "decodebin2");
 		if (!extractor->bin) {
-			g_error ("Failed to create decodebin");
-			return;
+			g_critical ("Failed to create GStreamer decodebin");
+			goto fail;
 		}
 		extractor->id = g_signal_connect (G_OBJECT (extractor->bin), 
 				       "new-decoded-pad",
@@ -714,8 +713,8 @@ tracker_extract_gstreamer (const gchar *uri,
 	} else {
 	        extractor->bin = gst_element_factory_make ("tagreadbin", "tagreadbin");
 		if (!extractor->bin) {
-			g_error ("Failed to create tagreadbin");
-			return;
+			g_error ("Failed to create GStreamer tagreadbin");
+			goto fail;
 		}
 		extractor->id = 0;
 	}
@@ -725,16 +724,16 @@ tracker_extract_gstreamer (const gchar *uri,
 
 	if (use_cache) {
 		gst_bin_add (GST_BIN (extractor->pipeline), extractor->cache);
-		if (! gst_element_link_many (extractor->filesrc, extractor->cache, extractor->bin, NULL)) {
-		g_error ("Can't link elements\n");
-		/* FIXME Clean up */
-		return;
+		if (!gst_element_link_many (extractor->filesrc, extractor->cache, extractor->bin, NULL)) {
+			g_critical ("Could not link GStreamer elements (using cache)");
+			/* FIXME Clean up */
+			goto fail;
 		}
 	} else {
 		if (!gst_element_link_many (extractor->filesrc, extractor->bin, NULL)) {
-			g_error ("Can't link elements\n");
+			g_critical ("Could not link GStreamer elements");
 			/* FIXME Clean up */
-			return;
+			goto fail;
 		}
 	}
 
@@ -788,20 +787,24 @@ tracker_extract_gstreamer (const gchar *uri,
 	g_main_loop_unref (extractor->loop);
 	g_slice_free (MetadataExtractor, extractor);
 
+fail:
 	if (type == EXTRACT_MIME_IMAGE) {
+		/* We fallback to the file's modified time for the
+		 * "Image:Date" metadata if it doesn't exist.
+		 *
+		 * FIXME: This shouldn't be necessary.
+		 */
 		if (!g_hash_table_lookup (metadata, "Image:Date")) {
-			struct stat st;
+			gchar *date;
+			guint64 mtime;
 			
-			if (g_lstat (uri, &st) >= 0) {
-				gchar *date;
-				
-				date = tracker_date_to_string (st.st_mtime);
-				
-				g_hash_table_insert (metadata,
-						     g_strdup ("Image:Date"),
-						     tracker_escape_metadata (date));
-				g_free (date);
-			}
+			mtime = tracker_file_get_mtime (uri);
+			date = tracker_date_to_string ((time_t) mtime);
+			
+			g_hash_table_insert (metadata,
+					     g_strdup ("Image:Date"),
+					     tracker_escape_metadata (date));
+			g_free (date);
 		}
 	} else if (type == EXTRACT_MIME_VIDEO) {
 		if (!g_hash_table_lookup (metadata, "Video:Title")) {
@@ -836,7 +839,6 @@ tracker_extract_gstreamer (const gchar *uri,
 			g_free (title);
 		}
 	}
-
 }
 
 
diff --git a/src/tracker-extract/tracker-extract-jpeg.c b/src/tracker-extract/tracker-extract-jpeg.c
index a78babb..42e8a8b 100644
--- a/src/tracker-extract/tracker-extract-jpeg.c
+++ b/src/tracker-extract/tracker-extract-jpeg.c
@@ -70,17 +70,19 @@ static TrackerExtractData data[] = {
 	{ NULL, NULL }
 };
 
-struct tej_error_mgr 
-{
+struct tej_error_mgr {
 	struct jpeg_error_mgr jpeg;
 	jmp_buf setjmp_buffer;
 };
 
-static void tracker_extract_jpeg_error_exit (j_common_ptr cinfo)
+static void 
+tracker_extract_jpeg_error_exit (j_common_ptr cinfo)
 {
-    struct tej_error_mgr *h = (struct tej_error_mgr *)cinfo->err;
+    struct tej_error_mgr *h = (struct tej_error_mgr *) cinfo->err;
+
     (*cinfo->err->output_message)(cinfo);
-    longjmp(h->setjmp_buffer, 1);
+
+    longjmp (h->setjmp_buffer, 1);
 }
 
 #ifdef HAVE_LIBEXIF
@@ -276,7 +278,6 @@ read_exif (const unsigned char *buffer,
 
 #endif /* HAVE_LIBEXIF */
 
-
 static void
 extract_jpeg (const gchar *filename,
 	      GHashTable  *metadata)
@@ -303,9 +304,14 @@ extract_jpeg (const gchar *filename,
 		gsize  sublen;
 #endif /* HAVE_LIBIPTCDATA */
 
+		/* So, if we don't use the jpeg.error_exit() here, the
+		 * JPEG library will abort() on error. So, we use
+		 * setjmp and longjmp here to avoid that.
+		 */
 		cinfo.err = jpeg_std_error (&tejerr.jpeg);
 		tejerr.jpeg.error_exit = tracker_extract_jpeg_error_exit;
-		if (setjmp(tejerr.setjmp_buffer)) {
+		if (setjmp (tejerr.setjmp_buffer)) {
+			tracker_file_close (f, FALSE);
 			goto fail;
 		}
 
@@ -393,25 +399,28 @@ extract_jpeg (const gchar *filename,
 				     g_strdup ("Image:Height"),
 				     tracker_escape_metadata_printf ("%u", cinfo.image_height));
 
-		/* Check that we have the minimum data. FIXME We should not need to do this */
-
-		if (!g_hash_table_lookup (metadata, "Image:Date")) {
-			gchar *date;
-			guint64 mtime;
-
-			mtime = tracker_file_get_mtime (filename);
-			date = tracker_date_to_string ((time_t) mtime);
-
-			g_hash_table_insert (metadata,
-					     g_strdup ("Image:Date"),
-					     tracker_escape_metadata (date));
-			g_free (date);
-		}
-
 		jpeg_destroy_decompress (&cinfo);
-	fail:
 		tracker_file_close (f, FALSE);
 	}
+
+fail:
+	/* We fallback to the file's modified time for the
+	 * "Image:Date" metadata if it doesn't exist.
+	 *
+	 * FIXME: This shouldn't be necessary.
+	 */
+	if (!g_hash_table_lookup (metadata, "Image:Date")) {
+		gchar *date;
+		guint64 mtime;
+		
+		mtime = tracker_file_get_mtime (filename);
+		date = tracker_date_to_string ((time_t) mtime);
+		
+		g_hash_table_insert (metadata,
+				     g_strdup ("Image:Date"),
+				     tracker_escape_metadata (date));
+		g_free (date);
+	}
 }
 
 TrackerExtractData *
diff --git a/src/tracker-extract/tracker-extract-png.c b/src/tracker-extract/tracker-extract-png.c
index 8727fc7..114236f 100644
--- a/src/tracker-extract/tracker-extract-png.c
+++ b/src/tracker-extract/tracker-extract-png.c
@@ -59,16 +59,16 @@ static void   extract_png	      (const gchar *filename,
 				       GHashTable  *metadata);
 
 static TagProcessors tag_processors[] = {
-	{ "Author",		"Image:Creator",      NULL},
-	{ "Creator",		"Image:Creator",      NULL},
-	{ "Description",	"Image:Description",  NULL},
-	{ "Comment",		"Image:Comments",     NULL},
-	{ "Copyright",		"File:Copyright",     NULL},
-	{ "Creation Time",	"Image:Date",	      rfc1123_to_iso8601_date},
-	{ "Title",		"Image:Title",	      NULL},
-	{ "Software",		"Image:Software",     NULL},
-	{ "Disclaimer",		"File:License",       NULL},
-	{ NULL,			NULL,		      NULL},
+	{ "Author",	   "Image:Creator",     NULL },
+	{ "Creator",	   "Image:Creator",     NULL },
+	{ "Description",   "Image:Description", NULL },
+	{ "Comment",	   "Image:Comments",    NULL },
+	{ "Copyright",	   "File:Copyright",    NULL },
+	{ "Creation Time", "Image:Date",	rfc1123_to_iso8601_date },
+	{ "Title",	   "Image:Title",	NULL },
+	{ "Software",	   "Image:Software",    NULL },
+	{ "Disclaimer",	   "File:License",      NULL },
+	{ NULL,		   NULL,		NULL },
 };
 
 static TrackerExtractData data[] = {
@@ -87,7 +87,9 @@ rfc1123_to_iso8601_date (gchar *date)
 }
 
 static void
-read_metadata (png_structp png_ptr, png_infop info_ptr, GHashTable *metadata)
+read_metadata (png_structp  png_ptr, 
+	       png_infop    info_ptr, 
+	       GHashTable  *metadata)
 {
 	gint	     num_text;
 	png_textp    text_ptr;
@@ -158,7 +160,7 @@ extract_png (const gchar *filename,
 	size = tracker_file_get_size (filename);
 
 	if (size < 64) {
-		return;
+		goto fail;
 	}
 
 	f = tracker_file_open (filename, "r", FALSE); 
@@ -170,27 +172,27 @@ extract_png (const gchar *filename,
 						  NULL);
 		if (!png_ptr) {
 			tracker_file_close (f, FALSE);
-			return;
+			goto fail;
 		}
 
 		info_ptr = png_create_info_struct (png_ptr);
 		if (!info_ptr) {
 			png_destroy_read_struct (&png_ptr, &info_ptr, NULL);
 			tracker_file_close (f, FALSE);
-			return;
+			goto fail;
 		}
 
 		end_ptr = png_create_info_struct (png_ptr);
 		if (!end_ptr) {
 			png_destroy_read_struct (&png_ptr, &info_ptr, NULL);
 			tracker_file_close (f, FALSE);
-			return;
+			goto fail;
 		}
 
 		if (setjmp (png_jmpbuf (png_ptr))) {
 			png_destroy_read_struct (&png_ptr, &info_ptr, &end_ptr);
 			tracker_file_close (f, FALSE);
-			return;
+			goto fail;
 		}
 
 		png_init_io (png_ptr, f);
@@ -207,7 +209,7 @@ extract_png (const gchar *filename,
 				   &filter_type)) {
 			png_destroy_read_struct (&png_ptr, &info_ptr, &end_ptr);
 			tracker_file_close (f, FALSE);
-			return;
+			goto fail;
 		}
 		
 		/* Read the image. FIXME We should be able to skip this step and
@@ -241,24 +243,29 @@ extract_png (const gchar *filename,
 		g_hash_table_insert (metadata,
 				     g_strdup ("Image:Height"),
 				     tracker_escape_metadata_printf ("%ld", height));
-		
-		/* Check that we have the minimum data. FIXME We should not need to do this */
-		if (!g_hash_table_lookup (metadata, "Image:Date")) {
-			gchar *date;
-			guint64 mtime;
-
-			mtime = tracker_file_get_mtime (filename);
-			date = tracker_date_to_string ((time_t) mtime);
-			
-			g_hash_table_insert (metadata,
-					     g_strdup ("Image:Date"),
-					     tracker_escape_metadata (date));
-			g_free (date);
-		}
 
 		png_destroy_read_struct (&png_ptr, &info_ptr, &end_ptr);
 		tracker_file_close (f, FALSE);
 	}
+	
+fail:
+	/* We fallback to the file's modified time for the
+	 * "Image:Date" metadata if it doesn't exist.
+	 *
+	 * FIXME: This shouldn't be necessary.
+	 */
+	if (!g_hash_table_lookup (metadata, "Image:Date")) {
+		gchar *date;
+		guint64 mtime;
+		
+		mtime = tracker_file_get_mtime (filename);
+		date = tracker_date_to_string ((time_t) mtime);
+		
+		g_hash_table_insert (metadata,
+				     g_strdup ("Image:Date"),
+				     tracker_escape_metadata (date));
+		g_free (date);
+	}
 }
 
 TrackerExtractData *
diff --git a/src/tracker-extract/tracker-extract-tiff.c b/src/tracker-extract/tracker-extract-tiff.c
index 65bba39..93d5567 100644
--- a/src/tracker-extract/tracker-extract-tiff.c
+++ b/src/tracker-extract/tracker-extract-tiff.c
@@ -135,7 +135,7 @@ extract_tiff (const gchar *filename,
 
 	if ((image = TIFFOpen (filename, "r")) == NULL){
 		g_critical ("Could not open image:'%s'\n", filename);
-		return;
+		goto fail;
 	}
 
 #ifdef HAVE_LIBIPTCDATA
@@ -264,8 +264,14 @@ extract_tiff (const gchar *filename,
 		}
 	}
 
-	/* Check that we have the minimum data. FIXME We should not need to do this */
-	
+	TIFFClose (image);
+
+fail:
+	/* We fallback to the file's modified time for the
+	 * "Image:Date" metadata if it doesn't exist.
+	 *
+	 * FIXME: This shouldn't be necessary.
+	 */
 	if (!g_hash_table_lookup (metadata, "Image:Date")) {
 		gchar *date;
 		guint64 mtime;
@@ -278,8 +284,6 @@ extract_tiff (const gchar *filename,
 				     tracker_escape_metadata (date));
 		g_free (date);
 	}
-
-	TIFFClose (image);
 }
 
 TrackerExtractData *