Add per-episode podcast cover art and fix nested album grouping

- podcast_feeds.py: fetch per-episode cover art from a feed's itunes:image (when it's genuinely distinct from the channel image) or, failing that, from the og:image on the episode's own linked page; extract audio from video-only enclosures via ffmpeg; match podcast-dl's filename convention (illegal characters become "_" instead of being dropped) so enabling feed.txt on an already-downloaded show doesn't re-download its back catalog - scanner.py: _cover_for_episode now checks for a same-stem sidecar cover image before falling back to embedded ID3 art and the shared folder cover - scanner.py/__init__.py: fixed a bug where an album folder nested one level deeper than usual (an age-range grouping folder, say) was mistaken for an empty album and skipped; added periodic progress logging for long scans and analysis passes Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-11 09:54:23 +02:00
parent 747c390303
commit 69119bb72a
5 changed files with 635 additions and 46 deletions
--- a/python-backend/tests/test_podcast_feeds.py
+++ b/python-backend/tests/test_podcast_feeds.py
@@ -7,13 +7,18 @@ from pathlib import Path
 import httpx2
 import pytest

+from musicmouse.library import podcast_feeds
 from musicmouse.library.podcast_feeds import (
+    AudioExtractionError,
    Episode,
+    _entry_enclosure,
    download_episode,
+    episode_cover_filename,
    episode_filename,
    find_feed_shows,
    missing_episodes,
    parse_feed,
+    resolve_episode_cover,
    sync_all_shows,
    sync_show,
 )
@@ -43,6 +48,46 @@ _FEED = b"""<?xml version="1.0"?>
 </rss>
 """

+_FEED_WITH_COVERS = b"""<?xml version="1.0"?>
+<rss version="2.0" xmlns:itunes="http://www.itunes.com/dtds/podcast-1.0.dtd">
+<channel>
+<title>Cover Show</title>
+<link>http://example.com/show</link>
+<itunes:image href="http://example.com/channel.jpg" />
+<item>
+  <title>Unique Cover</title>
+  <link>http://example.com/episodes/unique</link>
+  <pubDate>Wed, 01 Jan 2025 08:00:00 GMT</pubDate>
+  <itunes:image href="http://example.com/unique.jpg" />
+  <enclosure url="http://example.com/ep1.mp3" length="100" type="audio/mpeg" />
+  <guid>ep1</guid>
+</item>
+<item>
+  <title>Same As Channel</title>
+  <link>http://example.com/show</link>
+  <pubDate>Thu, 02 Jan 2025 08:00:00 GMT</pubDate>
+  <itunes:image href="http://example.com/channel.jpg" />
+  <enclosure url="http://example.com/ep2.mp3" length="100" type="audio/mpeg" />
+  <guid>ep2</guid>
+</item>
+</channel>
+</rss>
+"""
+
+_VIDEO_FEED = b"""<?xml version="1.0"?>
+<rss version="2.0">
+<channel>
+<title>Video Show</title>
+<item>
+  <title>Video Only</title>
+  <pubDate>Wed, 01 Jan 2025 08:00:00 GMT</pubDate>
+  <enclosure url="http://example.com/ep1.mp4" length="100" type="video/mpeg" />
+  <guid>ep1</guid>
+</item>
+</channel>
+</rss>
+"""
+

 def _root_with_show(
    tmp_path: Path, *, feed_url: str | None = "http://example.com/feed.xml"
@@ -99,7 +144,19 @@ def test_episode_filename_sanitizes_illegal_characters_and_falls_back_on_type()
    name = episode_filename(
        episode.published, episode.title, episode.enclosure_type, episode.enclosure_url
    )
-    assert name == "20250103 - Episode Two SpecialChars.mp3"
+    # Illegal characters become "_" rather than being dropped, matching the
+    # convention every episode already on disk (downloaded by hand via the
+    # `podcast-dl` CLI) was named with.
+    assert name == "20250103 - Episode Two_ Special_Chars_.mp3"
+
+
+def test_episode_filename_matches_the_podcast_dl_convention_on_a_real_title() -> None:
+    # Pinned against a real file already in the library: `podcast-dl` replaced the
+    # "?" with "_" rather than dropping it, so ours must too or the whole back
+    # catalog would look "new" the moment a feed marker is added for that show.
+    published = parse_feed(_FEED)[0].published
+    name = episode_filename(published, "Wie funktioniert Bogenschießen?", "audio/mpeg", "x.mp3")
+    assert name == "20250101 - Wie funktioniert Bogenschießen_.mp3"


 def test_missing_episodes_skips_files_already_on_disk(tmp_path: Path) -> None:
@@ -151,7 +208,7 @@ async def test_sync_show_downloads_new_episodes_and_is_a_noop_on_rerun(tmp_path:
    assert changed_first is True
    assert changed_second is False
    assert (folder / "20250101 - Episode One.mp3").exists()
-    assert (folder / "20250103 - Episode Two SpecialChars.mp3").exists()
+    assert (folder / "20250103 - Episode Two_ Special_Chars_.mp3").exists()


@pytest.mark.asyncio
@@ -211,3 +268,245 @@ async def test_sync_all_shows_keeps_going_past_one_broken_show(tmp_path: Path) -
    assert changed is True
    assert list(good.glob("*.mp3"))
    assert list(bad.glob("*.mp3")) == []
+
+
+# ------------------------------------------------------------------- per-episode art
+
+
+def test_parse_feed_extracts_cover_and_link_only_when_distinct_from_the_channel() -> None:
+    unique, same = parse_feed(_FEED_WITH_COVERS)
+    assert unique.cover_url == "http://example.com/unique.jpg"
+    assert unique.link == "http://example.com/episodes/unique"
+    # This item's image/link both just repeat the channel's own - not real per-episode
+    # art, so both come out None rather than a false positive.
+    assert same.cover_url is None
+    assert same.link is None
+
+
+def test_episode_cover_filename_shares_the_audio_stem_and_takes_the_url_extension() -> None:
+    episode = parse_feed(_FEED_WITH_COVERS)[0]
+    name = episode_cover_filename(episode.published, episode.title, episode.cover_url)
+    assert name == "20250101 - Unique Cover.jpg"
+
+
+@pytest.mark.asyncio
+async def test_resolve_episode_cover_prefers_the_feed_image_without_a_request() -> None:
+    episode = parse_feed(_FEED_WITH_COVERS)[0]
+
+    def handler(request: httpx2.Request) -> httpx2.Response:
+        raise AssertionError("must not fetch the episode page when cover_url is already set")
+
+    async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client:
+        cover = await resolve_episode_cover(client, episode)
+
+    assert cover == "http://example.com/unique.jpg"
+
+
+@pytest.mark.asyncio
+async def test_resolve_episode_cover_falls_back_to_og_image_on_the_linked_page() -> None:
+    episode = Episode(
+        title="No Feed Image",
+        published=parse_feed(_FEED)[0].published,
+        enclosure_url="http://example.com/ep.mp3",
+        enclosure_type="audio/mpeg",
+        link="http://example.com/episode-page",
+    )
+
+    def handler(request: httpx2.Request) -> httpx2.Response:
+        return httpx2.Response(
+            200,
+            content=b'<html><head>'
+            b'<meta property="og:image" content="http://example.com/scraped.jpg"/>'
+            b"</head></html>",
+        )
+
+    async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client:
+        cover = await resolve_episode_cover(client, episode)
+
+    assert cover == "http://example.com/scraped.jpg"
+
+
+@pytest.mark.asyncio
+async def test_resolve_episode_cover_returns_none_when_the_link_is_unreachable() -> None:
+    episode = Episode(
+        title="Broken Link",
+        published=parse_feed(_FEED)[0].published,
+        enclosure_url="http://example.com/ep.mp3",
+        enclosure_type="audio/mpeg",
+        link="http://example.com/missing",
+    )
+
+    async with httpx2.AsyncClient(
+        transport=httpx2.MockTransport(lambda _: httpx2.Response(404))
+    ) as client:
+        cover = await resolve_episode_cover(client, episode)
+
+    assert cover is None
+
+
+@pytest.mark.asyncio
+async def test_resolve_episode_cover_returns_none_without_a_cover_or_link() -> None:
+    episode = Episode(
+        title="Nothing",
+        published=parse_feed(_FEED)[0].published,
+        enclosure_url="http://example.com/ep.mp3",
+        enclosure_type="audio/mpeg",
+    )
+
+    async def handler(request: httpx2.Request) -> httpx2.Response:  # pragma: no cover
+        raise AssertionError("must not make a request with neither a cover nor a link")
+
+    async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client:
+        cover = await resolve_episode_cover(client, episode)
+
+    assert cover is None
+
+
+@pytest.mark.asyncio
+async def test_sync_show_downloads_the_episode_cover_alongside_the_audio(tmp_path: Path) -> None:
+    folder = tmp_path / "show"
+    folder.mkdir()
+
+    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_COVERS)
+        if url == "http://example.com/unique.jpg":
+            return httpx2.Response(200, content=b"cover-bytes")
+        return httpx2.Response(200, content=b"audio-bytes")
+
+    async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client:
+        changed = await sync_show(client, folder, "http://example.com/feed.xml")
+
+    assert changed is True
+    assert (folder / "20250101 - Unique Cover.jpg").read_bytes() == b"cover-bytes"
+    # No cover file for the episode whose image just repeats the channel's.
+    assert not (folder / "20250102 - Same As Channel.jpg").exists()
+    assert list(folder.glob(".downloading-*.tmp")) == []
+
+
+@pytest.mark.asyncio
+async def test_sync_show_survives_a_failed_cover_download(tmp_path: Path) -> None:
+    folder = tmp_path / "show"
+    folder.mkdir()
+
+    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_COVERS)
+        if url == "http://example.com/unique.jpg":
+            return httpx2.Response(404)
+        return httpx2.Response(200, content=b"audio-bytes")
+
+    async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client:
+        changed = await sync_show(client, folder, "http://example.com/feed.xml")
+
+    # The episode itself still downloaded fine - only its cover failed.
+    assert changed is True
+    assert (folder / "20250101 - Unique Cover.mp3").exists()
+    assert not (folder / "20250101 - Unique Cover.jpg").exists()
+
+
+# ---------------------------------------------------------------------- video shows
+
+
+def test_entry_enclosure_accepts_video_when_no_audio_is_offered() -> None:
+    entry = {"enclosures": [{"href": "http://example.com/ep.mp4", "type": "video/mpeg"}]}
+    assert _entry_enclosure(entry) == ("http://example.com/ep.mp4", "video/mpeg")
+
+
+def test_entry_enclosure_prefers_audio_over_video_when_both_are_offered() -> None:
+    entry = {
+        "enclosures": [
+            {"href": "http://example.com/ep.mp4", "type": "video/mpeg"},
+            {"href": "http://example.com/ep.mp3", "type": "audio/mpeg"},
+        ]
+    }
+    assert _entry_enclosure(entry) == ("http://example.com/ep.mp3", "audio/mpeg")
+
+
+def test_parse_feed_keeps_a_video_only_episode() -> None:
+    episodes = parse_feed(_VIDEO_FEED)
+    assert episodes[0].enclosure_url == "http://example.com/ep1.mp4"
+    assert episodes[0].enclosure_type == "video/mpeg"
+
+
+@pytest.mark.asyncio
+async def test_download_episode_raises_when_ffmpeg_is_not_installed(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    monkeypatch.setattr(podcast_feeds.shutil, "which", lambda _: None)
+    folder = tmp_path / "show"
+    folder.mkdir()
+    episode = parse_feed(_VIDEO_FEED)[0]
+
+    async with httpx2.AsyncClient(
+        transport=httpx2.MockTransport(lambda _: httpx2.Response(200))
+    ) as client:
+        with pytest.raises(AudioExtractionError):
+            await download_episode(client, folder, episode, "20250101 - Video Only.mp3")
+
+    assert list(folder.glob(".downloading-*.tmp")) == []
+
+
+@pytest.mark.asyncio
+async def test_download_episode_raises_when_ffmpeg_exits_nonzero(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    monkeypatch.setattr(podcast_feeds.shutil, "which", lambda _: "/usr/bin/ffmpeg")
+
+    class _FailingProcess:
+        returncode = 1
+
+        async def communicate(self) -> tuple[bytes, bytes]:
+            return b"", b"boom"
+
+    async def fake_exec(*args: object, **kwargs: object) -> _FailingProcess:
+        return _FailingProcess()
+
+    monkeypatch.setattr(podcast_feeds.asyncio, "create_subprocess_exec", fake_exec)
+
+    folder = tmp_path / "show"
+    folder.mkdir()
+    episode = parse_feed(_VIDEO_FEED)[0]
+
+    async with httpx2.AsyncClient(
+        transport=httpx2.MockTransport(lambda _: httpx2.Response(200))
+    ) as client:
+        with pytest.raises(AudioExtractionError, match="boom"):
+            await download_episode(client, folder, episode, "20250101 - Video Only.mp3")
+
+    assert list(folder.glob(".downloading-*.tmp")) == []
+
+
+@pytest.mark.asyncio
+async def test_download_episode_extracts_audio_via_ffmpeg(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    monkeypatch.setattr(podcast_feeds.shutil, "which", lambda _: "/usr/bin/ffmpeg")
+
+    class _SucceedingProcess:
+        returncode = 0
+
+        async def communicate(self) -> tuple[bytes, bytes]:
+            return b"", b""
+
+    async def fake_exec(*args: object, **kwargs: object) -> _SucceedingProcess:
+        # ffmpeg's destination path is the last positional argument; write to it here
+        # to stand in for what the real binary would have produced.
+        Path(str(args[-1])).write_bytes(b"extracted-audio")
+        return _SucceedingProcess()
+
+    monkeypatch.setattr(podcast_feeds.asyncio, "create_subprocess_exec", fake_exec)
+
+    folder = tmp_path / "show"
+    folder.mkdir()
+    episode = parse_feed(_VIDEO_FEED)[0]
+
+    async with httpx2.AsyncClient(
+        transport=httpx2.MockTransport(lambda _: httpx2.Response(200))
+    ) as client:
+        await download_episode(client, folder, episode, "20250101 - Video Only.mp3")
+
+    assert (folder / "20250101 - Video Only.mp3").read_bytes() == b"extracted-audio"
+    assert list(folder.glob(".downloading-*.tmp")) == []