diff options
Diffstat (limited to 'packages/meshbay-node')
3 files changed, 365 insertions, 31 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/indexer/enrich_audio.py b/packages/meshbay-node/src/meshbay_node/indexer/enrich_audio.py index d484e35..dd6b2c0 100644 --- a/packages/meshbay-node/src/meshbay_node/indexer/enrich_audio.py +++ b/packages/meshbay-node/src/meshbay_node/indexer/enrich_audio.py @@ -320,16 +320,30 @@ def _read_tags_and_cover( tags["title"] = title_parse.strip_track_prefix(tags["title"]) or tags["title"] if cover is None and not skip_cover: - sibling = _find_sibling_cover(path.parent) - if sibling is not None: - try: - cover = sibling.read_bytes() - except OSError: - cover = None + cover = _read_sibling_cover(path) return tags, duration, cover +def _read_sibling_cover(path: Path) -> bytes | None: + """ + The cover image sitting beside the track, as bytes. + + Its own function because it is the one part of the read that depends on the + *folder* rather than on the file's bytes: `audio_meta` remembers what the + bytes said and lets `AudioEnricher._run` skip opening the file at all, and + this still has to run on top of that, or a cover dropped in after the first + pass could never be found again. Blocking; called via asyncio.to_thread. + """ + sibling = _find_sibling_cover(path.parent) + if sibling is None: + return None + try: + return sibling.read_bytes() + except OSError: + return None + + # A folder name used as a last-resort artist/album, cleaned of the # punctuation-as-separator and release-tag noise this era of rip is full of # (underscores standing in for spaces, a bitrate/quality tag still attached @@ -450,33 +464,53 @@ class AudioEnricher: async with self._sem: fields: dict = {} - # entry.id is the file's own content hash — a cover already - # cached under it means this exact content's cover was already - # extracted (this run, an earlier one, even a previous daemon - # process), so the second file open (embedded APIC/covr scan) - # and the sibling-directory disk read are both skippable. + # entry.id is the file's own content hash, so everything a read of + # those bytes produced is cacheable under it — the tags, the + # duration, and whether the embedded-art scan has already run + # (media_cache.audio_meta, plus get_thumb_hash_by_file_id for the + # cover itself). With both in hand the file is not opened at all, + # which is the whole point: this pass used to re-read every audio + # file on the node at every start, and until it landed the index + # the node served carried no artist on any track. # - # Tags themselves are *not* skipped this way, deliberately: - # unlike a cover, artist/album/title/track_no can fall back to - # the filename or the folder name (_artist_album_from_ancestors - # above) when no tag is present, which is exactly what a rename - # needs re-derived — _reenrich_renamed_audio_entries exists for - # precisely that. Caching the *result* the same way enrich.py's - # duration/width/height is cached doesn't apply cleanly here: - # mutagen reads tags and duration in the same call as cover, so - # skipping that call to save time would also skip the - # rename-sensitive fields, and skipping only the parts that are - # safe to skip needs the cover check below, not a separate - # cache of the tag-derived fields. + # What is *not* cached, and must not be: the fallbacks below. + # artist/album/title/track_no can come from the filename or the + # folder name (_artist_album_from_ancestors above) when no tag is + # present, and those a rename has to re-derive — + # _reenrich_renamed_audio_entries exists for precisely that. The + # raw tag is not rename-sensitive, so the tag is what is stored and + # the chain still runs live on top of it. The sibling-image scan is + # the other one: it reads the folder, not the file, so a cover + # dropped in afterwards must still be found. cached_thumb_hash = await self._media_cache.get_thumb_hash_by_file_id(entry.id) - try: - tags, duration, cover = await asyncio.wait_for( - asyncio.to_thread( - _read_tags_and_cover, file_path, skip_cover=bool(cached_thumb_hash)), - timeout=READ_TIMEOUT_SECS) - except Exception as e: - log.warning("Tag read failed for %s: %s", file_path, e) - tags, duration, cover = {}, None, None + cached = await self._media_cache.get_audio_meta(entry.id) + cover_settled = bool(cached_thumb_hash) or bool(cached and cached["cover_seen"]) + if cached and cover_settled: + tags = {k: cached[k] for k in ("title", "artist", "album", "track_no") + if cached[k] is not None} + duration = cached["duration"] + cover = None if cached_thumb_hash else await asyncio.to_thread( + _read_sibling_cover, file_path) + else: + try: + tags, duration, cover = await asyncio.wait_for( + asyncio.to_thread( + _read_tags_and_cover, file_path, skip_cover=bool(cached_thumb_hash)), + timeout=READ_TIMEOUT_SECS) + except Exception as e: + # Not cached. A file that genuinely carries no tags reads + # fine and returns an empty dict, which is an answer worth + # keeping; this is a drive that did not answer, and writing + # "says nothing" for it would make one bad read permanent. + log.warning("Tag read failed for %s: %s", file_path, e) + tags, duration, cover = {}, None, None + else: + # `cover_seen` is false when the scan was skipped, so a file + # whose cover was cached and has since been evicted is + # looked at again rather than left without one for good. + await self._media_cache.put_audio_meta( + entry.id, tags, int(duration) if duration else None, + cover_seen=not cached_thumb_hash) if duration: fields["duration"] = int(duration) diff --git a/packages/meshbay-node/src/meshbay_node/media_cache.py b/packages/meshbay-node/src/meshbay_node/media_cache.py index 9dfdf73..9c692ca 100644 --- a/packages/meshbay-node/src/meshbay_node/media_cache.py +++ b/packages/meshbay-node/src/meshbay_node/media_cache.py @@ -107,6 +107,40 @@ CREATE TABLE IF NOT EXISTS mbid_meta ( -- hit had already proven unnecessary. thumb_hash is not duplicated here — -- get_thumb_hash_by_file_id(file_id) already answers that, and a second copy -- would just be one more place for the two to drift. +-- Music app (docs/MESHBAY_DESIGN.md §9.8): what a read of the file's own bytes +-- produced. Music was the one app whose index-time enrichment survived +-- nothing: `video_meta` and `photo_meta` are here, `thumbs` is here, and +-- artist/album/track_no/title lived only in the in-memory IndexEntry. So every +-- node start re-read the tags of every audio file it serves, and until that +-- pass landed the index it served carried no artist on any track — which is an +-- index the Music app cannot group, and looks from the outside like a tab that +-- lost its content. Measured over a real 6176-file library: the pass costs +-- 27.8s with nothing cached and 6.4s with this table populated. It does not +-- make the node's start-up window vanish — video enrichment and re-sealing an +-- 8845-entry index dominate that — it makes the artist and the album come back +-- in seconds rather than in tens of them. +-- +-- Only what the bytes decide, for the reason video_meta states: `display_title` +-- and `track_no` as the app finally sees them can also come from the *filename* +-- (title_parse, and _artist_album_from_ancestors for artist/album), and a +-- rename has to re-derive those — _reenrich_renamed_audio_entries exists for +-- it. The raw tag is not rename-sensitive, so it is what is stored, and the +-- fallback chain still runs live on top of it. +-- +-- `cover_seen` records that the *embedded* art scan ran for this content; +-- combined with get_thumb_hash_by_file_id it says whether the file has to be +-- opened again at all. The sibling-image scan is deliberately not covered by +-- it — that one reads the *folder*, so a cover dropped in later must still be +-- found, and it costs 0.8s across the same library. +CREATE TABLE IF NOT EXISTS audio_meta ( + file_id TEXT PRIMARY KEY, + title TEXT, + artist TEXT, + album TEXT, + track_no INTEGER, + duration INTEGER, + cover_seen INTEGER NOT NULL DEFAULT 0 +); CREATE TABLE IF NOT EXISTS photo_meta ( file_id TEXT PRIMARY KEY, width INTEGER, @@ -460,6 +494,34 @@ class MediaCache: ) await self._db.commit() + # ── audio tags (Music app) ─────────────────────────────────────────────── + # + # The tag as read, never the field as the app finally sees it — see the + # schema comment on why the filename-derived half stays out. + + async def get_audio_meta(self, file_id: str) -> dict | None: + async with self._db.execute( + "SELECT title, artist, album, track_no, duration, cover_seen " + "FROM audio_meta WHERE file_id = ?", + (file_id,), + ) as cur: + row = await cur.fetchone() + if not row: + return None + return {"title": row[0], "artist": row[1], "album": row[2], + "track_no": row[3], "duration": row[4], "cover_seen": bool(row[5])} + + async def put_audio_meta(self, file_id: str, tags: dict, duration: int | None, + cover_seen: bool) -> None: + await self._db.execute( + "INSERT OR REPLACE INTO audio_meta " + "(file_id, title, artist, album, track_no, duration, cover_seen) " + "VALUES (?, ?, ?, ?, ?, ?, ?)", + (file_id, tags.get("title"), tags.get("artist"), tags.get("album"), + tags.get("track_no"), duration, 1 if cover_seen else 0), + ) + await self._db.commit() + # ── video technical fields (Videos app) ────────────────────────────────── # # duration/width/height only — see the schema comment on why @@ -498,6 +560,8 @@ class MediaCache: await self._db.execute("DELETE FROM thumbs WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM photo_meta WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM video_meta WHERE file_id = ?", (file_id,)) + await self._db.execute( + "DELETE FROM audio_meta WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM file_tmdb WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM tmdb_override WHERE file_id = ?", (file_id,)) await self._db.execute("DELETE FROM file_mbid WHERE file_id = ?", (file_id,)) diff --git a/packages/meshbay-node/tests/test_audio_meta_cache.py b/packages/meshbay-node/tests/test_audio_meta_cache.py new file mode 100644 index 0000000..e6ee315 --- /dev/null +++ b/packages/meshbay-node/tests/test_audio_meta_cache.py @@ -0,0 +1,236 @@ +""" +The Music app's index-time enrichment survives a restart. + +`video_meta`, `photo_meta` and `thumbs` were all durable; the audio tags were +not, so every node start re-read the tags of every audio file it serves, and +until that pass landed the index it served carried no artist on any track. A +client that asked in that window got an index the Music app cannot group — a +toolbar over a blank page, for as long as the page stayed open. + +What is cached is what the file's *bytes* said. The filename and folder +fallbacks on top of it are not, and these tests are mostly about that line: +a cache that also remembered the derived fields would hand a renamed file the +old name's answer, which is the fault `_reenrich_renamed_audio_entries` exists +to prevent. +""" +import asyncio +import subprocess +from pathlib import Path + +import pytest +from meshbay_common.protocol import IndexEntry +from meshbay_node.indexer import enrich_audio +from meshbay_node.indexer.enrich_audio import AudioEnricher +from meshbay_node.media_cache import MediaCache + +_HAVE_FFMPEG = ( + subprocess.run(["which", "ffmpeg"], capture_output=True).returncode == 0) + +pytestmark = pytest.mark.skipif(not _HAVE_FFMPEG, reason="ffmpeg not installed") + + +def _make_clip(path: Path, *, title=None, artist=None, album=None, track=None) -> None: + subprocess.run( + ["ffmpeg", "-hide_banner", "-loglevel", "error", "-y", + "-f", "lavfi", "-i", "sine=frequency=440:duration=1", + "-c:a", "libmp3lame", "-b:a", "64k", + *(["-metadata", f"title={title}"] if title else []), + *(["-metadata", f"artist={artist}"] if artist else []), + *(["-metadata", f"album={album}"] if album else []), + *(["-metadata", f"track={track}"] if track else []), + str(path)], + check=True, capture_output=True, + ) + + +@pytest.fixture +async def media_cache(tmp_path): + c = MediaCache(db_path=tmp_path / "media_cache.db") + await c.open() + yield c + await c.close() + + +async def _enrich(enricher, entry, clip, root_path=None): + done = asyncio.get_event_loop().create_future() + + async def on_done(file_id, fields): + done.set_result(fields) + + enricher.spawn(entry, clip, on_done, root_path) + return await asyncio.wait_for(done, timeout=30) + + +@pytest.fixture +def counted_reads(monkeypatch): + """How many times the audio file itself was opened and parsed.""" + calls = [] + real = enrich_audio._read_tags_and_cover + + def counting(path, skip_cover=False): + calls.append(Path(path).name) + return real(path, skip_cover=skip_cover) + + monkeypatch.setattr(enrich_audio, "_read_tags_and_cover", counting) + return calls + + +@pytest.mark.asyncio +async def test_a_second_pass_over_the_same_content_does_not_open_the_file( + tmp_path, media_cache, counted_reads): + """The restart case, in one process: same bytes, no second read.""" + clip = tmp_path / "01 - a track.mp3" + _make_clip(clip, title="A Title", artist="An Act", album="A Record", track=2) + entry = IndexEntry(id="content1", name=clip.name, path=clip.name, + size=clip.stat().st_size, type="audio", added_at=0) + + first = await _enrich(AudioEnricher(media_cache), entry, clip) + assert len(counted_reads) == 1 + + # A different enricher, as a restarted daemon would build — only the cache + # on disk is shared. + second = await _enrich(AudioEnricher(media_cache), entry, clip) + assert len(counted_reads) == 1, "the file was opened again despite a cache hit" + assert second == first, "a cache hit must produce the fields the read produced" + + +@pytest.mark.asyncio +async def test_the_cached_answer_is_the_tags_and_the_duration(tmp_path, media_cache): + clip = tmp_path / "01 - a track.mp3" + _make_clip(clip, title="A Title", artist="An Act", album="A Record", track=2) + entry = IndexEntry(id="content2", name=clip.name, path=clip.name, + size=clip.stat().st_size, type="audio", added_at=0) + await _enrich(AudioEnricher(media_cache), entry, clip) + + row = await media_cache.get_audio_meta("content2") + assert row["title"] == "A Title" + assert row["artist"] == "An Act" + assert row["album"] == "A Record" + assert row["track_no"] == 2 + assert row["duration"] == 1 + assert row["cover_seen"] is True + + +@pytest.mark.asyncio +async def test_a_file_that_says_nothing_is_remembered_as_saying_nothing( + tmp_path, media_cache, counted_reads): + """ + The commonest row in a real library, and the one worth caching most: an + untagged file costs exactly the same open as a tagged one to learn nothing + from. + """ + folder = tmp_path / "Some Act" / "Some Record" + folder.mkdir(parents=True) + clip = folder / "05 - Filename Title.mp3" + _make_clip(clip) + entry = IndexEntry(id="content3", name=clip.name, + path=str(clip.relative_to(tmp_path)), + size=clip.stat().st_size, type="audio", added_at=0) + + first = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert await media_cache.get_audio_meta("content3") is not None + + second = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert len(counted_reads) == 1 + # Still resolved, and from the folder rather than from any tag. + assert second["artist"] == "Some Act" + assert second["album"] == "Some Record" + assert second == first + + +@pytest.mark.asyncio +async def test_a_renamed_file_re_derives_what_the_name_decides( + tmp_path, media_cache, counted_reads): + """ + The line the cache must not cross. Same content, so the tags are reused — + and `display_title`/`track_no`/artist/album still come from the new name + and the new folder, which is what `_reenrich_renamed_audio_entries` asks + for when it discards the attempt. + """ + first_folder = tmp_path / "Old Act" / "Old Record" + first_folder.mkdir(parents=True) + clip = first_folder / "05 - Old Name.mp3" + _make_clip(clip) # no tags at all: the name decides + entry = IndexEntry(id="content4", name=clip.name, + path=str(clip.relative_to(tmp_path)), + size=clip.stat().st_size, type="audio", added_at=0) + before = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert before["display_title"] == "Old Name" + assert before["artist"] == "Old Act" + + moved_folder = tmp_path / "New Act" / "New Record" + moved_folder.mkdir(parents=True) + moved = moved_folder / "07 - New Name.mp3" + clip.rename(moved) + renamed = IndexEntry(id="content4", name=moved.name, + path=str(moved.relative_to(tmp_path)), + size=moved.stat().st_size, type="audio", added_at=0) + + after = await _enrich(AudioEnricher(media_cache), renamed, moved, tmp_path) + assert len(counted_reads) == 1, "the bytes did not change; the file need not be reopened" + assert after["display_title"] == "New Name", "the cache answered for the old name" + assert after["track_no"] == 7 + assert after["artist"] == "New Act" + assert after["album"] == "New Record" + + +@pytest.mark.asyncio +async def test_a_cover_dropped_in_afterwards_is_still_found(tmp_path, media_cache): + """ + The sibling scan reads the *folder*, so it stays live on top of the cache. + Caching it would have made "no cover" permanent for content that later got + one — and the scan costs 0.8s across a 6000-file library, measured. + """ + folder = tmp_path / "An Act" / "A Record" + folder.mkdir(parents=True) + clip = folder / "01 - a track.mp3" + _make_clip(clip) + entry = IndexEntry(id="content5", name=clip.name, + path=str(clip.relative_to(tmp_path)), + size=clip.stat().st_size, type="audio", added_at=0) + + first = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert first.get("thumb_hash") is None + + (folder / "cover.jpg").write_bytes(b"\xff\xd8\xff\xe0 not really a jpeg") + second = await _enrich(AudioEnricher(media_cache), entry, clip, tmp_path) + assert second.get("thumb_hash"), "the cache made a missing cover permanent" + + +@pytest.mark.asyncio +async def test_an_unreadable_file_is_not_remembered_as_empty(tmp_path, media_cache): + """ + A drive that did not answer is not a file that carries no tags. Writing + "says nothing" for it would make one bad read permanent. + """ + clip = tmp_path / "01 - a track.mp3" + _make_clip(clip, title="A Title", artist="An Act") + entry = IndexEntry(id="content6", name=clip.name, path=clip.name, + size=clip.stat().st_size, type="audio", added_at=0) + + def boom(path, skip_cover=False): + raise OSError("the drive said no") + + real = enrich_audio._read_tags_and_cover + enrich_audio._read_tags_and_cover = boom + try: + await _enrich(AudioEnricher(media_cache), entry, clip) + finally: + enrich_audio._read_tags_and_cover = real + assert await media_cache.get_audio_meta("content6") is None + + fields = await _enrich(AudioEnricher(media_cache), entry, clip) + assert fields["artist"] == "An Act", "the failed read was cached and never retried" + + +@pytest.mark.asyncio +async def test_a_file_leaving_the_index_takes_its_row_with_it(tmp_path, media_cache): + clip = tmp_path / "01 - a track.mp3" + _make_clip(clip, artist="An Act") + entry = IndexEntry(id="content7", name=clip.name, path=clip.name, + size=clip.stat().st_size, type="audio", added_at=0) + await _enrich(AudioEnricher(media_cache), entry, clip) + assert await media_cache.get_audio_meta("content7") is not None + + await media_cache.prune_file("content7") + assert await media_cache.get_audio_meta("content7") is None |