diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 904220685..d8568bfd1 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -39,8 +39,7 @@ import os urllib3_logger = logging.getLogger('requests.packages.urllib3') urllib3_logger.setLevel(logging.CRITICAL) -USER_AGENT = 'beets/{0} +http://beets.radbox.org/'.format(beets.__version__) \ - .encode('utf8') +USER_AGENT = u'beets/{0} +http://beets.radbox.org/'.format(beets.__version__) # Exceptions that discogs_client should really handle but does not. CONNECTION_ERRORS = (ConnectionError, socket.error, httplib.HTTPException, @@ -66,8 +65,8 @@ class DiscogsPlugin(BeetsPlugin): def setup(self, session=None): """Create the `discogs_client` field. Authenticate if necessary. """ - c_key = self.config['apikey'].get(unicode).encode('utf8') - c_secret = self.config['apisecret'].get(unicode).encode('utf8') + c_key = self.config['apikey'].get(unicode) + c_secret = self.config['apisecret'].get(unicode) # Get the OAuth token from a file or log in. try: @@ -77,8 +76,8 @@ class DiscogsPlugin(BeetsPlugin): # No token yet. Generate one. token, secret = self.authenticate(c_key, c_secret) else: - token = tokendata['token'].encode('utf8') - secret = tokendata['secret'].encode('utf8') + token = tokendata['token'] + secret = tokendata['secret'] self.discogs_client = Client(USER_AGENT, c_key, c_secret, token, secret) @@ -121,7 +120,7 @@ class DiscogsPlugin(BeetsPlugin): with open(self._tokenfile(), 'w') as f: json.dump({'token': token, 'secret': secret}, f) - return token.encode('utf8'), secret.encode('utf8') + return token, secret def album_distance(self, items, album_info, mapping): """Returns the album distance. @@ -151,8 +150,8 @@ class DiscogsPlugin(BeetsPlugin): return self.candidates(items, artist, album, va_likely) else: return [] - except CONNECTION_ERRORS as e: - self._log.debug(u'HTTP Connection Error: {0}', e) + except CONNECTION_ERRORS: + self._log.debug('Connection error in album search', exc_info=True) return [] def album_for_id(self, album_id): @@ -182,8 +181,8 @@ class DiscogsPlugin(BeetsPlugin): self.reset_auth() return self.album_for_id(album_id) return None - except CONNECTION_ERRORS as e: - self._log.debug(u'HTTP Connection Error: {0}', e) + except CONNECTION_ERRORS: + self._log.debug('Connection error in album lookup', exc_info=True) return None return self.get_album_info(result) @@ -204,9 +203,9 @@ class DiscogsPlugin(BeetsPlugin): try: releases = self.discogs_client.search(query, type='release').page(1) - except CONNECTION_ERRORS as exc: - self._log.debug("Communication error while searching for {0!r}: " - "{1}".format(query, exc)) + except CONNECTION_ERRORS: + self._log.debug("Communication error while searching for {0!r}", + query, exc_info=True) return [] return [self.get_album_info(release) for release in releases[:5]] diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 10b30af0f..9c0efa51e 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -63,7 +63,6 @@ class EmbedCoverArtPlugin(BeetsPlugin): maxwidth = self.config['maxwidth'].get(int) compare_threshold = self.config['compare_threshold'].get(int) ifempty = self.config['ifempty'].get(bool) - remove_art_file = self.config['remove_art_file'].get(bool) def embed_func(lib, opts, args): if opts.file: @@ -79,14 +78,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): for album in lib.albums(decargs(args)): art.embed_album(self._log, album, maxwidth, False, compare_threshold, ifempty) - - if remove_art_file and album.artpath is not None: - if os.path.isfile(album.artpath): - self._log.debug(u'Removing album art file ' - u'for {0}', album) - os.remove(album.artpath) - album.artpath = None - album.store() + self.remove_artfile(album) embed_cmd.func = embed_func @@ -141,3 +133,15 @@ class EmbedCoverArtPlugin(BeetsPlugin): art.embed_album(self._log, album, max_width, True, self.config['compare_threshold'].get(int), self.config['ifempty'].get(bool)) + self.remove_artfile(album) + + def remove_artfile(self, album): + """Possibly delete the album art file for an album (if the + appropriate configuration option is enabled. + """ + if self.config['remove_art_file'] and album.artpath: + if os.path.isfile(album.artpath): + self._log.debug('Removing album art file for {0}', album) + os.remove(album.artpath) + album.artpath = None + album.store() diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 2c20dd698..6847d8828 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -41,6 +41,10 @@ IMAGE_EXTENSIONS = ['png', 'jpg', 'jpeg'] CONTENT_TYPES = ('image/jpeg', 'image/png') DOWNLOAD_EXTENSION = '.jpg' +CANDIDATE_BAD = 0 +CANDIDATE_EXACT = 1 +CANDIDATE_DOWNSCALE = 2 + def _logged_get(log, *args, **kwargs): """Like `requests.get`, but logs the effective URL to the specified @@ -508,11 +512,18 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): return None def _is_valid_image_candidate(self, candidate): - if not candidate: - return False + """Determine whether the given candidate artwork is valid based on + its dimensions (width and ratio). - if not (self.enforce_ratio or self.minwidth): - return True + Return `CANDIDATE_BAD` if the file is unusable. + Return `CANDIDATE_EXACT` if the file is usable as-is. + Return `CANDIDATE_DOWNSCALE` if the file must be resized. + """ + if not candidate: + return CANDIDATE_BAD + + if not (self.enforce_ratio or self.minwidth or self.maxwidth): + return CANDIDATE_EXACT # get_size returns None if no local imaging backend is available size = ArtResizer.shared.get_size(candidate) @@ -522,10 +533,14 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): u'documentation for dependencies. ' u'The configuration options `minwidth` and ' u'`enforce_ratio` may be violated.') - return True + return CANDIDATE_EXACT - return size and size[0] >= self.minwidth and \ - (not self.enforce_ratio or size[0] == size[1]) + if (not self.minwidth or size[0] >= self.minwidth) and ( + not self.enforce_ratio or size[0] == size[1]): + if not self.maxwidth or size[0] > self.maxwidth: + return CANDIDATE_DOWNSCALE + return CANDIDATE_EXACT + return CANDIDATE_BAD def art_for_album(self, album, paths, local_only=False): """Given an Album object, returns a path to downloaded art for the @@ -535,6 +550,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): are made. """ out = None + check = None # Local art. cover_names = self.config['cover_names'].as_str_seq() @@ -543,7 +559,8 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): if paths: for path in paths: candidate = self.fs_source.get(path, cover_names, cautious) - if self._is_valid_image_candidate(candidate): + check = self._is_valid_image_candidate(candidate) + if check: out = candidate self._log.debug('found local image {}', out) break @@ -555,12 +572,13 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): if self.maxwidth: url = ArtResizer.shared.proxy_url(self.maxwidth, url) candidate = self._fetch_image(url) - if self._is_valid_image_candidate(candidate): + check = self._is_valid_image_candidate(candidate) + if check: out = candidate self._log.debug('using remote image {}', out) break - if self.maxwidth and out: + if self.maxwidth and out and check == CANDIDATE_DOWNSCALE: out = ArtResizer.shared.resize(self.maxwidth, out) return out @@ -570,7 +588,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): fetchart CLI command. """ for album in albums: - if album.artpath and not force: + if album.artpath and not force and os.path.isfile(album.artpath): message = ui.colorize('text_highlight_minor', 'has album art') else: # In ordinary invocations, look for images on the diff --git a/beetsplug/thumbnails.py b/beetsplug/thumbnails.py index ce3382fc8..6fd8c8cbe 100644 --- a/beetsplug/thumbnails.py +++ b/beetsplug/thumbnails.py @@ -39,8 +39,8 @@ from beets.util.artresizer import ArtResizer, has_IM, has_PIL BASE_DIR = os.path.join(BaseDirectory.xdg_cache_home, "thumbnails") -NORMAL_DIR = os.path.join(BASE_DIR, "normal") -LARGE_DIR = os.path.join(BASE_DIR, "large") +NORMAL_DIR = util.bytestring_path(os.path.join(BASE_DIR, "normal")) +LARGE_DIR = util.bytestring_path(os.path.join(BASE_DIR, "large")) class ThumbnailsPlugin(BeetsPlugin): diff --git a/docs/changelog.rst b/docs/changelog.rst index d71efac17..666a76641 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -20,6 +20,14 @@ Fixes: :bug:`314` * :doc:`/plugins/fetchart`: In auto mode, skips albums that already have art attached to them so as not to interfere with re-imports. :bug:`314` +* :doc:`plugins/fetchart`: The plugin now only resizes album art if necessary, + rather than always by default. :bug:`1264` +* :doc:`plugins/fetchart`: Fix a bug where a database reference to a + non-existent album art file would prevent the command from fetching new art. + :bug:`1126` +* :doc:`/plugins/thumbnails`: Fix a crash with Unicode paths. :bug:`1686` +* :doc:`/plugins/embedart`: The ``remove_art_file`` option now works on import + (as well as with the explicit command). :bug:`1662` :bug:`1675` 1.3.15 (October 17, 2015) diff --git a/docs/faq.rst b/docs/faq.rst index 447be45e1..050461587 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -208,6 +208,23 @@ Use the ``%asciify{}`` function in your path formats. See :ref:`template-functions`. +.. _move-dir: + +…point beets at a new music directory? +-------------------------------------- + +If you want to move your music from one directory to another, the best way is +to let beets do it for you. First, edit your configuration and set the +``directory`` setting to the new place. Then, type ``beet move`` to have beets +move all your files. + +If you've already moved your music *outside* of beets, you have a few options: + +- Move the music back (with an ordinary ``mv``) and then use the above steps. +- Delete your database and re-create it from the new paths using ``beet import -AWMC``. +- Resort to manually modifying the SQLite database (not recommended). + + Why does beets… =============== diff --git a/test/test_art.py b/test/test_art.py index 3cc24a7a4..e4a070b1a 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -21,6 +21,7 @@ import os import shutil import responses +from mock import patch from test import _common from test._common import unittest @@ -30,6 +31,7 @@ from beets import library from beets import importer from beets import config from beets import logging +from beets import util from beets.util.artresizer import ArtResizer, WEBPROXY @@ -357,6 +359,18 @@ class ArtImporterTest(UseThePlugin): self.afa_response = artdest self._fetch_art(True) + def test_fetch_art_if_imported_file_deleted(self): + # See #1126. Test the following scenario: + # - Album art imported, `album.artpath` set. + # - Imported album art file subsequently deleted (by user or other + # program). + # `fetchart` should import album art again instead of printing the + # message " has album art". + self._fetch_art(True) + util.remove(self.album.artpath) + self.plugin.batch_fetch_art(self.lib, self.lib.albums(), force=False) + self.assertExists(self.album.artpath) + class ArtForAlbumTest(UseThePlugin): """ Tests that fetchart.art_for_album respects the size @@ -409,6 +423,12 @@ class ArtForAlbumTest(UseThePlugin): else: self.assertIsNone(local_artpath) + def _assertImageResized(self, image_file, should_resize): + self.image_file = image_file + with patch.object(ArtResizer.shared, 'resize') as mock_resize: + self.plugin.art_for_album(None, [''], True) + self.assertEqual(mock_resize.called, should_resize) + def _require_backend(self): """Skip the test if the art resizer doesn't have ImageMagick or PIL (so comparisons and measurements are unavailable). @@ -432,6 +452,12 @@ class ArtForAlbumTest(UseThePlugin): self.plugin.enforce_ratio = False self._assertImageIsValidArt(self.IMG_500x490, True) + def test_resize_if_necessary(self): + self._require_backend() + self.plugin.maxwidth = 300 + self._assertImageResized(self.IMG_225x225, False) + self._assertImageResized(self.IMG_348x348, True) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__)