Cover Art Filler: skip files that already have art (keep apply purely additive)
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).
This commit is contained in:
parent
33965c7cbd
commit
405b0988d6
2 changed files with 44 additions and 18 deletions
|
|
@ -53,29 +53,33 @@ def file_has_embedded_art(file_path: str) -> bool:
|
||||||
if not symbols:
|
if not symbols:
|
||||||
return False
|
return False
|
||||||
try:
|
try:
|
||||||
audio = symbols.File(file_path)
|
return _audio_has_art(symbols.File(file_path), symbols)
|
||||||
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
|
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.debug("art presence check failed for %s: %s", file_path, exc)
|
logger.debug("art presence check failed for %s: %s", file_path, exc)
|
||||||
return False
|
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:
|
def album_has_art_on_disk(rep_file_path: str) -> bool:
|
||||||
"""Does this album have art on disk?
|
"""Does this album have art on disk?
|
||||||
|
|
||||||
|
|
@ -123,6 +127,12 @@ def apply_art_to_album_files(
|
||||||
if audio is None:
|
if audio is None:
|
||||||
result["skipped"] += 1
|
result["skipped"] += 1
|
||||||
continue
|
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.
|
# ID3 needs a tag container before APIC can be added.
|
||||||
if getattr(audio, "tags", None) is None and hasattr(audio, "add_tags"):
|
if getattr(audio, "tags", None) is None and hasattr(audio, "add_tags"):
|
||||||
with contextlib.suppress(Exception):
|
with contextlib.suppress(Exception):
|
||||||
|
|
|
||||||
|
|
@ -119,6 +119,22 @@ def test_apply_embeds_into_each_file_and_writes_cover(tmp_path, monkeypatch):
|
||||||
assert res['cover_written'] is True
|
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):
|
def test_apply_counts_failures_without_raising(tmp_path, monkeypatch):
|
||||||
f1 = tmp_path / '01.flac'; f1.write_bytes(b'')
|
f1 = tmp_path / '01.flac'; f1.write_bytes(b'')
|
||||||
audio = SimpleNamespace(tags=object(), save=lambda: (_ for _ in ()).throw(OSError('read-only')))
|
audio = SimpleNamespace(tags=object(), save=lambda: (_ for _ in ()).throw(OSError('read-only')))
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue