Merge pull request #4029 from khnsky/deinterlace

Add option to fetchart to store cover art as non-progressive.

beets/util/artresizer.py

@@ -77,7 +77,10 @@ def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
             # Use PIL's default quality.
             quality = -1
 
-        im.save(util.py3_path(path_out), quality=quality)
+        # progressive=False only affects JPEGs and is the default,
+        # but we include it here for explicitness.
+        im.save(util.py3_path(path_out), quality=quality, progressive=False)
+
         if max_filesize > 0:
             # If maximum filesize is set, we attempt to lower the quality of
             # jpeg conversion by a proportional amount, up to 3 attempts
@@ -99,9 +102,8 @@ def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
                 if lower_qual < 10:
                     lower_qual = 10
                 # Use optimize flag to improve filesize decrease
-                im.save(
-                    util.py3_path(path_out), quality=lower_qual, optimize=True
-                )
+                im.save(util.py3_path(path_out), quality=lower_qual,
+                        optimize=True, progressive=False)
             log.warning("PIL Failed to resize file to below {0}B",
                         max_filesize)
             return path_out
@@ -127,9 +129,12 @@ def im_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
     # "-resize WIDTHx>" shrinks images with the width larger
     # than the given width while maintaining the aspect ratio
     # with regards to the height.
+    # ImageMagick already seems to default to no interlace, but we include it
+    # here for the sake of explicitness.
     cmd = ArtResizer.shared.im_convert_cmd + [
         util.syspath(path_in, prefix=False),
         '-resize', f'{maxwidth}x>',
+        '-interlace', 'none',
     ]
 
     if quality > 0:
@@ -195,6 +200,40 @@ BACKEND_GET_SIZE = {
 }
 
 
+def pil_deinterlace(path_in, path_out=None):
+    path_out = path_out or temp_file_for(path_in)
+    from PIL import Image
+
+    try:
+        im = Image.open(util.syspath(path_in))
+        im.save(util.py3_path(path_out), progressive=False)
+        return path_out
+    except IOError:
+        return path_in
+
+
+def im_deinterlace(path_in, path_out=None):
+    path_out = path_out or temp_file_for(path_in)
+
+    cmd = ArtResizer.shared.im_convert_cmd + [
+        util.syspath(path_in, prefix=False),
+        '-interlace', 'none',
+        util.syspath(path_out, prefix=False),
+    ]
+
+    try:
+        util.command_output(cmd)
+        return path_out
+    except subprocess.CalledProcessError:
+        return path_in
+
+
+DEINTERLACE_FUNCS = {
+    PIL: pil_deinterlace,
+    IMAGEMAGICK: im_deinterlace,
+}
+
+
 class Shareable(type):
     """A pseudo-singleton metaclass that allows both shared and
     non-shared instances. The ``MyClass.shared`` property holds a
@@ -251,6 +290,13 @@ class ArtResizer(metaclass=Shareable):
         else:
             return path_in
 
+    def deinterlace(self, path_in, path_out=None):
+        if self.local:
+            func = DEINTERLACE_FUNCS[self.method[0]]
+            return func(path_in, path_out)
+        else:
+            return path_in
+
     def proxy_url(self, maxwidth, url, quality=0):
         """Modifies an image URL according the method, returning a new
         URL. For WEBPROXY, a URL on the proxy server is returned.

beetsplug/fetchart.py

@@ -49,6 +49,7 @@ class Candidate:
     CANDIDATE_EXACT = 1
     CANDIDATE_DOWNSCALE = 2
     CANDIDATE_DOWNSIZE = 3
+    CANDIDATE_DEINTERLACE = 4
 
     MATCH_EXACT = 0
     MATCH_FALLBACK = 1
@@ -72,12 +73,13 @@ class Candidate:
         Return `CANDIDATE_DOWNSCALE` if the file must be rescaled.
         Return `CANDIDATE_DOWNSIZE` if the file must be resized, and possibly
             also rescaled.
