diff --git a/beetsplug/_utils/art.py b/beetsplug/_utils/art.py index a2053f00c2..2789ca9541 100644 --- a/beetsplug/_utils/art.py +++ b/beetsplug/_utils/art.py @@ -160,7 +160,6 @@ def extract(log, outpath, item): art = get_art(log, item) outpath = bytestring_path(outpath) if not art: - log.info("No album art present in {}, skipping.", item) return None # Add an extension to the filename. @@ -170,9 +169,6 @@ def extract(log, outpath, item): return None outpath += bytestring_path(f".{ext}") - log.info( - "Extracting album art from: {} to: {}", item, displayable_path(outpath) - ) with open(syspath(outpath), "wb") as f: f.write(art) return outpath @@ -182,7 +178,13 @@ def extract_first(log, outpath, items): for item in items: real_path = extract(log, outpath, item) if real_path: + log.info( + "Extracting album art from: {} to: {}", + item, + displayable_path(real_path), + ) return real_path + log.info("No album art present in {}, skipping.", item) return None diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index e8a4219b0f..572338c28b 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -21,6 +21,7 @@ from beets.util.artresizer import ArtResizer from beets.util.color import colorize from beets.util.config import UnknownPairError, sanitize_pairs +from beetsplug._utils import art if TYPE_CHECKING: from collections.abc import Iterable, Iterator, Sequence @@ -771,11 +772,11 @@ def get( matches = [] # can there be more than one releasegroupid per response? - for mbid, art in data.get("albums", {}).items(): + for mbid, images in data.get("albums", {}).items(): # there might be more art referenced, e.g. cdart, and an albumcover # might not be present, even if the request was successful - if album.mb_releasegroupid == mbid and "albumcover" in art: - matches.extend(art["albumcover"]) + if album.mb_releasegroupid == mbid and "albumcover" in images: + matches.extend(images["albumcover"]) # can this actually occur? else: self._log.debug( @@ -1285,9 +1286,50 @@ def get( return +class Embedded(LocalArtSource): + NAME = "Embedded" + ID = "embedded" + + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + + def cleanup(self, candidate: Candidate) -> None: + if candidate.path: + try: + util.remove(path=candidate.path) + except util.FilesystemError as exc: + self._log.debug("error cleaning up tmp art: {}", exc) + + def get( + self, + album: Album, + plugin: FetchArtPlugin, + paths: None | Sequence[bytes], + ) -> Iterator[Candidate]: + filename = get_temp_filename(__name__) + for item in album.items(): + if extracted_path := art.extract(self._log, filename, item): + self._log.debug("embedded: extracting art from {}", item) + yield self._candidate( + path=extracted_path, match=MetadataMatch.EXACT + ) + break + else: + self._log.debug("embedded: art not found for {}", album) + + try: + # Always remove tempfile because art.extract will have created a new + # file with the same basename but correct file extension added + util.remove(path=filename) + except util.FilesystemError as exc: + self._log.debug("error cleaning up tempfile: {}", exc) + return + + # All art sources. The order they will be tried in is specified by the config. ART_SOURCES: set[type[ArtSource]] = { FileSystem, + Embedded, CoverArtArchive, ITunesStore, AlbumArtOrg, @@ -1319,6 +1361,7 @@ def __init__(self) -> None: { "auto": True, "fetch_for_asis": False, + "skip_embedded": False, "minwidth": 0, "maxwidth": 0, "quality": 0, @@ -1443,6 +1486,10 @@ def _get_sources(self) -> list[tuple[str, str]]: def fetch_for_asis(self) -> bool: return self.config["fetch_for_asis"].get(bool) + @cached_property + def skip_embedded(self) -> bool: + return self.config["skip_embedded"].get(bool) + @staticmethod def _is_source_file_removal_enabled() -> bool: return config["import"]["delete"].get(bool) or config["import"][ @@ -1463,9 +1510,7 @@ def _is_candidate_fallback(self, candidate: Candidate) -> bool: def fetch_art(self, session: ImportSession, task: ImportTask) -> None: """Find art for the album being imported.""" if task.is_album: # Only fetch art for full albums. - if task.album.artpath and os.path.isfile( - syspath(task.album.artpath) - ): + if self.album_has_art(task.album): # Album already has art (probably a re-import); skip it. return if task.choice_flag == importer.Action.ASIS: @@ -1554,6 +1599,17 @@ def func(lib: Library, opts, args) -> None: cmd.func = func return [cmd] + def album_has_art(self, album: Album): + if album.artpath and os.path.isfile(syspath(album.artpath)): + self._log.debug("skipping {}: has stored artwork", album) + return True + if self.skip_embedded: + if any(art.get_art(self._log, item) for item in album.items()): + self._log.debug("skipping {}: has embedded artwork", album) + return True + + return False + # Utilities converted from functions to methods on logging overhaul def art_for_album( @@ -1607,11 +1663,7 @@ def batch_fetch_art( fetchart CLI command. """ for album in albums: - if ( - album.artpath - and not force - and os.path.isfile(syspath(album.artpath)) - ): + if not force and self.album_has_art(album): if not quiet: message = colorize("text_highlight_minor", "has album art") ui.print_(f"{album}: {message}") diff --git a/docs/changelog.rst b/docs/changelog.rst index 4d906f7778..2eb109e4de 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -33,6 +33,10 @@ New features album art from online sources even when imported files are not modified by the auto-tagger. Default is ``no`` which means ``fetchart`` looks for art only in the local filesystem when the user (or ``quiet_fallback``) chooses ``asis``. +- :doc:`plugins/fetchart`: Add ``embedded`` source that extracts embedded art + for an album into the file named by :ref:`art-filename`. Add ``skip_embedded`` + setting that allows ``fetchart`` to skip fetching art for files that already + have embedded art. Bug fixes ~~~~~~~~~ diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index 85498304b1..81071f732a 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -22,6 +22,11 @@ be embedded. Art will be embedded after each album has its cover art set. This behavior can be disabled with the ``auto`` config option (see below). +Extracting Art Automatically +---------------------------- + +See the ``embedded`` source of the :doc:`/plugins/fetchart`. + .. _image-similarity-check: Image Similarity diff --git a/docs/plugins/fetchart.rst b/docs/plugins/fetchart.rst index 7b965eb9f9..bb3ce3af0e 100644 --- a/docs/plugins/fetchart.rst +++ b/docs/plugins/fetchart.rst @@ -68,13 +68,16 @@ file. The available options are: of the longer edge (``enforce_ratio: 0.5%``). Default: ``no``. - **sources**: List of sources to search for images. An asterisk ``*`` expands to all available sources. Default: ``filesystem coverart itunes amazon - albumart``, i.e., everything but ``wikipedia``, ``google``, ``fanarttv`` and - ``lastfm``. Enable those sources for more matches at the cost of some speed. - They are searched in the given order, thus in the default config, no remote - (Web) art source are queried if local art is found in the filesystem. To use a - local image as fallback, move it to the end of the list. For even more - fine-grained control over the search order, see the section on - :ref:`album-art-sources` below. + albumart``, i.e., everything but :ref:`embedded `, + ``wikipedia``, ``google``, ``fanarttv`` and ``lastfm``. Enable those sources + for more matches at the cost of some speed. They are searched in the given + order, thus in the default config, no remote (Web) art source are queried if + local art is found in the filesystem. To use a local image as fallback, move + it to the end of the list. For even more fine-grained control over the search + order, see the section on :ref:`album-art-sources` below. +- **skip_embedded**: Don't fetch art for files that already have embedded + artwork. Enabling both ``skip_embedded`` and the ``embedded`` source + effectively cancel each other out. Default: ``no`` - **google_key**: Your Google API key (to enable the Google Custom Search backend). Default: None. - **google_engine**: The custom search engine to use. Default: The `beets custom @@ -217,6 +220,21 @@ When you choose to apply changes during an import, beets will search for art as described above. For "as-is" imports (and non-autotagged imports using the ``-A`` flag), beets only looks for art on the local filesystem. +.. _embedded-art-source: + +Embedded Art +~~~~~~~~~~~~ + +Enable the ``embedded`` source to extract embedded art into the album's +directory, to the file named by the :ref:`art-filename` config option. Placing +``embedded`` at the beginning of the ``sources`` list will prevent fetching from +other art sources if the album's files already have embedded art. + +To skip fetching art for files that have embedded art, enable the +``skip_embedded`` configuration option. + +See also: The :doc:`/plugins/embedart` for embedding fetched art into files. + Google custom search ~~~~~~~~~~~~~~~~~~~~ diff --git a/test/plugins/test_art.py b/test/plugins/test_art.py index a787c90341..9454ec127b 100644 --- a/test/plugins/test_art.py +++ b/test/plugins/test_art.py @@ -339,6 +339,31 @@ def test_is_candidate_fallback_os_error( assert not result +class TestEmbeddedArt(UseThePlugin): + def setup_beets(self): + super().setup_beets() + self.albums = [ + self.add_album_fixture(fname="image", ext=ext) + for ext in ["ape", "flac", "m4a", "mp3", "ogg", "wma"] + ] + + @pytest.fixture + def settings(self) -> Settings: + return Settings(fallback=None) + + @pytest.fixture + def source(self) -> fetchart.Embedded: + return fetchart.Embedded(logger, self.plugin.config) + + def test_extract_embedded_art(self, source, settings): + for album in self.albums: + candidate = next(source.get(album, settings, None)) + assert candidate + # image.* fixtures have embedded PNG data + assert os.path.splitext(candidate.path)[1] == b".png" + assert Path(os.fsdecode(candidate.path)).exists() + + class TestCombined(UseThePlugin, FetchImageHelper, CAAData): ASIN = "xxxx" MBID = "releaseid" diff --git a/test/plugins/test_fetchart.py b/test/plugins/test_fetchart.py index 9de3edab87..f7fb4b9a39 100644 --- a/test/plugins/test_fetchart.py +++ b/test/plugins/test_fetchart.py @@ -4,6 +4,8 @@ from typing import Any, ClassVar from unittest import mock +import pytest + from beets import importer, util from beets.test.helper import ( AutotagImportHelper, @@ -14,6 +16,16 @@ from beetsplug.fetchart import CoverArtArchive, FetchArtPlugin, FileSystem +@pytest.fixture(autouse=True) +def disable_http_requests(requests_mock): + """Disable all outgoing requests and raise an error on HTTP + + For test safety to ensure no changes cause tests to make + real, unmocked requests for cover art. + """ + return + + class TestFetchartImport(PluginMixin, AutotagImportHelper): plugin = "fetchart" preload_plugin = False @@ -63,6 +75,53 @@ def test_fetch_for_asis_uses_network_sources(self): self.remote_art_mock.assert_called() +class TestFetchartEmbeddedImport(PluginMixin, AutotagImportHelper): + plugin = "fetchart" + preload_plugin = False + + def setup_beets(self): + super().setup_beets() + + # Use track data fixture with embedded art + self.resource_path = self.resource_path.with_name("image.mp3") + + self.prepare_album_for_import(1) + self.setup_importer() + + @mock.patch.object( + FetchArtPlugin, + "art_for_album", + autospec=True, + wraps=FetchArtPlugin.art_for_album, + ) + def test_embedded_art_skips_fetch(self, art_for_album_mock): + self.importer.add_choice(importer.Action.APPLY) + with self.configure_plugin({"skip_embedded": True}): + self.importer.run() + art_for_album_mock.assert_not_called() + imported_album = self.lib.albums().get() + assert imported_album + assert not imported_album.art_filepath + + def test_embedded_art_extracted(self): + self.importer.add_choice(importer.Action.APPLY) + with self.configure_plugin({"sources": ["embedded"]}): + self.importer.run() + imported_album = self.lib.albums().get() + assert imported_album + assert imported_album.art_filepath + assert imported_album.art_filepath.exists() + + def test_embedded_art_extracted_when_asis(self): + self.importer.add_choice(importer.Action.ASIS) + with self.configure_plugin({"sources": ["embedded"]}): + self.importer.run() + imported_album = self.lib.albums().get() + assert imported_album + assert imported_album.art_filepath + assert imported_album.art_filepath.exists() + + class TestFetchartCli(IOMixin, PluginTestHelper): plugin = "fetchart" @@ -180,7 +239,7 @@ def test_sources_is_a_string(self): def test_sources_is_an_asterisk(self): self.config["fetchart"].set({"sources": "*"}) fa = FetchArtPlugin() - assert len(fa.sources) == 10 + assert len(fa.sources) == 11 def test_sources_is_a_string_list(self): self.config["fetchart"].set({"sources": ["filesystem", "coverart"]})