Refactor Candidate class in fetchart.py to improve resizing logic (#5152)

This commit is contained in:
Serene 2024-06-25 14:30:49 +10:00 committed by GitHub
commit 240c5fca51
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 80 additions and 25 deletions

View file

@ -891,7 +891,7 @@ def command_output(
if proc.returncode:
raise subprocess.CalledProcessError(
returncode=proc.returncode,
cmd=" ".join(cmd),
cmd=" ".join(map(str, cmd)),
output=stdout + stderr,
)
return CommandOutput(stdout, stderr)

View file

@ -67,10 +67,15 @@ class Candidate:
self.match = match
self.size = size
def _validate(self, plugin):
def _validate(self, plugin, skip_check_for=None):
"""Determine whether the candidate artwork is valid based on
its dimensions (width and ratio).
`skip_check_for` is a check or list of checks to skip. This is used to
avoid redundant checks when the candidate has already been
validated for a particular operation without changing
plugin configuration.
Return `CANDIDATE_BAD` if the file is unusable.
Return `CANDIDATE_EXACT` if the file is usable as-is.
Return `CANDIDATE_DOWNSCALE` if the file must be rescaled.
@ -82,6 +87,11 @@ class Candidate:
if not self.path:
return self.CANDIDATE_BAD
if skip_check_for is None:
skip_check_for = []
if isinstance(skip_check_for, int):
skip_check_for = [skip_check_for]
if not (
plugin.enforce_ratio
or plugin.minwidth
@ -180,30 +190,51 @@ class Candidate:
plugin.cover_format,
)
if downscale:
if downscale and (self.CANDIDATE_DOWNSCALE not in skip_check_for):
return self.CANDIDATE_DOWNSCALE
elif downsize:
return self.CANDIDATE_DOWNSIZE
elif plugin.deinterlace:
return self.CANDIDATE_DEINTERLACE
elif reformat:
if reformat and (self.CANDIDATE_REFORMAT not in skip_check_for):
return self.CANDIDATE_REFORMAT
else:
return self.CANDIDATE_EXACT
if plugin.deinterlace and (
self.CANDIDATE_DEINTERLACE not in skip_check_for
):
return self.CANDIDATE_DEINTERLACE
if downsize and (self.CANDIDATE_DOWNSIZE not in skip_check_for):
return self.CANDIDATE_DOWNSIZE
return self.CANDIDATE_EXACT
def validate(self, plugin):
self.check = self._validate(plugin)
def validate(self, plugin, skip_check_for=None):
self.check = self._validate(plugin, skip_check_for)
return self.check
def resize(self, plugin):
if self.check == self.CANDIDATE_DOWNSCALE:
"""Resize the candidate artwork according to the plugin's
configuration until it is valid or no further resizing is
possible.
"""
# validate the candidate in case it hasn't been done yet
current_check = self.validate(plugin)
checks_performed = []
# we don't want to resize the image if it's valid or bad
while current_check not in [self.CANDIDATE_BAD, self.CANDIDATE_EXACT]:
self._resize(plugin, current_check)
checks_performed.append(current_check)
current_check = self.validate(
plugin, skip_check_for=checks_performed
)
def _resize(self, plugin, check=None):
"""Resize the candidate artwork according to the plugin's
configuration and the specified check.
"""
if check == self.CANDIDATE_DOWNSCALE:
self.path = ArtResizer.shared.resize(
plugin.maxwidth,
self.path,
quality=plugin.quality,
max_filesize=plugin.max_filesize,
)
elif self.check == self.CANDIDATE_DOWNSIZE:
elif check == self.CANDIDATE_DOWNSIZE:
# dimensions are correct, so maxwidth is set to maximum dimension
self.path = ArtResizer.shared.resize(
max(self.size),
@ -211,9 +242,9 @@ class Candidate:
quality=plugin.quality,
max_filesize=plugin.max_filesize,
)
elif self.check == self.CANDIDATE_DEINTERLACE:
elif check == self.CANDIDATE_DEINTERLACE:
self.path = ArtResizer.shared.deinterlace(self.path)
elif self.check == self.CANDIDATE_REFORMAT:
elif check == self.CANDIDATE_REFORMAT:
self.path = ArtResizer.shared.reformat(
self.path,
plugin.cover_format,

View file

@ -350,6 +350,8 @@ Bug fixes:
:bug:`5130`
* Fix bug where some plugin commands hang indefinitely due to a missing
`requests` timeout.
* Fix cover art resizing logic to support multiple steps of resizing
:bug:`5151`
For plugin developers:

View file

@ -862,6 +862,10 @@ class ArtForAlbumTest(UseThePlugin):
IMG_225x225_SIZE = os.stat(util.syspath(IMG_225x225)).st_size
IMG_348x348_SIZE = os.stat(util.syspath(IMG_348x348)).st_size
RESIZE_OP = "resize"
DEINTERLACE_OP = "deinterlace"
REFORMAT_OP = "reformat"
def setUp(self):
super().setUp()
@ -892,11 +896,13 @@ class ArtForAlbumTest(UseThePlugin):
else:
self.assertIsNone(candidate)
def _assertImageResized(self, image_file, should_resize): # noqa
def _assert_image_operated(self, image_file, operation, should_operate):
self.image_file = image_file
with patch.object(ArtResizer.shared, "resize") as mock_resize:
with patch.object(
ArtResizer.shared, operation, return_value=self.image_file
) as mock_operation:
self.plugin.art_for_album(self.album, [""], True)
self.assertEqual(mock_resize.called, should_resize)
self.assertEqual(mock_operation.called, should_operate)
def _require_backend(self):
"""Skip the test if the art resizer doesn't have ImageMagick or
@ -948,31 +954,47 @@ class ArtForAlbumTest(UseThePlugin):
def test_resize_if_necessary(self):
self._require_backend()
self.plugin.maxwidth = 300
self._assertImageResized(self.IMG_225x225, False)
self._assertImageResized(self.IMG_348x348, True)
self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, False)
self._assert_image_operated(self.IMG_348x348, self.RESIZE_OP, True)
def test_fileresize(self):
self._require_backend()
self.plugin.max_filesize = self.IMG_225x225_SIZE // 2
self._assertImageResized(self.IMG_225x225, True)
self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, True)
def test_fileresize_if_necessary(self):
self._require_backend()
self.plugin.max_filesize = self.IMG_225x225_SIZE
self._assertImageResized(self.IMG_225x225, False)
self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, False)
self._assertImageIsValidArt(self.IMG_225x225, True)
def test_fileresize_no_scale(self):
self._require_backend()
self.plugin.maxwidth = 300
self.plugin.max_filesize = self.IMG_225x225_SIZE // 2
self._assertImageResized(self.IMG_225x225, True)
self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, True)
def test_fileresize_and_scale(self):
self._require_backend()
self.plugin.maxwidth = 200
self.plugin.max_filesize = self.IMG_225x225_SIZE // 2
self._assertImageResized(self.IMG_225x225, True)
self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, True)
def test_deinterlace(self):
self._require_backend()
self.plugin.deinterlace = True
self._assert_image_operated(self.IMG_225x225, self.DEINTERLACE_OP, True)
self.plugin.deinterlace = False
self._assert_image_operated(
self.IMG_225x225, self.DEINTERLACE_OP, False
)
def test_deinterlace_and_resize(self):
self._require_backend()
self.plugin.maxwidth = 300
self.plugin.deinterlace = True
self._assert_image_operated(self.IMG_348x348, self.DEINTERLACE_OP, True)
self._assert_image_operated(self.IMG_348x348, self.RESIZE_OP, True)
class DeprecatedConfigTest(_common.TestCase):