fix(inspect): video metadata carrier parity between Images and Inspect (#275)
* fix(video): select one prompt carrier from per-key or comment tags Current ComfyUI savers write one container tag per key (prompt, workflow, parameters, comfyassets_prompt_pairs); only the old VideoHelperSuite comment envelope was read. select_video_carrier picks exactly one carrier in the order per-key tags, comment envelope, description: the first with a role key wins, else the first with a workflow, and carriers never merge. Matroska upper-cased tag names fold on our side; differing fold-duplicates fail the read like duplicate PNG keys, identical ones are one record. Per-key values stay original-text; envelope objects are reserialized. * fix(inspect): read per-key video prompt tags The inspector asked ffprobe only for comment and description, so videos written by core SaveVideo/SaveWEBM and current VideoHelperSuite (one tag per key) were inspected as having no metadata. It now asks for prompt,workflow,parameters,comfyassets_prompt_pairs,comment,description and hands the tags to the shared carrier selector. ffprobe stdout is bounded at the role and workflow budgets plus 64 KiB of envelope; past that it probes once more without the workflow tag within the role budget and marks the per-key workflow limited, while an over-budget envelope still fails the read. Differing fold-duplicate tags fail like duplicate PNG keys. * fix(video): scan videos written by current ComfyUI savers The output scan and live monitor read container tags through metadata_from_tags, which now delegates to the shared carrier selector, so per-key prompt, workflow, parameters and comfyassets_prompt_pairs tags (including Matroska upper-cased names) are ingested with the default resolver profile exactly like the same graph in a PNG. Fold-duplicate tags and null markers are skipped quietly. Pins the behavior with a mocked ffprobe scan test and real MP4/MKV/WebM round trips when ffmpeg is present. * docs(testing): document video carrier selection Record the shared video carrier order (per-key tags, comment envelope, description), the fold-duplicate rule, the 6 MiB + 64 KiB probe bound with the role-only re-probe and limited workflow, and that scans read the same carrier as the inspector. * fix(inspect): size video probe bounds for JSON escaping, exact-case envelopes ffprobe JSON-escapes tag text, so bounds measured on its stdout could mark a video workflow limited, or fail a role record, that the identical PNG reads within its decoded budget. The first probe now allows twice the role and workflow budgets plus 64 KiB (12 MiB + 64 KiB) and the role-only re-probe twice the role budget plus 64 KiB, leaving the decoded per-record budgets as the only gate; a reader-level test reads near-budget quote-dense records as PNG and as video and asserts identical metadata and formats. Envelope keys (comment, description) are matched exactly as the legacy readers did; only the four per-key carriers fold Matroska case. Fixtures can write the legacy ilst comment atom (metadata_tags=False), and the docs now say the admin scan reads the carrier while the live monitor still processes images only, with a follow-up recorded in TESTING.md.
This commit is contained in:
Vito
+52
-1
@@ -2476,7 +2476,8 @@ def _write_mp4(path, comment):
|
||||
|
||||
|
||||
class TestScanReadsVideos(AdminAPITestCase):
|
||||
"""Videos saved by ComfyUI carry the graph in their comment tag."""
|
||||
"""Videos carry the graph in per-key container tags (current savers) or in
|
||||
the old VideoHelperSuite comment envelope; the scan reads one carrier."""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
@@ -2506,6 +2507,56 @@ class TestScanReadsVideos(AdminAPITestCase):
|
||||
images = self.api.db.get_prompt_images(prompts[0]["id"])
|
||||
self.assertTrue(images[0]["image_path"].endswith("clip.mp4"))
|
||||
|
||||
async def test_scan_links_per_key_tag_video_with_default_profile(self):
|
||||
"""A current saver's per-key tags scan exactly like the same graph as PNG."""
|
||||
from utils import video_metadata
|
||||
|
||||
_write_png(self.output_dir / "same.png", prompt_text="a cat")
|
||||
graph = {
|
||||
"1": {"class_type": "CLIPTextEncode", "inputs": {"text": "a cat"}},
|
||||
"2": {"class_type": "CLIPTextEncode", "inputs": {"text": "bad"}},
|
||||
"3": {
|
||||
"class_type": "KSampler",
|
||||
"inputs": {"positive": ["1", 0], "negative": ["2", 0]},
|
||||
},
|
||||
}
|
||||
(self.output_dir / "clip.mkv").write_bytes(b"\x1aE\xdf\xa3 fixture")
|
||||
probed = {
|
||||
"format": {
|
||||
"tags": {
|
||||
"PROMPT": json.dumps(graph),
|
||||
"WORKFLOW": '{"nodes":[]}',
|
||||
"COMMENT": json.dumps({"prompt": "must not win"}),
|
||||
}
|
||||
}
|
||||
}
|
||||
GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)]
|
||||
|
||||
with (
|
||||
patch.object(video_metadata.shutil, "which", return_value="/bin/ffprobe"),
|
||||
patch.object(
|
||||
video_metadata.subprocess,
|
||||
"run",
|
||||
return_value=MagicMock(returncode=0, stdout=json.dumps(probed)),
|
||||
) as run,
|
||||
self.assertNoLogs(level="ERROR"),
|
||||
):
|
||||
body = await (
|
||||
await self.client.request("POST", "/prompt_manager/scan")
|
||||
).text()
|
||||
|
||||
self.assertEqual(run.call_args.args[0][-1], str(self.output_dir / "clip.mkv"))
|
||||
self.assertIn('"found": 2', body)
|
||||
prompts = self.api.db.get_recent_prompts(limit=10)["prompts"]
|
||||
self.assertEqual(sorted(p["text"] for p in prompts), ["a cat", "bad"])
|
||||
positive = next(p for p in prompts if p["text"] == "a cat")
|
||||
images = self.api.db.get_prompt_images(positive["id"])
|
||||
self.assertEqual(
|
||||
sorted(os.path.basename(image["image_path"]) for image in images),
|
||||
["clip.mkv", "same.png"],
|
||||
)
|
||||
self.assertNotIn("must not win", json.dumps(prompts))
|
||||
|
||||
async def test_scan_without_ffprobe_skips_videos_quietly(self):
|
||||
from unittest.mock import patch
|
||||
from utils import video_metadata
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
"""Physical-file prompt inspection is bounded, conservative and read-only."""
|
||||
|
||||
import io
|
||||
import json
|
||||
from unittest.mock import patch
|
||||
import shutil
|
||||
import subprocess
|
||||
import unittest
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from PIL import Image, PngImagePlugin
|
||||
|
||||
@@ -15,6 +19,9 @@ except ImportError:
|
||||
from utils.pair_execution import EXECUTION_KEY, record_pair_execution
|
||||
from database.pair_ingestion import ingest_resolved_pair
|
||||
from utils.prompt_pair_resolver import resolve_prompt_pair
|
||||
from utils import video_metadata
|
||||
|
||||
HAVE_FFMPEG = bool(shutil.which("ffmpeg") and shutil.which("ffprobe"))
|
||||
|
||||
|
||||
def graph(positive="portrait", negative="blurry"):
|
||||
@@ -28,6 +35,54 @@ def graph(positive="portrait", negative="blurry"):
|
||||
}
|
||||
|
||||
|
||||
def write_video(path, metadata_tags=True, **tags):
|
||||
"""Tiny black clip carrying one container tag per keyword (needs ffmpeg).
|
||||
|
||||
MP4 keeps arbitrary keys only with ``use_metadata_tags`` (mdta atoms), the
|
||||
movflag the core SaveVideo node sets; without it ffmpeg writes the legacy
|
||||
ilst atoms that old VideoHelperSuite files carry. Matroska and WebM
|
||||
upper-case the key names.
|
||||
"""
|
||||
command = [
|
||||
"ffmpeg",
|
||||
"-v",
|
||||
"quiet",
|
||||
"-f",
|
||||
"lavfi",
|
||||
"-i",
|
||||
"color=c=black:s=16x16:d=0.2",
|
||||
]
|
||||
if metadata_tags and path.suffix.lower() in (".mp4", ".mov", ".m4v"):
|
||||
command += ["-movflags", "use_metadata_tags"]
|
||||
for key, value in tags.items():
|
||||
command += ["-metadata", f"{key}={value}"]
|
||||
subprocess.run(command + ["-y", str(path)], check=True, timeout=60)
|
||||
return path
|
||||
|
||||
|
||||
class ProbeOutput(io.BytesIO):
|
||||
"""ffprobe stdout that records the bounded read size asked of it."""
|
||||
|
||||
requested_size = None
|
||||
|
||||
def read(self, size=-1):
|
||||
self.requested_size = size
|
||||
return super().read(size)
|
||||
|
||||
|
||||
def probe_process(payload, poll=0, returncode=0):
|
||||
"""A fake ffprobe whose stdout is ``payload`` bytes."""
|
||||
process = MagicMock()
|
||||
process.stdout = ProbeOutput(payload)
|
||||
process.poll.return_value = poll
|
||||
process.returncode = returncode
|
||||
return process
|
||||
|
||||
|
||||
def probe_payload(tags):
|
||||
return json.dumps({"format": {"tags": tags}}).encode()
|
||||
|
||||
|
||||
def png(path, **metadata):
|
||||
info = PngImagePlugin.PngInfo()
|
||||
for key, value in metadata.items():
|
||||
@@ -312,7 +367,7 @@ class TestPromptInspector(ImageAPITestCase):
|
||||
async def test_corrupt_truncated_duplicate_and_expanding_metadata_fail_closed(self):
|
||||
import struct
|
||||
import zlib
|
||||
from utils.prompt_inspector import MAX_METADATA_BYTES
|
||||
from utils.prompt_inspector import VIDEO_REPROBE_BYTES
|
||||
|
||||
path = self.output_dir / "image.png"
|
||||
base = png(path, prompt=graph()).read_bytes()
|
||||
@@ -325,8 +380,8 @@ class TestPromptInspector(ImageAPITestCase):
|
||||
cases.append(bytes(broken))
|
||||
for kind, payload in (
|
||||
(b"tEXt", b"prompt\0{}"),
|
||||
(b"zTXt", b"prompt\0\0" + zlib.compress(b"x" * (MAX_METADATA_BYTES + 1))),
|
||||
(b"tEXt", b"prompt\0" + b"x" * (MAX_METADATA_BYTES + 1)),
|
||||
(b"zTXt", b"prompt\0\0" + zlib.compress(b"x" * (VIDEO_REPROBE_BYTES + 1))),
|
||||
(b"tEXt", b"prompt\0" + b"x" * (VIDEO_REPROBE_BYTES + 1)),
|
||||
(b"tEXt", b"bad-key-without-separator"),
|
||||
(b"zTXt", b"prompt\0\1bad-compression"),
|
||||
(b"iTXt", b"prompt\0\2\0\0\0text"),
|
||||
@@ -385,9 +440,7 @@ class TestPromptInspector(ImageAPITestCase):
|
||||
self.assertEqual(result["prompts"]["positive"]["status"], "absent")
|
||||
|
||||
async def test_video_inspection_uses_bounded_container_evidence(self):
|
||||
import io
|
||||
from unittest.mock import MagicMock
|
||||
from utils.prompt_inspector import MAX_METADATA_BYTES
|
||||
from utils.prompt_inspector import VIDEO_PROBE_BYTES, VIDEO_REPROBE_BYTES
|
||||
|
||||
(self.output_dir / "clip.mp4").write_bytes(b"video fixture")
|
||||
metadata = {"prompt": graph("Video portrait", "")}
|
||||
@@ -412,14 +465,25 @@ class TestPromptInspector(ImageAPITestCase):
|
||||
process.wait.assert_called()
|
||||
self.assertNotIn("shell", spawn.call_args.kwargs)
|
||||
self.assertIn(str(self.output_dir / "clip.mp4"), spawn.call_args.args[0])
|
||||
process.stdout = io.BytesIO(b"x" * (MAX_METADATA_BYTES + 1))
|
||||
# An envelope over the probe bound cannot be split: the role-only
|
||||
# re-probe overflows too and the read fails instead of guessing.
|
||||
processes = [
|
||||
probe_process(b"x" * (VIDEO_PROBE_BYTES + 1)),
|
||||
probe_process(b"x" * (VIDEO_REPROBE_BYTES + 1)),
|
||||
]
|
||||
with (
|
||||
patch("utils.prompt_inspector.ffprobe_path", return_value="/tool/ffprobe"),
|
||||
patch("utils.prompt_inspector.subprocess.Popen", return_value=process),
|
||||
patch(
|
||||
"utils.prompt_inspector.subprocess.Popen", side_effect=processes
|
||||
) as spawn,
|
||||
):
|
||||
_, result = await self.inspect("clip.mp4")
|
||||
self.assertEqual(spawn.call_count, 2)
|
||||
self.assertEqual(result["prompts"]["positive"]["status"], "unresolved")
|
||||
self.assertTrue(process.stdout.closed)
|
||||
self.assertEqual(result["metadata"]["status"], "unreadable")
|
||||
for process in processes:
|
||||
self.assertTrue(process.stdout.closed)
|
||||
process.wait.assert_called()
|
||||
|
||||
async def test_video_missing_probe_and_timeout_preserve_uncertainty(self):
|
||||
import io
|
||||
@@ -895,11 +959,9 @@ class TestPromptInspector(ImageAPITestCase):
|
||||
self.assertNotIn("\ufffd".encode(), body)
|
||||
self.assertLessEqual(len(body), 8 * 1024 * 1024)
|
||||
|
||||
async def video_inspect(self, comment=None, description=None):
|
||||
import io
|
||||
async def video_inspect(self, comment=None, description=None, **per_key):
|
||||
import threading
|
||||
from unittest.mock import MagicMock
|
||||
from utils.prompt_inspector import MAX_METADATA_BYTES
|
||||
from utils.prompt_inspector import VIDEO_PROBE_BYTES
|
||||
|
||||
path = self.output_dir / "clip.mp4"
|
||||
path.write_bytes(b"fixture")
|
||||
@@ -908,17 +970,8 @@ class TestPromptInspector(ImageAPITestCase):
|
||||
for key, value in (("comment", comment), ("description", description))
|
||||
if value is not None
|
||||
}
|
||||
payload = json.dumps({"format": {"tags": tags}}).encode()
|
||||
|
||||
class ProbeOutput(io.BytesIO):
|
||||
def read(self, size=-1):
|
||||
self.requested_size = size
|
||||
return super().read(size)
|
||||
|
||||
process = MagicMock()
|
||||
process.stdout = ProbeOutput(payload)
|
||||
process.poll.return_value = 0
|
||||
process.returncode = 0
|
||||
tags.update(per_key)
|
||||
process = probe_process(probe_payload(tags))
|
||||
timers = []
|
||||
timer_type = threading.Timer
|
||||
|
||||
@@ -937,11 +990,156 @@ class TestPromptInspector(ImageAPITestCase):
|
||||
response, result = await self.inspect("clip.mp4")
|
||||
self.assertEqual(spawn.call_count, 1)
|
||||
self.assertTrue(process.stdout.closed)
|
||||
self.assertEqual(process.stdout.requested_size, MAX_METADATA_BYTES + 1)
|
||||
self.assertEqual(process.stdout.requested_size, VIDEO_PROBE_BYTES + 1)
|
||||
self.assertEqual(len(timers), 1)
|
||||
self.assertTrue(timers[0].finished.is_set())
|
||||
self.assertEqual(
|
||||
spawn.call_args.args[0][spawn.call_args.args[0].index("-show_entries") + 1],
|
||||
"format_tags=prompt,workflow,parameters,"
|
||||
"comfyassets_prompt_pairs,comment,description",
|
||||
)
|
||||
return response, result
|
||||
|
||||
async def test_video_separate_prompt_workflow_tags_are_read(self):
|
||||
source = graph("per-key portrait", "per-key negative")
|
||||
workflow = '{"nodes":[{"id":"per-key"}],"cfg":6.5000000000000000001}'
|
||||
for tags in (
|
||||
{"prompt": json.dumps(source), "workflow": workflow},
|
||||
{"PROMPT": json.dumps(source), "WORKFLOW": workflow},
|
||||
):
|
||||
_, result = await self.video_inspect(
|
||||
json.dumps({"prompt": graph("comment must not win")}), **tags
|
||||
)
|
||||
self.assertEqual(
|
||||
result["prompts"]["positive"], self.side("per-key portrait")
|
||||
)
|
||||
self.assertEqual(
|
||||
result["prompts"]["negative"], self.side("per-key negative")
|
||||
)
|
||||
self.assertEqual(
|
||||
result["metadata"]["record_formats"],
|
||||
{"prompt": "original-text", "workflow": "original-text"},
|
||||
)
|
||||
self.assertEqual(result["metadata"]["raw"]["workflow"], workflow)
|
||||
self.assertEqual(
|
||||
result["metadata"]["workflow_graph"]["value"]["cfg"],
|
||||
"6.5000000000000000001",
|
||||
)
|
||||
self.assertNotIn("must not win", json.dumps(result))
|
||||
_, result = await self.video_inspect(
|
||||
**{"PROMPT": json.dumps(source), "prompt": json.dumps(graph("other"))}
|
||||
)
|
||||
self.assertEqual(result["prompts"]["positive"]["status"], "unresolved")
|
||||
self.assertEqual(result["metadata"]["status"], "unreadable")
|
||||
|
||||
@unittest.skipUnless(HAVE_FFMPEG, "ffmpeg/ffprobe not installed")
|
||||
async def test_video_per_key_and_comment_tags_read_from_real_containers(self):
|
||||
video_metadata.ffprobe_path.cache_clear()
|
||||
self.addCleanup(video_metadata.ffprobe_path.cache_clear)
|
||||
source = json.dumps(graph("real portrait", ""))
|
||||
workflow = '{"nodes":[{"id":"real"}]}'
|
||||
for name in ("per_key.mp4", "per_key.mkv", "per_key.webm"):
|
||||
write_video(self.output_dir / name, prompt=source, workflow=workflow)
|
||||
response, result = await self.inspect(name)
|
||||
self.assertEqual(response.status, 200, name)
|
||||
self.assertEqual(
|
||||
result["prompts"]["positive"], self.side("real portrait"), name
|
||||
)
|
||||
self.assertEqual(result["metadata"]["raw"]["workflow"], workflow, name)
|
||||
self.assertEqual(
|
||||
result["metadata"]["record_formats"],
|
||||
{"prompt": "original-text", "workflow": "original-text"},
|
||||
name,
|
||||
)
|
||||
# Old VideoHelperSuite envelope in the legacy ilst comment atom, unchanged.
|
||||
write_video(
|
||||
self.output_dir / "vhs.mp4",
|
||||
metadata_tags=False,
|
||||
comment=json.dumps({"prompt": json.loads(source), "workflow": {"v": 1}}),
|
||||
)
|
||||
_, result = await self.inspect("vhs.mp4")
|
||||
self.assertEqual(result["prompts"]["positive"], self.side("real portrait"))
|
||||
self.assertEqual(
|
||||
result["metadata"]["record_formats"],
|
||||
{"prompt": "reserialized-json", "workflow": "reserialized-json"},
|
||||
)
|
||||
self.assertEqual(result["metadata"]["workflow_graph"]["value"], {"v": 1})
|
||||
|
||||
async def test_video_oversize_workflow_reprobes_role_only_and_marks_limited(self):
|
||||
from utils.prompt_inspector import VIDEO_PROBE_BYTES, VIDEO_REPROBE_BYTES
|
||||
|
||||
path = self.output_dir / "clip.mp4"
|
||||
path.write_bytes(b"fixture")
|
||||
source = json.dumps(graph("bounded portrait", ""))
|
||||
processes = [
|
||||
probe_process(b"x" * (VIDEO_PROBE_BYTES + 1)),
|
||||
probe_process(probe_payload({"PROMPT": source})),
|
||||
]
|
||||
with (
|
||||
patch("utils.prompt_inspector.ffprobe_path", return_value="/tool/ffprobe"),
|
||||
patch(
|
||||
"utils.prompt_inspector.subprocess.Popen", side_effect=processes
|
||||
) as spawn,
|
||||
):
|
||||
response, result = await self.inspect("clip.mp4")
|
||||
self.assertEqual(response.status, 200)
|
||||
self.assertEqual(spawn.call_count, 2)
|
||||
first, second = (call.args[0] for call in spawn.call_args_list)
|
||||
self.assertEqual(
|
||||
first[first.index("-show_entries") + 1],
|
||||
"format_tags=prompt,workflow,parameters,"
|
||||
"comfyassets_prompt_pairs,comment,description",
|
||||
)
|
||||
self.assertEqual(
|
||||
second[second.index("-show_entries") + 1],
|
||||
"format_tags=prompt,parameters,comfyassets_prompt_pairs,"
|
||||
"comment,description",
|
||||
)
|
||||
self.assertEqual(processes[0].stdout.requested_size, VIDEO_PROBE_BYTES + 1)
|
||||
self.assertEqual(processes[1].stdout.requested_size, VIDEO_REPROBE_BYTES + 1)
|
||||
for process in processes:
|
||||
self.assertTrue(process.stdout.closed)
|
||||
process.wait.assert_called()
|
||||
self.assertEqual(result["prompts"]["positive"], self.side("bounded portrait"))
|
||||
self.assertEqual(result["metadata"]["status"], "available")
|
||||
self.assertEqual(
|
||||
result["metadata"]["record_formats"],
|
||||
{"prompt": "original-text", "workflow": "limited"},
|
||||
)
|
||||
self.assertNotIn("workflow", result["metadata"]["raw"])
|
||||
self.assertEqual(result["metadata"]["workflow_graph"]["status"], "limited")
|
||||
# The dropped per-key workflow belonged to an unselected carrier when
|
||||
# an envelope wins the re-probe: that envelope is whole, not limited.
|
||||
envelope = json.dumps({"prompt": json.loads(source), "workflow": {"v": 2}})
|
||||
processes = [
|
||||
probe_process(b"x" * (VIDEO_PROBE_BYTES + 1)),
|
||||
probe_process(probe_payload({"comment": envelope})),
|
||||
]
|
||||
with (
|
||||
patch("utils.prompt_inspector.ffprobe_path", return_value="/tool/ffprobe"),
|
||||
patch("utils.prompt_inspector.subprocess.Popen", side_effect=processes),
|
||||
):
|
||||
_, result = await self.inspect("clip.mp4")
|
||||
self.assertEqual(result["prompts"]["positive"], self.side("bounded portrait"))
|
||||
self.assertEqual(result["metadata"]["workflow_graph"]["value"], {"v": 2})
|
||||
self.assertEqual(
|
||||
result["metadata"]["record_formats"]["workflow"], "reserialized-json"
|
||||
)
|
||||
# A workflow-only per-key video over budget is limited, not absent.
|
||||
processes = [
|
||||
probe_process(b"x" * (VIDEO_PROBE_BYTES + 1)),
|
||||
probe_process(probe_payload({})),
|
||||
]
|
||||
with (
|
||||
patch("utils.prompt_inspector.ffprobe_path", return_value="/tool/ffprobe"),
|
||||
patch("utils.prompt_inspector.subprocess.Popen", side_effect=processes),
|
||||
):
|
||||
_, result = await self.inspect("clip.mp4")
|
||||
self.assertEqual(result["metadata"]["status"], "available")
|
||||
self.assertEqual(result["metadata"]["record_formats"], {"workflow": "limited"})
|
||||
self.assertEqual(result["metadata"]["workflow_graph"]["status"], "limited")
|
||||
self.assertEqual(result["prompts"]["positive"]["status"], "absent")
|
||||
|
||||
async def test_video_role_description_wins_over_workflow_only_comment(self):
|
||||
evidence = {}
|
||||
source = graph("chosen", "")
|
||||
|
||||
@@ -823,6 +823,88 @@ def _png_with_chunks(path, chunks):
|
||||
return path
|
||||
|
||||
|
||||
class TestVideoProbeBoundsMatchPngBudgets(unittest.TestCase):
|
||||
"""ffprobe escapes tag text; the stdout bounds must never gate a record
|
||||
that the identical PNG reads within its decoded budget (I3)."""
|
||||
|
||||
@staticmethod
|
||||
def _dense(size):
|
||||
# Quote-dense JSON text: every other byte escapes, so ffprobe's JSON
|
||||
# output is close to twice the decoded size.
|
||||
unit = '{"k":"v",' + "".join('"' for _ in range(6)) + "}"
|
||||
return (unit * (size // len(unit) + 1))[:size]
|
||||
|
||||
def _video_read(self, path, tags, bound):
|
||||
payload = json.dumps({"format": {"tags": tags}}).encode()
|
||||
self.assertGreater(len(payload), bound)
|
||||
process = MagicMock()
|
||||
process.stdout = io.BytesIO(payload)
|
||||
process.poll.return_value = 0
|
||||
process.returncode = 0
|
||||
with (
|
||||
patch("utils.prompt_inspector.ffprobe_path", return_value="/tool/ffprobe"),
|
||||
patch(
|
||||
"utils.prompt_inspector.subprocess.Popen", return_value=process
|
||||
) as spawn,
|
||||
):
|
||||
read = read_inspection_file(path)
|
||||
self.assertEqual(spawn.call_count, 1)
|
||||
self.assertTrue(process.stdout.closed)
|
||||
return read
|
||||
|
||||
def test_near_budget_records_read_the_same_as_png(self):
|
||||
from utils.prompt_inspector import (
|
||||
ROLE_METADATA_BYTES,
|
||||
VIDEO_PROBE_BYTES,
|
||||
VIDEO_REPROBE_BYTES,
|
||||
WORKFLOW_METADATA_BYTES,
|
||||
)
|
||||
|
||||
prompt = self._dense(ROLE_METADATA_BYTES - 1024)
|
||||
workflow = self._dense(WORKFLOW_METADATA_BYTES - 1024)
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
png = _png_with_chunks(
|
||||
Path(directory) / "same.png",
|
||||
[
|
||||
_text_chunk(b"tEXt", "prompt", prompt.encode("latin-1")),
|
||||
_text_chunk(b"tEXt", "workflow", workflow.encode("latin-1")),
|
||||
],
|
||||
)
|
||||
expected = read_inspection_file(png)
|
||||
self.assertEqual(
|
||||
expected.record_formats,
|
||||
{"prompt": "original-text", "workflow": "original-text"},
|
||||
)
|
||||
video = Path(directory) / "same.mp4"
|
||||
video.write_bytes(b"fixture")
|
||||
# Escaped output is over the old 6 MiB + 64 KiB stdout bound.
|
||||
read = self._video_read(
|
||||
video,
|
||||
{"prompt": prompt, "workflow": workflow},
|
||||
ROLE_METADATA_BYTES + WORKFLOW_METADATA_BYTES + 64 * 1024,
|
||||
)
|
||||
self.assertEqual(read.metadata, expected.metadata)
|
||||
self.assertEqual(read.record_formats, expected.record_formats)
|
||||
self.assertEqual(read.limited_records, frozenset())
|
||||
self.assertGreaterEqual(
|
||||
VIDEO_PROBE_BYTES,
|
||||
2 * (ROLE_METADATA_BYTES + WORKFLOW_METADATA_BYTES) + 64 * 1024,
|
||||
)
|
||||
self.assertGreaterEqual(
|
||||
VIDEO_REPROBE_BYTES, 2 * ROLE_METADATA_BYTES + 64 * 1024
|
||||
)
|
||||
# A role record near its budget alone escapes past 2 MiB and must
|
||||
# still read as the PNG does.
|
||||
png = _png_with_chunks(
|
||||
Path(directory) / "role.png",
|
||||
[_text_chunk(b"tEXt", "prompt", prompt.encode("latin-1"))],
|
||||
)
|
||||
expected = read_inspection_file(png)
|
||||
read = self._video_read(video, {"prompt": prompt}, ROLE_METADATA_BYTES)
|
||||
self.assertEqual(read.metadata, expected.metadata)
|
||||
self.assertEqual(read.record_formats, {"prompt": "original-text"})
|
||||
|
||||
|
||||
class TestPerRecordBudgets(unittest.TestCase):
|
||||
"""Role records keep the 2 MiB budget; a workflow has its own 4 MiB budget."""
|
||||
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
"""Tests for utils/video_metadata.py: prompt metadata from video containers.
|
||||
|
||||
ComfyUI's video savers store ``{"prompt": ..., "workflow": ...}`` as JSON in
|
||||
the container's ``comment`` tag. ffprobe reads it; when ffprobe is missing
|
||||
videos are skipped without noise.
|
||||
Current ComfyUI savers (core SaveVideo/SaveWEBM, VideoHelperSuite) write one
|
||||
container tag per key (``prompt``, ``workflow``, ``parameters``,
|
||||
``comfyassets_prompt_pairs``); older VideoHelperSuite wrote one ``comment``
|
||||
JSON envelope, with ``description`` as a fallback. One selector picks a single
|
||||
carrier for the inspector and the scan. ffprobe reads the tags; when ffprobe
|
||||
is missing videos are skipped without noise.
|
||||
"""
|
||||
|
||||
import json
|
||||
@@ -19,34 +22,44 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||
from utils import video_metadata # noqa: E402
|
||||
from utils.video_metadata import ( # noqa: E402
|
||||
VIDEO_EXTENSIONS,
|
||||
DuplicateVideoTagError,
|
||||
is_video_path,
|
||||
metadata_from_tags,
|
||||
read_video_metadata,
|
||||
select_video_carrier,
|
||||
)
|
||||
|
||||
HAVE_FFMPEG = bool(shutil.which("ffmpeg") and shutil.which("ffprobe"))
|
||||
GRAPH = json.dumps({"1": {"class_type": "CLIPTextEncode", "inputs": {"text": "a cat"}}})
|
||||
|
||||
|
||||
def write_video(path, metadata_tags=True, **tags):
|
||||
"""Write a tiny black video carrying one container tag per keyword.
|
||||
|
||||
MP4 keeps arbitrary keys only with ``use_metadata_tags`` (mdta atoms), the
|
||||
movflag the core SaveVideo node sets; without it ffmpeg writes the legacy
|
||||
ilst atoms (``comment`` becomes ``©cmt``) that old VideoHelperSuite files
|
||||
carry. Matroska and WebM upper-case the key names.
|
||||
"""
|
||||
command = [
|
||||
"ffmpeg",
|
||||
"-v",
|
||||
"quiet",
|
||||
"-f",
|
||||
"lavfi",
|
||||
"-i",
|
||||
"color=c=black:s=16x16:d=0.2",
|
||||
]
|
||||
if metadata_tags and str(path).lower().endswith((".mp4", ".mov", ".m4v")):
|
||||
command += ["-movflags", "use_metadata_tags"]
|
||||
for key, value in tags.items():
|
||||
command += ["-metadata", f"{key}={value}"]
|
||||
subprocess.run(command + ["-y", str(path)], check=True, timeout=60)
|
||||
|
||||
|
||||
def write_mp4(path, comment):
|
||||
"""Write a tiny black video carrying ``comment`` in its container tags."""
|
||||
subprocess.run(
|
||||
[
|
||||
"ffmpeg",
|
||||
"-v",
|
||||
"quiet",
|
||||
"-f",
|
||||
"lavfi",
|
||||
"-i",
|
||||
"color=c=black:s=16x16:d=0.2",
|
||||
"-metadata",
|
||||
f"comment={comment}",
|
||||
"-y",
|
||||
str(path),
|
||||
],
|
||||
check=True,
|
||||
timeout=60,
|
||||
)
|
||||
"""Write a tiny black MP4 with ``comment`` in its legacy ilst ``©cmt`` atom."""
|
||||
write_video(path, metadata_tags=False, comment=comment)
|
||||
|
||||
|
||||
class TestMetadataFromTags(unittest.TestCase):
|
||||
@@ -69,6 +82,158 @@ class TestMetadataFromTags(unittest.TestCase):
|
||||
self.assertEqual(metadata_from_tags(tags), {"prompt": GRAPH})
|
||||
|
||||
|
||||
class TestSelectVideoCarrier(unittest.TestCase):
|
||||
"""Spec 2026-10-19 resolution parity section 8: one carrier, never merged."""
|
||||
|
||||
WORKFLOW = json.dumps({"nodes": [{"id": "per-key"}]})
|
||||
|
||||
def test_per_key_tags_win_over_comment(self):
|
||||
tags = {
|
||||
"prompt": GRAPH,
|
||||
"workflow": self.WORKFLOW,
|
||||
"comment": json.dumps({"prompt": json.dumps({"9": {}})}),
|
||||
}
|
||||
metadata, formats, carrier = select_video_carrier(tags)
|
||||
self.assertEqual(metadata, {"prompt": GRAPH, "workflow": self.WORKFLOW})
|
||||
self.assertEqual(
|
||||
formats, {"prompt": "original-text", "workflow": "original-text"}
|
||||
)
|
||||
self.assertIsNone(carrier)
|
||||
|
||||
def test_matroska_uppercase_keys_fold(self):
|
||||
tags = {"PROMPT": GRAPH, "Workflow": self.WORKFLOW, "ENCODER": "Lavf"}
|
||||
metadata, formats, carrier = select_video_carrier(tags)
|
||||
self.assertEqual(metadata, {"prompt": GRAPH, "workflow": self.WORKFLOW})
|
||||
self.assertEqual(
|
||||
formats, {"prompt": "original-text", "workflow": "original-text"}
|
||||
)
|
||||
self.assertIsNone(carrier)
|
||||
# Envelope keys are read exactly as legacy did: no fold, so a Matroska
|
||||
# upper-cased COMMENT is not an envelope.
|
||||
self.assertIsNone(
|
||||
select_video_carrier({"COMMENT": json.dumps({"prompt": GRAPH})})
|
||||
)
|
||||
|
||||
def test_folded_duplicate_keys_fail(self):
|
||||
for tags in (
|
||||
{"PROMPT": GRAPH, "prompt": json.dumps({"9": {}})},
|
||||
{"Workflow": self.WORKFLOW, "WORKFLOW": "{}", "prompt": GRAPH},
|
||||
):
|
||||
with self.assertRaises(DuplicateVideoTagError):
|
||||
select_video_carrier(tags)
|
||||
# Byte-identical duplicates are one record, as for PNG text chunks.
|
||||
metadata, _, _ = select_video_carrier({"PROMPT": GRAPH, "prompt": GRAPH})
|
||||
self.assertEqual(metadata, {"prompt": GRAPH})
|
||||
# Envelope keys never fold, so a foreign COMMENT note is not a duplicate.
|
||||
envelope = json.dumps({"prompt": GRAPH})
|
||||
metadata, _, carrier = select_video_carrier(
|
||||
{"comment": envelope, "COMMENT": "a free-text note"}
|
||||
)
|
||||
self.assertEqual(metadata, {"prompt": GRAPH})
|
||||
self.assertEqual(carrier, envelope)
|
||||
|
||||
def test_comment_envelope_still_read(self):
|
||||
envelope = json.dumps({"prompt": GRAPH, "workflow": {"nodes": []}})
|
||||
metadata, formats, carrier = select_video_carrier(
|
||||
{"comment": envelope, "encoder": "Lavf"}
|
||||
)
|
||||
self.assertEqual(metadata["prompt"], GRAPH)
|
||||
self.assertEqual(json.loads(metadata["workflow"]), {"nodes": []})
|
||||
self.assertEqual(
|
||||
formats, {"prompt": "original-text", "workflow": "reserialized-json"}
|
||||
)
|
||||
self.assertEqual(carrier, envelope)
|
||||
|
||||
def test_description_fallback(self):
|
||||
envelope = json.dumps({"prompt": GRAPH})
|
||||
tags = {"comment": "not json", "description": envelope}
|
||||
self.assertEqual(
|
||||
select_video_carrier(tags),
|
||||
({"prompt": GRAPH}, {"prompt": "original-text"}, envelope),
|
||||
)
|
||||
self.assertIsNone(select_video_carrier({"comment": "note", "encoder": "x"}))
|
||||
self.assertIsNone(select_video_carrier({"comment": "[1]"}))
|
||||
self.assertIsNone(select_video_carrier({}))
|
||||
self.assertIsNone(select_video_carrier(None))
|
||||
self.assertIsNone(select_video_carrier({"comment": 7}))
|
||||
|
||||
def test_role_carrier_beats_workflow_only_carrier(self):
|
||||
envelope = json.dumps({"prompt": GRAPH, "workflow": {"nodes": ["c"]}})
|
||||
metadata, formats, carrier = select_video_carrier(
|
||||
{"workflow": self.WORKFLOW, "comment": envelope}
|
||||
)
|
||||
self.assertEqual(metadata["prompt"], GRAPH)
|
||||
self.assertEqual(json.loads(metadata["workflow"]), {"nodes": ["c"]})
|
||||
self.assertEqual(carrier, envelope)
|
||||
for role in ("parameters", "comfyassets_prompt_pairs"):
|
||||
metadata, _, carrier = select_video_carrier(
|
||||
{"comment": json.dumps({"workflow": {}}), role: "x"}
|
||||
)
|
||||
self.assertEqual(metadata, {role: "x"})
|
||||
self.assertIsNone(carrier)
|
||||
# Without a role anywhere, the first workflow-only carrier wins.
|
||||
metadata, _, carrier = select_video_carrier(
|
||||
{"workflow": self.WORKFLOW, "comment": json.dumps({"workflow": {}})}
|
||||
)
|
||||
self.assertEqual(metadata, {"workflow": self.WORKFLOW})
|
||||
self.assertIsNone(carrier)
|
||||
|
||||
def test_carriers_never_merge(self):
|
||||
metadata, _, carrier = select_video_carrier(
|
||||
{
|
||||
"prompt": GRAPH,
|
||||
"comment": json.dumps({"workflow": {"nodes": ["comment"]}}),
|
||||
"description": json.dumps({"parameters": "Steps: 1"}),
|
||||
}
|
||||
)
|
||||
self.assertEqual(metadata, {"prompt": GRAPH})
|
||||
self.assertIsNone(carrier)
|
||||
metadata, _, carrier = select_video_carrier(
|
||||
{
|
||||
"workflow": self.WORKFLOW,
|
||||
"comment": json.dumps({"prompt": GRAPH}),
|
||||
"description": json.dumps({"parameters": "Steps: 1"}),
|
||||
}
|
||||
)
|
||||
self.assertEqual(metadata, {"prompt": GRAPH})
|
||||
self.assertEqual(carrier, json.dumps({"prompt": GRAPH}))
|
||||
|
||||
def test_per_key_values_are_original_text_and_envelope_objects_reserialized(
|
||||
self,
|
||||
):
|
||||
exact = '{"3":{"inputs":{"cfg":6.5000000000000000001}}}'
|
||||
metadata, formats, _ = select_video_carrier({"prompt": exact})
|
||||
self.assertEqual(metadata, {"prompt": exact})
|
||||
self.assertEqual(formats, {"prompt": "original-text"})
|
||||
envelope = json.dumps(
|
||||
{"prompt": {"3": {}}, "workflow": {"nodes": []}, "parameters": "Steps: 1"}
|
||||
)
|
||||
metadata, formats, carrier = select_video_carrier({"comment": envelope})
|
||||
self.assertEqual(json.loads(metadata["prompt"]), {"3": {}})
|
||||
self.assertEqual(json.loads(metadata["workflow"]), {"nodes": []})
|
||||
self.assertEqual(metadata["parameters"], "Steps: 1")
|
||||
self.assertEqual(
|
||||
formats,
|
||||
{
|
||||
"prompt": "reserialized-json",
|
||||
"workflow": "reserialized-json",
|
||||
"parameters": "original-text",
|
||||
},
|
||||
)
|
||||
self.assertEqual(carrier, envelope)
|
||||
|
||||
def test_null_role_markers_and_invalid_workflow_keep_their_shape(self):
|
||||
# The inspector refuses a null role marker rather than falling back.
|
||||
metadata, formats, _ = select_video_carrier(
|
||||
{"comment": json.dumps({"prompt": None, "workflow": [1]})}
|
||||
)
|
||||
self.assertEqual(metadata, {"prompt": None})
|
||||
self.assertEqual(formats, {"workflow": "invalid"})
|
||||
metadata, formats, _ = select_video_carrier({"prompt": 7, "workflow": 1})
|
||||
self.assertEqual(metadata, {"prompt": None})
|
||||
self.assertEqual(formats, {"workflow": "invalid"})
|
||||
|
||||
|
||||
class TestIsVideoPath(unittest.TestCase):
|
||||
|
||||
def test_extensions(self):
|
||||
@@ -125,6 +290,37 @@ class TestReadVideoMetadataWithoutFfprobe(unittest.TestCase):
|
||||
):
|
||||
self.assertEqual(read_video_metadata("clip.mp4"), {})
|
||||
|
||||
def test_read_video_metadata_returns_per_key_prompt(self):
|
||||
payload = {
|
||||
"format": {
|
||||
"tags": {
|
||||
"PROMPT": GRAPH,
|
||||
"WORKFLOW": '{"nodes":[]}',
|
||||
"COMFYASSETS_PROMPT_PAIRS": '{"version":1}',
|
||||
"COMMENT": json.dumps({"prompt": "must not win"}),
|
||||
"ENCODER": "Lavf",
|
||||
}
|
||||
}
|
||||
}
|
||||
with mock.patch.object(
|
||||
video_metadata.shutil, "which", return_value="/bin/ffprobe"
|
||||
):
|
||||
with self._run_returning(stdout=json.dumps(payload)):
|
||||
self.assertEqual(
|
||||
read_video_metadata("clip.mkv"),
|
||||
{
|
||||
"prompt": GRAPH,
|
||||
"workflow": '{"nodes":[]}',
|
||||
"comfyassets_prompt_pairs": '{"version":1}',
|
||||
},
|
||||
)
|
||||
duplicates = {"format": {"tags": {"PROMPT": GRAPH, "prompt": "{}"}}}
|
||||
with self._run_returning(stdout=json.dumps(duplicates)):
|
||||
self.assertEqual(read_video_metadata("clip.mkv"), {})
|
||||
markers = {"format": {"tags": {"comment": json.dumps({"prompt": None})}}}
|
||||
with self._run_returning(stdout=json.dumps(markers)):
|
||||
self.assertEqual(read_video_metadata("clip.mp4"), {})
|
||||
|
||||
def test_ffprobe_output_is_parsed(self):
|
||||
payload = {"format": {"tags": {"comment": json.dumps({"prompt": GRAPH})}}}
|
||||
with mock.patch.object(
|
||||
@@ -155,6 +351,39 @@ class TestReadVideoMetadataWithRealFfprobe(unittest.TestCase):
|
||||
self.assertEqual(meta["prompt"], GRAPH)
|
||||
self.assertEqual(json.loads(meta["workflow"]), {"v": 1})
|
||||
|
||||
def test_per_key_tags_round_trip_through_real_containers(self):
|
||||
for name in ("per_key.mp4", "per_key.mkv", "per_key.webm"):
|
||||
path = os.path.join(self.tmpdir, name)
|
||||
write_video(path, prompt=GRAPH, workflow='{"nodes":[]}')
|
||||
self.assertEqual(
|
||||
read_video_metadata(path),
|
||||
{"prompt": GRAPH, "workflow": '{"nodes":[]}'},
|
||||
name,
|
||||
)
|
||||
|
||||
def test_legacy_ilst_comment_atom_is_read(self):
|
||||
path = os.path.join(self.tmpdir, "vhs.mp4")
|
||||
write_mp4(path, json.dumps({"prompt": GRAPH}))
|
||||
probe = subprocess.run(
|
||||
[
|
||||
shutil.which("ffprobe"),
|
||||
"-v",
|
||||
"quiet",
|
||||
"-show_entries",
|
||||
"format_tags",
|
||||
"-of",
|
||||
"json",
|
||||
path,
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
timeout=60,
|
||||
)
|
||||
tags = json.loads(probe.stdout)["format"]["tags"]
|
||||
self.assertIn("comment", tags)
|
||||
self.assertEqual(read_video_metadata(path), {"prompt": GRAPH})
|
||||
|
||||
def test_video_without_a_comment_is_empty(self):
|
||||
path = os.path.join(self.tmpdir, "plain.mp4")
|
||||
write_mp4(path, "")
|
||||
|
||||
Reference in New Issue
Block a user