Merge pull request #436 from ABCbum/grab-cover-art

Grab cover art from MusicBrainz/Cover Art Archive and add it to the resulting whipper rips

.travis.yml

@@ -24,7 +24,7 @@ install:
     # Dependencies
     - sudo apt-get -qq update
     - pip install --upgrade -qq pip
-    - sudo apt-get -qq install cdparanoia cdrdao flac gir1.2-glib-2.0 libcdio-dev libgirepository1.0-dev libiso9660-dev libsndfile1-dev sox swig libcdio-utils libdiscid0
+    - sudo apt-get -qq install cdparanoia cdrdao flac gir1.2-glib-2.0 libcdio-dev libgirepository1.0-dev libiso9660-dev libsndfile1-dev sox swig libcdio-utils libdiscid0 python3-pil
     # newer version of pydcio requires newer version of libcdio than travis has
     - pip install pycdio==0.21
     # install rest of dependencies

Dockerfile

@@ -21,6 +21,7 @@ RUN apt-get update && apt-get install --no-install-recommends -y \
     python3-gi \
     python3-musicbrainzngs \
     python3-mutagen \
+    python3-pil \
     python3-pip \
     python3-requests \
     python3-ruamel.yaml \

README.md

@@ -156,6 +156,13 @@ PyPI installable dependencies are listed in the [requirements.txt](https://githu
 
 `pip install -r requirements.txt`
 
+### Optional dependencies
+- [pillow](https://pypi.org/project/Pillow/), for completely supporting the cover art feature (`embed` and `complete` option values won't work otherwise).
+
+This dependency isn't listed in the `requirements.txt`, to install it just issue the following command:
+
+`pip install Pillow`
+
 ### Fetching the source code
 
 Change to a directory where you want to put whipper source code (for example, `$HOME/dev/ext` or `$HOME/prefix/src`)

setup.py

@@ -16,6 +16,9 @@
                   libraries=['sndfile'],
                   sources=['src/accuraterip-checksum.c'])
     ],
+    extras_require={
+        'cover_art': ["pillow"]
+    },
     entry_points={
         'console_scripts': [
             'whipper = whipper.command.main:main'

whipper/command/cd.py

@@ -20,6 +20,7 @@
 
 import argparse
 import cdio
+import importlib.util
 import os
 import glob
 import logging
@@ -290,6 +291,14 @@ def add_arguments(self):
                                  help="whether to continue ripping if "
                                  "the disc is a CD-R",
                                  default=False)
+        self.parser.add_argument('-C', '--cover-art',
+                                 action="store", dest="fetch_cover_art",
+                                 help="Fetch cover art and save it as "
+                                 "standalone file, embed into FLAC files "
+                                 "or perform both actions: file, embed, "
+                                 "complete option values respectively",
+                                 choices=['file', 'embed', 'complete'],
+                                 default=None)
 
     def handle_arguments(self):
         self.options.output_directory = os.path.expanduser(
@@ -342,6 +351,19 @@ def doCommand(self):
             logger.info("creating output directory %s", dirname)
             os.makedirs(dirname)
 
+        self.coverArtPath = None
+        if (self.options.fetch_cover_art in {"embed", "complete"} and
+                importlib.util.find_spec("PIL") is None):
+            logger.warning("the cover art option '%s' won't be honored "
+                           "because the 'pillow' module isn't available",
+                           self.options.fetch_cover_art)
+        elif self.options.fetch_cover_art in {"file", "embed", "complete"}:
+            self.coverArtPath = self.program.getCoverArt(
+                                    dirname,
+                                    self.program.metadata.mbid)
+        if self.options.fetch_cover_art == "file":
+            self.coverArtPath = None  # NOTE: avoid image embedding (hacky)
+
         # FIXME: turn this into a method
         def _ripIfNotRipped(number):
             logger.debug('ripIfNotRipped for track %d', number)
@@ -412,7 +434,8 @@ def _ripIfNotRipped(number):
                                               what='track %d of %d%s' % (
                                                   number,
                                                   len(self.itable.tracks),
-                                                  extra))
+                                                  extra),
+                                              coverArtPath=self.coverArtPath)
                         break
                     # FIXME: catching too general exception (Exception)
                     except Exception as e:
@@ -474,6 +497,15 @@ def _ripIfNotRipped(number):
                 continue
             _ripIfNotRipped(i + 1)
 
