Merge pull request #1683 from sampsyo/1264-unnecessary-resize

Fix #1264: Avoid unnecessary art resizes

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
@@ -505,11 +509,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)
@@ -519,10 +530,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
@@ -532,6 +547,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin):
         are made.
         """
         out = None
+        check = None
 
         # Local art.
         cover_names = self.config['cover_names'].as_str_seq()
@@ -540,7 +556,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
@@ -552,12 +569,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

docs/changelog.rst

@@ -16,6 +16,8 @@ Fixes:
   when there were not. :bug:`1652`
 * :doc:`plugins/lastgenre`: Clean up the reggae related genres somewhat.
   Thanks to :user:`Freso`. :bug:`1661`
+* :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`

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
@@ -422,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).
@@ -445,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__)