From bac93e10951242c569af86d8ff8db94a9cc17608 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 11:23:35 +0100
Subject: [PATCH 01/17] artresizer: add a few comments

---
 beets/util/artresizer.py | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 0041db230..7cfa6b69e 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -439,13 +439,19 @@ class ArtResizer(metaclass=Shareable):
             return func(self, maxwidth, path_in, path_out,
                         quality=quality, max_filesize=max_filesize)
         else:
+            # Handled by `proxy_url` already.
             return path_in
 
     def deinterlace(self, path_in, path_out=None):
+        """Deinterlace an image.
+
+        Only available locally.
+        """
         if self.local:
             func = DEINTERLACE_FUNCS[self.method[0]]
             return func(self, path_in, path_out)
         else:
+            # FIXME: Should probably issue a warning?
             return path_in
 
     def proxy_url(self, maxwidth, url, quality=0):
@@ -454,6 +460,7 @@ class ArtResizer(metaclass=Shareable):
         Otherwise, the URL is returned unmodified.
         """
         if self.local:
+            # Going to be handled by `resize()`.
             return url
         else:
             return resize_url(url, maxwidth, quality)
@@ -474,7 +481,9 @@ class ArtResizer(metaclass=Shareable):
         if self.local:
             func = BACKEND_GET_SIZE[self.method[0]]
             return func(self, path_in)
-        return None
+        else:
+            # FIXME: Should probably issue a warning?
+            return None
 
     def get_format(self, path_in):
         """Returns the format of the image as a string.
@@ -484,7 +493,9 @@ class ArtResizer(metaclass=Shareable):
         if self.local:
             func = BACKEND_GET_FORMAT[self.method[0]]
             return func(self, path_in)
-        return None
+        else:
+            # FIXME: Should probably issue a warning?
+            return None
 
     def reformat(self, path_in, new_format, deinterlaced=True):
         """Converts image to desired format, updating its extension, but
@@ -493,6 +504,7 @@ class ArtResizer(metaclass=Shareable):
         Only available locally.
         """
         if not self.local:
+            # FIXME: Should probably issue a warning?
             return path_in
 
         new_format = new_format.lower()
@@ -529,7 +541,9 @@ class ArtResizer(metaclass=Shareable):
         if self.local:
             func = BACKEND_COMPARE[self.method[0]]
             return func(self, im1, im2, compare_threshold)
-        return None
+        else:
+            # FIXME: Should probably issue a warning?
+            return None
 
     @staticmethod
     def _check_method():

From 8a969e30410d0b3b47a67de080bf75c4baa21b5e Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 11:23:54 +0100
Subject: [PATCH 02/17] artresizer: backend classes part 1: stub classes,
 version checks

---
 beets/util/artresizer.py | 244 +++++++++++++++++++++++----------------
 beetsplug/thumbnails.py  |  26 +++--
 test/test_art.py         |   4 +-
 test/test_art_resize.py  |  53 ++++++---
 test/test_embedart.py    |   6 +-
 test/test_thumbnails.py  |  40 ++++---
 6 files changed, 222 insertions(+), 151 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 7cfa6b69e..4369dccd8 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -61,7 +61,98 @@ def temp_file_for(path):
         return bytestring_path(f.name)
 
 
