diff --git a/.travis.yml b/.travis.yml index 3dd3a75..7425673 100644 --- a/.travis.yml +++ b/.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 diff --git a/Dockerfile b/Dockerfile index 73cb13d..ba85f15 100644 --- a/Dockerfile +++ b/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 \ diff --git a/README.md b/README.md index 02681bc..788d695 100644 --- a/README.md +++ b/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`) diff --git a/setup.py b/setup.py index 96bebf5..d5d41e3 100644 --- a/setup.py +++ b/setup.py @@ -16,6 +16,9 @@ setup( libraries=['sndfile'], sources=['src/accuraterip-checksum.c']) ], + extras_require={ + 'cover_art': ["pillow"] + }, entry_points={ 'console_scripts': [ 'whipper = whipper.command.main:main' diff --git a/whipper/command/cd.py b/whipper/command/cd.py index a5ecc49..7060f86 100644 --- a/whipper/command/cd.py +++ b/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 @@ Log files will log the path to tracks relative to this directory. 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 @@ Log files will log the path to tracks relative to this directory. 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 @@ Log files will log the path to tracks relative to this directory. 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 @@ Log files will log the path to tracks relative to this directory. 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. + # + 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) diff --git a/whipper/common/encode.py b/whipper/common/encode.py index 237daa9..667ab51 100644 --- a/whipper/common/encode.py +++ b/whipper/common/encode.py @@ -19,7 +19,8 @@ # along with whipper. If not, see . -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 @@ class TaggingTask(task.Task): 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() diff --git a/whipper/common/program.py b/whipper/common/program.py index 1d829ae..dd1f824 100644 --- a/whipper/common/program.py +++ b/whipper/common/program.py @@ -27,6 +27,7 @@ import re 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 @@ class Program: 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 @@ class Program: 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 @@ class Program: offset=offset, device=device, taglist=taglist, - what=what) + what=what, + coverArtPath=coverArtPath) runner.run(t) diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py index f071674..d410d0c 100644 --- a/whipper/program/cdparanoia.py +++ b/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 @@ class ReadVerifyTrackTask(task.MultiSeparateTask): 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 diff --git a/whipper/test/76df3287-6cda-33eb-8e9a-044b5e15ffdd.jpg b/whipper/test/76df3287-6cda-33eb-8e9a-044b5e15ffdd.jpg new file mode 100644 index 0000000..07179a3 Binary files /dev/null and b/whipper/test/76df3287-6cda-33eb-8e9a-044b5e15ffdd.jpg differ diff --git a/whipper/test/test_common_program.py b/whipper/test/test_common_program.py index 36cc7a6..8871de9 100644 --- a/whipper/test/test_common_program.py +++ b/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 @@ class PathTestCase(unittest.TestCase): 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))