GNOME.org
 

[rhythmbox] metadata: move media type and file size checks out to rhythmdb

commit 8139664724a58253273bb869e8689e12f4fc0532
Author: Jonathan Matthew <jonathan d14n org>
Date:   Mon Jun 8 19:38:48 2009 +1000

    metadata: move media type and file size checks out to rhythmdb
    
    This leaves us with no file type checks for podcast downloads, so m4a
    podcasts containing image sequences will no longer show up as 'failed'.
    (bug #479897)
---
 metadata/rb-metadata-gst.c |  138 +-------------------------------------------
 rhythmdb/rhythmdb.c        |   99 ++++++++++++++++++++++++++-----
 2 files changed, 85 insertions(+), 152 deletions(-)

diff --git a/metadata/rb-metadata-gst.c b/metadata/rb-metadata-gst.c
index 9c4edce..bfadd5b 100644
--- a/metadata/rb-metadata-gst.c
+++ b/metadata/rb-metadata-gst.c
@@ -51,30 +51,6 @@ static void rb_metadata_finalize (GObject *object);
 
 typedef GstElement *(*RBAddTaggerElem) (RBMetaData *, GstElement *);
 
-/*
- * The list of mime-type prefixes for files that shouldn't display errors about being non-audio.
- * Useful for people who have cover art, et cetera, in their music directories
- */
-const char * ignore_mime_types[] = {
-	"image/",
-	"text/",
-	"application/",
-};
-
-const char * non_ignore_mime_types[] = {
-	"application/x-id3",
-	"application/ogg",
-	"application/x-apetag",
-	"application/x-3gp"
-};
-
-/*
- * File size below which we will simply ignore files that can't be identified.
- * This is mostly here so we ignore the various text files that are packaged
- * with many netlabel releases and other downloads.
- */
-#define REALLY_SMALL_FILE_SIZE	(4096)
-
 struct RBMetadataGstType
 {
 	char *mimetype;
@@ -97,7 +73,6 @@ struct RBMetaDataPrivate
 	GPtrArray *supported_types;
 
 	char *type;
-	gboolean handoff;
 	gboolean eos;
 	gboolean has_audio;
 	gboolean has_non_audio;
@@ -443,18 +418,6 @@ free_gvalue (GValue *val)
 	g_free (val);
 }
 
-static const char *
-rb_metadata_gst_type_to_name (RBMetaData *md, const char *mimetype)
-{
-	int i;
-	for (i = 0; i < md->priv->supported_types->len; i++) {
-		struct RBMetadataGstType *type = g_ptr_array_index (md->priv->supported_types, i);
-		if (!strcmp (type->mimetype, mimetype))
-			return type->human_name;
-	}
-	return NULL;
-}
-
 static RBAddTaggerElem
 rb_metadata_gst_type_to_tag_function (RBMetaData *md, const char *mimetype)
 {
@@ -467,29 +430,6 @@ rb_metadata_gst_type_to_tag_function (RBMetaData *md, const char *mimetype)
 	return NULL;
 }
 
-static char *
-make_undecodable_error (RBMetaData *md)
-{
-	const char *human_name;
-	char *free_name = NULL;
-
-	human_name = rb_metadata_gst_type_to_name (md, md->priv->type);
-	if (human_name == NULL) {
-		free_name = rb_mime_get_friendly_name (md->priv->type);
-		human_name = free_name;
-	}
-
-	if (human_name) {
-		return g_strdup_printf (_("The GStreamer plugins to decode \"%s\" files cannot be found"),
-					human_name);
-	} else {
-		return g_strdup_printf (_("The file contains a stream of type %s, which is not decodable"),
-					md->priv->type);
-	}
-
-	g_free (free_name);
-}
-
 static void
 rb_metadata_gst_load_tag (const GstTagList *list, const gchar *tag, RBMetaData *md)
 {
@@ -815,23 +755,6 @@ rb_metadata_event_loop (RBMetaData *md, GstElement *element, gboolean block)
 	gst_object_unref (bus);
 }
 
-static gboolean
-ignore_mimetype (const char *type)
-{
-	guint i;
-
-	for (i = 0; i < G_N_ELEMENTS (non_ignore_mime_types); i++) {
-		if (g_str_equal (type, non_ignore_mime_types[i]) != FALSE)
-			return FALSE;
-	}
-	for (i = 0; i < G_N_ELEMENTS (ignore_mime_types); i++) {
-		if (g_str_has_prefix (type, ignore_mime_types[i]) != FALSE)
-			return TRUE;
-	}
-
-	return FALSE;
-}
-
 void
 rb_metadata_load (RBMetaData *md,
 		  const char *uri,
@@ -853,7 +776,6 @@ rb_metadata_load (RBMetaData *md,
 	md->priv->type = NULL;
 	md->priv->error = NULL;
 	md->priv->eos = FALSE;
-	md->priv->handoff = FALSE;
 	md->priv->has_audio = FALSE;
 	md->priv->has_non_audio = FALSE;
 	md->priv->has_video = FALSE;
@@ -994,75 +916,19 @@ rb_metadata_load (RBMetaData *md,
 	if (state_ret == GST_STATE_CHANGE_ASYNC) {
 		g_warning ("Failed to return metadata reader to NULL state");
 	}
-	md->priv->handoff = (state_ret == GST_STATE_CHANGE_SUCCESS);
-
-	/* report errors for various failure cases.
-	 * these don't include the URI as the import errors source
-	 * already displays it.
-	 */
-	if ((md->priv->has_video || !md->priv->has_audio) &&
-	    (md->priv->has_non_audio || !md->priv->handoff)) {
-		gboolean ignore = FALSE;
 
-		if (md->priv->has_video) {
-			ignore = TRUE;
-		} else {
-			ignore = ignore_mimetype (md->priv->type);
-		}
-
-		if (!ignore) {
-			char *msg = make_undecodable_error (md);
-			g_set_error (error,
-				     RB_METADATA_ERROR,
-				     RB_METADATA_ERROR_NOT_AUDIO,
-				     "%s", msg);
-			g_free (msg);
-		} else {
-			/* we don't need an error message here (it'll never be
-			 * displayed).  using NULL causes crashes with some C
-			 * libraries, and gcc doesn't like zero-length format
-			 * strings, so we use a single space instead.
-			 */
-			g_set_error (error,
-				     RB_METADATA_ERROR,
-				     RB_METADATA_ERROR_NOT_AUDIO_IGNORE,
-				     " ");
-		}
-	} else if (md->priv->error != NULL) {
+	if (md->priv->error != NULL) {
 		g_propagate_error (error, md->priv->error);
 		md->priv->error = NULL;
 	} else if (!md->priv->type) {
-		/* ignore really small files that can't be identified */
-		gint error_code = RB_METADATA_ERROR_UNRECOGNIZED;
-		if (file_size > 0 && file_size < REALLY_SMALL_FILE_SIZE) {
-			rb_debug ("ignoring %s because it's too small to care about", md->priv->uri);
-			error_code = RB_METADATA_ERROR_NOT_AUDIO_IGNORE;
-		} else if (file_size == 0) {
-			g_clear_error (error);
-			g_set_error (error,
-				     RB_METADATA_ERROR,
-				     RB_METADATA_ERROR_EMPTY_FILE,
-				     _("Empty file"));
-			goto out;
-		}
-
 		g_clear_error (error);
 		g_set_error (error,
 			     RB_METADATA_ERROR,
-			     error_code,
+			     RB_METADATA_ERROR_UNRECOGNIZED,
 			     _("The MIME type of the file could not be identified"));
 	} else {
 		/* yay, it worked */
 		rb_debug ("successfully read metadata for %s", uri);
-
-		/* it doesn't matter if we don't recognise the format,
-		 * as long as gstreamer can play it, we can put it in
-		 * the library.
-		 */
-		if (!rb_metadata_gst_type_to_name (md, md->priv->type)) {
-			rb_debug ("we don't know what type %s (from file %s) is, but we'll play it anyway",
-				  md->priv->type, uri);
-		}
 	}
 
  out:
diff --git a/rhythmdb/rhythmdb.c b/rhythmdb/rhythmdb.c
index 64f5b91..ba1f31e 100644
--- a/rhythmdb/rhythmdb.c
+++ b/rhythmdb/rhythmdb.c
@@ -80,6 +80,33 @@ G_DEFINE_ABSTRACT_TYPE(RhythmDB, rhythmdb, G_TYPE_OBJECT)
 	RHYTHMDB_FILE_INFO_ATTRIBUTES ","		\
 	G_FILE_ATTRIBUTE_STANDARD_NAME
 
+/*
+ * Filters for MIME/media types to ignore.
+ * The only complication here is that there are some application/ types that
+ * are used for audio/video files.  Otherwise, we'd ignore everything except
+ * audio/ and video/.
+ */
+struct media_type_filter {
+	const char *prefix;
+	gboolean ignore;
+} media_type_filters[] = {
+	{ "image/", TRUE },
+	{ "text/", TRUE },
+	{ "application/ogg", FALSE },
+	{ "application/x-id3", FALSE },
+	{ "application/x-apetag", FALSE },
+	{ "application/x-3gp", FALSE },
+	{ "application/", TRUE },
+};
+
+/*
+ * File size below which we will simply ignore files that can't be identified.
+ * This is mostly here so we ignore the various text files that are packaged
+ * with many netlabel releases and other downloads.
+ */
+#define REALLY_SMALL_FILE_SIZE	(4096)
+
+
 typedef struct
 {
 	RhythmDB *db;
@@ -730,6 +757,19 @@ make_access_failed_error (const char *uri, GError *access_error)
 	return error;
 }
 
+static gboolean
+rhythmdb_ignore_media_type (const char *media_type)
+{
+	int i;
+
+	for (i = 0; i < G_N_ELEMENTS (media_type_filters); i++) {
+		if (g_str_has_prefix (media_type, media_type_filters[i].prefix)) {
+			return media_type_filters[i].ignore;
+		}
+	}
+	return FALSE;
+}
+
 typedef struct {
 	RhythmDBEvent *event;
 	GMutex *mutex;
@@ -2153,20 +2193,14 @@ typedef struct
 
 static void
 rhythmdb_add_import_error_entry (RhythmDB *db,
-				 RhythmDBEvent *event)
+				 RhythmDBEvent *event,
+				 RhythmDBEntryType error_entry_type)
 {
 	RhythmDBEntry *entry;
 	GValue value = {0,};
-	RhythmDBEntryType error_entry_type = event->error_type;
 
-	rb_debug ("adding import error for %s: %s", rb_refstring_get (event->real_uri), event->error->message);
-	if (g_error_matches (event->error, RB_METADATA_ERROR, RB_METADATA_ERROR_NOT_AUDIO_IGNORE)) {
-		/* only add an ignore entry if we have an entry type for it */
-		if (event->ignore_type == RHYTHMDB_ENTRY_TYPE_INVALID)
-			return;
-
-		error_entry_type = event->ignore_type;
-	} else if (event->error_type == RHYTHMDB_ENTRY_TYPE_INVALID) {
+	rb_debug ("adding import error for %s: %s", rb_refstring_get (event->real_uri), event->error ? event->error->message : "<no error>");
+	if (error_entry_type == RHYTHMDB_ENTRY_TYPE_INVALID) {
 		/* we don't have an error entry type, so we can't add an import error */
 		return;
 	}
@@ -2246,18 +2280,51 @@ rhythmdb_process_metadata_load_real (RhythmDBEvent *event)
 {
 	RhythmDBEntry *entry;
 	GValue value = {0,};
-	const char *mime;
 	GTimeVal time;
+	const char *media_type;
+
+	/*
+	 * always ignore anything with video in it, or anything
+	 * matching one of the media types we don't care about.
+	 * if we can identify it that much, we know it's not interesting.
+	 * otherwise, add an import error entry if there was an error,
+	 * or just ignore it if it doesn't contain audio.
+	 */
+
+	media_type = rb_metadata_get_mime (event->metadata);
+	if (rb_metadata_has_video (event->metadata) ||
+	    (media_type != NULL && rhythmdb_ignore_media_type (media_type))) {
+		rhythmdb_add_import_error_entry (event->db, event, event->ignore_type);
+		return TRUE;
+	}
+
+	/* also ignore really small files we can't identify */
+	if (event->error && event->error->code == RB_METADATA_ERROR_UNRECOGNIZED) {
+		guint64 file_size;
+
+		file_size = g_file_info_get_attribute_uint64 (event->file_info,
+							      G_FILE_ATTRIBUTE_STANDARD_SIZE);
+		if (file_size == 0) {
+			/* except for empty files */
+			g_clear_error (&event->error);
+			g_set_error (&event->error,
+				     RB_METADATA_ERROR,
+				     RB_METADATA_ERROR_EMPTY_FILE,
+				     _("Empty file"));
+		} else if (file_size < REALLY_SMALL_FILE_SIZE) {
+			rhythmdb_add_import_error_entry (event->db, event, event->ignore_type);
+			return TRUE;
+		}
+	}
 
 	if (event->error) {
-		rhythmdb_add_import_error_entry (event->db, event);
+		rhythmdb_add_import_error_entry (event->db, event, event->error_type);
 		return TRUE;
 	}
 
-	/* do we really need to do this? */
-	mime = rb_metadata_get_mime (event->metadata);
-	if (!mime) {
-		rb_debug ("unsupported file");
+	/* check if this is something we want in the library */
+	if (rb_metadata_has_audio (event->metadata) == FALSE) {
+		rhythmdb_add_import_error_entry (event->db, event, event->ignore_type);
 		return TRUE;
 	}
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]