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