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 760378 - Cover art fetching broken for albums
Cover art fetching broken for albums
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: plugins
unspecified
Other Linux
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2016-01-09 22:33 UTC by Carlos Garnacho
Modified: 2016-01-10 11:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
lua-factory: Make containers match any supported_media (1.41 KB, patch)
2016-01-09 22:34 UTC, Carlos Garnacho
committed Details | Review

Description Carlos Garnacho 2016-01-09 22:33:08 UTC
After the switch to grilo 0.3, gnome-music seems unable to fetch any cover art, this can be seen moving/deleting ~/.cache/media-art and running gnome-music from master.

AFAICT, the culprit is around https://git.gnome.org/browse/grilo-plugins/tree/src/lua-factory/grl-lua-factory.c#n1656 . The grilo tracker source query used in gnome-music returns nmm:MusicAlbums, so we get GrlMedia that only pass the grl_media_is_container() check, however the lua plugins that implement cover downloading for music specify "audio" as the only supported media, so we bail out here.

Arguably, containers should pass an "any" check, rather than an "all" check, I'm attaching a patch going that way.
Comment 1 Carlos Garnacho 2016-01-09 22:34:23 UTC
Created attachment 318609 [details] [review]
lua-factory: Make containers match any supported_media

Individually, containers are not likely to represent all 3 of
images/audio/video types at once, it feels a bit backwards that
lua plugins must use "all" in order to provide information for
containers.

Fixes album cover fetching on gnome-music after the port to
grilo 0.3.
Comment 2 Victor Toso 2016-01-09 23:25:25 UTC
Review of attachment 318609 [details] [review]:

It should be any, not all. Indeed!

::: src/lua-factory/grl-lua-factory.c
@@ +1656,3 @@
   /* Verify if the source resolve type and media type match */
   res_type = lua_source->priv->resolve_type;
+  if ((grl_media_is_container (media) && !(res_type & GRL_SUPPORTED_MEDIA_ALL))

Yes, indeed.
Comment 3 Carlos Garnacho 2016-01-10 11:30:51 UTC
Thanks! pushed to master.

Attachment 318609 [details] pushed as 56f96c0 - lua-factory: Make containers match any supported_media