From 405b0988d6fc546e0818f0b277a9e6816db27da8 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Wed, 3 Jun 2026 22:17:20 -0700 Subject: [PATCH] Cover Art Filler: skip files that already have art (keep apply purely additive) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verification found a non-additive edge: embed_album_art_metadata uses FLAC add_picture(), which APPENDS — so applying to an album where some tracks already had art would have added a duplicate embedded picture. The apply now checks each file and skips any that already carry art (shared _audio_has_art helper), so it only ever ADDS art to files missing it. Test covers the skip (no re-embed). --- core/metadata/art_apply.py | 46 +++++++++++++++++++++++--------------- tests/test_art_apply.py | 16 +++++++++++++ 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/core/metadata/art_apply.py b/core/metadata/art_apply.py index 5b9a8bf9..e267a3e5 100644 --- a/core/metadata/art_apply.py +++ b/core/metadata/art_apply.py @@ -53,29 +53,33 @@ def file_has_embedded_art(file_path: str) -> bool: if not symbols: return False try: - audio = symbols.File(file_path) - if audio is None: - return False - # FLAC / Ogg expose picture blocks directly. - if getattr(audio, "pictures", None): - return True - tags = getattr(audio, "tags", None) - if isinstance(audio, symbols.MP4): - return bool(audio.get("covr")) - if tags is None: - return False - with contextlib.suppress(Exception): - if isinstance(tags, symbols.ID3): - return bool(tags.getall("APIC")) - with contextlib.suppress(Exception): - if "metadata_block_picture" in tags: - return True - return False + return _audio_has_art(symbols.File(file_path), symbols) except Exception as exc: logger.debug("art presence check failed for %s: %s", file_path, exc) return False +def _audio_has_art(audio, symbols) -> bool: + """True if an already-open mutagen object carries embedded cover art.""" + if audio is None: + return False + # FLAC / Ogg expose picture blocks directly. + if getattr(audio, "pictures", None): + return True + if isinstance(audio, symbols.MP4): + return bool(audio.get("covr")) + tags = getattr(audio, "tags", None) + if tags is None: + return False + with contextlib.suppress(Exception): + if isinstance(tags, symbols.ID3): + return bool(tags.getall("APIC")) + with contextlib.suppress(Exception): + if "metadata_block_picture" in tags: + return True + return False + + def album_has_art_on_disk(rep_file_path: str) -> bool: """Does this album have art on disk? @@ -123,6 +127,12 @@ def apply_art_to_album_files( if audio is None: result["skipped"] += 1 continue + # Purely additive: never touch a file that already has art. Embedding + # again would APPEND a duplicate picture on FLAC (add_picture doesn't + # replace), so leave already-arted files alone. + if _audio_has_art(audio, symbols): + result["skipped"] += 1 + continue # ID3 needs a tag container before APIC can be added. if getattr(audio, "tags", None) is None and hasattr(audio, "add_tags"): with contextlib.suppress(Exception): diff --git a/tests/test_art_apply.py b/tests/test_art_apply.py index d160c73b..8e240e41 100644 --- a/tests/test_art_apply.py +++ b/tests/test_art_apply.py @@ -119,6 +119,22 @@ def test_apply_embeds_into_each_file_and_writes_cover(tmp_path, monkeypatch): assert res['cover_written'] is True +def test_apply_skips_files_that_already_have_art(tmp_path, monkeypatch): + """Purely additive: a file that already has art is left untouched (no + duplicate FLAC picture, no re-embed).""" + f1 = tmp_path / '01.flac'; f1.write_bytes(b'') + audio = SimpleNamespace(pictures=['existing'], tags=None, save=lambda: None) + monkeypatch.setattr(aa, 'get_mutagen_symbols', lambda: _fake_symbols(audio)) + embed_calls = [] + monkeypatch.setattr(aa, 'embed_album_art_metadata', lambda a, m: embed_calls.append(m) or True) + monkeypatch.setattr(aa, 'download_cover_art', lambda *a, **k: None) + + res = aa.apply_art_to_album_files([str(f1)], {'album': 'B'}, {'album_name': 'B'}, folder=str(tmp_path)) + assert res['embedded'] == 0 + assert res['skipped'] == 1 + assert embed_calls == [] # never re-embedded → no duplicate picture + + def test_apply_counts_failures_without_raising(tmp_path, monkeypatch): f1 = tmp_path / '01.flac'; f1.write_bytes(b'') audio = SimpleNamespace(tags=object(), save=lambda: (_ for _ in ()).throw(OSError('read-only')))