From 5af5c54c62caa557813b0a08153a495b445052f1 Mon Sep 17 00:00:00 2001 From: Martin Bauer Date: Fri, 2 Oct 2026 21:53:06 +0200 Subject: [PATCH] Backfill podcast covers for episodes already on disk A podcast's episode cover was only fetched right after the feed sync downloaded that episode, so anything placed by hand (or downloaded before the og:image fallback existed) never got one - in Wissen macht Ah only the three newest episodes had their own art. The sync now fetches the cover of every on-disk episode in the feed window that lacks a sidecar image. One that can't be found or fetched is recorded in .missing-covers.json and left alone for the same 7-day backoff as failed downloads, so a feed without per-episode art doesn't cost a page fetch per episode every poll. The scanner's per-episode fingerprint now includes the sidecar cover, so one added after the episode was first scanned invalidates its cached entry instead of being ignored until the index is rebuilt. Episodes without a sidecar fingerprint exactly as before, so existing caches stay valid. Co-Authored-By: Claude Opus 5.5 --- .../musicmouse/library/podcast_feeds.py | 107 +++++++++++++----- python-backend/musicmouse/library/scanner.py | 6 +- python-backend/tests/test_library.py | 19 ++++ python-backend/tests/test_podcast_feeds.py | 77 +++++++++++++ 4 files changed, 181 insertions(+), 28 deletions(-) diff --git a/python-backend/musicmouse/library/podcast_feeds.py b/python-backend/musicmouse/library/podcast_feeds.py index a2b3ec7..18b0db6 100644 --- a/python-backend/musicmouse/library/podcast_feeds.py +++ b/python-backend/musicmouse/library/podcast_feeds.py @@ -116,6 +116,9 @@ _FFMPEG_TIMEOUT: Final = 600.0 #: Where a show folder remembers episodes that failed to download, so a permanently #: dead enclosure (e.g. pulled from the host's CDN) isn't retried every poll. _FAILED_DOWNLOADS_FILENAME: Final = ".failed-downloads.json" +#: The same, for episodes on disk whose cover could not be found or fetched - so an +#: episode with no art anywhere doesn't cost a page fetch on every poll. +_MISSING_COVERS_FILENAME: Final = ".missing-covers.json" #: How long a failed episode is left alone before it's given another chance - long #: enough to stop hammering a dead URL every 6 hours, short enough that a genuinely #: transient failure (a host outage, a flaky network) still recovers on its own. @@ -379,15 +382,16 @@ def prune_show(folder: Path, keep: int | None, extensions: frozenset[str] | None return removed -def _load_failed_downloads(folder: Path) -> dict[str, datetime]: - """Filename -> when it last failed to download, for episodes ``sync_show`` should - leave alone until :data:`_RETRY_BACKOFF` has passed. +def _load_failed(folder: Path, record_name: str) -> dict[str, datetime]: + """Filename -> when it last failed, for episodes ``sync_show`` should leave alone + until :data:`_RETRY_BACKOFF` has passed. ``record_name`` picks which record: failed + downloads, or covers that could not be found. A missing or corrupt record is just an empty one - nothing here is precious enough to raise over, matching the module's overall rule that bad input is logged and skipped rather than fatal. """ - path = folder / _FAILED_DOWNLOADS_FILENAME + path = folder / record_name try: raw = json.loads(path.read_text(encoding="utf-8")) except FileNotFoundError: @@ -402,12 +406,12 @@ def _load_failed_downloads(folder: Path) -> dict[str, datetime]: return {} -def _save_failed_downloads(folder: Path, failed: dict[str, datetime]) -> None: - path = folder / _FAILED_DOWNLOADS_FILENAME +def _save_failed(folder: Path, record_name: str, failed: dict[str, datetime]) -> None: + path = folder / record_name if not failed: path.unlink(missing_ok=True) return - temp_path = folder / f".{_FAILED_DOWNLOADS_FILENAME}.tmp" + temp_path = folder / f".{record_name}.tmp" payload = {filename: when.isoformat() for filename, when in failed.items()} temp_path.write_text(json.dumps(payload), encoding="utf-8") temp_path.replace(path) @@ -529,6 +533,65 @@ async def resolve_episode_cover(client: httpx2.AsyncClient, episode: Episode) -> return _extract_og_image(response.text) +def _has_sidecar_cover(audio_path: Path) -> bool: + return any(audio_path.with_suffix(ext).is_file() for ext in _IMAGE_EXTENSIONS) + + +async def _sync_covers( + client: httpx2.AsyncClient, folder: Path, episodes: list[Episode], now: datetime +) -> bool: + """Fetch the sidecar cover of every episode in ``episodes`` that is on disk without + one. Returns whether any cover was added. + + Covers every episode on disk, not just the ones this pass downloaded: episodes + placed by hand, or downloaded before a cover route existed (Wissen macht Ah's + ``og:image`` fallback), would otherwise never get theirs. An episode whose cover + can't be found or fetched is remembered and left alone for :data:`_RETRY_BACKOFF`, + like a failed download, so a feed with no per-episode art doesn't cost one page + fetch per episode on every poll. + """ + missing = _load_failed(folder, _MISSING_COVERS_FILENAME) + on_disk: set[str] = set() + added = False + for episode in episodes: + filename = episode_filename( + episode.published, episode.title, episode.enclosure_type, episode.enclosure_url + ) + audio_path = folder / filename + if not audio_path.is_file(): + continue + on_disk.add(filename) + if _has_sidecar_cover(audio_path): + missing.pop(filename, None) + continue + last_miss = missing.get(filename) + if last_miss is not None and now - last_miss < _RETRY_BACKOFF: + continue + + cover_url = await resolve_episode_cover(client, episode) + if cover_url is None: + missing[filename] = now + continue + cover_filename = episode_cover_filename(episode.published, episode.title, cover_url) + try: + await download_episode_cover(client, folder, cover_url, cover_filename) + except (httpx2.HTTPError, OSError) as exc: + _log.warning( + "Could not download cover for episode %r in %s: %s", + episode.title, + folder.name, + exc, + ) + missing[filename] = now + continue + missing.pop(filename, None) + added = True + + missing = {filename: when for filename, when in missing.items() if filename in on_disk} + _save_failed(folder, _MISSING_COVERS_FILENAME, missing) + return added + + async def sync_show( client: httpx2.AsyncClient, folder: Path, @@ -542,9 +605,9 @@ async def sync_show( Both halves use the same ``keep``, which is what makes the folder settle - see :data:`DEFAULT_EPISODE_LIMIT`. - Returns whether anything changed, deletions included: a pruned folder needs a - rescan just as much as a downloaded episode does, or the library keeps offering - tracks whose files are gone. + Returns whether anything changed, deletions and backfilled covers included: a + pruned folder needs a rescan just as much as a downloaded episode does, or the + library keeps offering tracks whose files are gone. Errors - an unreachable feed, a malformed one, a single broken enclosure - are logged and swallowed here so one bad show never stops the others or takes down the @@ -559,8 +622,9 @@ async def sync_show( _log.warning("Could not fetch podcast feed %s for %s: %s", feed_url, folder.name, exc) return False - pending = missing_episodes(folder, newest_episodes(parse_feed(response.content), keep)) - failed = _load_failed_downloads(folder) + window = newest_episodes(parse_feed(response.content), keep) + pending = missing_episodes(folder, window) + failed = _load_failed(folder, _FAILED_DOWNLOADS_FILENAME) now = datetime.now(UTC) changed = False for episode, filename in pending: @@ -579,23 +643,12 @@ async def sync_show( _log.info("Downloaded new episode %r for %s", episode.title, folder.name) changed = True - cover_url = await resolve_episode_cover(client, episode) - if cover_url: - cover_filename = episode_cover_filename(episode.published, episode.title, cover_url) - if not (folder / cover_filename).exists(): - try: - await download_episode_cover(client, folder, cover_url, cover_filename) - except (httpx2.HTTPError, OSError) as exc: - _log.warning( - "Could not download cover for episode %r in %s: %s", - episode.title, - folder.name, - exc, - ) - pending_filenames = {filename for _episode, filename in pending} failed = {filename: when for filename, when in failed.items() if filename in pending_filenames} - _save_failed_downloads(folder, failed) + _save_failed(folder, _FAILED_DOWNLOADS_FILENAME, failed) + + if await _sync_covers(client, folder, window, now): + changed = True # After downloading, not before: an episode that just arrived is one of the newest # and must be counted when deciding what falls off the end. diff --git a/python-backend/musicmouse/library/scanner.py b/python-backend/musicmouse/library/scanner.py index 7da4050..992f3b3 100644 --- a/python-backend/musicmouse/library/scanner.py +++ b/python-backend/musicmouse/library/scanner.py @@ -262,7 +262,11 @@ def scan_episodes( out: dict[str, tuple[Album, Fingerprint]] = {} for path in paths: identifier = album_id(root, path) - fingerprint = Fingerprint.of([path]) + # The sidecar cover counts too: one fetched after the episode was first scanned + # (``podcast_feeds.py`` backfills them) must invalidate the cached entry, or the + # episode keeps showing the show's shared cover. + sidecars = [path.with_suffix(ext) for ext in _SIDECAR_COVER_EXTENSIONS] + fingerprint = Fingerprint.of([path, *(p for p in sidecars if p.is_file())]) cached = known.get(identifier) if cached is not None and cached[1] == fingerprint: out[identifier] = cached diff --git a/python-backend/tests/test_library.py b/python-backend/tests/test_library.py index 5367678..da264bb 100644 --- a/python-backend/tests/test_library.py +++ b/python-backend/tests/test_library.py @@ -284,6 +284,25 @@ async def test_an_episodes_sidecar_cover_wins_over_the_shows_shared_one(config_d assert red < 60 and blue > red, "falls back to the show's shared cover" +async def test_a_sidecar_cover_added_later_replaces_the_cached_one(config_dir: Path) -> None: + """`podcast_feeds.py` backfills covers for episodes already on disk - the episode's + audio is untouched, so only the sidecar can tell the cache its entry is stale.""" + from PIL import Image + + podcast = config_dir / "music" / "Kinderpodcasts" / "Wissen macht Ah" + Image.new("RGB", (32, 32), (10, 20, 30)).save(podcast / "cover.jpg") + first = album_named(await build(config_dir), "Alt") + + alt_audio = first.tracks[0].path + Image.new("RGB", (32, 32), (200, 40, 30)).save(alt_audio.with_suffix(".jpg")) + second = album_named(await build(config_dir), "Alt") + + assert second.cover is not None + with Image.open(second.cover) as image: + red, green, _blue = image.convert("RGB").getpixel((0, 0)) # type: ignore[misc] + assert red > 150 and green < 90, "the new sidecar should replace the shared cover" + + # --------------------------------------------------------------------------- cache diff --git a/python-backend/tests/test_podcast_feeds.py b/python-backend/tests/test_podcast_feeds.py index 193d889..155cee9 100644 --- a/python-backend/tests/test_podcast_feeds.py +++ b/python-backend/tests/test_podcast_feeds.py @@ -411,6 +411,83 @@ async def test_sync_show_survives_a_failed_cover_download(tmp_path: Path) -> Non assert not (folder / "20250101 - Unique Cover.jpg").exists() +_FEED_WITH_LINKS = b""" + + +Link Show +http://example.com/show + + Old One + http://example.com/episodes/old + Wed, 01 Jan 2025 08:00:00 GMT + + ep1 + + + +""" + + +@pytest.mark.asyncio +async def test_sync_show_backfills_the_cover_of_an_episode_already_on_disk( + tmp_path: Path, +) -> None: + # Placed by hand (or downloaded before covers were fetched): the audio is there, + # so nothing is downloaded - but its cover still has to be fetched. + folder = tmp_path / "show" + folder.mkdir() + (folder / "20250101 - Old One.mp3").write_bytes(b"audio-bytes") + requests: list[str] = [] + + def handler(request: httpx2.Request) -> httpx2.Response: + url = str(request.url) + requests.append(url) + if url == "http://example.com/feed.xml": + return httpx2.Response(200, content=_FEED_WITH_LINKS) + if url == "http://example.com/episodes/old": + html = '' + return httpx2.Response(200, text=html) + if url == "http://example.com/old.png": + return httpx2.Response(200, content=b"cover-bytes") + return httpx2.Response(500) + + async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client: + changed = await sync_show(client, folder, "http://example.com/feed.xml") + requests.clear() + changed_again = await sync_show(client, folder, "http://example.com/feed.xml") + + assert changed is True + assert (folder / "20250101 - Old One.png").read_bytes() == b"cover-bytes" + # Once the sidecar exists, a later poll doesn't go looking for it again. + assert changed_again is False + assert requests == ["http://example.com/feed.xml"] + + +@pytest.mark.asyncio +async def test_sync_show_does_not_look_for_a_missing_cover_again_within_the_backoff( + tmp_path: Path, +) -> None: + folder = tmp_path / "show" + folder.mkdir() + (folder / "20250101 - Old One.mp3").write_bytes(b"audio-bytes") + page_requests: list[str] = [] + + def handler(request: httpx2.Request) -> httpx2.Response: + url = str(request.url) + if url == "http://example.com/feed.xml": + return httpx2.Response(200, content=_FEED_WITH_LINKS) + page_requests.append(url) + return httpx2.Response(200, text="no og:image here") + + async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client: + changed = await sync_show(client, folder, "http://example.com/feed.xml") + await sync_show(client, folder, "http://example.com/feed.xml") + + assert changed is False + assert page_requests == ["http://example.com/episodes/old"] + assert (folder / podcast_feeds._MISSING_COVERS_FILENAME).exists() + + # ---------------------------------------------------------------------- video shows