From 497b91606241c70e2e7b9f462f7ab07ee8aeb67c Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 17:35:18 +0100
Subject: [PATCH 01/12] artresizer: import *path helpers directly

shortens lines a bit, and should pose no problem for understanding the
code given the prevalence of these functions in our code
---
 beets/util/artresizer.py | 51 ++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 8683e2287..a5ccaa4e9 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -24,6 +24,7 @@ from tempfile import NamedTemporaryFile
 from urllib.parse import urlencode
 from beets import logging
 from beets import util
+from beets.util import bytestring_path, displayable_path, py3_path, syspath
 
 # Resizing methods
 PIL = 1
@@ -55,8 +56,8 @@ def temp_file_for(path):
     specified path.
     """
     ext = os.path.splitext(path)[1]
-    with NamedTemporaryFile(suffix=util.py3_path(ext), delete=False) as f:
-        return util.bytestring_path(f.name)
+    with NamedTemporaryFile(suffix=py3_path(ext), delete=False) as f:
+        return bytestring_path(f.name)
 
 
 def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
@@ -67,10 +68,10 @@ def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
     from PIL import Image
 
     log.debug('artresizer: PIL resizing {0} to {1}',
-              util.displayable_path(path_in), util.displayable_path(path_out))
+              displayable_path(path_in), displayable_path(path_out))
 
     try:
-        im = Image.open(util.syspath(path_in))
+        im = Image.open(syspath(path_in))
         size = maxwidth, maxwidth
         im.thumbnail(size, Image.ANTIALIAS)
 
@@ -80,7 +81,7 @@ def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
 
         # progressive=False only affects JPEGs and is the default,
         # but we include it here for explicitness.
-        im.save(util.py3_path(path_out), quality=quality, progressive=False)
+        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
@@ -92,7 +93,7 @@ def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
                 lower_qual = 95
             for i in range(5):
                 # 5 attempts is an abitrary choice
-                filesize = os.stat(util.syspath(path_out)).st_size
+                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
@@ -103,7 +104,7 @@ def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
                 if lower_qual < 10:
                     lower_qual = 10
                 # Use optimize flag to improve filesize decrease
-                im.save(util.py3_path(path_out), quality=lower_qual,
+                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)
@@ -113,7 +114,7 @@ def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
             return path_out
     except OSError:
         log.error("PIL cannot create thumbnail for '{0}'",
-                  util.displayable_path(path_in))
+                  displayable_path(path_in))
         return path_in
 
 
@@ -125,7 +126,7 @@ def im_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
     """
     path_out = path_out or temp_file_for(path_in)
     log.debug('artresizer: ImageMagick resizing {0} to {1}',
-              util.displayable_path(path_in), util.displayable_path(path_out))
+              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
@@ -133,7 +134,7 @@ def im_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
     # ImageMagick already seems to default to no interlace, but we include it
     # here for the sake of explicitness.
     cmd = ArtResizer.shared.im_convert_cmd + [
-        util.syspath(path_in, prefix=False),
+        syspath(path_in, prefix=False),
         '-resize', f'{maxwidth}x>',
         '-interlace', 'none',
     ]
@@ -146,13 +147,13 @@ def im_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
     if max_filesize > 0:
         cmd += ['-define', f'jpeg:extent={max_filesize}b']
 
-    cmd.append(util.syspath(path_out, prefix=False))
+    cmd.append(syspath(path_out, prefix=False))
 
     try:
         util.command_output(cmd)
     except subprocess.CalledProcessError:
         log.warning('artresizer: IM convert failed for {0}',
-                    util.displayable_path(path_in))
+                    displayable_path(path_in))
         return path_in
 
     return path_out
@@ -168,16 +169,16 @@ def pil_getsize(path_in):
     from PIL import Image
 
     try:
-        im = Image.open(util.syspath(path_in))
+        im = Image.open(syspath(path_in))
         return im.size
     except OSError as exc:
         log.error("PIL could not read file {}: {}",
-                  util.displayable_path(path_in), exc)
+                  displayable_path(path_in), exc)
 
 
 def im_getsize(path_in):
     cmd = ArtResizer.shared.im_identify_cmd + \