+        Return `CANDIDATE_DEINTERLACE` if the file must be deinterlaced.
         """
         if not self.path:
             return self.CANDIDATE_BAD
 
         if (not (plugin.enforce_ratio or plugin.minwidth or plugin.maxwidth
-                 or plugin.max_filesize)):
+                 or plugin.max_filesize or plugin.deinterlace)):
             return self.CANDIDATE_EXACT
 
         # get_size returns None if no local imaging backend is available
@@ -144,6 +146,8 @@ class Candidate:
             return self.CANDIDATE_DOWNSCALE
         elif downsize:
             return self.CANDIDATE_DOWNSIZE
+        elif plugin.deinterlace:
+            return self.CANDIDATE_DEINTERLACE
         else:
             return self.CANDIDATE_EXACT
 
@@ -163,6 +167,8 @@ class Candidate:
                 ArtResizer.shared.resize(max(self.size), self.path,
                                          quality=plugin.quality,
                                          max_filesize=plugin.max_filesize)
+        elif self.check == self.CANDIDATE_DEINTERLACE:
+            self.path = ArtResizer.shared.deinterlace(self.path)
 
 
 def _logged_get(log, *args, **kwargs):
@@ -916,6 +922,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin):
             'lastfm_key': None,
             'store_source': False,
             'high_resolution': False,
+            'deinterlace': False,
         })
         self.config['google_key'].redact = True
         self.config['fanarttv_key'].redact = True
@@ -933,6 +940,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin):
                            confuse.String(pattern=self.PAT_PERCENT)]))
         self.margin_px = None
         self.margin_percent = None
+        self.deinterlace = self.config['deinterlace'].get(bool)
         if type(self.enforce_ratio) is str:
             if self.enforce_ratio[-1] == '%':
                 self.margin_percent = float(self.enforce_ratio[:-1]) / 100

docs/changelog.rst

@@ -46,6 +46,9 @@ Other new things:
   using the target path. This gets us closer to always updating files
   atomically. Thanks to :user:`catap`.
   :bug:`4060`
+* :doc:`/plugins/fetchart`: A new option to store cover art as non-progressive
+  image. Useful for DAPs that support progressive images. Set ``deinterlace:
+  yes`` in your configuration to enable.
 
 For plugin developers:
 

docs/plugins/fetchart.rst

@@ -86,6 +86,10 @@ file. The available options are:
 - **high_resolution**: If enabled, fetchart retrieves artwork in the highest
   resolution it can find (warning: image files can sometimes reach >20MB).
   Default: ``no``.
+- **deinterlace**: If enabled, `Pillow`_ or `ImageMagick`_ backends are
+  instructed to store cover art as non-progressive JPEG. You might need this if
+  you use DAPs that don't support progressive images.
+  Default: ``no``.
 
 Note: ``maxwidth`` and ``enforce_ratio`` options require either `ImageMagick`_
 or `Pillow`_.

test/test_art_resize.py

@@ -20,12 +20,15 @@ import os
 
 from test import _common
 from test.helper import TestHelper
-from beets.util import syspath
+from beets.util import command_output, syspath
 from beets.util.artresizer import (
     pil_resize,
     im_resize,
     get_im_version,
     get_pil_version,
+    pil_deinterlace,
+    im_deinterlace,
+    ArtResizer,
 )
 
 
@@ -97,6 +100,32 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         """Test IM resize function is lowering file size."""
         self._test_img_resize(im_resize)
 
+    @unittest.skipUnless(get_pil_version(), "PIL not available")
+    def test_pil_file_deinterlace(self):
+        """Test PIL deinterlace function.
+
+        Check if pil_deinterlace function returns images
+        that are non-progressive
+        """
+        path = pil_deinterlace(self.IMG_225x225)
+        from PIL import Image
+        with Image.open(path) as img:
+            self.assertFalse('progression' in img.info)
+
+    @unittest.skipUnless(get_im_version(), "ImageMagick not available")
+    def test_im_file_deinterlace(self):
+        """Test ImageMagick deinterlace function.
+
+        Check if im_deinterlace function returns images
+        that are non-progressive.
+        """
+        path = im_deinterlace(self.IMG_225x225)
+        cmd = ArtResizer.shared.im_identify_cmd + [
+            '-format', '%[interlace]', syspath(path, prefix=False),
+        ]
+        out = command_output(cmd).stdout
+        self.assertTrue(out == b'None')
+
 
 def suite():
     """Run this suite of tests."""