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 779584 - Handle coverart extraction in gnome-music
Handle coverart extraction in gnome-music
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-music-maint
gnome-music-maint
Depends on:
Blocks: 778870
 
 
Reported: 2017-03-04 18:18 UTC by Carlos Garnacho
Modified: 2017-03-13 20:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
albumartcache: Lookup embedded/local cover art (6.26 KB, patch)
2017-03-04 18:19 UTC, Carlos Garnacho
none Details | Review
albumartcache: Lookup embedded/local cover art (6.49 KB, patch)
2017-03-13 19:54 UTC, Marinus Schraal
none Details | Review
albumartcache: Lookup embedded/local cover art (7.05 KB, patch)
2017-03-13 20:21 UTC, Marinus Schraal
committed Details | Review

Description Carlos Garnacho 2017-03-04 18:18:54 UTC
I'm attaching a patch doing what Tracker used to do for gnome-music: trying to fetch albumart from embedded images (through gstreamer), and external image files in the same directory where the disc is (through libmediaart).

The patch however requires libmediaart from git ATM, an introspection fix was necessary for it.
Comment 1 Carlos Garnacho 2017-03-04 18:19:38 UTC
Created attachment 347227 [details] [review]
albumartcache: Lookup embedded/local cover art

This behavior is similar in result to what gnome-music used to obtained
from Tracker albumart management, if the albumart cache file does not
exist, it will first lookup embedded, then local images.

If none of those exist, the Grilo lookup paths will kick in as it used
to do.

As we now need an URI to pull embedded albumart from, the "all albums"
query will now return one of the songs from the album.
Comment 2 Marinus Schraal 2017-03-05 08:49:52 UTC
Review of attachment 347227 [details] [review]:

looks good

Some minor pep8 fixes needed, but I can do that on commit.

::: gnomemusic/albumartcache.py
@@ +220,3 @@
+        Gst.init(None)
+        self.discoverer = GstPbutils.Discoverer.new(Gst.SECOND)
+        self.discoverer.connect('discovered', self.discovered_cb)

self._discovered_cb

@@ +304,3 @@
+    @log
+    def discovered_cb(self, discoverer, info, error):
+        item, callback, itr, art_size, cache_path, album, artist = self._discoverer_items[info.get_uri()]

line too long (pep8)

@@ +341,3 @@
+            try:
+                mime = sample.get_caps().get_structure(0).get_name()
+                MediaArt.buffer_to_jpeg(map_info.data, mime, cache_path)

this needs another argument for me
(map_info.data, map_info.size, mime, cache_path)

@@ +345,3 @@
+                return
+            except Exception as err:
+                logger.warn("Error: %s, %s", err.__class__, err)

On some files (with mediaart) I get 

09:41:12 WARNING	Error: <class 'TypeError'>, Must be a single character

This comes from libmediaart.
Comment 3 Carlos Garnacho 2017-03-05 10:51:15 UTC
(In reply to Marinus Schraal from comment #2)
> Review of attachment 347227 [details] [review] [review]:
> @@ +341,3 @@
> +            try:
> +                mime = sample.get_caps().get_structure(0).get_name()
> +                MediaArt.buffer_to_jpeg(map_info.data, mime, cache_path)
> 
> this needs another argument for me
> (map_info.data, map_info.size, mime, cache_path)
> 
> @@ +345,3 @@
> +                return
> +            except Exception as err:
> +                logger.warn("Error: %s, %s", err.__class__, err)
> 
> On some files (with mediaart) I get 
> 
> 09:41:12 WARNING	Error: <class 'TypeError'>, Must be a single character
> 
> This comes from libmediaart.

These last two errors are gone with the libmediaart introspection fix at:

https://git.gnome.org/browse/libmediaart/commit/?id=65a8f317a7155303a7208808234570f32c721207

I guess nobody tried to use buffer_to_jpeg() from introspection before :(. I can roll a tarball soon so gnome-music 3.24 can bump the required version.
Comment 4 Marinus Schraal 2017-03-13 19:54:56 UTC
Created attachment 347868 [details] [review]
albumartcache: Lookup embedded/local cover art

This behavior is similar in result to what gnome-music used to obtained
from Tracker albumart management, if the albumart cache file does not
exist, it will first lookup embedded, then local images.

If none of those exist, the Grilo lookup paths will kick in as it used
to do.

As we now need an URI to pull embedded albumart from, the "all albums"
query will now return one of the songs from the album.
Comment 5 Marinus Schraal 2017-03-13 20:21:29 UTC
Created attachment 347871 [details] [review]
albumartcache: Lookup embedded/local cover art

This behavior is similar in result to what gnome-music used to obtained
from Tracker albumart management, if the albumart cache file does not
exist, it will first lookup embedded, then local images.

If none of those exist, the Grilo lookup paths will kick in as it used
to do.

As we now need an URI to pull embedded albumart from, the "all albums"
query will now return one of the songs from the album.

Bump libmediaart version to contain a gir fix.
Comment 6 Marinus Schraal 2017-03-13 20:23:34 UTC
Did some fixes and cleanups. Thanks for the patch.

Attachment 347871 [details] pushed as 38320cd - albumartcache: Lookup embedded/local cover art