+        # NOTE: Seems like some kind of with … or try: … finally: … clause
+        # would be more appropriate, since otherwise this would potentially
+        # leave stray files lying around in case of crashes etc.
+        # <Freso 2020-01-03, GitHub comment>
+        if (self.options.fetch_cover_art == "embed" and
+                self.coverArtPath is not None):
+            logger.debug('deleting cover art file at: %r', self.coverArtPath)
+            os.remove(self.coverArtPath)
+
         logger.debug('writing cue file for %r', discName)
         self.program.writeCue(discName)
 

whipper/common/encode.py

@@ -19,7 +19,8 @@
 # along with whipper.  If not, see <http://www.gnu.org/licenses/>.
 
 
-from mutagen.flac import FLAC
+from mutagen.flac import FLAC, Picture
+from mutagen.id3 import PictureType
 
 from whipper.extern.task import task
 
@@ -89,3 +90,71 @@ def _tag(self):
         w.save()
 
         self.stop()
+
+
+class EmbedPictureTask(task.Task):
+    description = 'Embed picture to FLAC'
+
+    def __init__(self, track_path, cover_art_path):
+        self.track_path = track_path
+        self.cover_art_path = cover_art_path
+
+    def start(self, runner):
+        task.Task.start(self, runner)
+        self.schedule(0.0, self._embed_picture)
+
+    def _make_flac_picture(self, cover_art_filename):
+        """
+        Given a path to a jpg/png file, return a FLAC picture for embedding.
+
+        The embedding will be performed using the mutagen module.
+
+        :param cover_art_filename: path to cover art image file
+        :type cover_art_filename: str
+        :returns: a valid FLAC picture for embedding
+        :rtype: mutagen.flac.Picture or None
+        """
+        if not cover_art_filename:
+            return
+
+        from PIL import Image
+
+        im = Image.open(cover_art_filename)
+        # NOTE: the cover art thumbnails we're getting from the Cover Art
+        # Archive should be always in the JPEG format: this check is currently
+        # useless but will leave it here to better handle unexpected formats.
+        if im.format == 'JPEG':
+            mime = 'image/jpeg'
+        elif im.format == 'PNG':
+            mime = 'image/png'
+        else:
+            # we only support png and jpeg
+            logger.warning("no cover art will be added because the fetched "
+                           "image format is unsupported")
+            return
+
+        pic = Picture()
+        with open(cover_art_filename, 'rb') as f:
+            pic.data = f.read()
+
+        pic.type = PictureType.COVER_FRONT
+        pic.mime = mime
+        pic.width, pic.height = im.size
+        if im.mode not in ('P', 'RGB', 'SRGB'):
+            logger.warning("no cover art will be added because the fetched "
+                           "image mode is unsupported")
+            return
+
+        return pic
+
+    def _embed_picture(self):
+        """
+        Get flac picture generated from mutagen.flac.Picture then embed
+        it to given track if the flac picture exists.
+        """
+        flac_pic = self._make_flac_picture(self.cover_art_path)
+        if flac_pic:
+            w = FLAC(self.track_path)
+            w.add_picture(flac_pic)
+
+        self.stop()

whipper/common/program.py

@@ -27,6 +27,7 @@
 import os
 import time
 
+from tempfile import NamedTemporaryFile
 from whipper.common import accurip, cache, checksum, common, mbngs, path
 from whipper.program import cdrdao, cdparanoia
 from whipper.image import image
@@ -470,6 +471,35 @@ def getHTOA(self):
         stop = track.getIndex(1).absolute - 1
         return start, stop
 
+    def getCoverArt(self, path, release_id):
+        """
+        Get cover art image from Cover Art Archive.
+
+        :param path: where to store the fetched image
+        :type  path: str
+        :param release_id: a release id (self.program.metadata.mbid)
+        :type  release_id: str
+        :returns: path to the downloaded cover art, else `None`
+        :rtype: str or None
+        """
+        cover_art_path = os.path.join(path, 'cover.jpg')
+
+        logger.debug('fetching cover art for release: %r', release_id)
+        try:
+            data = musicbrainzngs.get_image_front(release_id, 500)
+        except musicbrainzngs.ResponseError as e:
+            logger.error('error fetching cover art: %r', e)
+            return
+
+        if data:
+            with NamedTemporaryFile(suffix='.cover.jpg', delete=False) as f:
+                f.write(data)
+            os.chmod(f.name, 0o644)
+            os.replace(f.name, cover_art_path)
+            logger.debug('cover art fetched at: %r', cover_art_path)
+            return cover_art_path
+        return
+
     @staticmethod
     def verifyTrack(runner, trackResult):
         is_wave = not trackResult.filename.endswith('.flac')