-        ['-format', '%w %h', util.syspath(path_in, prefix=False)]
+        ['-format', '%w %h', syspath(path_in, prefix=False)]
 
     try:
         out = util.command_output(cmd).stdout
@@ -206,8 +207,8 @@ def pil_deinterlace(path_in, path_out=None):
     from PIL import Image
 
     try:
-        im = Image.open(util.syspath(path_in))
-        im.save(util.py3_path(path_out), progressive=False)
+        im = Image.open(syspath(path_in))
+        im.save(py3_path(path_out), progressive=False)
         return path_out
     except IOError:
         return path_in
@@ -217,9 +218,9 @@ def im_deinterlace(path_in, path_out=None):
     path_out = path_out or temp_file_for(path_in)
 
     cmd = ArtResizer.shared.im_convert_cmd + [
-        util.syspath(path_in, prefix=False),
+        syspath(path_in, prefix=False),
         '-interlace', 'none',
-        util.syspath(path_out, prefix=False),
+        syspath(path_out, prefix=False),
     ]
 
     try:
@@ -238,7 +239,7 @@ DEINTERLACE_FUNCS = {
 def im_get_format(filepath):
     cmd = ArtResizer.shared.im_identify_cmd + [
         '-format', '%[magick]',
-        util.syspath(filepath)
+        syspath(filepath)
     ]
 
     try:
@@ -251,7 +252,7 @@ def pil_get_format(filepath):
     from PIL import Image, UnidentifiedImageError
 
     try:
-        with Image.open(util.syspath(filepath)) as im:
+        with Image.open(syspath(filepath)) as im:
             return im.format
     except (ValueError, TypeError, UnidentifiedImageError, FileNotFoundError):
         log.exception("failed to detect image format for {}", filepath)
@@ -266,9 +267,9 @@ BACKEND_GET_FORMAT = {
 
 def im_convert_format(source, target, deinterlaced):
     cmd = ArtResizer.shared.im_convert_cmd + [
-        util.syspath(source),
+        syspath(source),
         *(["-interlace", "none"] if deinterlaced else []),
-        util.syspath(target),
+        syspath(target),
     ]
 
     try:
@@ -286,8 +287,8 @@ def pil_convert_format(source, target, deinterlaced):
     from PIL import Image, UnidentifiedImageError
 
     try:
-        with Image.open(util.syspath(source)) as im:
-            im.save(util.py3_path(target), progressive=not deinterlaced)
+        with Image.open(syspath(source)) as im:
+            im.save(py3_path(target), progressive=not deinterlaced)
             return target
     except (ValueError, TypeError, UnidentifiedImageError, FileNotFoundError,
             OSError):

From 890662f93d80bfbb9a226f7c1a967dee4f4b2a5b Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 17:41:47 +0100
Subject: [PATCH 02/12] artresizer: don't manually cache can_compare

it's only computed once on startup anyway (see the embedart plugin,
which is the only user)
---
 beets/util/artresizer.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index a5ccaa4e9..b366df330 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -329,7 +329,6 @@ class ArtResizer(metaclass=Shareable):
         """
         self.method = self._check_method()
         log.debug("artresizer: method is {0}", self.method)
-        self.can_compare = self._can_compare()
 
         # Use ImageMagick's magick binary when it's available. If it's
         # not, fall back to the older, separate convert and identify
@@ -430,7 +429,8 @@ class ArtResizer(metaclass=Shareable):
                 os.unlink(path_in)
         return result_path
 
-    def _can_compare(self):
+    @property
+    def can_compare(self):
         """A boolean indicating whether image comparison is available"""
 
         return self.method[0] == IMAGEMAGICK and self.method[1] > (6, 8, 7)

From a17a5f2fa6e7fddd9df51219176207f539435163 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 17:54:38 +0100
Subject: [PATCH 03/12] art: move the backend-specific code to util.artresizer

all the other backend-specific code is already there.
---
 beets/art.py             | 68 ++------------------------------
 beets/util/artresizer.py | 83 ++++++++++++++++++++++++++++++++++++++++
 test/test_embedart.py    |  2 +-
 3 files changed, 87 insertions(+), 66 deletions(-)

diff --git a/beets/art.py b/beets/art.py
index 13d5dfbd4..1dff8b39a 100644
--- a/beets/art.py
+++ b/beets/art.py
@@ -17,8 +17,6 @@ music and items' embedded album art.
 """
 
 
-import subprocess
-import platform
 from tempfile import NamedTemporaryFile
 import os
 
@@ -121,70 +119,10 @@ def check_art_similarity(log, item, imagepath, compare_threshold):
     with NamedTemporaryFile(delete=True) as f:
         art = extract(log, f.name, item)
 
-        if art:
-            is_windows = platform.system() == "Windows"
+        if not art:
+            return True
 
-            # 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 = ['convert', syspath(imagepath, prefix=False),
-                           syspath(art, prefix=False),
-                           '-colorspace', 'gray', 'MIFF:-']
-            compare_cmd = ['compare', '-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
-
-            # 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(imagepath),
-                              displayable_path(art))
-                    return
-                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
-
-            log.debug('ImageMagick compare score: {0}', phash_diff)
-            return phash_diff <= compare_threshold
-
-    return True
+        return ArtResizer.shared.compare(art, imagepath, compare_threshold)
 
 
 def extract(log, outpath, item):
diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index b366df330..552f21140 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -19,6 +19,7 @@ public resizing proxy if neither is available.
 import subprocess
 import os
 import os.path
+import platform
 import re
 from tempfile import NamedTemporaryFile
 from urllib.parse import urlencode
@@ -301,6 +302,79 @@ BACKEND_CONVERT_IMAGE_FORMAT = {
     IMAGEMAGICK: im_convert_format,
 }
 
+def im_compare(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 = ['convert', syspath(im2, prefix=False),
+                   syspath(im1, prefix=False),
+                   '-colorspace', 'gray', 'MIFF:-']
+    compare_cmd = ['compare', '-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
+
+    # 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
+        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
+
+    log.debug('ImageMagick compare score: {0}', phash_diff)
+    return phash_diff <= compare_threshold
+
+
+def pil_compare(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,
+}
+
 
 class Shareable(type):
     """A pseudo-singleton metaclass that allows both shared and
@@ -435,6 +509,15 @@ class ArtResizer(metaclass=Shareable):
 
         return self.method[0] == IMAGEMAGICK and self.method[1] > (6, 8, 7)
 
+    def compare(self, im1, im2, compare_threshold):
+        """Return a boolean indicating whether two images are similar.
+
+        Only available locally.
+        """
+        if self.local:
+            func = BACKEND_COMPARE[self.method[0]]
+            return func(im1, im2, compare_threshold)
+
     @staticmethod
     def _check_method():
         """Return a tuple indicating an available method and its version.
diff --git a/test/test_embedart.py b/test/test_embedart.py
index 6b6d61614..b02989175 100644
--- a/test/test_embedart.py
+++ b/test/test_embedart.py
@@ -216,7 +216,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper):
         self.assertEqual(mediafile.images[0].data, self.image_data)
 
 
-@patch('beets.art.subprocess')
+@patch('beets.util.artresizer.subprocess')
 @patch('beets.art.extract')
 class ArtSimilarityTest(unittest.TestCase):
     def setUp(self):

From 8ca19de6bccd4567ec5e27755bf57a3cd2c3e425 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 17:57:51 +0100
Subject: [PATCH 04/12] artresizer: switch ImageMagick compare commmand
 depending on IM version

similar to other commands

Fixes #4272
---
 beets/util/artresizer.py | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 552f21140..f076b3ab5 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -310,10 +310,13 @@ def im_compare(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 = ['convert', syspath(im2, prefix=False),
-                   syspath(im1, prefix=False),
-                   '-colorspace', 'gray', 'MIFF:-']
-    compare_cmd = ['compare', '-metric', 'PHASH', '-', 'null:']
+    convert_cmd = ArtResizer.shared.im_convert_cmd + [
+        syspath(im2, prefix=False), syspath(im1, prefix=False),
+        '-colorspace', 'gray', 'MIFF:-'
+    ]
+    compare_cmd = ArtResizer.shared.im_compare_cmd + [
+        '-metric', 'PHASH', '-', 'null:',
+    ]
     log.debug('comparing images with pipeline {} | {}',
               convert_cmd, compare_cmd)
     convert_proc = subprocess.Popen(
@@ -412,9 +415,11 @@ class ArtResizer(metaclass=Shareable):
             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']
 
     def resize(
         self, maxwidth, path_in, path_out=None, quality=0, max_filesize=0

From 0125b1cd428f79552ee8c9ab358793bb5440f8ca Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 18:05:24 +0100
Subject: [PATCH 05/12] artresizer: in backends, always use the appropriate
 ArtResizer instance

This didn't cause any issues since we only use the shared instance
anyway, but logically it doesn't make a lot of sense for the backends
always using ArtResizer.shared (which they should be oblivious of).
---
 beets/util/artresizer.py | 52 +++++++++++++++++++++-------------------
 test/test_art_resize.py  |  8 +++++--
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index f076b3ab5..0260c6f82 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -61,7 +61,8 @@ def temp_file_for(path):
         return bytestring_path(f.name)
 
 
-def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
+def pil_resize(artresizer, maxwidth, path_in, path_out=None, quality=0,
+        max_filesize=0):
     """Resize using Python Imaging Library (PIL).  Return the output path
     of resized image.
     """
@@ -119,7 +120,8 @@ def pil_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
         return path_in
 
 
-def im_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=0):
+def im_resize(artresizer, maxwidth, path_in, path_out=None, quality=0,
+        max_filesize=0):
     """Resize using ImageMagick.
 
     Use the ``magick`` program or ``convert`` on older versions. Return
@@ -134,7 +136,7 @@ def im_resize(maxwidth, path_in, path_out=None, quality=0, max_filesize=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.shared.im_convert_cmd + [
+    cmd = artresizer.im_convert_cmd + [
         syspath(path_in, prefix=False),
         '-resize', f'{maxwidth}x>',
         '-interlace', 'none',
@@ -166,7 +168,7 @@ BACKEND_FUNCS = {
 }
 
 
-def pil_getsize(path_in):
+def pil_getsize(artresizer, path_in):
     from PIL import Image
 
     try:
@@ -177,8 +179,8 @@ def pil_getsize(path_in):
                   displayable_path(path_in), exc)
 
 
-def im_getsize(path_in):
-    cmd = ArtResizer.shared.im_identify_cmd + \
+def im_getsize(artresizer, path_in):
+    cmd = artresizer.im_identify_cmd + \
         ['-format', '%w %h', syspath(path_in, prefix=False)]
 
     try:
@@ -203,7 +205,7 @@ BACKEND_GET_SIZE = {
 }
 
 
-def pil_deinterlace(path_in, path_out=None):
+def pil_deinterlace(artresizer, path_in, path_out=None):
     path_out = path_out or temp_file_for(path_in)
     from PIL import Image
 
@@ -215,10 +217,10 @@ def pil_deinterlace(path_in, path_out=None):
         return path_in
 
 
-def im_deinterlace(path_in, path_out=None):
+def im_deinterlace(artresizer, path_in, path_out=None):
     path_out = path_out or temp_file_for(path_in)
 
-    cmd = ArtResizer.shared.im_convert_cmd + [
+    cmd = artresizer.im_convert_cmd + [
         syspath(path_in, prefix=False),
         '-interlace', 'none',
         syspath(path_out, prefix=False),
@@ -237,8 +239,8 @@ DEINTERLACE_FUNCS = {
 }
 
 
-def im_get_format(filepath):
-    cmd = ArtResizer.shared.im_identify_cmd + [
+def im_get_format(artresizer, filepath):
+    cmd = artresizer.im_identify_cmd + [
         '-format', '%[magick]',
         syspath(filepath)
     ]
@@ -249,7 +251,7 @@ def im_get_format(filepath):
         return None
 
 
-def pil_get_format(filepath):
+def pil_get_format(artresizer, filepath):
     from PIL import Image, UnidentifiedImageError
 
     try:
@@ -266,8 +268,8 @@ BACKEND_GET_FORMAT = {
 }
 
 
-def im_convert_format(source, target, deinterlaced):
-    cmd = ArtResizer.shared.im_convert_cmd + [
+def im_convert_format(artresizer, source, target, deinterlaced):
+    cmd = artresizer.im_convert_cmd + [
         syspath(source),
         *(["-interlace", "none"] if deinterlaced else []),
         syspath(target),
@@ -284,7 +286,7 @@ def im_convert_format(source, target, deinterlaced):
         return source
 
 
-def pil_convert_format(source, target, deinterlaced):
+def pil_convert_format(artresizer, source, target, deinterlaced):
     from PIL import Image, UnidentifiedImageError
 
     try:
@@ -302,7 +304,7 @@ BACKEND_CONVERT_IMAGE_FORMAT = {
     IMAGEMAGICK: im_convert_format,
 }
 
-def im_compare(im1, im2, compare_threshold):
+def im_compare(artresizer, im1, im2, compare_threshold):
     is_windows = platform.system() == "Windows"
 
     # Converting images to grayscale tends to minimize the weight
@@ -310,11 +312,11 @@ def im_compare(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.shared.im_convert_cmd + [
+    convert_cmd = artresizer.im_convert_cmd + [
         syspath(im2, prefix=False), syspath(im1, prefix=False),
         '-colorspace', 'gray', 'MIFF:-'
     ]
-    compare_cmd = ArtResizer.shared.im_compare_cmd + [
+    compare_cmd = artresizer.im_compare_cmd + [
         '-metric', 'PHASH', '-', 'null:',
     ]
     log.debug('comparing images with pipeline {} | {}',
@@ -368,7 +370,7 @@ def im_compare(im1, im2, compare_threshold):
     return phash_diff <= compare_threshold
 
 
-def pil_compare(im1, im2, compare_threshold):
+def pil_compare(artresizer, im1, im2, compare_threshold):
     # It is an error to call this when ArtResizer.can_compare is not True.
     raise NotImplementedError()
 
@@ -431,7 +433,7 @@ class ArtResizer(metaclass=Shareable):
         """
         if self.local:
             func = BACKEND_FUNCS[self.method[0]]
-            return func(maxwidth, path_in, path_out,
+            return func(self, maxwidth, path_in, path_out,
                         quality=quality, max_filesize=max_filesize)
         else:
             return path_in
@@ -439,7 +441,7 @@ class ArtResizer(metaclass=Shareable):
     def deinterlace(self, path_in, path_out=None):
         if self.local:
             func = DEINTERLACE_FUNCS[self.method[0]]
-            return func(path_in, path_out)
+            return func(self, path_in, path_out)
         else:
             return path_in
 
@@ -468,7 +470,7 @@ class ArtResizer(metaclass=Shareable):
         """
         if self.local:
             func = BACKEND_GET_SIZE[self.method[0]]
-            return func(path_in)
+            return func(self, path_in)
 
     def get_format(self, path_in):
         """Returns the format of the image as a string.
@@ -477,7 +479,7 @@ class ArtResizer(metaclass=Shareable):
         """
         if self.local:
             func = BACKEND_GET_FORMAT[self.method[0]]
-            return func(path_in)
+            return func(self, path_in)
 
     def reformat(self, path_in, new_format, deinterlaced=True):
         """Converts image to desired format, updating its extension, but
@@ -502,7 +504,7 @@ class ArtResizer(metaclass=Shareable):
         # file path was removed
         result_path = path_in
         try:
-            result_path = func(path_in, path_new, deinterlaced)
+            result_path = func(self, path_in, path_new, deinterlaced)
         finally:
             if result_path != path_in:
                 os.unlink(path_in)
@@ -521,7 +523,7 @@ class ArtResizer(metaclass=Shareable):
         """
         if self.local:
             func = BACKEND_COMPARE[self.method[0]]
-            return func(im1, im2, compare_threshold)
+            return func(self, im1, im2, compare_threshold)
 
     @staticmethod
     def _check_method():
diff --git a/test/test_art_resize.py b/test/test_art_resize.py
index 73847e0a6..4600bab77 100644
--- a/test/test_art_resize.py
+++ b/test/test_art_resize.py
@@ -50,6 +50,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         """Test resizing based on file size, given a resize_func."""
         # Check quality setting unaffected by new parameter
         im_95_qual = resize_func(
+            ArtResizer.shared,
             225,
             self.IMG_225x225,
             quality=95,
@@ -60,6 +61,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
 
         # Attempt a lower filesize with same quality
         im_a = resize_func(
+            ArtResizer.shared,
             225,
             self.IMG_225x225,
             quality=95,
@@ -72,6 +74,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
 
         # Attempt with lower initial quality
         im_75_qual = resize_func(
+            ArtResizer.shared,
             225,
             self.IMG_225x225,
             quality=75,
@@ -80,6 +83,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         self.assertExists(im_75_qual)
 
         im_b = resize_func(
+            ArtResizer.shared,
             225,
             self.IMG_225x225,
             quality=95,
@@ -107,7 +111,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         Check if pil_deinterlace function returns images
         that are non-progressive
         """
-        path = pil_deinterlace(self.IMG_225x225)
+        path = pil_deinterlace(ArtResizer.shared, self.IMG_225x225)
         from PIL import Image
         with Image.open(path) as img:
             self.assertFalse('progression' in img.info)
@@ -119,7 +123,7 @@ class ArtResizerFileSizeTest(_common.TestCase, TestHelper):
         Check if im_deinterlace function returns images
         that are non-progressive.
         """
-        path = im_deinterlace(self.IMG_225x225)
+        path = im_deinterlace(ArtResizer.shared, self.IMG_225x225)
         cmd = ArtResizer.shared.im_identify_cmd + [
             '-format', '%[interlace]', syspath(path, prefix=False),
         ]

From fa967f3efc7bccd9d6d6b6991d8aab5e35e45730 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 22:59:25 +0100
Subject: [PATCH 06/12] artresizer: add a comment

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

diff --git a/beets/art.py b/beets/art.py
index 1dff8b39a..eb283d600 100644
--- a/beets/art.py
+++ b/beets/art.py
@@ -115,6 +115,10 @@ def resize_image(log, imagepath, maxwidth, quality):
 
 def check_art_similarity(log, item, imagepath, compare_threshold):
     """A boolean indicating if an image is similar to embedded item art.
+
+    If no embedded art exists, always return `True`.
+
+    This must only be called if `ArtResizer.shared.can_compare` is `True`.
     """
     with NamedTemporaryFile(delete=True) as f:
         art = extract(log, f.name, item)

From 2fa37aa22b46946719479fe8f0f7d80a76d4f0d1 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 22:59:52 +0100
Subject: [PATCH 07/12] artresizer: return None explicitly

`None` is used both as a marker when errors occured, and when a function
is not implemented by the backend. That is already confusing enough
without there being implicit `None` return values when falling of the
tail of a method
---
 beets/util/artresizer.py | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index 0260c6f82..d1a544bd8 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -177,6 +177,7 @@ def pil_getsize(artresizer, path_in):
     except OSError as exc:
         log.error("PIL could not read file {}: {}",
                   displayable_path(path_in), exc)
+        return None
 
 
 def im_getsize(artresizer, path_in):
@@ -192,11 +193,12 @@ def im_getsize(artresizer, path_in):
             'getting size with command {}:\n{}',
             exc.returncode, cmd, exc.output.strip()
         )
-        return
+        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 = {
@@ -347,7 +349,7 @@ def im_compare(artresizer, im1, im2, compare_threshold):
             convert_proc.returncode,
             convert_stderr,
         )
-        return
+        return None
 
     # Check the compare output.
     stdout, stderr = compare_proc.communicate()
@@ -355,7 +357,7 @@ def im_compare(artresizer, im1, im2, compare_threshold):
         if compare_proc.returncode != 1:
             log.debug('ImageMagick compare failed: {0}, {1}',
                       displayable_path(im2), displayable_path(im1))
-            return
+            return None
         out_str = stderr
     else:
         out_str = stdout
@@ -364,7 +366,7 @@ def im_compare(artresizer, im1, im2, compare_threshold):
         phash_diff = float(out_str)
     except ValueError:
         log.debug('IM output is not a number: {0!r}', out_str)
-        return
+        return None
 
     log.debug('ImageMagick compare score: {0}', phash_diff)
     return phash_diff <= compare_threshold
@@ -471,6 +473,7 @@ class ArtResizer(metaclass=Shareable):
         if self.local:
             func = BACKEND_GET_SIZE[self.method[0]]
             return func(self, path_in)
+        return None
 
     def get_format(self, path_in):
         """Returns the format of the image as a string.
@@ -480,6 +483,7 @@ class ArtResizer(metaclass=Shareable):
         if self.local:
             func = BACKEND_GET_FORMAT[self.method[0]]
             return func(self, path_in)
+        return None
 
     def reformat(self, path_in, new_format, deinterlaced=True):
         """Converts image to desired format, updating its extension, but
@@ -524,6 +528,7 @@ class ArtResizer(metaclass=Shareable):
         if self.local:
             func = BACKEND_COMPARE[self.method[0]]
             return func(self, im1, im2, compare_threshold)
+        return None
 
     @staticmethod
     def _check_method():

From 03ce35f4d3e01862fddb6f527c9d3a58ac2f528b Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 23:10:22 +0100
Subject: [PATCH 08/12] art: whitespace/comment improvements

---
 beets/art.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/beets/art.py b/beets/art.py
index eb283d600..619c472a5 100644
--- a/beets/art.py
+++ b/beets/art.py
@@ -51,14 +51,17 @@ def embed_item(log, item, imagepath, maxwidth=None, itempath=None,
                quality=0):
     """Embed an image into the item's media file.
     """
-    # Conditions and filters.
+    # Conditions.
     if compare_threshold:
         if not check_art_similarity(log, item, imagepath, compare_threshold):
             log.info('Image not similar; skipping.')
             return
+
     if ifempty and get_art(log, item):
         log.info('media file already contained art')
         return
+
+    # Filters.
     if maxwidth and not as_album:
         imagepath = resize_image(log, imagepath, maxwidth, quality)
 

From 8235af9770d876bf2152493806f1ae744af72f39 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 23:16:05 +0100
Subject: [PATCH 09/12] art: log errors more explicitly, add some comments

error handling was previously implicit in the
`if not check_art_similarity(...)`
check, which is pretty unintuitive
---
 beets/art.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/beets/art.py b/beets/art.py
index 619c472a5..e7f18ee1d 100644
--- a/beets/art.py
+++ b/beets/art.py
@@ -53,7 +53,12 @@ def embed_item(log, item, imagepath, maxwidth=None, itempath=None,
     """
     # Conditions.
     if compare_threshold:
-        if not check_art_similarity(log, item, imagepath, compare_threshold):
+        is_similar = check_art_similarity(
+            log, item, imagepath, compare_threshold)
+        if is_similar is None:
+            log.warning('Error while checking art similarity; skipping.')
+            return
+        elif not is_similar:
             log.info('Image not similar; skipping.')
             return
 
@@ -119,7 +124,8 @@ def resize_image(log, imagepath, maxwidth, quality):
 def check_art_similarity(log, item, imagepath, compare_threshold):
     """A boolean indicating if an image is similar to embedded item art.
 
-    If no embedded art exists, always return `True`.
+    If no embedded art exists, always return `True`. If the comparison fails
+    for some reason, the return value is `None`.
 
     This must only be called if `ArtResizer.shared.can_compare` is `True`.
     """

From f558c091b47c5eca256704c0e07c48d974d338e3 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 23:26:25 +0100
Subject: [PATCH 10/12] update changelog

---
 docs/changelog.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index a28a992b9..bbe617638 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -46,6 +46,9 @@ Bug fixes:
 * :doc:`plugins/replaygain`: Correctly handle the ``overwrite`` config option,
   which forces recomputing ReplayGain values on import even for tracks
   that already have the tags.
+* :doc:`plugins/embedart`: Fix a crash when using recent versions of
+  ImageMagick and the ``compare_threshold`` option.
+  :bug:`4272`
 
 For packagers:
 

From 959e24e463c764da1cd8d05535d50222209573b5 Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Sat, 12 Feb 2022 23:34:35 +0100
Subject: [PATCH 11/12] artresizer: whitespace fixes

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

diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py
index d1a544bd8..0041db230 100644
--- a/beets/util/artresizer.py
+++ b/beets/util/artresizer.py
@@ -62,7 +62,7 @@ def temp_file_for(path):
 
 
 def pil_resize(artresizer, 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.
     """
@@ -121,7 +121,7 @@ def pil_resize(artresizer, maxwidth, path_in, path_out=None, quality=0,
 
 
 def im_resize(artresizer, 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
@@ -306,6 +306,7 @@ BACKEND_CONVERT_IMAGE_FORMAT = {
     IMAGEMAGICK: im_convert_format,
 }
 
+
 def im_compare(artresizer, im1, im2, compare_threshold):
     is_windows = platform.system() == "Windows"
 

From 1815d083925846ff1e7b5223d2b00f1bd9f7efbb Mon Sep 17 00:00:00 2001
From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Date: Mon, 14 Feb 2022 22:52:00 +0100
Subject: [PATCH 12/12] artresizer: fix image comparison test

Since the ImageMagick based comparison is now abstracted via ArtResizer,
it becomes a little more involved to mock it for testing.
---
 beets/art.py          | 13 +++++++++++--
 test/test_embedart.py | 21 ++++++++++++++++++---
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/beets/art.py b/beets/art.py
index e7f18ee1d..6e0a5f82b 100644
--- a/beets/art.py
+++ b/beets/art.py
@@ -121,7 +121,13 @@ def resize_image(log, imagepath, maxwidth, quality):
     return imagepath
 
 
-def check_art_similarity(log, item, imagepath, compare_threshold):
+def check_art_similarity(
+    log,
+    item,
+    imagepath,
+    compare_threshold,
+    artresizer=None,
+):
     """A boolean indicating if an image is similar to embedded item art.
 
     If no embedded art exists, always return `True`. If the comparison fails
@@ -135,7 +141,10 @@ def check_art_similarity(log, item, imagepath, compare_threshold):
         if not art:
             return True
 
-        return ArtResizer.shared.compare(art, imagepath, compare_threshold)
+        if artresizer is None:
+            artresizer = ArtResizer.shared
+
+        return artresizer.compare(art, imagepath, compare_threshold)
 
 
 def extract(log, outpath, item):
diff --git a/test/test_embedart.py b/test/test_embedart.py
index b02989175..0fed08f98 100644
--- a/test/test_embedart.py
+++ b/test/test_embedart.py
@@ -24,7 +24,7 @@ from test.helper import TestHelper
 
 from mediafile import MediaFile
 from beets import config, logging, ui
-from beets.util import syspath, displayable_path
+from beets.util import artresizer, syspath, displayable_path
 from beets.util.artresizer import ArtResizer
 from beets import art
 
@@ -216,16 +216,31 @@ class EmbedartCliTest(_common.TestCase, TestHelper):
         self.assertEqual(mediafile.images[0].data, self.image_data)
 
 
+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
+
+
 @patch('beets.util.artresizer.subprocess')
 @patch('beets.art.extract')
 class ArtSimilarityTest(unittest.TestCase):
     def setUp(self):
         self.item = _common.item()
         self.log = logging.getLogger('beets.embedart')
+        self.artresizer = DummyArtResizer()
 
     def _similarity(self, threshold):
-        return art.check_art_similarity(self.log, self.item, b'path',
-                                        threshold)
+        return art.check_art_similarity(
+            self.log,
+            self.item,
+            b'path',
+            threshold,
+            artresizer=self.artresizer,
+        )
 
     def _popen(self, status=0, stdout="", stderr=""):
         """Create a mock `Popen` object."""