From ff4f11d51eecde584d2941fd8997ed7bc1c2cb42 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 20 Sep 2026 11:40:10 -0400 Subject: [PATCH 1/3] feat(video): read the JSON sidecar for videos generated before InvokeAI 7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-7 releases kept a generated video's record in a sidecar under {outputs}/videos/sidecars/, mirroring the video's own subfolder, rather than inside the MP4. InvokeAI 7 still writes one when the embedding remux fails, so this is a live fallback and not only a legacy path. The extractor now reads the MP4's keyed metadata first and the sidecar second, which is InvokeAI's own order and means a v7 video costs no sidecar lookup at all. Finding the sidecar is the interesting part: PhotoMapAI indexes absolute paths and never learns where an `outputs` directory begins, so each ancestor of the video is tried as the videos root, nearest first, mirroring the video's relative path under `sidecars/`. Bounded, and required to carry an `invokeai_metadata` key, so an unrelated directory called `sidecars` cannot be mistaken for InvokeAI's. Measured against a real install (668 videos, 662 sidecars): every sidecar resolved at ancestor depth 1, 132 carried a record and 530 carried a null one (a workflow but no parameters), 3 videos had the record embedded, and the whole scan cost 1 ms per video including the MP4 walk. All 132 recovered records parse, are recognised as video generations, and render a panel with no undeclared fields — which also validates the v5 video profile against real data rather than only against the synthetic fixture. Co-Authored-By: Claude Opus 5 (1M context) --- docs/user-guide/invokeai-integration.md | 20 +- photomap/backend/invokeai_sidecar.py | 140 ++++++++++++ photomap/backend/metadata_extraction.py | 29 ++- tests/backend/test_invokeai_sidecar.py | 275 ++++++++++++++++++++++++ 4 files changed, 453 insertions(+), 11 deletions(-) create mode 100644 photomap/backend/invokeai_sidecar.py create mode 100644 tests/backend/test_invokeai_sidecar.py diff --git a/docs/user-guide/invokeai-integration.md b/docs/user-guide/invokeai-integration.md index d497d1ae..d4080537 100644 --- a/docs/user-guide/invokeai-integration.md +++ b/docs/user-guide/invokeai-integration.md @@ -201,11 +201,23 @@ clickable thumbnail, exactly like an image's reference images. still shows no generation parameters, re-index the album: an update only re-reads files whose modification time has changed. +Videos generated before InvokeAI 7 carry no record inside the file, but +InvokeAI kept one in a JSON sidecar under `outputs/videos/sidecars/`, and +PhotoMapAI reads that too — so an older clip shows the same panel. The file +itself is always preferred; the sidecar is consulted only when the video +carries nothing, which is also what happens on the rare occasion InvokeAI 7 +could not embed the record and fell back to writing one. + +!!! note + Not every older video has recoverable parameters. A sidecar is written + for each generated video, but it records the *workflow* and only + sometimes the generation parameters — in one real collection, about a + fifth of them carried a usable record. The rest show the still-frame + panel alone. + !!! note - Only videos generated by InvokeAI 7 or later carry this record. - Earlier releases kept it in a separate sidecar file next to the - video, which PhotoMapAI does not read. The **Remix** and **Recall** - buttons are not offered for videos. + The **Remix** and **Recall** buttons are not offered for videos: + InvokeAI's recall endpoint does not yet accept video parameters. --- diff --git a/photomap/backend/invokeai_sidecar.py b/photomap/backend/invokeai_sidecar.py new file mode 100644 index 00000000..18100b55 --- /dev/null +++ b/photomap/backend/invokeai_sidecar.py @@ -0,0 +1,140 @@ +"""The JSON sidecar InvokeAI wrote beside a video before it embedded metadata. + +Releases before InvokeAI 7 kept a generated video's generation record in a +separate file under ``{outputs}/videos/sidecars/``, mirroring the video's own +subfolder: + + outputs/videos/general/.mp4 + outputs/videos/sidecars/general/.json + + outputs/videos/.mp4 # no subfolder + outputs/videos/sidecars/.json + +The file holds the same three strings an InvokeAI 7 MP4 carries as keyed +metadata — ``invokeai_metadata``, ``invokeai_workflow`` and +``invokeai_graph`` — each a *stringified* JSON document, or null when that +generation produced none. In a 662-sidecar sample from a real install, 530 +carried a null record (workflow only) and 132 a real one, so "the file exists +but has nothing for us" is the common case, not an error. + +InvokeAI 7 still writes a sidecar when the metadata remux fails, so this is +not purely a legacy path. + +**Finding the sidecar from the video alone.** PhotoMapAI indexes absolute +file paths and never learns where an InvokeAI ``outputs`` directory begins, +so the videos root has to be guessed: each ancestor of the video is tried as +the root, nearest first, and the video's path relative to it is mirrored +under ``sidecars/``. The walk is bounded because InvokeAI's own subfolder is +one level deep in practice (the ``general`` / ``intermediate`` / ``user`` +category) even though its validator permits more. + +A false positive would need a directory literally named ``sidecars`` holding +a JSON file with the video's stem *and* an ``invokeai_metadata`` key, so the +key is required rather than assumed. +""" + +from __future__ import annotations + +import json +import logging +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +logger = logging.getLogger(__name__) + +# The directory InvokeAI puts sidecars in, relative to the videos root. +SIDECAR_DIRNAME = "sidecars" + +# How many ancestors of the video to try as the videos root. Measured +# against a real install: all 661 sidecars there were found at depth 1, the +# category directory. Depth 0 covers a videos root with no subfolder at all, +# and the rest is headroom, because ``video_subfolder`` is a path and +# InvokeAI's validator accepts more than one segment in it. Each unused +# level costs one ``is_file`` on a video that has no sidecar — the whole +# scan measured 1 ms per video including the MP4 walk. +MAX_SUBFOLDER_DEPTH = 3 + +# A sidecar holds a graph, which runs to a few hundred KiB. The cap only +# stops a hostile or corrupt file from being read into memory whole — which +# JSON parsing requires, unlike the MP4 walk. +MAX_SIDECAR_BYTES = 32 * 1024 * 1024 + +# The key holding the generation record. Required, so that an unrelated +# ``sidecars`` directory cannot be mistaken for InvokeAI's. +METADATA_KEY = "invokeai_metadata" + + +def sidecar_candidates(video_path: Path) -> Iterator[Path]: + """Where ``video_path``'s sidecar would be, nearest videos root first. + + Yields at most ``MAX_SUBFOLDER_DEPTH + 1`` paths and stops early at the + filesystem root. Nothing is touched on disk here. + """ + filename = video_path.stem + ".json" + parent = video_path.parent + for depth in range(MAX_SUBFOLDER_DEPTH + 1): + try: + root = video_path.parents[depth] + except IndexError: + # Ran out of ancestors before the depth limit. + return + # "" at depth 0, the category at depth 1, and so on. pathlib drops + # the "." that ``relative_to`` returns for the former. + subpath = parent.relative_to(root) + yield root / SIDECAR_DIRNAME / subpath / filename + + +def read_sidecar_metadata(video_path: Path) -> dict[str, Any]: + """The generation record from ``video_path``'s sidecar, or ``{}``. + + Returns ``{}`` when there is no sidecar, when the one found carries a + null record (the common case — 530 of 662 in the sample install), or + when it is unreadable, oversized, not JSON, or not an InvokeAI sidecar + at all. Nothing here raises: this runs once per video while indexing a + collection that contains whatever is on the user's disk. + + A candidate that exists but yields nothing usable does not end the + search — the next ancestor is still tried. + """ + for candidate in sidecar_candidates(video_path): + try: + if not candidate.is_file(): + continue + if candidate.stat().st_size > MAX_SIDECAR_BYTES: + logger.warning("Ignoring oversized sidecar %s", candidate) + continue + payload = json.loads(candidate.read_text(encoding="utf-8")) + except OSError as e: + logger.debug("Could not read sidecar %s: %s", candidate, e) + continue + except ValueError as e: + logger.warning("Sidecar %s is not valid JSON: %s", candidate, e) + continue + + if not isinstance(payload, dict) or METADATA_KEY not in payload: + # Not an InvokeAI sidecar — some other file that happens to sit + # where one would. + continue + + record = payload[METADATA_KEY] + if isinstance(record, str): + # The written shape: stringified, as into a PNG chunk or an MP4 + # tag. Every sidecar in the sample install is this or null. + try: + record = json.loads(record) + except ValueError as e: + logger.warning("Sidecar record in %s is not valid JSON: %s", candidate, e) + continue + if isinstance(record, dict): + return record + if record is not None: + logger.warning( + "Sidecar record in %s is a %s, not an object", + candidate, + type(record).__name__, + ) + # `null` is a real and common shape: a generation that produced a + # workflow but no record. Keep looking rather than treating the + # file's existence as the answer. + return {} diff --git a/photomap/backend/metadata_extraction.py b/photomap/backend/metadata_extraction.py index 7af1ada2..3969a3e1 100644 --- a/photomap/backend/metadata_extraction.py +++ b/photomap/backend/metadata_extraction.py @@ -14,6 +14,7 @@ from PIL import ExifTags, Image +from .invokeai_sidecar import read_sidecar_metadata from .mp4_metadata import INVOKEAI_METADATA_KEY, read_mp4_tags logger = logging.getLogger(__name__) @@ -81,22 +82,36 @@ def _json_from_chunk(key: str): @staticmethod def extract_video_metadata(video_path: Path) -> dict[str, Any]: - """The InvokeAI generation record embedded in a video, or ``{}``. + """The InvokeAI generation record for a video, or ``{}``. The video counterpart of :meth:`extract_image_metadata`: InvokeAI 7 writes the same ``invokeai_metadata`` JSON a generated PNG carries as a text chunk into a generated MP4 as keyed metadata, so both return the same flat record and everything downstream is media-agnostic. + Two sources, in InvokeAI's own reading order: the MP4's keyed + metadata first, then the JSON sidecar. The sidecar covers videos + generated before InvokeAI 7 embedded anything, and also the modern + case where the embedding remux failed and InvokeAI fell back to + writing one. Looking in the file first means an InvokeAI 7 video + costs no sidecar lookup at all. + Only the record is read, not the workflow or graph: PhotoMapAI - renders neither, and skipping them keeps the read to the few KiB of - the record rather than the few hundred KiB of a graph. + renders neither, and skipping them keeps the MP4 read to the few KiB + of the record rather than the few hundred KiB of a graph. - Returns ``{}`` for every failure — a video with no tags, a - non-InvokeAI video, an unreadable file, a tag that is not JSON, or - JSON that is not an object. The caller indexes the video either way; - metadata is never worth failing an index over. + Returns ``{}`` for every failure — no tags and no sidecar, a + non-InvokeAI video, an unreadable file, a payload that is not JSON, + or JSON that is not an object. The caller indexes the video either + way; metadata is never worth failing an index over. """ + return MetadataExtractor._embedded_video_metadata( + video_path + ) or read_sidecar_metadata(video_path) + + @staticmethod + def _embedded_video_metadata(video_path: Path) -> dict[str, Any]: + """The record in the MP4's own keyed metadata, or ``{}``.""" try: tags = read_mp4_tags(video_path, keys=(INVOKEAI_METADATA_KEY,)) except OSError as e: diff --git a/tests/backend/test_invokeai_sidecar.py b/tests/backend/test_invokeai_sidecar.py new file mode 100644 index 00000000..7232f8d9 --- /dev/null +++ b/tests/backend/test_invokeai_sidecar.py @@ -0,0 +1,275 @@ +"""The JSON sidecar fallback for videos generated before InvokeAI 7. + +Two things are under test and they fail differently. The *path derivation* +has to find a sidecar that is really there, from nothing but the video's +absolute path — PhotoMapAI never learns where an ``outputs`` directory +begins. The *reader* has to come back empty rather than raise for everything +else on a user's disk, because it runs once per video over whole +collections. + +The fixtures mirror the layout measured on a real install: + + /general/.mp4 + /sidecars/general/.json + +with `invokeai_metadata` a *stringified* JSON document or ``null``. In that +install 530 of 662 sidecars carried null and 132 a real record, so "exists +but has nothing for us" is the ordinary case rather than an error. +""" + +from __future__ import annotations + +import json +import shutil +from pathlib import Path + +import pytest +from fixtures import media_fixture_path + +from photomap.backend import invokeai_sidecar +from photomap.backend.invokeai_sidecar import ( + MAX_SUBFOLDER_DEPTH, + read_sidecar_metadata, + sidecar_candidates, +) +from photomap.backend.metadata_extraction import MetadataExtractor + +RECORD = { + "app_version": "6.14.0-alpha", + "generation_mode": "minimax_h3_extend_video", + "positive_prompt": "a paper boat", + "seed": 11, + "num_frames": 97, + "model": {"name": "MiniMax H3", "base": "minimax", "type": "main"}, +} + + +def write_sidecar(path, record=RECORD, workflow='{"name":"w"}'): + """A sidecar in InvokeAI's shape: the record *stringified*, or null.""" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps( + { + "invokeai_metadata": None if record is None else json.dumps(record), + "invokeai_workflow": workflow, + "invokeai_graph": None, + } + ), + encoding="utf-8", + ) + return path + + +@pytest.fixture +def invoke_outputs(tmp_path): + """``/general/clip.mp4`` with a real, untagged MP4 — the layout a + pre-7 install actually has.""" + root = tmp_path / "videos" + video = root / "general" / "clip.mp4" + video.parent.mkdir(parents=True) + shutil.copy(media_fixture_path("clip.mp4"), video) + return root, video + + +# -------------------------------------------------------------------------- +# Path derivation +# -------------------------------------------------------------------------- + + +def test_the_category_layout_is_the_second_candidate(tmp_path): + """Depth 1 is where every sidecar in the sampled install lives.""" + video = tmp_path / "videos" / "general" / "abc.mp4" + + candidates = list(sidecar_candidates(video)) + + assert candidates[0] == tmp_path / "videos" / "general" / "sidecars" / "abc.json" + assert candidates[1] == tmp_path / "videos" / "sidecars" / "general" / "abc.json" + + +def test_a_videos_root_with_no_subfolder_is_the_first_candidate(tmp_path): + video = tmp_path / "videos" / "abc.mp4" + + assert next(sidecar_candidates(video)) == ( + tmp_path / "videos" / "sidecars" / "abc.json" + ) + + +def test_the_walk_is_bounded(tmp_path): + video = tmp_path / "a" / "b" / "c" / "d" / "e" / "clip.mp4" + + assert len(list(sidecar_candidates(video))) == MAX_SUBFOLDER_DEPTH + 1 + + +def test_a_shallow_path_stops_at_the_filesystem_root(): + """Fewer ancestors than the depth limit must not raise.""" + candidates = list(sidecar_candidates(Path("/clip.mp4"))) + + assert candidates == [Path("/sidecars/clip.json")] + + +def test_the_extension_is_replaced_not_appended(tmp_path): + video = tmp_path / "videos" / "general" / "clip.mov" + + assert all(c.name == "clip.json" for c in sidecar_candidates(video)) + + +# -------------------------------------------------------------------------- +# Reading +# -------------------------------------------------------------------------- + + +def test_reads_a_record_from_the_category_layout(invoke_outputs): + root, video = invoke_outputs + write_sidecar(root / "sidecars" / "general" / "clip.json") + + assert read_sidecar_metadata(video)["generation_mode"] == ( + "minimax_h3_extend_video" + ) + + +def test_reads_a_record_from_a_flat_videos_root(tmp_path): + video = tmp_path / "videos" / "clip.mp4" + video.parent.mkdir(parents=True) + video.touch() + write_sidecar(tmp_path / "videos" / "sidecars" / "clip.json") + + assert read_sidecar_metadata(video)["seed"] == 11 + + +def test_a_null_record_yields_nothing(invoke_outputs): + """The majority case: a workflow was saved but no record.""" + root, video = invoke_outputs + write_sidecar(root / "sidecars" / "general" / "clip.json", record=None) + + assert read_sidecar_metadata(video) == {} + + +def test_no_sidecar_at_all_yields_nothing(invoke_outputs): + _root, video = invoke_outputs + + assert read_sidecar_metadata(video) == {} + + +def test_a_sidecar_for_a_different_video_is_not_used(invoke_outputs): + root, video = invoke_outputs + write_sidecar(root / "sidecars" / "general" / "other.json") + + assert read_sidecar_metadata(video) == {} + + +def test_a_sidecar_under_the_wrong_subfolder_is_not_used(invoke_outputs): + """The mirror has to be exact, or an ``intermediate`` record could be + served for a ``general`` video of the same name.""" + root, video = invoke_outputs + write_sidecar(root / "sidecars" / "intermediate" / "clip.json") + + assert read_sidecar_metadata(video) == {} + + +def test_a_json_file_that_is_not_an_invokeai_sidecar_is_ignored(invoke_outputs): + """A directory named ``sidecars`` need not be InvokeAI's.""" + root, video = invoke_outputs + path = root / "sidecars" / "general" / "clip.json" + path.parent.mkdir(parents=True) + path.write_text(json.dumps({"camera": "Pixel", "iso": 400}), encoding="utf-8") + + assert read_sidecar_metadata(video) == {} + + +@pytest.mark.parametrize( + "content", ["{not json", "[]", '"a string"', "null", ""] +) +def test_malformed_or_non_object_sidecars_yield_nothing(invoke_outputs, content): + root, video = invoke_outputs + path = root / "sidecars" / "general" / "clip.json" + path.parent.mkdir(parents=True) + path.write_text(content, encoding="utf-8") + + assert read_sidecar_metadata(video) == {} + + +def test_a_record_that_is_not_an_object_yields_nothing(invoke_outputs): + root, video = invoke_outputs + path = root / "sidecars" / "general" / "clip.json" + path.parent.mkdir(parents=True) + path.write_text(json.dumps({"invokeai_metadata": "[1,2,3]"}), encoding="utf-8") + + assert read_sidecar_metadata(video) == {} + + +def test_a_record_stored_unstringified_is_still_read(invoke_outputs): + """InvokeAI always stringifies it, but discarding a perfectly good + record over its encoding would be gratuitous.""" + root, video = invoke_outputs + path = root / "sidecars" / "general" / "clip.json" + path.parent.mkdir(parents=True) + path.write_text(json.dumps({"invokeai_metadata": RECORD}), encoding="utf-8") + + assert read_sidecar_metadata(video)["seed"] == 11 + + +def test_an_oversized_sidecar_is_not_read(invoke_outputs, monkeypatch): + """A sidecar must be read whole to be parsed, unlike the MP4 walk.""" + monkeypatch.setattr(invokeai_sidecar, "MAX_SIDECAR_BYTES", 16) + root, video = invoke_outputs + write_sidecar(root / "sidecars" / "general" / "clip.json") + + assert read_sidecar_metadata(video) == {} + + +def test_a_directory_where_the_sidecar_would_be_is_ignored(invoke_outputs): + root, video = invoke_outputs + (root / "sidecars" / "general" / "clip.json").mkdir(parents=True) + + assert read_sidecar_metadata(video) == {} + + +def test_a_useless_near_candidate_does_not_end_the_search(invoke_outputs): + """A null sidecar at depth 0 must not mask a real one at depth 1.""" + root, video = invoke_outputs + write_sidecar(root / "general" / "sidecars" / "clip.json", record=None) + write_sidecar(root / "sidecars" / "general" / "clip.json") + + assert read_sidecar_metadata(video)["seed"] == 11 + + +# -------------------------------------------------------------------------- +# Precedence, through the extractor +# -------------------------------------------------------------------------- + + +def test_the_sidecar_is_used_when_the_mp4_carries_nothing(invoke_outputs): + root, video = invoke_outputs + write_sidecar(root / "sidecars" / "general" / "clip.json") + + assert MetadataExtractor.extract_video_metadata(video)["seed"] == 11 + + +def test_embedded_metadata_wins_over_a_sidecar(tmp_path): + """InvokeAI 7 still writes a sidecar when its remux fails, so both can + exist; the file's own copy is the newer one.""" + root = tmp_path / "videos" + video = root / "general" / "invoke_video.mp4" + video.parent.mkdir(parents=True) + shutil.copy(media_fixture_path("invoke_video.mp4"), video) + write_sidecar(root / "sidecars" / "general" / "invoke_video.json") + + record = MetadataExtractor.extract_video_metadata(video) + + assert record["generation_mode"] == "wan_i2v" + assert record["seed"] == 1234567 + + +def test_a_tagged_video_never_looks_for_a_sidecar(tmp_path, monkeypatch): + """The fallback must cost an InvokeAI 7 video nothing.""" + video = tmp_path / "invoke_video.mp4" + shutil.copy(media_fixture_path("invoke_video.mp4"), video) + + called = [] + monkeypatch.setattr( + "photomap.backend.metadata_extraction.read_sidecar_metadata", + lambda path: called.append(path) or {}, + ) + + assert MetadataExtractor.extract_video_metadata(video) + assert called == [] From 2029f1e188ed09bbdcf753d5eadd7b278790bb50 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 20 Sep 2026 11:56:48 -0400 Subject: [PATCH 2/3] fix(video): close the gaps an adversarial review found in the sidecar reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six defects, each reproduced first, and the fixes mutation-tested after. - RecursionError escaped the reader. json.loads raises it on deeply nested JSON; it is a RuntimeError, so neither `except OSError` nor `except ValueError` caught it, and ~120 KB of brackets is three orders of magnitude under the size cap. The damage was not a lost record but a lost *video*: _load_video catches it, returns None, and the file is recorded as bad and left out of the album — exactly what the docstring promises cannot happen. Both json.loads sites were exposed; they now share a helper. - The `invokeai_metadata` key gate was weak and untested: it handed back whatever object another tool stored under that name, and removing the gate entirely left all 25 tests passing. The record must now also satisfy looks_like_invoke_metadata — the same test the drawer routes on, and one all 132 records in the reference install pass. - MAX_SUBFOLDER_DEPTH cut from 3 to 1. The bound had no real test (the old one imported the constant it was checking, so it passed at 1 and at 9) and no evidence: 661 of 661 real sidecars resolve at depth 1. The extra levels only bought reach *outside* the configured album — at depth 3, a path at the filesystem root. That also removes the `..` escape the review found, which needed depth 2. - The reader resolves the video path first, so a symlinked video finds the sidecar beside its target. sidecar_candidates stays pure and now documents that it expects a resolved path. - Size cap 32 MiB -> 8 MiB, matching mp4_metadata.MAX_TAG_BYTES, so the same record is not rejected from one source and accepted from the other. - UnicodeDecodeError was caught (it is a ValueError) but logged as a JSON error; split so the message names the right stage. Also pinned the empty-embedded-record fall-through, which the `or` cannot distinguish from "no tag", in both a test and the docstring. Re-measured against the reference install after the fixes: the same 135 records recovered from 668 videos, at 0.2 ms per video. Co-Authored-By: Claude Opus 5 (1M context) --- docs/user-guide/invokeai-integration.md | 8 +- photomap/backend/invokeai_sidecar.py | 85 +++++++++---- photomap/backend/metadata_extraction.py | 5 + tests/backend/test_invokeai_sidecar.py | 155 +++++++++++++++++++++++- 4 files changed, 222 insertions(+), 31 deletions(-) diff --git a/docs/user-guide/invokeai-integration.md b/docs/user-guide/invokeai-integration.md index d4080537..228eabb5 100644 --- a/docs/user-guide/invokeai-integration.md +++ b/docs/user-guide/invokeai-integration.md @@ -203,10 +203,10 @@ clickable thumbnail, exactly like an image's reference images. Videos generated before InvokeAI 7 carry no record inside the file, but InvokeAI kept one in a JSON sidecar under `outputs/videos/sidecars/`, and -PhotoMapAI reads that too — so an older clip shows the same panel. The file -itself is always preferred; the sidecar is consulted only when the video -carries nothing, which is also what happens on the rare occasion InvokeAI 7 -could not embed the record and fell back to writing one. +PhotoMapAI reads that too — so an older clip shows the same panel. The +sidecar is consulted only when the video itself carries no parameters, which +is also what happens on the rare occasion InvokeAI 7 could not embed the +record and fell back to writing one. !!! note Not every older video has recoverable parameters. A sidecar is written diff --git a/photomap/backend/invokeai_sidecar.py b/photomap/backend/invokeai_sidecar.py index 18100b55..0969561a 100644 --- a/photomap/backend/invokeai_sidecar.py +++ b/photomap/backend/invokeai_sidecar.py @@ -29,8 +29,9 @@ category) even though its validator permits more. A false positive would need a directory literally named ``sidecars`` holding -a JSON file with the video's stem *and* an ``invokeai_metadata`` key, so the -key is required rather than assumed. +a JSON file with the video's stem, an ``invokeai_metadata`` key, *and* a +record that looks like InvokeAI's — all three are required rather than +assumed, and the last is the same test the drawer routes on. """ from __future__ import annotations @@ -41,24 +42,31 @@ from pathlib import Path from typing import Any +from .metadata_modules.invokemetadata import looks_like_invoke_metadata + logger = logging.getLogger(__name__) # The directory InvokeAI puts sidecars in, relative to the videos root. SIDECAR_DIRNAME = "sidecars" # How many ancestors of the video to try as the videos root. Measured -# against a real install: all 661 sidecars there were found at depth 1, the -# category directory. Depth 0 covers a videos root with no subfolder at all, -# and the rest is headroom, because ``video_subfolder`` is a path and -# InvokeAI's validator accepts more than one segment in it. Each unused -# level costs one ``is_file`` on a video that has no sidecar — the whole -# scan measured 1 ms per video including the MP4 walk. -MAX_SUBFOLDER_DEPTH = 3 - -# A sidecar holds a graph, which runs to a few hundred KiB. The cap only -# stops a hostile or corrupt file from being read into memory whole — which -# JSON parsing requires, unlike the MP4 walk. -MAX_SIDECAR_BYTES = 32 * 1024 * 1024 +# against a real install: all 661 sidecars there resolved at depth 1, the +# category directory, and none at any other depth. Depth 0 covers a videos +# root with no subfolder at all. +# +# Deliberately not deeper. InvokeAI's ``video_subfolder`` is a path and its +# validator would accept more than one segment, but no caller writes one, +# and each extra level is a file the reader opens *outside* the album the +# user configured — at depth 3 that is a path at the filesystem root. Since +# the headroom buys nothing measurable, it is not worth that reach. +MAX_SUBFOLDER_DEPTH = 1 + +# A sidecar holds all three strings, the graph being the large one, and runs +# to a few hundred KiB. The cap only stops a hostile or corrupt file from +# being read into memory whole, which JSON parsing requires and the MP4 walk +# does not. Matched to ``mp4_metadata.MAX_TAG_BYTES`` so the same record is +# not rejected from one source and accepted from the other. +MAX_SIDECAR_BYTES = 8 * 1024 * 1024 # The key holding the generation record. Required, so that an unrelated # ``sidecars`` directory cannot be mistaken for InvokeAI's. @@ -69,7 +77,10 @@ def sidecar_candidates(video_path: Path) -> Iterator[Path]: """Where ``video_path``'s sidecar would be, nearest videos root first. Yields at most ``MAX_SUBFOLDER_DEPTH + 1`` paths and stops early at the - filesystem root. Nothing is touched on disk here. + filesystem root. Nothing is touched on disk here — pass a *resolved* + path, or a ``..`` in it will survive into the candidate and let the + kernel resolve it back out of the ``sidecars`` directory at open time. + :func:`read_sidecar_metadata` resolves before calling this. """ filename = video_path.stem + ".json" parent = video_path.parent @@ -97,21 +108,24 @@ def read_sidecar_metadata(video_path: Path) -> dict[str, Any]: A candidate that exists but yields nothing usable does not end the search — the next ancestor is still tried. """ - for candidate in sidecar_candidates(video_path): + for candidate in sidecar_candidates(Path(video_path).resolve()): try: if not candidate.is_file(): continue if candidate.stat().st_size > MAX_SIDECAR_BYTES: logger.warning("Ignoring oversized sidecar %s", candidate) continue - payload = json.loads(candidate.read_text(encoding="utf-8")) + text = candidate.read_text(encoding="utf-8") except OSError as e: logger.debug("Could not read sidecar %s: %s", candidate, e) continue - except ValueError as e: - logger.warning("Sidecar %s is not valid JSON: %s", candidate, e) + except UnicodeDecodeError as e: + # Split from the JSON failure below purely so the log names the + # right stage — this never reached the parser. + logger.warning("Sidecar %s is not UTF-8 text: %s", candidate, e) continue + payload = _loads(text, candidate) if not isinstance(payload, dict) or METADATA_KEY not in payload: # Not an InvokeAI sidecar — some other file that happens to sit # where one would. @@ -121,12 +135,18 @@ def read_sidecar_metadata(video_path: Path) -> dict[str, Any]: if isinstance(record, str): # The written shape: stringified, as into a PNG chunk or an MP4 # tag. Every sidecar in the sample install is this or null. - try: - record = json.loads(record) - except ValueError as e: - logger.warning("Sidecar record in %s is not valid JSON: %s", candidate, e) - continue + record = _loads(record, candidate) if isinstance(record, dict): + if not looks_like_invoke_metadata(record): + # The key alone is a weak gate: it would hand back whatever + # object some other tool stored under that name. Require the + # record to look like InvokeAI's, which is the same test the + # drawer routes on — and which all 132 records in the sample + # install pass. + logger.debug( + "Sidecar %s holds no recognisable InvokeAI record", candidate + ) + continue return record if record is not None: logger.warning( @@ -138,3 +158,20 @@ def read_sidecar_metadata(video_path: Path) -> dict[str, Any]: # workflow but no record. Keep looking rather than treating the # file's existence as the answer. return {} + + +def _loads(text: str, source: Path) -> Any: + """``json.loads``, returning ``None`` instead of raising. + + ``RecursionError`` is the reason this is a helper rather than one more + ``except`` clause: deeply nested JSON raises it, it is a ``RuntimeError`` + and so is caught by neither of the obvious guards, and ~120 KB of nested + brackets is three orders of magnitude under ``MAX_SIDECAR_BYTES``. It + escaping here does not merely lose the metadata — ``_load_video`` catches + it, returns ``None``, and the video is dropped from the album entirely. + """ + try: + return json.loads(text) + except (ValueError, RecursionError) as e: + logger.warning("Sidecar %s does not hold valid JSON: %s", source, e) + return None diff --git a/photomap/backend/metadata_extraction.py b/photomap/backend/metadata_extraction.py index 3969a3e1..8384346d 100644 --- a/photomap/backend/metadata_extraction.py +++ b/photomap/backend/metadata_extraction.py @@ -96,6 +96,11 @@ def extract_video_metadata(video_path: Path) -> dict[str, Any]: writing one. Looking in the file first means an InvokeAI 7 video costs no sidecar lookup at all. + Precedence is on the *record*, not on the source: a file carrying an + empty record falls through to the sidecar, because "the MP4 says + nothing" and "the MP4 has no tag" are worth the same and the sidecar + may still have something to show. + Only the record is read, not the workflow or graph: PhotoMapAI renders neither, and skipping them keeps the MP4 read to the few KiB of the record rather than the few hundred KiB of a graph. diff --git a/tests/backend/test_invokeai_sidecar.py b/tests/backend/test_invokeai_sidecar.py index 7232f8d9..a4604a91 100644 --- a/tests/backend/test_invokeai_sidecar.py +++ b/tests/backend/test_invokeai_sidecar.py @@ -28,7 +28,6 @@ from photomap.backend import invokeai_sidecar from photomap.backend.invokeai_sidecar import ( - MAX_SUBFOLDER_DEPTH, read_sidecar_metadata, sidecar_candidates, ) @@ -94,10 +93,33 @@ def test_a_videos_root_with_no_subfolder_is_the_first_candidate(tmp_path): ) -def test_the_walk_is_bounded(tmp_path): +def test_the_walk_stops_above_the_videos_root(tmp_path): + """Pinned by value, not by ``MAX_SUBFOLDER_DEPTH``. + + Asserting against the constant the loop uses proves only that the loop + uses it: the old form of this test passed with the bound set to 1 and + equally with it set to 9. What matters is the *reach* — every extra + level is a file opened outside the album the user configured, and at + depth 3 that is a path at the filesystem root. + """ video = tmp_path / "a" / "b" / "c" / "d" / "e" / "clip.mp4" - assert len(list(sidecar_candidates(video))) == MAX_SUBFOLDER_DEPTH + 1 + candidates = list(sidecar_candidates(video)) + + assert candidates == [ + tmp_path / "a/b/c/d/e" / "sidecars" / "clip.json", + tmp_path / "a/b/c/d" / "sidecars" / "e" / "clip.json", + ] + + +def test_a_sidecar_two_levels_up_is_out_of_reach(tmp_path): + """The bound is real, and this is the file it declines to open.""" + video = tmp_path / "videos" / "general" / "clip.mp4" + video.parent.mkdir(parents=True) + video.touch() + write_sidecar(tmp_path / "sidecars" / "videos" / "general" / "clip.json") + + assert read_sidecar_metadata(video) == {} def test_a_shallow_path_stops_at_the_filesystem_root(): @@ -176,6 +198,112 @@ def test_a_json_file_that_is_not_an_invokeai_sidecar_is_ignored(invoke_outputs): assert read_sidecar_metadata(video) == {} +def test_a_record_under_a_different_key_is_not_accepted(invoke_outputs): + """Pins the key gate itself. + + The test above passes without any key check at all, because its payload + has no dict record under *any* key — it is rejected further down. This + one puts a perfectly good-looking record under the wrong key, so only + the gate can reject it. + """ + root, video = invoke_outputs + path = root / "sidecars" / "general" / "clip.json" + path.parent.mkdir(parents=True) + path.write_text( + json.dumps({"metadata": {"positive_prompt": "x", "app_version": "6.0.0"}}), + encoding="utf-8", + ) + + assert read_sidecar_metadata(video) == {} + + +def test_a_record_that_does_not_look_like_invokeais_is_rejected(invoke_outputs): + """The key alone is a weak gate — it would hand back whatever object + another tool happened to store under that name.""" + root, video = invoke_outputs + path = root / "sidecars" / "general" / "clip.json" + path.parent.mkdir(parents=True) + path.write_text( + json.dumps({"invokeai_metadata": {"anything": "at all"}}), encoding="utf-8" + ) + + assert read_sidecar_metadata(video) == {} + + +def test_every_shape_of_real_record_still_passes_the_gate(invoke_outputs): + """The gate must not reject the records it exists to admit. These are + the marker keys the sampled pre-7 records actually carry.""" + root, video = invoke_outputs + path = root / "sidecars" / "general" / "clip.json" + for marker in ("app_version", "generation_mode", "num_frames"): + write_sidecar(path, record={marker: "6.14.0" if marker == "app_version" else 1}) + assert read_sidecar_metadata(video), marker + + +def test_deeply_nested_json_does_not_drop_the_video_from_the_index(invoke_outputs): + """``RecursionError`` is a ``RuntimeError``, so neither ``except OSError`` + nor ``except ValueError`` catches it. + + Escaping here is worse than losing the metadata: ``_load_video`` catches + it, returns None, and the video is recorded as a bad file and left out + of the album. ~120 KB of brackets, far under the size cap. + """ + root, video = invoke_outputs + path = root / "sidecars" / "general" / "clip.json" + path.parent.mkdir(parents=True) + path.write_text( + '{"invokeai_metadata": ' + "[" * 60000 + "]" * 60000 + "}", encoding="utf-8" + ) + + assert read_sidecar_metadata(video) == {} + assert MetadataExtractor.extract_video_metadata(video) == {} + + +def test_deep_nesting_inside_the_stringified_record_is_also_caught(invoke_outputs): + """The inner ``json.loads`` is a second, separate exposure.""" + root, video = invoke_outputs + path = root / "sidecars" / "general" / "clip.json" + path.parent.mkdir(parents=True) + path.write_text( + json.dumps({"invokeai_metadata": "[" * 60000 + "]" * 60000}), encoding="utf-8" + ) + + assert read_sidecar_metadata(video) == {} + + +def test_candidates_inherit_a_dot_dot_from_an_unresolved_path(tmp_path): + """Pins the precondition ``sidecar_candidates`` documents. + + It is pure and does no I/O, so a ``..`` in the input survives into the + candidate and the *kernel* resolves it at open time — which can land + outside any ``sidecars`` directory. That is why the reader resolves + before calling it, and this is the behaviour that makes it necessary. + """ + candidates = list(sidecar_candidates(Path("/a/b/../c/clip.mp4"))) + + assert ".." in str(candidates[0]) + + +def test_the_reader_resolves_the_video_path_first(tmp_path): + """A symlinked video must find the sidecar next to its *target*. + + Without resolution the lookup uses the link's own name and directory, + so a collection that symlinks InvokeAI output in finds nothing. + """ + root = tmp_path / "videos" + real = root / "general" / "real.mp4" + real.parent.mkdir(parents=True) + shutil.copy(media_fixture_path("clip.mp4"), real) + write_sidecar(root / "sidecars" / "general" / "real.json") + + album = tmp_path / "album" + album.mkdir() + link = album / "link.mp4" + link.symlink_to(real) + + assert read_sidecar_metadata(link)["seed"] == 11 + + @pytest.mark.parametrize( "content", ["{not json", "[]", '"a string"', "null", ""] ) @@ -273,3 +401,24 @@ def test_a_tagged_video_never_looks_for_a_sidecar(tmp_path, monkeypatch): assert MetadataExtractor.extract_video_metadata(video) assert called == [] + + +def test_an_empty_embedded_record_falls_through_to_the_sidecar(tmp_path, monkeypatch): + """Precedence is on the record, not the source. + + "The MP4 has no tag" and "the MP4's tag is an empty object" are worth + the same, and the sidecar may still have something to show. Pinned + because the ``or`` that implements it cannot tell the two apart, so the + behaviour is easy to change by accident. + """ + root = tmp_path / "videos" + video = root / "general" / "clip.mp4" + video.parent.mkdir(parents=True) + shutil.copy(media_fixture_path("clip.mp4"), video) + write_sidecar(root / "sidecars" / "general" / "clip.json") + monkeypatch.setattr( + "photomap.backend.metadata_extraction.read_mp4_tags", + lambda path, keys=None: {"invokeai_metadata": "{}"}, + ) + + assert MetadataExtractor.extract_video_metadata(video)["seed"] == 11 From 598e3e51f17d275ac83a9d4bd63203a929473685 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 20 Sep 2026 13:26:50 -0400 Subject: [PATCH 3/3] docs: an existing album needs a full re-index, not an Update Index The note shipped in #400 said an update "only re-reads files whose modification time has changed", which implies Update Index might pick up newly-readable metadata. It never will: _get_new_and_missing_images is a set difference on paths, and modification_times is stored and sorted but never consulted to decide re-processing, so a file already in the index is not re-read whatever its mtime. Confirmed on a real album: 363 indexed videos, 4 carrying a record, 89 more recovered only after rebuilding. Points at the Rebuild Index button from #402 rather than telling people to delete the index file by hand, and links to that button's own section, which covers what a rebuild costs and what it leaves alone. Co-Authored-By: Claude Opus 5 (1M context) --- docs/user-guide/invokeai-integration.md | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/docs/user-guide/invokeai-integration.md b/docs/user-guide/invokeai-integration.md index 228eabb5..bd93688a 100644 --- a/docs/user-guide/invokeai-integration.md +++ b/docs/user-guide/invokeai-integration.md @@ -195,11 +195,18 @@ panels together cover both the request and the result. Any keyframe or source clip that is also in the current album becomes a clickable thumbnail, exactly like an image's reference images. -!!! note - Videos already in an album when you upgrade keep whatever metadata - they were indexed with. Press **Update Index** and then, if a video - still shows no generation parameters, re-index the album: an update - only re-reads files whose modification time has changed. +!!! warning "Existing albums need a full re-index, not an update" + Generation metadata is read once, when a file is indexed, and stored in + the index — so videos that were already indexed keep whatever they were + indexed with, and show no parameters until the album is rebuilt. + + Update Index will **not** do it. + An update compares the files on disk against the ones in the index and + processes only what was added or removed; a file already in the index is + never re-read, whatever its modification time. Press the red + Rebuild Index button underneath it + instead — see [Rebuilding an index from + scratch](albums.md#rebuilding-an-index-from-scratch). Videos generated before InvokeAI 7 carry no record inside the file, but InvokeAI kept one in a JSON sidecar under `outputs/videos/sidecars/`, and