@@ -490,7 +520,7 @@ def verifyTrack(runner, trackResult):
         return ret
 
     def ripTrack(self, runner, trackResult, offset, device, taglist,
-                 overread, what=None):
+                 overread, what=None, coverArtPath=None):
         """
         Ripping the track may change the track's filename as stored in
         trackResult.
@@ -516,7 +546,8 @@ def ripTrack(self, runner, trackResult, offset, device, taglist,
                                            offset=offset,
                                            device=device,
                                            taglist=taglist,
-                                           what=what)
+                                           what=what,
+                                           coverArtPath=coverArtPath)
 
         runner.run(t)
 

whipper/program/cdparanoia.py

@@ -427,7 +427,7 @@ class ReadVerifyTrackTask(task.MultiSeparateTask):
     _tmppath = None
 
     def __init__(self, path, table, start, stop, overread, offset=0,
-                 device=None, taglist=None, what="track"):
+                 device=None, taglist=None, what="track", coverArtPath=None):
         """
         :param path:    where to store the ripped track
         :type  path:    str
@@ -493,8 +493,9 @@ def __init__(self, path, table, start, stop, overread, offset=0,
         self.tasks.append(checksum.CRC32Task(tmppath))
         self.tasks.append(encode.SoxPeakTask(tmppath))
 
-        # TODO: Move tagging outside of cdparanoia
+        # TODO: Move tagging and embed picture outside of cdparanoia
         self.tasks.append(encode.TaggingTask(tmpoutpath, taglist))
+        self.tasks.append(encode.EmbedPictureTask(tmpoutpath, coverArtPath))
 
         self.checksum = None
 

whipper/test/test_common_program.py

@@ -2,8 +2,10 @@
 # vi:si:et:sw=4:sts=4:ts=4
 
 
+import os
 import unittest
 
+from tempfile import NamedTemporaryFile
 from whipper.common import program, mbngs, config
 from whipper.command.cd import DEFAULT_DISC_TEMPLATE
 
@@ -38,3 +40,54 @@ def testIssue66TemplateFilled(self):
         path = prog.getPath('/tmp', '%A/%d', 'mbdiscid', md, 0)
         self.assertEqual(path,
                          '/tmp/Jeff Buckley/Grace')
+
+
+# TODO: Test cover art embedding too.
+class CoverArtTestCase(unittest.TestCase):
+
+    @staticmethod
+    def _mock_get_front_image(release_id):
+        """
+        Mock `musicbrainzngs.get_front_image` function.
+
+        Reads a local cover art image and returns its binary data.
+
+        :param release_id: a release id (self.program.metadata.mbid)
+        :type  release_id: str
+        :returns: the binary content of the local cover art image
+        :rtype: bytes
+        """
+        filename = '%s.jpg' % release_id
+        path = os.path.join(os.path.dirname(__file__), filename)
+        with open(path, 'rb') as f:
+            return f.read()
+
+    def _mock_getCoverArt(self, path, release_id):
+        """
+        Mock `common.program.getCoverArt` function.
+
+        :param path: where to store the fetched image
+        :type  path: str
+        :param release_id: a release id (self.program.metadata.mbid)
+        :type  release_id: str
+        :returns: path to the downloaded cover art
+        :rtype: str
+        """
+        cover_art_path = os.path.join(path, 'cover.jpg')
+
+        data = self._mock_get_front_image(release_id)
+
+        with NamedTemporaryFile(suffix='.cover.jpg', delete=False) as f:
+            f.write(data)
+        os.chmod(f.name, 0o644)
+        os.replace(f.name, cover_art_path)
+        return cover_art_path
+
+    def testCoverArtPath(self):
+        """Test whether a fetched cover art is saved properly."""
+        # Using: Dummy by Portishead
+        # https://musicbrainz.org/release/76df3287-6cda-33eb-8e9a-044b5e15ffdd
+        path = os.path.dirname(__file__)
+        release_id = "76df3287-6cda-33eb-8e9a-044b5e15ffdd"
+        coverArtPath = self._mock_getCoverArt(path, release_id)
+        self.assertTrue(os.path.isfile(coverArtPath))