-def pil_resize(artresizer, maxwidth, path_in, path_out=None, quality=0,
+class LocalBackendNotAvailableError(Exception):
+    pass
+
+
+_NOT_AVAILABLE = object()
+
+
+class LocalBackend:
+    @classmethod
+    def available(cls):
+        try:
+            cls.version()
+            return True
+        except LocalBackendNotAvailableError:
+            return False
+
+
+class IMBackend(LocalBackend):
+    NAME="ImageMagick"
+    ID=IMAGEMAGICK
+    _version = None
+    _legacy = None
+
+    @classmethod
+    def version(cls):
+        """Obtain and cache ImageMagick version.
+
+        Raises `LocalBackendNotAvailableError` if not available.
+        """
+        if cls._version is None:
+            for cmd_name, legacy in (('magick', False), ('convert', True)):
+                try:
+                    out = util.command_output([cmd_name, "--version"]).stdout
+                except (subprocess.CalledProcessError, OSError) as exc:
+                    log.debug('ImageMagick version check failed: {}', exc)
+                    cls._version = _NOT_AVAILABLE
+                else:
+                    if b'imagemagick' in out.lower():
+                        pattern = br".+ (\d+)\.(\d+)\.(\d+).*"
+                        match = re.search(pattern, out)
+                        if match:
+                            cls._version = (int(match.group(1)),
+                                       int(match.group(2)),
+                                       int(match.group(3)))
+                            cls._legacy = legacy
+
+        if cls._version is _NOT_AVAILABLE:
+            raise LocalBackendNotAvailableError()
+        else:
+            return cls._version
+
+    def __init__(self):
+        """Initialize a wrapper around ImageMagick for local image operations.
+
+        Stores the ImageMagick version and legacy flag. If ImageMagick is not
+        available, raise an Exception.
+        """
+        self.version()
+
+        # Use ImageMagick's magick binary when it's available.
+        # If it's not, fall back to the older, separate convert
+        # and identify commands.
+        if self._legacy:
+            self.convert_cmd = ['convert']
+            self.identify_cmd = ['identify']
+            self.compare_cmd = ['compare']
+        else:
+            self.convert_cmd = ['magick']
+            self.identify_cmd = ['magick', 'identify']
+            self.compare_cmd = ['magick', 'compare']
+
+
+class PILBackend(LocalBackend):
+    NAME="PIL"
+    ID=PIL
+
+    @classmethod
+    def version(cls):
+        try:
+            __import__('PIL', fromlist=['Image'])
+        except ImportError:
+            raise LocalBackendNotAvailableError()
+
+    def __init__(self):
+        """Initialize a wrapper around PIL for local image operations.
+
+        If PIL is not available, raise an Exception.
+        """
+        self.version()
+
+
+def pil_resize(backend, maxwidth, path_in, path_out=None, quality=0,
                max_filesize=0):
     """Resize using Python Imaging Library (PIL).  Return the output path
     of resized image.
@@ -120,7 +211,7 @@ def pil_resize(artresizer, maxwidth, path_in, path_out=None, quality=0,
         return path_in
 
 
-def im_resize(artresizer, maxwidth, path_in, path_out=None, quality=0,
+def im_resize(backend, maxwidth, path_in, path_out=None, quality=0,
               max_filesize=0):
     """Resize using ImageMagick.
 
@@ -136,7 +227,7 @@ def im_resize(artresizer, maxwidth, path_in, path_out=None, quality=0,
     # 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.im_convert_cmd + [
+    cmd = backend.convert_cmd + [
         syspath(path_in, prefix=False),
         '-resize', f'{maxwidth}x>',
         '-interlace', 'none',
@@ -168,7 +259,7 @@ BACKEND_FUNCS = {
 }
 
 
-def pil_getsize(artresizer, path_in):
+def pil_getsize(backend, path_in):
     from PIL import Image
 
     try:
@@ -180,8 +271,8 @@ def pil_getsize(artresizer, path_in):
         return None
 
 
-def im_getsize(artresizer, path_in):
-    cmd = artresizer.im_identify_cmd + \
+def im_getsize(backend, path_in):
+    cmd = backend.identify_cmd + \
         ['-format', '%w %h', syspath(path_in, prefix=False)]
 
     try:
@@ -207,7 +298,7 @@ BACKEND_GET_SIZE = {
 }
 
 
-def pil_deinterlace(artresizer, path_in, path_out=None):
+def pil_deinterlace(backend, path_in, path_out=None):
     path_out = path_out or temp_file_for(path_in)
     from PIL import Image
 
@@ -219,10 +310,10 @@ def pil_deinterlace(artresizer, path_in, path_out=None):
         return path_in
 
 
-def im_deinterlace(artresizer, path_in, path_out=None):
+def im_deinterlace(backend, path_in, path_out=None):
     path_out = path_out or temp_file_for(path_in)
 
-    cmd = artresizer.im_convert_cmd + [
+    cmd = backend.convert_cmd + [
         syspath(path_in, prefix=False),
         '-interlace', 'none',
         syspath(path_out, prefix=False),
@@ -241,8 +332,8 @@ DEINTERLACE_FUNCS = {
 }
 
 
-def im_get_format(artresizer, filepath):
-    cmd = artresizer.im_identify_cmd + [
+def im_get_format(backend, filepath):
+    cmd = backend.identify_cmd + [
         '-format', '%[magick]',
         syspath(filepath)
     ]
@@ -253,7 +344,7 @@ def im_get_format(artresizer, filepath):
         return None
 
 
-def pil_get_format(artresizer, filepath):
+def pil_get_format(backend, filepath):
     from PIL import Image, UnidentifiedImageError
 
     try:
@@ -270,8 +361,8 @@ BACKEND_GET_FORMAT = {
 }
 
 
-def im_convert_format(artresizer, source, target, deinterlaced):
-    cmd = artresizer.im_convert_cmd + [
+def im_convert_format(backend, source, target, deinterlaced):
+    cmd = backend.convert_cmd + [
         syspath(source),
         *(["-interlace", "none"] if deinterlaced else []),
         syspath(target),
@@ -288,7 +379,7 @@ def im_convert_format(artresizer, source, target, deinterlaced):
         return source
 
 
-def pil_convert_format(artresizer, source, target, deinterlaced):
+def pil_convert_format(backend, source, target, deinterlaced):
     from PIL import Image, UnidentifiedImageError
 
     try:
@@ -307,7 +398,7 @@ BACKEND_CONVERT_IMAGE_FORMAT = {
 }
 
 
-def im_compare(artresizer, im1, im2, compare_threshold):
+def im_compare(backend, im1, im2, compare_threshold):
     is_windows = platform.system() == "Windows"
 
     # Converting images to grayscale tends to minimize the weight
@@ -315,11 +406,11 @@ def im_compare(artresizer, im1, im2, compare_threshold):
     # to grayscale and then pipe them into the `compare` command.
     # On Windows, ImageMagick doesn't support the magic \\?\ prefix
     # on paths, so we pass `prefix=False` to `syspath`.
-    convert_cmd = artresizer.im_convert_cmd + [
+    convert_cmd = backend.convert_cmd + [
         syspath(im2, prefix=False), syspath(im1, prefix=False),
         '-colorspace', 'gray', 'MIFF:-'
     ]
-    compare_cmd = artresizer.im_compare_cmd + [
+    compare_cmd = backend.compare_cmd + [
         '-metric', 'PHASH', '-', 'null:',
     ]
     log.debug('comparing images with pipeline {} | {}',
@@ -373,7 +464,7 @@ def im_compare(artresizer, im1, im2, compare_threshold):
     return phash_diff <= compare_threshold
 
 
-def pil_compare(artresizer, im1, im2, compare_threshold):
+def pil_compare(backend, im1, im2, compare_threshold):
     # It is an error to call this when ArtResizer.can_compare is not True.
     raise NotImplementedError()
 
@@ -409,22 +500,11 @@ class ArtResizer(metaclass=Shareable):
     def __init__(self):
         """Create a resizer object with an inferred method.
         """
-        self.method = self._check_method()
-        log.debug("artresizer: method is {0}", self.method)
-
-        # Use ImageMagick's magick binary when it's available. If it's
-        # not, fall back to the older, separate convert and identify
-        # commands.
-        if self.method[0] == IMAGEMAGICK:
-            self.im_legacy = self.method[2]
-            if self.im_legacy:
-                self.im_convert_cmd = ['convert']
-                self.im_identify_cmd = ['identify']
-                self.im_compare_cmd = ['compare']
-            else:
-                self.im_convert_cmd = ['magick']
-                self.im_identify_cmd = ['magick', 'identify']
-                self.im_compare_cmd = ['magick', 'compare']
+        self.local_method = self._check_method()
+        if self.local_method is None:
+            log.debug(f"artresizer: method is WEBPROXY")
+        else:
+            log.debug(f"artresizer: method is {self.local_method.NAME}")
 
     def resize(
         self, maxwidth, path_in, path_out=None, quality=0, max_filesize=0
@@ -435,8 +515,8 @@ class ArtResizer(metaclass=Shareable):
         For WEBPROXY, returns `path_in` unmodified.
         """
         if self.local:
-            func = BACKEND_FUNCS[self.method[0]]
-            return func(self, maxwidth, path_in, path_out,
+            func = BACKEND_FUNCS[self.local_method]
+            return func(self.local_method, maxwidth, path_in, path_out,
                         quality=quality, max_filesize=max_filesize)
         else:
             # Handled by `proxy_url` already.
@@ -448,8 +528,8 @@ class ArtResizer(metaclass=Shareable):
         Only available locally.
         """
         if self.local:
-            func = DEINTERLACE_FUNCS[self.method[0]]
-            return func(self, path_in, path_out)
+            func = DEINTERLACE_FUNCS[self.local_method.ID]
+            return func(self.local_method, path_in, path_out)
         else:
             # FIXME: Should probably issue a warning?
             return path_in
@@ -470,7 +550,7 @@ class ArtResizer(metaclass=Shareable):
         """A boolean indicating whether the resizing method is performed
         locally (i.e., PIL or ImageMagick).
         """
-        return self.method[0] in BACKEND_FUNCS
+        return self.local_method is not None
 
     def get_size(self, path_in):
         """Return the size of an image file as an int couple (width, height)
@@ -479,11 +559,11 @@ class ArtResizer(metaclass=Shareable):
         Only available locally.
         """
         if self.local:
-            func = BACKEND_GET_SIZE[self.method[0]]
-            return func(self, path_in)
+            func = BACKEND_GET_SIZE[self.local_method.ID]
+            return func(self.local_method, path_in)
         else:
             # FIXME: Should probably issue a warning?
-            return None
+            return path_in
 
     def get_format(self, path_in):
         """Returns the format of the image as a string.
@@ -491,8 +571,8 @@ class ArtResizer(metaclass=Shareable):
         Only available locally.
         """
         if self.local:
-            func = BACKEND_GET_FORMAT[self.method[0]]
-            return func(self, path_in)
+            func = BACKEND_GET_FORMAT[self.local_method.ID]
+            return func(self.local_method, path_in)
         else:
             # FIXME: Should probably issue a warning?
             return None
@@ -503,7 +583,7 @@ class ArtResizer(metaclass=Shareable):
 
         Only available locally.
         """
-        if not self.local:
+        if self.local:
             # FIXME: Should probably issue a warning?
             return path_in
 
@@ -515,13 +595,13 @@ class ArtResizer(metaclass=Shareable):
 
         fname, ext = os.path.splitext(path_in)
         path_new = fname + b'.' + new_format.encode('utf8')
-        func = BACKEND_CONVERT_IMAGE_FORMAT[self.method[0]]
+        func = BACKEND_CONVERT_IMAGE_FORMAT[self.local_method.ID]
 
         # allows the exception to propagate, while still making sure a changed
         # file path was removed
         result_path = path_in
         try:
-            result_path = func(self, path_in, path_new, deinterlaced)
+            result_path = func(self.local_method, path_in, path_new, deinterlaced)
         finally:
             if result_path != path_in:
                 os.unlink(path_in)
@@ -531,7 +611,11 @@ class ArtResizer(metaclass=Shareable):
     def can_compare(self):
         """A boolean indicating whether image comparison is available"""
 
-        return self.method[0] == IMAGEMAGICK and self.method[1] > (6, 8, 7)
+        return (
+            self.local
+            and self.local_method.ID == IMAGEMAGICK
+            and self.local_method.version() > (6, 8, 7)
+        )
 
     def compare(self, im1, im2, compare_threshold):
         """Return a boolean indicating whether two images are similar.
@@ -539,65 +623,23 @@ class ArtResizer(metaclass=Shareable):
         Only available locally.
         """
         if self.local:
-            func = BACKEND_COMPARE[self.method[0]]
-            return func(self, im1, im2, compare_threshold)
+            func = BACKEND_COMPARE[self.local_method.ID]
+            return func(self.local_method, im1, im2, compare_threshold)
         else:
             # FIXME: Should probably issue a warning?
             return None
 
     @staticmethod
     def _check_method():
-        """Return a tuple indicating an available method and its version.
+        """Search availabe methods.
 
-        The result has at least two elements:
-        - The method, eitehr WEBPROXY, PIL, or IMAGEMAGICK.
-        - The version.
-
-        If the method is IMAGEMAGICK, there is also a third element: a
-        bool flag indicating whether to use the `magick` binary or
-        legacy single-purpose executables (`convert`, `identify`, etc.)
+        If a local backend is availabe, return an instance of the backend
+        class. Otherwise, when fallback to the web proxy is requird, return
+        None.
         """
-        version = get_im_version()
-        if version:
-            version, legacy = version
-            return IMAGEMAGICK, version, legacy
-
-        version = get_pil_version()
-        if version:
-            return PIL, version
-
-        return WEBPROXY, (0)
-
-
-def get_im_version():
-    """Get the ImageMagick version and legacy flag as a pair. Or return
-    None if ImageMagick is not available.
-    """
-    for cmd_name, legacy in ((['magick'], False), (['convert'], True)):
-        cmd = cmd_name + ['--version']
-
         try:
-            out = util.command_output(cmd).stdout
-        except (subprocess.CalledProcessError, OSError) as exc:
-            log.debug('ImageMagick version check failed: {}', exc)
-        else:
-            if b'imagemagick' in out.lower():
-                pattern = br".+ (\d+)\.(\d+)\.(\d+).*"
-                match = re.search(pattern, out)
-                if match:
-                    version = (int(match.group(1)),
-                               int(match.group(2)),
-                               int(match.group(3)))
-                    return version, legacy
+            return IMBackend()
+            return PILBackend()
+        except LocalBackendNotAvailableError:
+            return None
 
-    return None
-
-
-def get_pil_version():
-    """Get the PIL/Pillow version, or None if it is unavailable.
-    """
-    try:
-        __import__('PIL', fromlist=['Image'])
-        return (0,)
-    except ImportError:
-        return None
diff --git a/beetsplug/thumbnails.py b/beetsplug/thumbnails.py
index 6bd9cbac6..e3cf6e6a2 100644
--- a/beetsplug/thumbnails.py
+++ b/beetsplug/thumbnails.py
@@ -32,7 +32,7 @@ from xdg import BaseDirectory
 from beets.plugins import BeetsPlugin
 from beets.ui import Subcommand, decargs
 from beets import util
-from beets.util.artresizer import ArtResizer, get_im_version, get_pil_version
+from beets.util.artresizer import ArtResizer, IMBackend, PILBackend
 
 
 BASE_DIR = os.path.join(BaseDirectory.xdg_cache_home, "thumbnails")
@@ -90,14 +90,18 @@ class ThumbnailsPlugin(BeetsPlugin):
             if not os.path.exists(dir):
                 os.makedirs(dir)
 
-        if get_im_version():
+        # FIXME: Should we have our own backend instance?
+        self.backend = ArtResizer.shared.local_method
+        if isinstance(self.backend, IMBackend):
             self.write_metadata = write_metadata_im
-            tool = "IM"
-        else:
-            assert get_pil_version()  # since we're local
+        elif isinstance(self.backend, PILBackend):
             self.write_metadata = write_metadata_pil
-            tool = "PIL"
-        self._log.debug("using {0} to write metadata", tool)
+        else:
+            # since we're local
+            raise RuntimeError(
+                f"Thumbnails: Unexpected ArtResizer backend {self.backend!r}."
+            )
+        self._log.debug(f"using {self.backend.NAME} to write metadata")
 
         uri_getter = GioURI()
         if not uri_getter.available:
@@ -171,7 +175,7 @@ class ThumbnailsPlugin(BeetsPlugin):
         metadata = {"Thumb::URI": self.get_uri(album.artpath),
                     "Thumb::MTime": str(mtime)}
         try:
-            self.write_metadata(image_path, metadata)
+            self.write_metadata(self.backend, image_path, metadata)
         except Exception:
             self._log.exception("could not write metadata to {0}",
                                 util.displayable_path(image_path))
@@ -188,16 +192,16 @@ class ThumbnailsPlugin(BeetsPlugin):
         self._log.debug("Wrote file {0}", util.displayable_path(outfilename))
 
 
-def write_metadata_im(file, metadata):
+def write_metadata_im(im_backend, file, metadata):
     """Enrich the file metadata with `metadata` dict thanks to IM."""
-    command = ['convert', file] + \
+    command = im_backend.convert_cmd + [file] + \
         list(chain.from_iterable(('-set', k, v)
                                  for k, v in metadata.items())) + [file]
     util.command_output(command)
     return True
 
 
-def write_metadata_pil(file, metadata):
+def write_metadata_pil(pil_backend, file, metadata):
     """Enrich the file metadata with `metadata` dict thanks to PIL."""
     from PIL import Image, PngImagePlugin
     im = Image.open(file)
diff --git a/test/test_art.py b/test/test_art.py
index 498c4cedc..b32285e70 100644
--- a/test/test_art.py
+++ b/test/test_art.py
@@ -31,7 +31,7 @@ from beets import library
 from beets import importer
 from beets import logging
 from beets import util
-from beets.util.artresizer import ArtResizer, WEBPROXY
+from beets.util.artresizer import ArtResizer
 import confuse
 
 
@@ -787,7 +787,7 @@ class ArtForAlbumTest(UseThePlugin):
         """Skip the test if the art resizer doesn't have ImageMagick or
         PIL (so comparisons and measurements are unavailable).
         """
-        if ArtResizer.shared.method[0] == WEBPROXY:
+        if not ArtResizer.shared.local:
             self.skipTest("ArtResizer has no local imaging backend available")
 
     def test_respect_minwidth(self):
diff --git a/test/test_art_resize.py b/test/test_art_resize.py
index 4600bab77..9d3be19e7 100644
--- a/test/test_art_resize.py
+++ b/test/test_art_resize.py
@@ -22,16 +22,34 @@ from test import _common
 from test.helper import TestHelper
 from beets.util import command_output, syspath
 from beets.util.artresizer import (
+    IMBackend,
+    PILBackend,
     pil_resize,
     im_resize,
-    get_im_version,
-    get_pil_version,
     pil_deinterlace,
     im_deinterlace,
-    ArtResizer,
 )
 
 
+class DummyIMBackend(IMBackend):
+    """An `IMBackend` which pretends that ImageMagick is available, and has
+    a sufficiently recent version to support image comparison.
+    """
+    def __init__(self):
+        self.version = (7, 0, 0)
+        self.legacy = False
+        self.convert_cmd = ['magick']
+        self.identify_cmd = ['magick', 'identify']
+        self.compare_cmd = ['magick', 'compare']
+
+
+class DummyPILBackend(PILBackend):
+    """An `PILBackend` which pretends that PIL is available.
+    """
+    def __init__(self):
+        pass
+
+
 class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
     """Unittest test case for Art Resizer to a specific filesize."""
 
@@ -46,11 +64,11 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         """Called after each test, unloading all plugins."""
         self.teardown_beets()
 
-    def _test_img_resize(self, resize_func):
+    def _test_img_resize(self, backend, resize_func):
         """Test resizing based on file size, given a resize_func."""
         # Check quality setting unaffected by new parameter
         im_95_qual = resize_func(
-            ArtResizer.shared,
+            backend,
             225,
             self.IMG_225x225,
             quality=95,
@@ -61,7 +79,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
 
         # Attempt a lower filesize with same quality
         im_a = resize_func(
-            ArtResizer.shared,
+            backend,
             225,
             self.IMG_225x225,
             quality=95,
@@ -74,7 +92,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
 
         # Attempt with lower initial quality
         im_75_qual = resize_func(
-            ArtResizer.shared,
+            backend,
             225,
             self.IMG_225x225,
             quality=75,
@@ -83,7 +101,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         self.assertExists(im_75_qual)
 
         im_b = resize_func(
-            ArtResizer.shared,
+            backend,
             225,
             self.IMG_225x225,
             quality=95,
@@ -94,37 +112,38 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         self.assertLess(os.stat(syspath(im_b)).st_size,
                         os.stat(syspath(im_75_qual)).st_size)
 
-    @unittest.skipUnless(get_pil_version(), "PIL not available")
+    @unittest.skipUnless(PILBackend.available(), "PIL not available")
     def test_pil_file_resize(self):
         """Test PIL resize function is lowering file size."""
-        self._test_img_resize(pil_resize)
+        self._test_img_resize(PILBackend(), pil_resize)
 
-    @unittest.skipUnless(get_im_version(), "ImageMagick not available")
+    @unittest.skipUnless(IMBackend.available(), "ImageMagick not available")
     def test_im_file_resize(self):
         """Test IM resize function is lowering file size."""
-        self._test_img_resize(im_resize)
+        self._test_img_resize(IMBackend(), im_resize)
 
-    @unittest.skipUnless(get_pil_version(), "PIL not available")
+    @unittest.skipUnless(PILBackend.available(), "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(ArtResizer.shared, self.IMG_225x225)
+        path = pil_deinterlace(PILBackend(), 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")
+    @unittest.skipUnless(IMBackend.available(), "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(ArtResizer.shared, self.IMG_225x225)
-        cmd = ArtResizer.shared.im_identify_cmd + [
+        im = IMBackend()
+        path = im_deinterlace(im, self.IMG_225x225)
+        cmd = im.identify_cmd + [
             '-format', '%[interlace]', syspath(path, prefix=False),
         ]
         out = command_output(cmd).stdout
diff --git a/test/test_embedart.py b/test/test_embedart.py
index 0fed08f98..b3f61babe 100644
--- a/test/test_embedart.py
+++ b/test/test_embedart.py
@@ -21,6 +21,7 @@ import unittest
 
 from test import _common
 from test.helper import TestHelper
+from test.test_art_resize import DummyIMBackend
 
 from mediafile import MediaFile
 from beets import config, logging, ui
@@ -220,9 +221,8 @@ class DummyArtResizer(ArtResizer):
     """An `ArtResizer` which pretends that ImageMagick is available, and has
     a sufficiently recent version to support image comparison.
     """
-    @staticmethod
-    def _check_method():
-        return artresizer.IMAGEMAGICK, (7, 0, 0), True
+    def __init__(self):
+        self.local_method = DummyIMBackend()
 
 
 @patch('beets.util.artresizer.subprocess')
diff --git a/test/test_thumbnails.py b/test/test_thumbnails.py
index e8ab21d72..376969e93 100644
--- a/test/test_thumbnails.py
+++ b/test/test_thumbnails.py
@@ -20,6 +20,7 @@ from shutil import rmtree
 import unittest
 
 from test.helper import TestHelper
+from test.test_art_resize import DummyIMBackend, DummyPILBackend
 
 from beets.util import bytestring_path
 from beetsplug.thumbnails import (ThumbnailsPlugin, NORMAL_DIR, LARGE_DIR,
@@ -37,18 +38,21 @@ class ThumbnailsTest(unittest.TestCase, TestHelper):
     @patch('beetsplug.thumbnails.util')
     def test_write_metadata_im(self, mock_util):
         metadata = {"a": "A", "b": "B"}
-        write_metadata_im("foo", metadata)
+        im = DummyIMBackend()
+        write_metadata_im(im, "foo", metadata)
         try:
-            command = "convert foo -set a A -set b B foo".split(' ')
+            command = im.convert_cmd + "foo -set a A -set b B foo".split()
             mock_util.command_output.assert_called_once_with(command)
         except AssertionError:
-            command = "convert foo -set b B -set a A foo".split(' ')
+            command = im.convert_cmd + "foo -set b B -set a A foo".split()
             mock_util.command_output.assert_called_once_with(command)
 
     @patch('beetsplug.thumbnails.ThumbnailsPlugin._check_local_ok')
     @patch('beetsplug.thumbnails.os.stat')
     def test_add_tags(self, mock_stat, _):
         plugin = ThumbnailsPlugin()
+        # backend is not set due to _check_local_ok being mocked
+        plugin.backend = "DummyBackend"
         plugin.write_metadata = Mock()
         plugin.get_uri = Mock(side_effect={b"/path/to/cover":
                                            "COVER_URI"}.__getitem__)
@@ -59,24 +63,26 @@ class ThumbnailsTest(unittest.TestCase, TestHelper):
 
         metadata = {"Thumb::URI": "COVER_URI",
                     "Thumb::MTime": "12345"}
-        plugin.write_metadata.assert_called_once_with(b"/path/to/thumbnail",
-                                                      metadata)
+        plugin.write_metadata.assert_called_once_with(
+            plugin.backend,
+            b"/path/to/thumbnail",
+            metadata,
+        )
         mock_stat.assert_called_once_with(album.artpath)
 
     @patch('beetsplug.thumbnails.os')
     @patch('beetsplug.thumbnails.ArtResizer')
-    @patch('beetsplug.thumbnails.get_im_version')
-    @patch('beetsplug.thumbnails.get_pil_version')
     @patch('beetsplug.thumbnails.GioURI')
-    def test_check_local_ok(self, mock_giouri, mock_pil, mock_im,
-                            mock_artresizer, mock_os):
+    def test_check_local_ok(self, mock_giouri, mock_artresizer, mock_os):
         # test local resizing capability
         mock_artresizer.shared.local = False
+        mock_artresizer.shared.local_method = None
         plugin = ThumbnailsPlugin()
         self.assertFalse(plugin._check_local_ok())
 
         # test dirs creation
         mock_artresizer.shared.local = True
+        mock_artresizer.shared.local_method = DummyIMBackend()
 
         def exists(path):
             if path == NORMAL_DIR:
@@ -91,18 +97,18 @@ class ThumbnailsTest(unittest.TestCase, TestHelper):
 
         # test metadata writer function
         mock_os.path.exists = lambda _: True
-        mock_pil.return_value = False
-        mock_im.return_value = False
-        with self.assertRaises(AssertionError):
+
+        mock_artresizer.shared.local = True
+        mock_artresizer.shared.local_method = None
+        with self.assertRaises(RuntimeError):
             ThumbnailsPlugin()
 
-        mock_pil.return_value = True
+        mock_artresizer.shared.local = True
+        mock_artresizer.shared.local_method = DummyPILBackend()
         self.assertEqual(ThumbnailsPlugin().write_metadata, write_metadata_pil)
 
-        mock_im.return_value = True
-        self.assertEqual(ThumbnailsPlugin().write_metadata, write_metadata_im)
-
-        mock_pil.return_value = False
+        mock_artresizer.shared.local = True
+        mock_artresizer.shared.local_method = DummyIMBackend()
         self.assertEqual(ThumbnailsPlugin().write_metadata, write_metadata_im)
 
         self.assertTrue(ThumbnailsPlugin()._check_local_ok())

From c45d2e28a61ccdb5f78c8f0f8c50fa13c5fd8ab3 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 20:23:37 +0100
Subject: [PATCH 03/17] artresizer: move resize functions to backend classes

---
 beets/util/artresizer.py | 193 +++++++++++++++++++--------------------
 test/test_art_resize.py  |  20 ++--
 2 files changed, 99 insertions(+), 114 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 4369dccd8..801f2fdb5 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -132,6 +132,47 @@ class IMBackend(LocalBackend):
             self.identify_cmd = ['magick', 'identify']
             self.compare_cmd = ['magick', 'compare']
 
+    def resize(self, maxwidth, path_in, path_out=None, quality=0,
+                  max_filesize=0):
+        """Resize using ImageMagick.
+
+        Use the ``magick`` program or ``convert`` on older versions. Return
+        the output path of resized image.
+        """
+        path_out = path_out or temp_file_for(path_in)
+        log.debug('artresizer: ImageMagick resizing {0} to {1}',
+                  displayable_path(path_in), displayable_path(path_out))
+
+        # "-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 = self.convert_cmd + [
+            syspath(path_in, prefix=False),
+            '-resize', f'{maxwidth}x>',
+            '-interlace', 'none',
+        ]
+
+        if quality > 0:
+            cmd += ['-quality', f'{quality}']
+
+        # "-define jpeg:extent=SIZEb" sets the target filesize for imagemagick to
+        # SIZE in bytes.
+        if max_filesize > 0:
+            cmd += ['-define', f'jpeg:extent={max_filesize}b']
+
+        cmd.append(syspath(path_out, prefix=False))
+
+        try:
+            util.command_output(cmd)
+        except subprocess.CalledProcessError:
+            log.warning('artresizer: IM convert failed for {0}',
+                        displayable_path(path_in))
+            return path_in
+
+        return path_out
+
 
 class PILBackend(LocalBackend):
     NAME="PIL"
@@ -151,112 +192,63 @@ class PILBackend(LocalBackend):
         """
         self.version()
 
+    def resize(self, maxwidth, path_in, path_out=None, quality=0,
+                   max_filesize=0):
+        """Resize using Python Imaging Library (PIL).  Return the output path
+        of resized image.
+        """
+        path_out = path_out or temp_file_for(path_in)
+        from PIL import Image
 
-def pil_resize(backend, maxwidth, path_in, path_out=None, quality=0,
-               max_filesize=0):
-    """Resize using Python Imaging Library (PIL).  Return the output path
-    of resized image.
-    """
-    path_out = path_out or temp_file_for(path_in)
-    from PIL import Image
+        log.debug('artresizer: PIL resizing {0} to {1}',
+                  displayable_path(path_in), displayable_path(path_out))
 
-    log.debug('artresizer: PIL resizing {0} to {1}',
-              displayable_path(path_in), displayable_path(path_out))
+        try:
+            im = Image.open(syspath(path_in))
+            size = maxwidth, maxwidth
+            im.thumbnail(size, Image.ANTIALIAS)
 
-    try:
-        im = Image.open(syspath(path_in))
-        size = maxwidth, maxwidth
-        im.thumbnail(size, Image.ANTIALIAS)
+            if quality == 0:
+                # Use PIL's default quality.
+                quality = -1
 
-        if quality == 0:
-            # Use PIL's default quality.
-            quality = -1
+            # progressive=False only affects JPEGs and is the default,
+            # but we include it here for explicitness.
+            im.save(py3_path(path_out), quality=quality, progressive=False)
 
-        # progressive=False only affects JPEGs and is the default,
-        # but we include it here for explicitness.
-        im.save(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
+                # First, set the maximum quality to either provided, or 95
+                if quality > 0:
+                    lower_qual = quality
+                else:
+                    lower_qual = 95
+                for i in range(5):
+                    # 5 attempts is an abitrary choice
+                    filesize = os.stat(syspath(path_out)).st_size
+                    log.debug("PIL Pass {0} : Output size: {1}B", i, filesize)
+                    if filesize <= max_filesize:
+                        return path_out
+                    # The relationship between filesize & quality will be
+                    # image dependent.
+                    lower_qual -= 10
+                    # Restrict quality dropping below 10
+                    if lower_qual < 10:
+                        lower_qual = 10
+                    # Use optimize flag to improve filesize decrease
+                    im.save(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
 
-        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
-            # First, set the maximum quality to either provided, or 95
-            if quality > 0:
-                lower_qual = quality
             else:
-                lower_qual = 95
-            for i in range(5):
-                # 5 attempts is an abitrary choice
-                filesize = os.stat(syspath(path_out)).st_size
-                log.debug("PIL Pass {0} : Output size: {1}B", i, filesize)
-                if filesize <= max_filesize:
-                    return path_out
-                # The relationship between filesize & quality will be
-                # image dependent.
-                lower_qual -= 10
-                # Restrict quality dropping below 10
-                if lower_qual < 10:
-                    lower_qual = 10
-                # Use optimize flag to improve filesize decrease
-                im.save(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
-
-        else:
-            return path_out
-    except OSError:
-        log.error("PIL cannot create thumbnail for '{0}'",
-                  displayable_path(path_in))
-        return path_in
-
-
-def im_resize(backend, maxwidth, path_in, path_out=None, quality=0,
-              max_filesize=0):
-    """Resize using ImageMagick.
-
-    Use the ``magick`` program or ``convert`` on older versions. Return
-    the output path of resized image.
-    """
-    path_out = path_out or temp_file_for(path_in)
-    log.debug('artresizer: ImageMagick resizing {0} to {1}',
-              displayable_path(path_in), displayable_path(path_out))
-
-    # "-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 = backend.convert_cmd + [
-        syspath(path_in, prefix=False),
-        '-resize', f'{maxwidth}x>',
-        '-interlace', 'none',
-    ]
-
-    if quality > 0:
-        cmd += ['-quality', f'{quality}']
-
-    # "-define jpeg:extent=SIZEb" sets the target filesize for imagemagick to
-    # SIZE in bytes.
-    if max_filesize > 0:
-        cmd += ['-define', f'jpeg:extent={max_filesize}b']
-
-    cmd.append(syspath(path_out, prefix=False))
-
-    try:
-        util.command_output(cmd)
-    except subprocess.CalledProcessError:
-        log.warning('artresizer: IM convert failed for {0}',
-                    displayable_path(path_in))
-        return path_in
-
-    return path_out
-
-
-BACKEND_FUNCS = {
-    PIL: pil_resize,
-    IMAGEMAGICK: im_resize,
-}
+                return path_out
+        except OSError:
+            log.error("PIL cannot create thumbnail for '{0}'",
+                      displayable_path(path_in))
+            return path_in
 
 
 def pil_getsize(backend, path_in):
@@ -515,8 +507,7 @@ class ArtResizer(metaclass=Shareable):
         For WEBPROXY, returns `path_in` unmodified.
         """
         if self.local:
-            func = BACKEND_FUNCS[self.local_method]
-            return func(self.local_method, maxwidth, path_in, path_out,
+            return self.local_method.resize(maxwidth, path_in, path_out,
                         quality=quality, max_filesize=max_filesize)
         else:
             # Handled by `proxy_url` already.
diff --git a/test/test_art_resize.py b/test/test_art_resize.py
index 9d3be19e7..80604ba76 100644
--- a/test/test_art_resize.py
+++ b/test/test_art_resize.py
@@ -24,8 +24,6 @@ from beets.util import command_output, syspath
 from beets.util.artresizer import (
     IMBackend,
     PILBackend,
-    pil_resize,
-    im_resize,
     pil_deinterlace,
     im_deinterlace,
 )
@@ -64,11 +62,10 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         """Called after each test, unloading all plugins."""
         self.teardown_beets()
 
-    def _test_img_resize(self, backend, resize_func):
+    def _test_img_resize(self, backend):
         """Test resizing based on file size, given a resize_func."""
         # Check quality setting unaffected by new parameter
-        im_95_qual = resize_func(
-            backend,
+        im_95_qual = backend.resize(
             225,
             self.IMG_225x225,
             quality=95,
@@ -78,8 +75,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         self.assertExists(im_95_qual)
 
         # Attempt a lower filesize with same quality
-        im_a = resize_func(
-            backend,
+        im_a = backend.resize(
             225,
             self.IMG_225x225,
             quality=95,
@@ -91,8 +87,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
                         os.stat(syspath(im_95_qual)).st_size)
 
         # Attempt with lower initial quality
-        im_75_qual = resize_func(
-            backend,
+        im_75_qual = backend.resize(
             225,
             self.IMG_225x225,
             quality=75,
@@ -100,8 +95,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         )
         self.assertExists(im_75_qual)
 
-        im_b = resize_func(
-            backend,
+        im_b = backend.resize(
             225,
             self.IMG_225x225,
             quality=95,
@@ -115,12 +109,12 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
     @unittest.skipUnless(PILBackend.available(), "PIL not available")
     def test_pil_file_resize(self):
         """Test PIL resize function is lowering file size."""
-        self._test_img_resize(PILBackend(), pil_resize)
+        self._test_img_resize(PILBackend())
 
     @unittest.skipUnless(IMBackend.available(), "ImageMagick not available")
     def test_im_file_resize(self):
         """Test IM resize function is lowering file size."""
-        self._test_img_resize(IMBackend(), im_resize)
+        self._test_img_resize(IMBackend())
 
     @unittest.skipUnless(PILBackend.available(), "PIL not available")
     def test_pil_file_deinterlace(self):

From 279c081f23c5962be36243c399a5eb7c89f764e9 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 20:27:14 +0100
Subject: [PATCH 04/17] artresizer: move get_size functions to backend classes

also rename getsize -> get_size for consistency with the ArtResizer
method
---
 beets/util/artresizer.py | 70 ++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 39 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 801f2fdb5..566087568 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -173,6 +173,27 @@ class IMBackend(LocalBackend):
 
         return path_out
 
+    def get_size(self, path_in):
+        cmd = self.identify_cmd + [
+            '-format', '%w %h', syspath(path_in, prefix=False)
+        ]
+
+        try:
+            out = util.command_output(cmd).stdout
+        except subprocess.CalledProcessError as exc:
+            log.warning('ImageMagick size query failed')
+            log.debug(
+                '`convert` exited with (status {}) when '
+                'getting size with command {}:\n{}',
+                exc.returncode, cmd, exc.output.strip()
+            )
+            return None
+        try:
+            return tuple(map(int, out.split(b' ')))
+        except IndexError:
+            log.warning('Could not understand IM output: {0!r}', out)
+            return None
+
 
 class PILBackend(LocalBackend):
     NAME="PIL"
@@ -250,44 +271,16 @@ class PILBackend(LocalBackend):
                       displayable_path(path_in))
             return path_in
 
+    def get_size(self, path_in):
+        from PIL import Image
 
-def pil_getsize(backend, path_in):
-    from PIL import Image
-
-    try:
-        im = Image.open(syspath(path_in))
-        return im.size
-    except OSError as exc:
-        log.error("PIL could not read file {}: {}",
-                  displayable_path(path_in), exc)
-        return None
-
-
-def im_getsize(backend, path_in):
-    cmd = backend.identify_cmd + \
-        ['-format', '%w %h', syspath(path_in, prefix=False)]
-
-    try:
-        out = util.command_output(cmd).stdout
-    except subprocess.CalledProcessError as exc:
-        log.warning('ImageMagick size query failed')
-        log.debug(
-            '`convert` exited with (status {}) when '
-            'getting size with command {}:\n{}',
-            exc.returncode, cmd, exc.output.strip()
-        )
-        return None
-    try:
-        return tuple(map(int, out.split(b' ')))
-    except IndexError:
-        log.warning('Could not understand IM output: {0!r}', out)
-        return None
-
-
-BACKEND_GET_SIZE = {
-    PIL: pil_getsize,
-    IMAGEMAGICK: im_getsize,
-}
+        try:
+            im = Image.open(syspath(path_in))
+            return im.size
+        except OSError as exc:
+            log.error("PIL could not read file {}: {}",
+                      displayable_path(path_in), exc)
+            return None
 
 
 def pil_deinterlace(backend, path_in, path_out=None):
@@ -550,8 +543,7 @@ class ArtResizer(metaclass=Shareable):
         Only available locally.
         """
         if self.local:
-            func = BACKEND_GET_SIZE[self.local_method.ID]
-            return func(self.local_method, path_in)
+            return self.local_method.get_size(path_in)
         else:
             # FIXME: Should probably issue a warning?
             return path_in

From 1b92dea9955c447a90ed6cf817460c81119e4bf0 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 20:30:39 +0100
Subject: [PATCH 05/17] artresizer: move deinterlace functions to backend
 classes

---
 beets/util/artresizer.py | 60 +++++++++++++++++-----------------------
 test/test_art_resize.py  | 10 +++----
 2 files changed, 30 insertions(+), 40 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 566087568..d7fa115e9 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -194,6 +194,22 @@ class IMBackend(LocalBackend):
             log.warning('Could not understand IM output: {0!r}', out)
             return None
 
+    def deinterlace(self, path_in, path_out=None):
+        path_out = path_out or temp_file_for(path_in)
+
+        cmd = self.convert_cmd + [
+            syspath(path_in, prefix=False),
+            '-interlace', 'none',
+            syspath(path_out, prefix=False),
+        ]
+
+        try:
+            util.command_output(cmd)
+            return path_out
+        except subprocess.CalledProcessError:
+            # FIXME: add a warning
+            return path_in
+
 
 class PILBackend(LocalBackend):
     NAME="PIL"
@@ -282,39 +298,16 @@ class PILBackend(LocalBackend):
                       displayable_path(path_in), exc)
             return None
 
+    def deinterlace(self, path_in, path_out=None):
+        path_out = path_out or temp_file_for(path_in)
+        from PIL import Image
 
-def pil_deinterlace(backend, path_in, path_out=None):
-    path_out = path_out or temp_file_for(path_in)
-    from PIL import Image
-
-    try:
-        im = Image.open(syspath(path_in))
-        im.save(py3_path(path_out), progressive=False)
-        return path_out
-    except IOError:
-        return path_in
-
-
-def im_deinterlace(backend, path_in, path_out=None):
-    path_out = path_out or temp_file_for(path_in)
-
-    cmd = backend.convert_cmd + [
-        syspath(path_in, prefix=False),
-        '-interlace', 'none',
-        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,
-}
+        try:
+            im = Image.open(syspath(path_in))
+            im.save(py3_path(path_out), progressive=False)
+            return path_out
+        except IOError:
+            return path_in
 
 
 def im_get_format(backend, filepath):
@@ -512,8 +505,7 @@ class ArtResizer(metaclass=Shareable):
         Only available locally.
         """
         if self.local:
-            func = DEINTERLACE_FUNCS[self.local_method.ID]
-            return func(self.local_method, path_in, path_out)
+            return self.local_method.deinterlace(path_in, path_out)
         else:
             # FIXME: Should probably issue a warning?
             return path_in
diff --git a/test/test_art_resize.py b/test/test_art_resize.py
index 80604ba76..57a7121f7 100644
--- a/test/test_art_resize.py
+++ b/test/test_art_resize.py
@@ -24,8 +24,6 @@ from beets.util import command_output, syspath
 from beets.util.artresizer import (
     IMBackend,
     PILBackend,
-    pil_deinterlace,
-    im_deinterlace,
 )
 
 
@@ -120,10 +118,10 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
     def test_pil_file_deinterlace(self):
         """Test PIL deinterlace function.
 
-        Check if pil_deinterlace function returns images
+        Check if the `PILBackend.deinterlace()` function returns images
         that are non-progressive
         """
-        path = pil_deinterlace(PILBackend(), self.IMG_225x225)
+        path = PILBackend().deinterlace(self.IMG_225x225)
         from PIL import Image
         with Image.open(path) as img:
             self.assertFalse('progression' in img.info)
@@ -132,11 +130,11 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
     def test_im_file_deinterlace(self):
         """Test ImageMagick deinterlace function.
 
-        Check if im_deinterlace function returns images
+        Check if the `IMBackend.deinterlace()` function returns images
         that are non-progressive.
         """
         im = IMBackend()
-        path = im_deinterlace(im, self.IMG_225x225)
+        path = im.deinterlace(self.IMG_225x225)
         cmd = im.identify_cmd + [
             '-format', '%[interlace]', syspath(path, prefix=False),
         ]

From a781e6a9bf4870b9c93edd27619382198a890285 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 20:37:31 +0100
Subject: [PATCH 06/17] artresizer: move get_format functions to backend
 classes

---
 beets/util/artresizer.py | 49 ++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index d7fa115e9..d413c9906 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -210,6 +210,17 @@ class IMBackend(LocalBackend):
             # FIXME: add a warning
             return path_in
 
+    def get_format(self, filepath):
+        cmd = self.identify_cmd + [
+            '-format', '%[magick]',
+            syspath(filepath)
+        ]
+
+        try:
+            return util.command_output(cmd).stdout
+        except subprocess.CalledProcessError:
+            return None
+
 
 class PILBackend(LocalBackend):
     NAME="PIL"
@@ -309,34 +320,15 @@ class PILBackend(LocalBackend):
         except IOError:
             return path_in
 
+    def get_format(self, filepath):
+        from PIL import Image, UnidentifiedImageError
 
-def im_get_format(backend, filepath):
-    cmd = backend.identify_cmd + [
-        '-format', '%[magick]',
-        syspath(filepath)
-    ]
-
-    try:
-        return util.command_output(cmd).stdout
-    except subprocess.CalledProcessError:
-        return None
-
-
-def pil_get_format(backend, filepath):
-    from PIL import Image, UnidentifiedImageError
-
-    try:
-        with Image.open(syspath(filepath)) as im:
-            return im.format
-    except (ValueError, TypeError, UnidentifiedImageError, FileNotFoundError):
-        log.exception("failed to detect image format for {}", filepath)
-        return None
-
-
-BACKEND_GET_FORMAT = {
-    PIL: pil_get_format,
-    IMAGEMAGICK: im_get_format,
-}
+        try:
+            with Image.open(syspath(filepath)) as im:
+                return im.format
+        except (ValueError, TypeError, UnidentifiedImageError, FileNotFoundError):
+            log.exception("failed to detect image format for {}", filepath)
+            return None
 
 
 def im_convert_format(backend, source, target, deinterlaced):
@@ -546,8 +538,7 @@ class ArtResizer(metaclass=Shareable):
         Only available locally.
         """
         if self.local:
-            func = BACKEND_GET_FORMAT[self.local_method.ID]
-            return func(self.local_method, path_in)
+            return self.local_method.get_format(path_in)
         else:
             # FIXME: Should probably issue a warning?
             return None

From 68e762c203ed3137eba85ee438355f3014cceea0 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 20:39:40 +0100
Subject: [PATCH 07/17] artresizer: move convert_format functions to backend
 classes

---
 beets/util/artresizer.py | 66 ++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 36 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index d413c9906..671a286bb 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -221,6 +221,23 @@ class IMBackend(LocalBackend):
         except subprocess.CalledProcessError:
             return None
 
+    def convert_format(self, source, target, deinterlaced):
+        cmd = self.convert_cmd + [
+            syspath(source),
+            *(["-interlace", "none"] if deinterlaced else []),
+            syspath(target),
+        ]
+
+        try:
+            subprocess.check_call(
+                cmd,
+                stderr=subprocess.DEVNULL,
+                stdout=subprocess.DEVNULL
+            )
+            return target
+        except subprocess.CalledProcessError:
+            return source
+
 
 class PILBackend(LocalBackend):
     NAME="PIL"
@@ -331,41 +348,17 @@ class PILBackend(LocalBackend):
             return None
 
 
-def im_convert_format(backend, source, target, deinterlaced):
-    cmd = backend.convert_cmd + [
-        syspath(source),
-        *(["-interlace", "none"] if deinterlaced else []),
-        syspath(target),
-    ]
+    def convert_format(self, source, target, deinterlaced):
+        from PIL import Image, UnidentifiedImageError
 
-    try:
-        subprocess.check_call(
-            cmd,
-            stderr=subprocess.DEVNULL,
-            stdout=subprocess.DEVNULL
-        )
-        return target
-    except subprocess.CalledProcessError:
-        return source
-
-
-def pil_convert_format(backend, source, target, deinterlaced):
-    from PIL import Image, UnidentifiedImageError
-
-    try:
-        with Image.open(syspath(source)) as im:
-            im.save(py3_path(target), progressive=not deinterlaced)
-            return target
-    except (ValueError, TypeError, UnidentifiedImageError, FileNotFoundError,
-            OSError):
-        log.exception("failed to convert image {} -> {}", source, target)
-        return source
-
-
-BACKEND_CONVERT_IMAGE_FORMAT = {
-    PIL: pil_convert_format,
-    IMAGEMAGICK: im_convert_format,
-}
+        try:
+            with Image.open(syspath(source)) as im:
+                im.save(py3_path(target), progressive=not deinterlaced)
+                return target
+        except (ValueError, TypeError, UnidentifiedImageError, FileNotFoundError,
+                OSError):
+            log.exception("failed to convert image {} -> {}", source, target)
+            return source
 
 
 def im_compare(backend, im1, im2, compare_threshold):
@@ -561,13 +554,14 @@ class ArtResizer(metaclass=Shareable):
 
         fname, ext = os.path.splitext(path_in)
         path_new = fname + b'.' + new_format.encode('utf8')
-        func = BACKEND_CONVERT_IMAGE_FORMAT[self.local_method.ID]
 
         # allows the exception to propagate, while still making sure a changed
         # file path was removed
         result_path = path_in
         try:
-            result_path = func(self.local_method, path_in, path_new, deinterlaced)
+            result_path = self.local_method.convert_format(
+                path_in, path_new, deinterlaced
+            )
         finally:
             if result_path != path_in:
                 os.unlink(path_in)

From cb9a765997e920a4061504426260776431ec29de Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 20:42:35 +0100
Subject: [PATCH 08/17] artresizer: move compare functions to backend classes

---
 beets/util/artresizer.py | 147 ++++++++++++++++++---------------------
 1 file changed, 69 insertions(+), 78 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 671a286bb..ac03e335d 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -238,6 +238,71 @@ class IMBackend(LocalBackend):
         except subprocess.CalledProcessError:
             return source
 
+    def compare(self, im1, im2, compare_threshold):
+        is_windows = platform.system() == "Windows"
+
+        # Converting images to grayscale tends to minimize the weight
+        # of colors in the diff score. So we first convert both images
+        # to grayscale and then pipe them into the `compare` command.
+        # On Windows, ImageMagick doesn't support the magic \\?\ prefix
+        # on paths, so we pass `prefix=False` to `syspath`.
+        convert_cmd = self.convert_cmd + [
+            syspath(im2, prefix=False), syspath(im1, prefix=False),
+            '-colorspace', 'gray', 'MIFF:-'
+        ]
+        compare_cmd = self.compare_cmd + [
+            '-metric', 'PHASH', '-', 'null:',
+        ]
+        log.debug('comparing images with pipeline {} | {}',
+                  convert_cmd, compare_cmd)
+        convert_proc = subprocess.Popen(
+            convert_cmd,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            close_fds=not is_windows,
+        )
+        compare_proc = subprocess.Popen(
+            compare_cmd,
+            stdin=convert_proc.stdout,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            close_fds=not is_windows,
+        )
+
+        # Check the convert output. We're not interested in the
+        # standard output; that gets piped to the next stage.
+        convert_proc.stdout.close()
+        convert_stderr = convert_proc.stderr.read()
+        convert_proc.stderr.close()
+        convert_proc.wait()
+        if convert_proc.returncode:
+            log.debug(
+                'ImageMagick convert failed with status {}: {!r}',
+                convert_proc.returncode,
+                convert_stderr,
+            )
+            return None
+
+        # Check the compare output.
+        stdout, stderr = compare_proc.communicate()
+        if compare_proc.returncode:
+            if compare_proc.returncode != 1:
+                log.debug('ImageMagick compare failed: {0}, {1}',
+                          displayable_path(im2), displayable_path(im1))
+                return None
+            out_str = stderr
+        else:
+            out_str = stdout
+
+        try:
+            phash_diff = float(out_str)
+        except ValueError:
+            log.debug('IM output is not a number: {0!r}', out_str)
+            return None
+
+        log.debug('ImageMagick compare score: {0}', phash_diff)
+        return phash_diff <= compare_threshold
+
 
 class PILBackend(LocalBackend):
     NAME="PIL"
@@ -360,82 +425,9 @@ class PILBackend(LocalBackend):
             log.exception("failed to convert image {} -> {}", source, target)
             return source
 
-
-def im_compare(backend, im1, im2, compare_threshold):
-    is_windows = platform.system() == "Windows"
-
-    # Converting images to grayscale tends to minimize the weight
-    # of colors in the diff score. So we first convert both images
-    # to grayscale and then pipe them into the `compare` command.
-    # On Windows, ImageMagick doesn't support the magic \\?\ prefix
-    # on paths, so we pass `prefix=False` to `syspath`.
-    convert_cmd = backend.convert_cmd + [
-        syspath(im2, prefix=False), syspath(im1, prefix=False),
-        '-colorspace', 'gray', 'MIFF:-'
-    ]
-    compare_cmd = backend.compare_cmd + [
-        '-metric', 'PHASH', '-', 'null:',
-    ]
-    log.debug('comparing images with pipeline {} | {}',
-              convert_cmd, compare_cmd)
-    convert_proc = subprocess.Popen(
-        convert_cmd,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        close_fds=not is_windows,
-    )
-    compare_proc = subprocess.Popen(
-        compare_cmd,
-        stdin=convert_proc.stdout,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        close_fds=not is_windows,
-    )
-
-    # Check the convert output. We're not interested in the
-    # standard output; that gets piped to the next stage.
-    convert_proc.stdout.close()
-    convert_stderr = convert_proc.stderr.read()
-    convert_proc.stderr.close()
-    convert_proc.wait()
-    if convert_proc.returncode:
-        log.debug(
-            'ImageMagick convert failed with status {}: {!r}',
-            convert_proc.returncode,
-            convert_stderr,
-        )
-        return None
-
-    # Check the compare output.
-    stdout, stderr = compare_proc.communicate()
-    if compare_proc.returncode:
-        if compare_proc.returncode != 1:
-            log.debug('ImageMagick compare failed: {0}, {1}',
-                      displayable_path(im2), displayable_path(im1))
-            return None
-        out_str = stderr
-    else:
-        out_str = stdout
-
-    try:
-        phash_diff = float(out_str)
-    except ValueError:
-        log.debug('IM output is not a number: {0!r}', out_str)
-        return None
-
-    log.debug('ImageMagick compare score: {0}', phash_diff)
-    return phash_diff <= compare_threshold
-
-
-def pil_compare(backend, im1, im2, compare_threshold):
-    # It is an error to call this when ArtResizer.can_compare is not True.
-    raise NotImplementedError()
-
-
-BACKEND_COMPARE = {
-    PIL: pil_compare,
-    IMAGEMAGICK: im_compare,
-}
+    def compare(self, im1, im2, compare_threshold):
+        # It is an error to call this when ArtResizer.can_compare is not True.
+        raise NotImplementedError()
 
 
 class Shareable(type):
@@ -583,8 +575,7 @@ class ArtResizer(metaclass=Shareable):
         Only available locally.
         """
         if self.local:
-            func = BACKEND_COMPARE[self.local_method.ID]
-            return func(self.local_method, im1, im2, compare_threshold)
+            return self.local_method.compare(im1, im2, compare_threshold)
         else:
             # FIXME: Should probably issue a warning?
             return None

From b097b1950616504cb5ba7cd60e9a2f7878b4a5a9 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 20:44:41 +0100
Subject: [PATCH 09/17] artresizer: move can_compare property to backend
 classes

---
 beets/util/artresizer.py | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index ac03e335d..5dad38dfc 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -238,6 +238,10 @@ class IMBackend(LocalBackend):
         except subprocess.CalledProcessError:
             return source
 
+    @property
+    def can_compare(self):
+        return self.version() > (6, 8, 7)
+
     def compare(self, im1, im2, compare_threshold):
         is_windows = platform.system() == "Windows"
 
@@ -425,6 +429,10 @@ class PILBackend(LocalBackend):
             log.exception("failed to convert image {} -> {}", source, target)
             return source
 
+    @property
+    def can_compare(self):
+        return False
+
     def compare(self, im1, im2, compare_threshold):
         # It is an error to call this when ArtResizer.can_compare is not True.
         raise NotImplementedError()
@@ -563,11 +571,10 @@ class ArtResizer(metaclass=Shareable):
     def can_compare(self):
         """A boolean indicating whether image comparison is available"""
 
-        return (
-            self.local
-            and self.local_method.ID == IMAGEMAGICK
-            and self.local_method.version() > (6, 8, 7)
-        )
+        if self.local:
+            return self.local_method.can_compare
+        else:
+            return False
 
     def compare(self, im1, im2, compare_threshold):
         """Return a boolean indicating whether two images are similar.

From bc364bc7d42edccc4bd3628e0d1d266c1513eecb Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 20:51:06 +0100
Subject: [PATCH 10/17] artresizer: merge _check_method into
 ArtResizer.__init__

more concise since _check_method became much shorter due to introduction
of backend classes
---
 beets/util/artresizer.py | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 5dad38dfc..77226612e 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -456,6 +456,11 @@ class Shareable(type):
         return cls._instance
 
 
+BACKEND_CLASSES = [
+    IMBackend,
+    PILBackend,
+]
+
 class ArtResizer(metaclass=Shareable):
     """A singleton class that performs image resizes.
     """
@@ -463,11 +468,18 @@ class ArtResizer(metaclass=Shareable):
     def __init__(self):
         """Create a resizer object with an inferred method.
         """
-        self.local_method = self._check_method()
-        if self.local_method is None:
-            log.debug(f"artresizer: method is WEBPROXY")
+        # Check if a local backend is availabe, and store an instance of the
+        # backend class. Otherwise, fallback to the web proxy.
+        for backend_cls in BACKEND_CLASSES:
+            try:
+                self.local_method = backend_cls()
+                log.debug(f"artresizer: method is {self.local_method.NAME}")
+                break
+            except LocalBackendNotAvailableError:
+                continue
         else:
-            log.debug(f"artresizer: method is {self.local_method.NAME}")
+            log.debug("artresizer: method is WEBPROXY")
+            self.local_method = None
 
     def resize(
         self, maxwidth, path_in, path_out=None, quality=0, max_filesize=0
@@ -587,17 +599,3 @@ class ArtResizer(metaclass=Shareable):
             # FIXME: Should probably issue a warning?
             return None
 
-    @staticmethod
-    def _check_method():
-        """Search availabe methods.
-
-        If a local backend is availabe, return an instance of the backend
-        class. Otherwise, when fallback to the web proxy is requird, return
-        None.
-        """
-        try:
-            return IMBackend()
-            return PILBackend()
-        except LocalBackendNotAvailableError:
-            return None
-

From f751893be23e37df26eb7b25e697105f4867dff1 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 20:59:25 +0100
Subject: [PATCH 11/17] artresizer: add FIXME notes where we should probably
 add some warnings

There's a lot of places where ArtResizer fails or falls back to
completely silently. We should at least log this, or maybe even raise
exceptions in some cases so that the caller can take a decision on how
to handle the failure.
---
 beets/util/artresizer.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 77226612e..ea8b6add8 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -207,7 +207,7 @@ class IMBackend(LocalBackend):
             util.command_output(cmd)
             return path_out
         except subprocess.CalledProcessError:
-            # FIXME: add a warning
+            # FIXME: Should probably issue a warning?
             return path_in
 
     def get_format(self, filepath):
@@ -219,6 +219,7 @@ class IMBackend(LocalBackend):
         try:
             return util.command_output(cmd).stdout
         except subprocess.CalledProcessError:
+            # FIXME: Should probably issue a warning?
             return None
 
     def convert_format(self, source, target, deinterlaced):
@@ -236,6 +237,7 @@ class IMBackend(LocalBackend):
             )
             return target
         except subprocess.CalledProcessError:
+            # FIXME: Should probably issue a warning?
             return source
 
     @property
@@ -404,6 +406,7 @@ class PILBackend(LocalBackend):
             im.save(py3_path(path_out), progressive=False)
             return path_out
         except IOError:
+            # FIXME: Should probably issue a warning?
             return path_in
 
     def get_format(self, filepath):
@@ -574,6 +577,9 @@ class ArtResizer(metaclass=Shareable):
             result_path = self.local_method.convert_format(
                 path_in, path_new, deinterlaced
             )
+        except Exception:
+            # FIXME: Should probably issue a warning?
+            pass
         finally:
             if result_path != path_in:
                 os.unlink(path_in)

From 5d07b5390c55fc052e64bea4d92dfeb5fb13b20c Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 21:02:38 +0100
Subject: [PATCH 12/17] artresizer: remove unused enum

---
 beets/util/artresizer.py | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index ea8b6add8..a08a922d5 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -27,11 +27,6 @@ from beets import logging
 from beets import util
 from beets.util import bytestring_path, displayable_path, py3_path, syspath
 
-# Resizing methods
-PIL = 1
-IMAGEMAGICK = 2
-WEBPROXY = 3
-
 PROXY_URL = 'https://images.weserv.nl/'
 
 log = logging.getLogger('beets')
@@ -80,7 +75,6 @@ class LocalBackend:
 
 class IMBackend(LocalBackend):
     NAME="ImageMagick"
-    ID=IMAGEMAGICK
     _version = None
     _legacy = None
 
@@ -312,7 +306,6 @@ class IMBackend(LocalBackend):
 
 class PILBackend(LocalBackend):
     NAME="PIL"
-    ID=PIL
 
     @classmethod
     def version(cls):

From e44a08eeb60f23e342d4ae0ac1c51234d8b8ea45 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 21:09:23 +0100
Subject: [PATCH 13/17] artresizer: style fixes

---
 beets/util/artresizer.py | 40 +++++++++++++++++++++-------------------
 test/test_art_resize.py  | 17 +++++++++--------
 test/test_embedart.py    |  2 +-
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index a08a922d5..a5e5f99d1 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -74,7 +74,7 @@ class LocalBackend:
 
 
 class IMBackend(LocalBackend):
-    NAME="ImageMagick"
+    NAME = "ImageMagick"
     _version = None
     _legacy = None
 
@@ -97,8 +97,8 @@ class IMBackend(LocalBackend):
                         match = re.search(pattern, out)
                         if match:
                             cls._version = (int(match.group(1)),
-                                       int(match.group(2)),
-                                       int(match.group(3)))
+                                            int(match.group(2)),
+                                            int(match.group(3)))
                             cls._legacy = legacy
 
         if cls._version is _NOT_AVAILABLE:
@@ -127,7 +127,7 @@ class IMBackend(LocalBackend):
             self.compare_cmd = ['magick', 'compare']
 
     def resize(self, maxwidth, path_in, path_out=None, quality=0,
-                  max_filesize=0):
+               max_filesize=0):
         """Resize using ImageMagick.
 
         Use the ``magick`` program or ``convert`` on older versions. Return
@@ -140,8 +140,8 @@ class IMBackend(LocalBackend):
         # "-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.
+        # ImageMagick already seems to default to no interlace, but we include
+        # it here for the sake of explicitness.
         cmd = self.convert_cmd + [
             syspath(path_in, prefix=False),
             '-resize', f'{maxwidth}x>',
@@ -151,8 +151,8 @@ class IMBackend(LocalBackend):
         if quality > 0:
             cmd += ['-quality', f'{quality}']
 
-        # "-define jpeg:extent=SIZEb" sets the target filesize for imagemagick to
-        # SIZE in bytes.
+        # "-define jpeg:extent=SIZEb" sets the target filesize for imagemagick
+        # to SIZE in bytes.
         if max_filesize > 0:
             cmd += ['-define', f'jpeg:extent={max_filesize}b']
 
@@ -305,7 +305,7 @@ class IMBackend(LocalBackend):
 
 
 class PILBackend(LocalBackend):
-    NAME="PIL"
+    NAME = "PIL"
 
     @classmethod
     def version(cls):
@@ -322,7 +322,7 @@ class PILBackend(LocalBackend):
         self.version()
 
     def resize(self, maxwidth, path_in, path_out=None, quality=0,
-                   max_filesize=0):
+               max_filesize=0):
         """Resize using Python Imaging Library (PIL).  Return the output path
         of resized image.
         """
@@ -346,8 +346,8 @@ class PILBackend(LocalBackend):
             im.save(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
+                # If maximum filesize is set, we attempt to lower the quality
+                # of jpeg conversion by a proportional amount, up to 3 attempts
                 # First, set the maximum quality to either provided, or 95
                 if quality > 0:
                     lower_qual = quality
@@ -408,11 +408,11 @@ class PILBackend(LocalBackend):
         try:
             with Image.open(syspath(filepath)) as im:
                 return im.format
-        except (ValueError, TypeError, UnidentifiedImageError, FileNotFoundError):
+        except (ValueError, TypeError, UnidentifiedImageError,
+                FileNotFoundError):
             log.exception("failed to detect image format for {}", filepath)
             return None
 
-
     def convert_format(self, source, target, deinterlaced):
         from PIL import Image, UnidentifiedImageError
 
@@ -420,8 +420,8 @@ class PILBackend(LocalBackend):
             with Image.open(syspath(source)) as im:
                 im.save(py3_path(target), progressive=not deinterlaced)
                 return target
-        except (ValueError, TypeError, UnidentifiedImageError, FileNotFoundError,
-                OSError):
+        except (ValueError, TypeError, UnidentifiedImageError,
+                FileNotFoundError, OSError):
             log.exception("failed to convert image {} -> {}", source, target)
             return source
 
@@ -457,6 +457,7 @@ BACKEND_CLASSES = [
     PILBackend,
 ]
 
+
 class ArtResizer(metaclass=Shareable):
     """A singleton class that performs image resizes.
     """
@@ -486,8 +487,10 @@ class ArtResizer(metaclass=Shareable):
         For WEBPROXY, returns `path_in` unmodified.
         """
         if self.local:
-            return self.local_method.resize(maxwidth, path_in, path_out,
-                        quality=quality, max_filesize=max_filesize)
+            return self.local_method.resize(
+                maxwidth, path_in, path_out,
+                quality=quality, max_filesize=max_filesize
+            )
         else:
             # Handled by `proxy_url` already.
             return path_in
@@ -597,4 +600,3 @@ class ArtResizer(metaclass=Shareable):
         else:
             # FIXME: Should probably issue a warning?
             return None
-
diff --git a/test/test_art_resize.py b/test/test_art_resize.py
index 57a7121f7..b6707ce41 100644
--- a/test/test_art_resize.py
+++ b/test/test_art_resize.py
@@ -21,17 +21,17 @@ import os
 from test import _common
 from test.helper import TestHelper
 from beets.util import command_output, syspath
-from beets.util.artresizer import (
-    IMBackend,
-    PILBackend,
-)
+from beets.util.artresizer import IMBackend, PILBackend
 
 
 class DummyIMBackend(IMBackend):
-    """An `IMBackend` which pretends that ImageMagick is available, and has
-    a sufficiently recent version to support image comparison.
+    """An `IMBackend` which pretends that ImageMagick is available.
+
+    The version is sufficiently recent to support image comparison.
     """
+
     def __init__(self):
+        """Init a dummy backend class for mocked ImageMagick tests."""
         self.version = (7, 0, 0)
         self.legacy = False
         self.convert_cmd = ['magick']
@@ -40,9 +40,10 @@ class DummyIMBackend(IMBackend):
 
 
 class DummyPILBackend(PILBackend):
-    """An `PILBackend` which pretends that PIL is available.
-    """
+    """An `PILBackend` which pretends that PIL is available."""
+
     def __init__(self):
+        """Init a dummy backend class for mocked PIL tests."""
         pass
 
 
diff --git a/test/test_embedart.py b/test/test_embedart.py
index b3f61babe..f41180ec1 100644
--- a/test/test_embedart.py
+++ b/test/test_embedart.py
@@ -25,7 +25,7 @@ from test.test_art_resize import DummyIMBackend
 
 from mediafile import MediaFile
 from beets import config, logging, ui
-from beets.util import artresizer, syspath, displayable_path
+from beets.util import syspath, displayable_path
 from beets.util.artresizer import ArtResizer
 from beets import art
 

From b76a3fcaa45a1adbeff499a424895c4793809213 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Mar 2022 23:03:08 +0100
Subject: [PATCH 14/17] artresizer: move ImageMagick/PIL code from thumbnails
 plugin to ArtResizer

Makes the dispatch to the chosen backend simpler in the thumbnails
plugin. Given that ArtResizer is not only about resizing art anymore,
these methods fit there quite nicely.
---
 beets/util/artresizer.py | 55 ++++++++++++++++++++++++++++++++++++++++
 beetsplug/thumbnails.py  | 40 +++++------------------------
 test/test_art_resize.py  | 14 ++++++++++
 test/test_thumbnails.py  | 37 +++++----------------------
 4 files changed, 82 insertions(+), 64 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index a5e5f99d1..5e619b5ca 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -16,6 +16,7 @@
 public resizing proxy if neither is available.
 """
 
+from itertools import chain
 import subprocess
 import os
 import os.path
@@ -303,6 +304,18 @@ class IMBackend(LocalBackend):
         log.debug('ImageMagick compare score: {0}', phash_diff)
         return phash_diff <= compare_threshold
 
+    @property
+    def can_write_metadata(self):
+        return True
+
+    def write_metadata(self, file, metadata):
+        assignments = list(chain.from_iterable(
+            ('-set', k, v) for k, v in metadata.items()
+        ))
+        command = self.convert_cmd + [file, *assignments, file]
+
+        util.command_output(command)
+
 
 class PILBackend(LocalBackend):
     NAME = "PIL"
@@ -433,6 +446,21 @@ class PILBackend(LocalBackend):
         # It is an error to call this when ArtResizer.can_compare is not True.
         raise NotImplementedError()
 
+    @property
+    def can_write_metadata(self):
+        return True
+
+    def write_metadata(self, file, metadata):
+        from PIL import Image, PngImagePlugin
+
+        # FIXME: Detect and handle other file types (currently, the only user
+        # is the thumbnails plugin, which generates PNG images).
+        im = Image.open(file)
+        meta = PngImagePlugin.PngInfo()
+        for k, v in metadata.items():
+            meta.add_text(k, v, 0)
+        im.save(file, "PNG", pnginfo=meta)
+
 
 class Shareable(type):
     """A pseudo-singleton metaclass that allows both shared and
@@ -478,6 +506,13 @@ class ArtResizer(metaclass=Shareable):
             log.debug("artresizer: method is WEBPROXY")
             self.local_method = None
 
+    @property
+    def method(self):
+        if self.local:
+            return self.local_method.NAME
+        else:
+            return "WEBPROXY"
+
     def resize(
         self, maxwidth, path_in, path_out=None, quality=0, max_filesize=0
     ):
@@ -600,3 +635,23 @@ class ArtResizer(metaclass=Shareable):
         else:
             # FIXME: Should probably issue a warning?
             return None
+
+    @property
+    def can_write_metadata(self):
+        """A boolean indicating whether writing image metadata is supported."""
+
+        if self.local:
+            return self.local_method.can_write_metadata
+        else:
+            return False
+
+    def write_metadata(self, file, metadata):
+        """Write key-value metadata to the image file.
+
+        Only available locally. Currently, expects the image to be a PNG file.
+        """
+        if self.local:
+            self.local_method.write_metadata(file, metadata)
+        else:
+            # FIXME: Should probably issue a warning?
+            pass
diff --git a/beetsplug/thumbnails.py b/beetsplug/thumbnails.py
index e3cf6e6a2..b81957593 100644
--- a/beetsplug/thumbnails.py
+++ b/beetsplug/thumbnails.py
@@ -22,7 +22,6 @@ Spec: standards.freedesktop.org/thumbnail-spec/latest/index.html
 from hashlib import md5
 import os
 import shutil
-from itertools import chain
 from pathlib import PurePosixPath
 import ctypes
 import ctypes.util
@@ -32,7 +31,7 @@ from xdg import BaseDirectory
 from beets.plugins import BeetsPlugin
 from beets.ui import Subcommand, decargs
 from beets import util
-from beets.util.artresizer import ArtResizer, IMBackend, PILBackend
+from beets.util.artresizer import ArtResizer
 
 
 BASE_DIR = os.path.join(BaseDirectory.xdg_cache_home, "thumbnails")
@@ -49,7 +48,6 @@ class ThumbnailsPlugin(BeetsPlugin):
             'dolphin': False,
         })
 
-        self.write_metadata = None
         if self.config['auto'] and self._check_local_ok():
             self.register_listener('art_set', self.process_album)
 
@@ -90,18 +88,12 @@ class ThumbnailsPlugin(BeetsPlugin):
             if not os.path.exists(dir):
                 os.makedirs(dir)
 
-        # FIXME: Should we have our own backend instance?
-        self.backend = ArtResizer.shared.local_method
-        if isinstance(self.backend, IMBackend):
-            self.write_metadata = write_metadata_im
-        elif isinstance(self.backend, PILBackend):
-            self.write_metadata = write_metadata_pil
-        else:
-            # since we're local
+        if not ArtResizer.shared.can_write_metadata:
             raise RuntimeError(
-                f"Thumbnails: Unexpected ArtResizer backend {self.backend!r}."
+                f"Thumbnails: ArtResizer backend {ArtResizer.shared.method}"
+                f" unexpectedly cannot write image metadata."
             )
-        self._log.debug(f"using {self.backend.NAME} to write metadata")
+        self._log.debug(f"using {ArtResizer.shared.method} to write metadata")
 
         uri_getter = GioURI()
         if not uri_getter.available:
@@ -175,7 +167,7 @@ class ThumbnailsPlugin(BeetsPlugin):
         metadata = {"Thumb::URI": self.get_uri(album.artpath),
                     "Thumb::MTime": str(mtime)}
         try:
-            self.write_metadata(self.backend, image_path, metadata)
+            ArtResizer.shared.write_metadata(image_path, metadata)
         except Exception:
             self._log.exception("could not write metadata to {0}",
                                 util.displayable_path(image_path))
@@ -192,26 +184,6 @@ class ThumbnailsPlugin(BeetsPlugin):
         self._log.debug("Wrote file {0}", util.displayable_path(outfilename))
 
 
-def write_metadata_im(im_backend, file, metadata):
-    """Enrich the file metadata with `metadata` dict thanks to IM."""
-    command = im_backend.convert_cmd + [file] + \
-        list(chain.from_iterable(('-set', k, v)
-                                 for k, v in metadata.items())) + [file]
-    util.command_output(command)
-    return True
-
-
-def write_metadata_pil(pil_backend, file, metadata):
-    """Enrich the file metadata with `metadata` dict thanks to PIL."""
-    from PIL import Image, PngImagePlugin
-    im = Image.open(file)
-    meta = PngImagePlugin.PngInfo()
-    for k, v in metadata.items():
-        meta.add_text(k, v, 0)
-    im.save(file, "PNG", pnginfo=meta)
-    return True
-
-
 class URIGetter:
     available = False
     name = "Abstract base"
diff --git a/test/test_art_resize.py b/test/test_art_resize.py
index b6707ce41..9660d96a2 100644
--- a/test/test_art_resize.py
+++ b/test/test_art_resize.py
@@ -16,6 +16,7 @@
 
 
 import unittest
+from unittest.mock import patch
 import os
 
 from test import _common
@@ -142,6 +143,19 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         out = command_output(cmd).stdout
         self.assertTrue(out == b'None')
 
+    @patch('beets.util.artresizer.util')
+    def test_write_metadata_im(self, mock_util):
+        """Test writing image metadata."""
+        metadata = {"a": "A", "b": "B"}
+        im = DummyIMBackend()
+        im.write_metadata("foo", metadata)
+        try:
+            command = im.convert_cmd + "foo -set a A -set b B foo".split()
+            mock_util.command_output.assert_called_once_with(command)
+        except AssertionError:
+            command = im.convert_cmd + "foo -set b B -set a A foo".split()
+            mock_util.command_output.assert_called_once_with(command)
+
 
 def suite():
     """Run this suite of tests."""
diff --git a/test/test_thumbnails.py b/test/test_thumbnails.py
index 376969e93..891411535 100644
--- a/test/test_thumbnails.py
+++ b/test/test_thumbnails.py
@@ -20,11 +20,9 @@ from shutil import rmtree
 import unittest
 
 from test.helper import TestHelper
-from test.test_art_resize import DummyIMBackend, DummyPILBackend
 
 from beets.util import bytestring_path
 from beetsplug.thumbnails import (ThumbnailsPlugin, NORMAL_DIR, LARGE_DIR,
-                                  write_metadata_im, write_metadata_pil,
                                   PathlibURI, GioURI)
 
 
@@ -35,25 +33,11 @@ class ThumbnailsTest(unittest.TestCase, TestHelper):
     def tearDown(self):
         self.teardown_beets()
 
-    @patch('beetsplug.thumbnails.util')
-    def test_write_metadata_im(self, mock_util):
-        metadata = {"a": "A", "b": "B"}
-        im = DummyIMBackend()
-        write_metadata_im(im, "foo", metadata)
-        try:
-            command = im.convert_cmd + "foo -set a A -set b B foo".split()
-            mock_util.command_output.assert_called_once_with(command)
-        except AssertionError:
-            command = im.convert_cmd + "foo -set b B -set a A foo".split()
-            mock_util.command_output.assert_called_once_with(command)
-
+    @patch('beetsplug.thumbnails.ArtResizer')
     @patch('beetsplug.thumbnails.ThumbnailsPlugin._check_local_ok')
     @patch('beetsplug.thumbnails.os.stat')
-    def test_add_tags(self, mock_stat, _):
+    def test_add_tags(self, mock_stat, _, mock_artresizer):
         plugin = ThumbnailsPlugin()
-        # backend is not set due to _check_local_ok being mocked
-        plugin.backend = "DummyBackend"
-        plugin.write_metadata = Mock()
         plugin.get_uri = Mock(side_effect={b"/path/to/cover":
                                            "COVER_URI"}.__getitem__)
         album = Mock(artpath=b"/path/to/cover")
@@ -63,8 +47,7 @@ class ThumbnailsTest(unittest.TestCase, TestHelper):
 
         metadata = {"Thumb::URI": "COVER_URI",
                     "Thumb::MTime": "12345"}
-        plugin.write_metadata.assert_called_once_with(
-            plugin.backend,
+        mock_artresizer.shared.write_metadata.assert_called_once_with(
             b"/path/to/thumbnail",
             metadata,
         )
@@ -76,13 +59,13 @@ class ThumbnailsTest(unittest.TestCase, TestHelper):
     def test_check_local_ok(self, mock_giouri, mock_artresizer, mock_os):
         # test local resizing capability
         mock_artresizer.shared.local = False
-        mock_artresizer.shared.local_method = None
+        mock_artresizer.shared.can_write_metadata = False
         plugin = ThumbnailsPlugin()
         self.assertFalse(plugin._check_local_ok())
 
         # test dirs creation
         mock_artresizer.shared.local = True
-        mock_artresizer.shared.local_method = DummyIMBackend()
+        mock_artresizer.shared.can_write_metadata = True
 
         def exists(path):
             if path == NORMAL_DIR:
@@ -99,18 +82,12 @@ class ThumbnailsTest(unittest.TestCase, TestHelper):
         mock_os.path.exists = lambda _: True
 
         mock_artresizer.shared.local = True
-        mock_artresizer.shared.local_method = None
+        mock_artresizer.shared.can_write_metadata = False
         with self.assertRaises(RuntimeError):
             ThumbnailsPlugin()
 
         mock_artresizer.shared.local = True
-        mock_artresizer.shared.local_method = DummyPILBackend()
-        self.assertEqual(ThumbnailsPlugin().write_metadata, write_metadata_pil)
-
-        mock_artresizer.shared.local = True
-        mock_artresizer.shared.local_method = DummyIMBackend()
-        self.assertEqual(ThumbnailsPlugin().write_metadata, write_metadata_im)
-
+        mock_artresizer.shared.can_write_metadata = True
         self.assertTrue(ThumbnailsPlugin()._check_local_ok())
 
         # test URI getter function

From 40b3dd7152d4303394c9b65d9e8718a0ff55d703 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Tue, 22 Mar 2022 21:22:08 +0100
Subject: [PATCH 15/17] artresizer: add a comment to clarify the meaning of
 class variables

---
 beets/util/artresizer.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 5e619b5ca..904559830 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -76,6 +76,10 @@ class LocalBackend:
 
 class IMBackend(LocalBackend):
     NAME = "ImageMagick"
+
+    # These fields are used as a cache for `version()`. `_legacy` indicates
+    # whether the modern `magick` binary is available or whether to fall back
+    # to the old-style `convert`, `identify`, etc. commands.
     _version = None
     _legacy = None
 

From 09101de397f4fcfad5e47aaef68539524b8a269f Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Tue, 22 Mar 2022 21:26:53 +0100
Subject: [PATCH 16/17] artresizer: restore incorrect change to exception
 handling from f751893b

---
 beets/util/artresizer.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 904559830..8a08bf6de 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -612,9 +612,6 @@ class ArtResizer(metaclass=Shareable):
             result_path = self.local_method.convert_format(
                 path_in, path_new, deinterlaced
             )
-        except Exception:
-            # FIXME: Should probably issue a warning?
-            pass
         finally:
             if result_path != path_in:
                 os.unlink(path_in)

From af71cebeeb4c67c5471d60648ad33be81e970be5 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Tue, 22 Mar 2022 22:08:04 +0100
Subject: [PATCH 17/17] artresizer: fixup 8a969e30

this crept in from a previous version of this PR which didn't make it
---
 beets/util/artresizer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 8a08bf6de..225280a94 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -592,7 +592,7 @@ class ArtResizer(metaclass=Shareable):
 
         Only available locally.
         """
-        if self.local:
+        if not self.local:
             # FIXME: Should probably issue a warning?
             return path_in