From 4d085934279728f6ac0d0cdda50def4a06515d77 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Fri, 11 Oct 2013 16:02:26 +0100 Subject: cache: Support more media art types than just 'album' The media art storage spec https://wiki.gnome.org/MediaArtStorageSpec#Identifiers specifies the following possible values for prefix: 'album', 'artist', 'podcast', 'radio', 'track'. This commit adds support for all of these except 'track', which will currently raise a g_warning() if used to advise the caller that it is not supported. Rather than only supporting a fixed list of identifiers, any unknown prefix is accepted and handled in the same way as 'artist', 'radio' and 'podcast' media art. This commit also fixes https://bugzilla.gnome.org/show_bug.cgi?id=705834 where the path for 'album' media art where artist == NULL was calculated incorrectly. --- libmediaart/cache.c | 39 +++++++++++++++++------- tests/mediaarttest.c | 84 ++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 88 insertions(+), 35 deletions(-) diff --git a/libmediaart/cache.c b/libmediaart/cache.c index e2592b6..ef3426b 100644 --- a/libmediaart/cache.c +++ b/libmediaart/cache.c @@ -228,7 +228,6 @@ media_art_get_file (const gchar *artist, GFile **local_file) { const gchar *space_checksum = "7215ee9c7d9dc229d2921a40e899ec5f"; - const gchar *a, *b; gchar *art_filename; gchar *dir, *filename; @@ -236,6 +235,7 @@ media_art_get_file (const gchar *artist, gchar *artist_stripped, *title_stripped; gchar *artist_norm, *title_norm; gchar *artist_checksum = NULL, *title_checksum = NULL; + gboolean title_only; /* http://live.gnome.org/MediaArtStorageSpec */ @@ -247,7 +247,21 @@ media_art_get_file (const gchar *artist, *local_file = NULL; } - if (!artist && !title) { + g_return_if_fail (prefix != NULL); + + if (strcmp(prefix, "track") == 0) { + /* Specified by media art storage spec, but not implemented yet */ + g_warning ("libmediaart does not support 'track' type media art."); + return; + } + + if (strcmp (prefix, "album") == 0) { + title_only = FALSE; + } else { + title_only = TRUE; + } + + if ((title_only && !title) || (!title && !artist)) { return; } @@ -277,16 +291,21 @@ media_art_get_file (const gchar *artist, g_mkdir_with_parents (dir, 0770); } - if (artist) { - a = artist_checksum; - b = title ? title_checksum : space_checksum; + if (title_only) { + /* The media art spec describes two types other than "artist": + * "podcast", and "radio", both of which have just the "title" + * attribute. For the sake of flexibility we allow any prefix to be + * used here, so applications can implement simple extentions like a + * 'video' media art type. + */ + art_filename = g_strdup_printf ("%s-%s-%s.jpeg", prefix, + title_checksum, space_checksum); } else { - a = title_checksum; - b = space_checksum; + art_filename = g_strdup_printf ("%s-%s-%s.jpeg", prefix, + artist ? artist_checksum : space_checksum, + title ? title_checksum : space_checksum); } - art_filename = g_strdup_printf ("%s-%s-%s.jpeg", prefix ? prefix : "album", a, b); - if (artist) { g_free (artist_checksum); g_free (artist_stripped); @@ -392,7 +411,7 @@ media_art_remove (const gchar *artist, gchar *target = NULL; gchar *album_path = NULL; - g_return_if_fail (artist != NULL && artist[0] != '\0'); + g_return_val_if_fail (artist != NULL && artist[0] != '\0', FALSE); dirname = g_build_filename (g_get_user_cache_dir (), "media-art", NULL); diff --git a/tests/mediaarttest.c b/tests/mediaarttest.c index e236713..f95afc7 100644 --- a/tests/mediaarttest.c +++ b/tests/mediaarttest.c @@ -71,27 +71,45 @@ test_mediaart_stripping_null (void) //g_assert (!mediaart_strip_invalid_entities (NULL)); } -struct { +typedef struct { + const gchar *prefix; const gchar *artist; - const gchar *album; + const gchar *title; const gchar *filename; -} mediaart_test_cases [] = { - {"Beatles", "Sgt. Pepper", - "album-2a9ea35253dbec60e76166ec8420fbda-cfba4326a32b44b8760b3a2fc827a634.jpeg"}, +} MediaArtTestCase; + +MediaArtTestCase mediaart_test_cases [] = { + { "album", "Beatles", "Sgt. Pepper", + "album-2a9ea35253dbec60e76166ec8420fbda-cfba4326a32b44b8760b3a2fc827a634.jpeg"}, - { "", "sgt. pepper", + { "album", "", "sgt. pepper", "album-d41d8cd98f00b204e9800998ecf8427e-cfba4326a32b44b8760b3a2fc827a634.jpeg"}, - { " ", "sgt. pepper", + { "album", " ", "sgt. pepper", "album-d41d8cd98f00b204e9800998ecf8427e-cfba4326a32b44b8760b3a2fc827a634.jpeg"}, - { NULL, "sgt. pepper", - "album-cfba4326a32b44b8760b3a2fc827a634-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"}, + { "album", NULL, "sgt. pepper", + "album-7215ee9c7d9dc229d2921a40e899ec5f-cfba4326a32b44b8760b3a2fc827a634.jpeg"}, - { "Beatles", NULL, + { "album", "Beatles", NULL, "album-2a9ea35253dbec60e76166ec8420fbda-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"}, - { NULL, NULL, NULL } + { "album", NULL, NULL, NULL }, + + { "podcast", NULL, "Podcast example", + "podcast-10ca71a13bbd1a2af179f6d5a4dea118-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"}, + + { "radio", NULL, "Radio Free Europe", + "radio-79577732dda605d0f953f6479ff1f42e-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"}, + + { "radio", "Artist is ignored", NULL, NULL }, + + { "x-video", NULL, "Test extension of spec", + "x-video-51110ae14ce4bbeb68335366289acdd1-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"}, + + /* Currently we raise a warning to advise the user that we don't + * support this part of the media art spec, which isn't tested here. */ + /*{ "track", "Ignored", "Ignored", NULL},*/ }; static void @@ -100,21 +118,37 @@ test_mediaart_location (void) gchar *path = NULL, *local_uri = NULL; gchar *expected; gint i; - - for (i = 0; mediaart_test_cases[i].filename != NULL; i++) { - media_art_get_path (mediaart_test_cases[i].artist, - mediaart_test_cases[i].album, - "album", + + for (i = 0; i < G_N_ELEMENTS(mediaart_test_cases); i++) { + MediaArtTestCase *testcase = &mediaart_test_cases[i]; + + media_art_get_path (testcase->artist, + testcase->title, + testcase->prefix, "file:///home/test/a.mp3", &path, &local_uri); - expected = g_build_path (G_DIR_SEPARATOR_S, - g_get_user_cache_dir (), - "media-art", - mediaart_test_cases[i].filename, - NULL); - g_assert_cmpstr (path, ==, expected); - + + if (testcase->filename == NULL) { + expected = NULL; + } else { + expected = g_build_path (G_DIR_SEPARATOR_S, + g_get_user_cache_dir (), + "media-art", + testcase->filename, + NULL); + } + + if (g_strcmp0(path, expected) != 0) { + g_error ("Incorrect path for prefix: '%s' artist: '%s' " + "title: '%s'. Expected %s, got %s", + testcase->prefix, + testcase->artist, + testcase->title, + expected, + path); + } + g_free (expected); g_free (path); g_free (local_uri); @@ -142,8 +176,8 @@ test_mediaart_location_path (void) /* Use path instead of URI */ media_art_get_path (mediaart_test_cases[0].artist, - mediaart_test_cases[0].album, - "album", + mediaart_test_cases[0].title, + mediaart_test_cases[0].prefix, "/home/test/a.mp3", &path, &local_uri); -- cgit v1.2.1