After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705834 - Behavior of Tracker and Media Art Storage spec do not match when the artist name is not found
Behavior of Tracker and Media Art Storage spec do not match when the artist n...
Status: RESOLVED FIXED
Product: tracker
Classification: Core
Component: Extractor
git master
Other Linux
: Normal normal
: ---
Assigned To: tracker-extractor
Depends on:
Blocks:
 
 
Reported: 2013-08-12 12:20 UTC by Arnel Borja
Modified: 2013-10-11 16:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch if the later case is the prefered one (1.39 KB, patch)
2013-08-12 12:25 UTC, Arnel Borja
committed Details | Review

Description Arnel Borja 2013-08-12 12:20:50 UTC
According to the Media Art Spec, B should be a space if the artist name is not found, and the title would then be C:
https://wiki.gnome.org/MediaArtStorageSpec#Identifiers

But tracker sets B to the title then sets C as space if it can't find the artist name.
Line 275 of file src/libtracker-common/tracker-media-art.c:
-----
        if (artist) {
                a = artist_checksum;
                b = title ? title_checksum : space_checksum;
        } else {
                a = title_checksum;
                b = space_checksum;
        }
-----

This behavior in tracker was introduced by this commit:

commit 439c662b1a5561d2dcfb29a824f2a5409d540156
Author: Sam Thursfield <sam.thursfield@codethink.co.uk>
Date:   Thu Oct 20 17:11:14 2011 +0100

    libtracker-common: Fix media art paths when artist is not known
    
    Correct behaviour is prefix-md5sum(title)-md5sum( ) but previously
    we were generating prefix-md5sum( )-md5sum(title)

So the solution is either to amend the spec, or tracker should follow the spec.
Comment 1 Arnel Borja 2013-08-12 12:25:29 UTC
Created attachment 251344 [details] [review]
Patch if the later case is the prefered one
Comment 2 Sam Thursfield 2013-08-13 11:36:50 UTC
Most media art with no artist name is 'video-' rather than 'album-'. That type of media art is actually not in the spec at all, so I don't think we need to worry about this change breaking existing code in places outside of GNOME.

The patch itself looks correct (and clearer than the code was!)
Comment 3 Arnel Borja 2013-08-13 11:51:56 UTC
Review of attachment 251344 [details] [review]:

Pushed as 12656e475 to master.
Comment 4 Sam Thursfield 2013-10-02 11:26:15 UTC
I realised today why I introduced the seemingly incorrect behaviour in the first place.

For "album" media art, the new behaviour is correct according to the spec:
  A = "album"
  B = MD5(artist), or MD5(" ") if there is no artist name
  C = MD5(album), or MD5(" ") if there is no album name (title)

However, media art of other types doesn't *have* an "artist" field. So if you wanted to calculate the media art filename of a "radio" or "podcast" (or "video", although this isn't in the spec), you'd would probably decide to call:

    tracker_media_art_get_path(NULL, "my title", "podcast", ...)

That would return the wrong thing though: you'd get:

    podcast-MD5(" ")-MD5("my title")

When what the spec says to do is:

    podcast-MD5("my title")-MD5(" ")

You could work around this in Tracker by doing this:

    tracker_media_art_get_path("my title", NULL, "podcast", ...)

i.e. passing the title for 'artist' and NULL for 'title'. The API is clearly not designed for this, though.

I suggest that we ignore this problem in Tracker (since it doesn't extract podcast art or any such thing anyway), but come up with a fix for this in  'libmediaart'.


Also (this is the actual reason I revisited this change), tests/libtracker-common/tracker-media-art-test.c should have been updated when we made this change. I shall push a fix the test :)
Comment 5 Sam Thursfield 2013-10-11 16:05:39 UTC
I've pushed a branch to libmediaart.git (media-art-types) that will fixes the problem there, and adds support for the media art types which include just the title in the cache path.