From 4c2ed8f3b5ad969810f920eb22e991f5b8f8c923 Mon Sep 17 00:00:00 2001 From: Vadim Melnik Date: Sat, 23 May 2026 11:42:28 +0300 Subject: [PATCH 1/4] Add additional attributes to AudioInfo model to generate codec in RFC6381 format --- src/reprostim/qr/video_audit.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/reprostim/qr/video_audit.py b/src/reprostim/qr/video_audit.py index 8a8cee7b..9116210b 100644 --- a/src/reprostim/qr/video_audit.py +++ b/src/reprostim/qr/video_audit.py @@ -59,8 +59,13 @@ class AudioInfo(BaseModel): bits_per_sample: Optional[int] = None # Bits per sample channels: Optional[int] = None # Number of audio channels codec: Optional[str] = None # Audio codec used + codec_long: Optional[str] = None # Audio codec detailed name + codec_rfc6381: Optional[str] = None # Audio codec in RFC 6381 format duration_sec: Optional[float] = None # Duration in seconds + profile: Optional[str] = None # Audio codec profile (e.g., "LC") sample_rate: Optional[int] = None # Sample rate in Hz + start_time: Optional[float] = None # Start time of the audio stream in seconds + tag_str: Optional[str] = None # Codec tag string (e.g., "[0][0][0][0]") class VaMode(str, Enum): From 512468d76c1c46aa8612df3ae2e783cc98f4b39a Mon Sep 17 00:00:00 2001 From: Vadim Melnik Date: Sat, 23 May 2026 17:11:05 +0300 Subject: [PATCH 2/4] Add VideoInfo model to extract video stream information with ffprobe --- src/reprostim/qr/video_audit.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/reprostim/qr/video_audit.py b/src/reprostim/qr/video_audit.py index 9116210b..b7dba26e 100644 --- a/src/reprostim/qr/video_audit.py +++ b/src/reprostim/qr/video_audit.py @@ -54,7 +54,7 @@ # NB: move in future to audio package or tool? class AudioInfo(BaseModel): - """Audio information extracted from the video file.""" + """Audio stream information extracted from the video file with ffprobe.""" bits_per_sample: Optional[int] = None # Bits per sample channels: Optional[int] = None # Number of audio channels @@ -68,6 +68,24 @@ class AudioInfo(BaseModel): tag_str: Optional[str] = None # Codec tag string (e.g., "[0][0][0][0]") +class VideoInfo(BaseModel): + """Video stream information extracted from the video file with ffprobe.""" + + bit_depth: Optional[int] = None # Bit depth per channel, 8, 10, 12, 16 etc. + codec: Optional[str] = None # Video codec used + codec_long: Optional[str] = None # Video codec detailed name + codec_rfc6381: Optional[str] = None # Video codec in RFC 6381 format + duration_sec: Optional[float] = None # Duration in seconds + fps: Optional[float] = None # Frames per second + height: Optional[int] = None # Video frame height in pixels + level: Optional[int] = None # Video codec level + pix_fmt: Optional[str] = None # Pixel format (e.g., "yuv420p") + profile: Optional[str] = None # Video codec profile (e.g., "High") + start_time: Optional[float] = None # Start time of the video stream in seconds + tag_str: Optional[str] = None # Codec tag string (e.g., "[0][0][0][0]") + width: Optional[int] = None # Video frame width in pixels + + class VaMode(str, Enum): """Video audit processing mode constants.""" From 912ecc940bb375d284cc1ad97988ffb059235c5b Mon Sep 17 00:00:00 2001 From: Vadim Melnik Date: Sat, 23 May 2026 22:46:47 +0300 Subject: [PATCH 3/4] Add VideoCodecRFC6381 and AudioCodecRFC6381 to sidecar_metadata via ffprobe --- .ai/task-bids-inject.md | 9 ++ .ai/task-video-audit.md | 30 +++- src/reprostim/qr/bids_inject.py | 17 +- src/reprostim/qr/split_video.py | 11 +- src/reprostim/qr/video_audit.py | 159 +++++++++++++++---- tests/data/video_audit/ffprobe_h264_aac.json | 106 +++++++++++++ tests/qr/test_bids_inject.py | 66 ++++++++ tests/qr/test_split_video.py | 22 +++ tests/qr/test_video_audit.py | 156 ++++++++++++++---- 9 files changed, 503 insertions(+), 73 deletions(-) create mode 100644 tests/data/video_audit/ffprobe_h264_aac.json diff --git a/.ai/task-bids-inject.md b/.ai/task-bids-inject.md index 7b5ee5ca..8c907b56 100644 --- a/.ai/task-bids-inject.md +++ b/.ai/task-bids-inject.md @@ -79,6 +79,7 @@ Tracks implementation progress against [spec-bids-inject.md](spec-bids-inject.md - [x] Media suffix determination (`_video` / `_audio` / `_audiovideo`) from `videos.tsv` - [x] Delegate to `split-video` Python API (`do_main`) - [x] Build `sidecar_metadata` dict from `record.metadata.TaskName` and pass to `do_main` +- [x] Populate `VideoCodecRFC6381` / `AudioCodecRFC6381` in `sidecar_metadata` via `get_audio_video_info_ffprobe` (TODO: future — read from `videos.tsv` columns instead) ### Dry-run mode - [x] Skip `split-video` call and file writes when `--dry-run` @@ -210,6 +211,14 @@ Test file location: `tests/qr/test_bids_inject.py` (mirrors `tests/audio/test_au - [x] `_is_scans_file` — `*_scans.tsv` existing file → `True` - [x] `_is_scans_file` — directory or non-matching name → `False` +### sidecar_metadata propagation + +- [x] `_call_split_video` passes `sidecar_metadata` with `TaskName` to `split-video` +- [x] `_call_split_video` passes empty `sidecar_metadata` when `TaskName` absent +- [x] `_call_split_video` adds `VideoCodecRFC6381` / `AudioCodecRFC6381` to `sidecar_metadata` via ffprobe +- [x] `_to_bids_model` uses `VideoCodecRFC6381` / `AudioCodecRFC6381` from `sidecar_metadata` when provided +- [x] `_to_bids_model` defaults `VideoCodecRFC6381` / `AudioCodecRFC6381` to `"n/a"` when absent + ### Integration tests (with synthetic BIDS fixture) > Requires a small synthetic BIDS dataset fixture (a few `_scans.tsv` files, stub JSON diff --git a/.ai/task-video-audit.md b/.ai/task-video-audit.md index 54dad5d5..34161deb 100644 --- a/.ai/task-video-audit.md +++ b/.ai/task-video-audit.md @@ -82,6 +82,11 @@ Tracks implementation progress against [spec-video-audit.md](spec-video-audit.md - [x] `VaContext` — Pydantic model carrying all processing options - [x] `VaMode` — enum of operation modes - [x] `VaSource` — enum of audit sources +- [x] `AudioInfo` — Pydantic model for audio stream info (codec, codec_long, codec_rfc6381, profile, sample_rate, channels, bits_per_sample, duration_sec, start_time, tag_str) +- [x] `VideoInfo` — Pydantic model for video stream info (codec, codec_long, codec_rfc6381, profile, level, width, height, pix_fmt, bit_depth, fps, duration_sec, start_time, tag_str) +- [x] `audio_codec_to_rfc6381` — RFC 6381 string for audio codec +- [x] `video_codec_to_rfc6381` — RFC 6381 string for video codec +- [x] `get_audio_video_info_ffprobe` — single ffprobe call returning `Tuple[AudioInfo, VideoInfo]` - [x] `do_audit_file` — audit a single video file (INTERNAL) - [x] `do_audit_dir` — audit all videos in a directory - [x] `do_audit_internal` — entry point for INTERNAL source @@ -128,12 +133,27 @@ Test file location: `tests/qr/test_video_audit.py` - [x] `ffprobe` available (subprocess mock) → `True` - [x] `ffprobe` not found (`FileNotFoundError` mock) → `False` -#### `get_audio_info_ffprobe` -- [x] Success with DURATION tag → codec/sample_rate/channels/bits_per_sample/duration set +#### `audio_codec_to_rfc6381` +- [x] AAC LC → `"mp4a.40.2"` +- [x] AAC HE-AAC → `"mp4a.40.5"` +- [x] AAC unknown/None profile → defaults to `"mp4a.40.2"` +- [x] MP3 → `"mp4a.69"` +- [x] Opus → `"opus"` +- [x] Unknown codec → `None` + +#### `video_codec_to_rfc6381` +- [x] H.264 High@L4.2 → `"avc1.64002A"` +- [x] H.264 Baseline@L3.0 → `"avc1.42001E"` +- [x] H.264 None level → `"avc1.4D0000"` +- [x] Unknown codec → `None` + +#### `get_audio_video_info_ffprobe` (replaces `get_audio_info_ffprobe`) +- [x] Success — audio fields: codec/codec_long/profile/sample_rate/channels/start_time/tag_str/codec_rfc6381/duration set +- [x] Success — video fields: codec/codec_long/profile/level/width/height/pix_fmt/bit_depth/fps/codec_rfc6381/duration/start_time/tag_str set - [x] Duration read from stream `duration` field (no DURATION tag) -- [x] No audio streams → all fields `None` -- [x] `FileNotFoundError` → returns empty `AudioInfo` -- [x] `CalledProcessError` → returns empty `AudioInfo` +- [x] No audio streams → AudioInfo all `None`, VideoInfo populated +- [x] `FileNotFoundError` → returns empty AudioInfo and VideoInfo +- [x] `CalledProcessError` → returns empty AudioInfo and VideoInfo #### `_compare_rec_ts` - [x] Both `n/a` → `0` diff --git a/src/reprostim/qr/bids_inject.py b/src/reprostim/qr/bids_inject.py index c939458a..744ec66e 100644 --- a/src/reprostim/qr/bids_inject.py +++ b/src/reprostim/qr/bids_inject.py @@ -23,7 +23,10 @@ from pydantic import BaseModel, Field -from reprostim.qr.video_audit import find_video_audit_by_timerange +from reprostim.qr.video_audit import ( + find_video_audit_by_timerange, + get_audio_video_info_ffprobe, +) # initialize the logger logger = logging.getLogger(__name__) @@ -892,6 +895,18 @@ def _capturing_out_func(msg: str) -> None: if record.metadata and record.metadata.TaskName: sidecar_metadata["TaskName"] = record.metadata.TaskName + # TODO: in the future, read VideoCodecRFC6381 / AudioCodecRFC6381 from + # dedicated columns in videos.tsv populated by video-audit, avoiding + # the extra ffprobe call here. + try: + ai, vi = get_audio_video_info_ffprobe(input_path) + if vi.codec_rfc6381: + sidecar_metadata["VideoCodecRFC6381"] = vi.codec_rfc6381 + if ai.codec_rfc6381: + sidecar_metadata["AudioCodecRFC6381"] = ai.codec_rfc6381 + except Exception as e: + logger.error(f"Failed to get codec RFC6381 info for {input_path}: {e}") + ret, split_results = split_video_main( input_path=input_path, output_path=output_path, diff --git a/src/reprostim/qr/split_video.py b/src/reprostim/qr/split_video.py index 68c87bbb..50989fb6 100644 --- a/src/reprostim/qr/split_video.py +++ b/src/reprostim/qr/split_video.py @@ -113,7 +113,8 @@ def _to_bids_model(sr: "SplitResult", sidecar_metadata: dict | None = None) -> d :param sr: SplitResult to convert :param sidecar_metadata: Optional dict with extra BIDS fields to inject. - Currently supports ``TaskName`` (written as the first field when present). + Supports ``TaskName`` (written as the first field when present), + ``VideoCodecRFC6381``, and ``AudioCodecRFC6381``. :return: Dict with BIDS-compliant metadata field names """ result = {} @@ -134,7 +135,9 @@ def _to_bids_model(sr: "SplitResult", sidecar_metadata: dict | None = None) -> d if sr.video_codec != "n/a": result["VideoCodec"] = sr.video_codec - result["VideoCodecRFC6381"] = "n/a" + result["VideoCodecRFC6381"] = (sidecar_metadata or {}).get( + "VideoCodecRFC6381", "n/a" + ) if sr.video_frame_rate is not None: result["FrameRate"] = sr.video_frame_rate @@ -153,7 +156,9 @@ def _to_bids_model(sr: "SplitResult", sidecar_metadata: dict | None = None) -> d if sr.audio_codec != "n/a": result["AudioCodec"] = sr.audio_codec - result["AudioCodecRFC6381"] = "n/a" + result["AudioCodecRFC6381"] = (sidecar_metadata or {}).get( + "AudioCodecRFC6381", "n/a" + ) if sr.audio_sample_rate != "n/a": try: diff --git a/src/reprostim/qr/video_audit.py b/src/reprostim/qr/video_audit.py index b7dba26e..c9a69d74 100644 --- a/src/reprostim/qr/video_audit.py +++ b/src/reprostim/qr/video_audit.py @@ -22,7 +22,7 @@ from datetime import datetime from enum import Enum from time import time -from typing import Dict, Generator, List, Optional, Set +from typing import Dict, Generator, List, Optional, Set, Tuple from filelock import FileLock, Timeout from pydantic import BaseModel @@ -418,19 +418,84 @@ def find_metadata_json(path: str, key: str, value) -> Optional[Dict]: ) +def audio_codec_to_rfc6381(codec: str, profile: Optional[str]) -> Optional[str]: + """Return the RFC 6381 codec string for an audio stream. + + :param codec: Short codec name as reported by ffprobe (e.g. ``"aac"``). + :type codec: str + + :param profile: Codec profile string (e.g. ``"LC"``), or ``None``. + :type profile: Optional[str] + + :return: RFC 6381 string (e.g. ``"mp4a.40.2"``), or ``None`` when the + codec is not recognized. + :rtype: Optional[str] + """ + if codec == "aac": + aot = {"LC": 2, "HE-AAC": 5, "HE-AACv2": 29, "LD": 23, "ELD": 39}.get( + profile, 2 + ) + return f"mp4a.40.{aot}" + if codec in ("mp3", "mp2"): + return "mp4a.69" + if codec == "opus": + return "opus" + return None + + +def video_codec_to_rfc6381( + codec: str, profile: Optional[str], level: Optional[int] +) -> Optional[str]: + """Return the RFC 6381 codec string for a video stream. + + :param codec: Short codec name as reported by ffprobe (e.g. ``"h264"``). + :type codec: str + + :param profile: Codec profile string (e.g. ``"High"``), or ``None``. + :type profile: Optional[str] + + :param level: Codec level integer as reported by ffprobe (e.g. ``42``), + or ``None``. + :type level: Optional[int] + + :return: RFC 6381 string (e.g. ``"avc1.64002A"``), or ``None`` when the + codec is not recognised. + :rtype: Optional[str] + """ + if codec == "h264": + profile_idc = { + "Baseline": 0x42, + "Main": 0x4D, + "High": 0x64, + "High 10": 0x6E, + "High 4:2:2": 0x7A, + "High 4:4:4 Predictive": 0xF4, + }.get(profile, 0x42) + level_idc = level if level is not None else 0 + return f"avc1.{profile_idc:02X}00{level_idc:02X}" + return None + + # NB: move in future to audio package or tool? -def get_audio_info_ffprobe(path: str) -> AudioInfo: - """Extract audio information from the video file using ffprobe. +def get_audio_video_info_ffprobe(path: str) -> Tuple[AudioInfo, VideoInfo]: + """Extract audio and video stream information from the video file using ffprobe. - :param path: Path to the video file(.mkv, .mp4, .avi) + Issues a single ffprobe call that reads all streams, then splits results + into an :class:`AudioInfo` (first audio stream) and a :class:`VideoInfo` + (first video stream). + + :param path: Path to the video file (.mkv, .mp4, .avi) :type path: str - :return: AudioInfo object with extracted audio information - :rtype: AudioInfo + :return: Tuple of (AudioInfo, VideoInfo) with extracted stream information. + Fields are ``None`` when the corresponding stream is absent or a + value cannot be parsed. + :rtype: Tuple[AudioInfo, VideoInfo] """ - logger.debug(f"get_audio_info: {path}") + logger.debug(f"get_audio_video_info_ffprobe: {path}") ai: AudioInfo = AudioInfo() + vi: VideoInfo = VideoInfo() try: cmd = [ @@ -440,8 +505,6 @@ def get_audio_info_ffprobe(path: str) -> AudioInfo: "-print_format", "json", # JSON output "-show_streams", # streams - "-select_streams", - "a", # filter all audio streams path, ] # cmd = ["ffprobe", "-h"] @@ -453,35 +516,72 @@ def get_audio_info_ffprobe(path: str) -> AudioInfo: streams = o.get("streams", []) audio_streams = [s for s in streams if s.get("codec_type") == "audio"] + video_streams = [s for s in streams if s.get("codec_type") == "video"] if audio_streams: - stream = audio_streams[ - 0 - ] # take first audio stream (or iterate if multiple) - - bps = stream.get("bits_per_sample") + s = audio_streams[0] + bps = s.get("bits_per_sample") if bps is not None and bps != 0: ai.bits_per_sample = bps - ai.channels = stream.get("channels") - ai.codec = stream.get("codec_name") - ai.sample_rate = ( - int(stream["sample_rate"]) if "sample_rate" in stream else None + ai.channels = s.get("channels") + ai.codec = s.get("codec_name") + ai.codec_long = s.get("codec_long_name") + ai.profile = s.get("profile") + ai.sample_rate = int(s["sample_rate"]) if "sample_rate" in s else None + ai.start_time = float(s["start_time"]) if "start_time" in s else None + ai.tag_str = s.get("codec_tag_string") + ai.codec_rfc6381 = audio_codec_to_rfc6381(ai.codec or "", ai.profile) + if "tags" in s and "DURATION" in s["tags"]: + h, m, sec = s["tags"]["DURATION"].split(":") + ai.duration_sec = int(h) * 3600 + int(m) * 60 + float(sec) + elif "duration" in s: + ai.duration_sec = float(s["duration"]) + + if video_streams: + s = video_streams[0] + vi.codec = s.get("codec_name") + vi.codec_long = s.get("codec_long_name") + vi.profile = s.get("profile") + vi.level = s.get("level") + vi.width = s.get("width") + vi.height = s.get("height") + vi.pix_fmt = s.get("pix_fmt") + vi.start_time = float(s["start_time"]) if "start_time" in s else None + vi.tag_str = s.get("codec_tag_string") + bprs = s.get("bits_per_raw_sample") + if bprs is not None: + try: + v = int(bprs) + if v != 0: + vi.bit_depth = v + except (ValueError, TypeError): + pass + for fps_key in ("avg_frame_rate", "r_frame_rate"): + fps_str = s.get(fps_key) + if fps_str and fps_str != "0/0": + try: + num, den = fps_str.split("/") + den_int = int(den) + if den_int != 0: + vi.fps = float(int(num)) / den_int + break + except (ValueError, ZeroDivisionError): + pass + vi.codec_rfc6381 = video_codec_to_rfc6381( + vi.codec or "", vi.profile, vi.level ) - - # duration is usually in tags, but sometimes in stream - if "tags" in stream and "DURATION" in stream["tags"]: - # Example: "00:00:17.150000000" - h, m, s = stream["tags"]["DURATION"].split(":") - ai.duration_sec = int(h) * 3600 + int(m) * 60 + float(s) - elif "duration" in stream: - ai.duration_sec = float(stream["duration"]) + if "tags" in s and "DURATION" in s["tags"]: + h, m, sec = s["tags"]["DURATION"].split(":") + vi.duration_sec = int(h) * 3600 + int(m) * 60 + float(sec) + elif "duration" in s: + vi.duration_sec = float(s["duration"]) except FileNotFoundError: logger.error("ffprobe is not installed or not in PATH") except subprocess.CalledProcessError as e: logger.error(f"ffprobe error: {e} {e.stdout} {e.stderr}") - return ai + return ai, vi def _save_tsv(records: List[VaRecord], path_out: str): @@ -821,8 +921,9 @@ def do_audit_file(ctx: VaContext, path: str) -> Generator[VaRecord, None, None]: ) vr.video_fps_recorded = f"{round(ps.video_frame_rate, 1)}" - # try to get audio info - ai: AudioInfo = get_audio_info_ffprobe(path) + # try to get audio and video info + ai: AudioInfo + ai, _ = get_audio_video_info_ffprobe(path) if ai is not None: logger.debug(f"ai: {ai}") if ai.sample_rate is not None: diff --git a/tests/data/video_audit/ffprobe_h264_aac.json b/tests/data/video_audit/ffprobe_h264_aac.json new file mode 100644 index 00000000..cd7e3317 --- /dev/null +++ b/tests/data/video_audit/ffprobe_h264_aac.json @@ -0,0 +1,106 @@ +{ + "streams": [ + { + "index": 0, + "codec_name": "h264", + "codec_long_name": "H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10", + "profile": "High", + "codec_type": "video", + "codec_tag_string": "[0][0][0][0]", + "codec_tag": "0x0000", + "width": 1920, + "height": 1080, + "coded_width": 1920, + "coded_height": 1080, + "closed_captions": 0, + "film_grain": 0, + "has_b_frames": 1, + "pix_fmt": "yuv420p", + "level": 42, + "color_range": "tv", + "chroma_location": "left", + "field_order": "progressive", + "refs": 1, + "is_avc": "true", + "nal_length_size": "4", + "r_frame_rate": "60/1", + "avg_frame_rate": "60/1", + "time_base": "1/1000", + "start_pts": 21, + "start_time": "0.021000", + "bits_per_raw_sample": "8", + "extradata_size": 46, + "disposition": { + "default": 0, + "dub": 0, + "original": 0, + "comment": 0, + "lyrics": 0, + "karaoke": 0, + "forced": 0, + "hearing_impaired": 0, + "visual_impaired": 0, + "clean_effects": 0, + "attached_pic": 0, + "timed_thumbnails": 0, + "non_diegetic": 0, + "captions": 0, + "descriptions": 0, + "metadata": 0, + "dependent": 0, + "still_image": 0, + "multilayer": 0 + }, + "tags": { + "ENCODER": "Lavc59.37.100 h264_vaapi", + "DURATION": "02:50:16.438000000" + } + }, + { + "index": 1, + "codec_name": "aac", + "codec_long_name": "AAC (Advanced Audio Coding)", + "profile": "LC", + "codec_type": "audio", + "codec_tag_string": "[0][0][0][0]", + "codec_tag": "0x0000", + "sample_fmt": "fltp", + "sample_rate": "48000", + "channels": 2, + "channel_layout": "stereo", + "bits_per_sample": 0, + "initial_padding": 0, + "r_frame_rate": "0/0", + "avg_frame_rate": "0/0", + "time_base": "1/1000", + "start_pts": 0, + "start_time": "0.000000", + "extradata_size": 5, + "disposition": { + "default": 0, + "dub": 0, + "original": 0, + "comment": 0, + "lyrics": 0, + "karaoke": 0, + "forced": 0, + "hearing_impaired": 0, + "visual_impaired": 0, + "clean_effects": 0, + "attached_pic": 0, + "timed_thumbnails": 0, + "non_diegetic": 0, + "captions": 0, + "descriptions": 0, + "metadata": 0, + "dependent": 0, + "still_image": 0, + "multilayer": 0 + }, + "tags": { + "ENCODER": "Lavc59.37.100 aac", + "DURATION": "02:50:16.420000000" + } + } + ] +} diff --git a/tests/qr/test_bids_inject.py b/tests/qr/test_bids_inject.py index c494dde7..1666ca9c 100644 --- a/tests/qr/test_bids_inject.py +++ b/tests/qr/test_bids_inject.py @@ -1034,6 +1034,72 @@ def _fake_split_video_main(**kwargs): assert captured["sidecar_metadata"] == {} +def test_call_split_video_passes_rfc6381_in_sidecar_metadata(tmp_path): + """_call_split_video adds VideoCodecRFC6381 and AudioCodecRFC6381 + to sidecar_metadata.""" + scans_tsv = _copy_bids_fixture(tmp_path) + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + + captured = {} + + def _fake_split_video_main(**kwargs): + captured.update(kwargs) + return 0, [] + + from reprostim.qr.video_audit import AudioInfo, VideoInfo + + fake_ai = AudioInfo(codec="aac", codec_rfc6381="mp4a.40.2") + fake_vi = VideoInfo(codec="h264", codec_rfc6381="avc1.640028") + + with patch("reprostim.qr.split_video.do_main", side_effect=_fake_split_video_main): + with patch( + "reprostim.qr.bids_inject.get_audio_video_info_ffprobe", + return_value=(fake_ai, fake_vi), + ): + ret, _ = _run( + [str(scans_tsv)], + videos_tsv, + match=_MATCH_TASK_REST, + overwrite="skip", + dry_run=False, + ) + + assert ret == 0 + assert "sidecar_metadata" in captured + assert captured["sidecar_metadata"].get("VideoCodecRFC6381") == "avc1.640028" + assert captured["sidecar_metadata"].get("AudioCodecRFC6381") == "mp4a.40.2" + + +def test_call_split_video_rfc6381_ffprobe_error_continues(tmp_path): + """ffprobe failure logs an error but injection still proceeds.""" + scans_tsv = _copy_bids_fixture(tmp_path) + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + + captured = {} + + def _fake_split_video_main(**kwargs): + captured.update(kwargs) + return 0, [] + + with patch("reprostim.qr.split_video.do_main", side_effect=_fake_split_video_main): + with patch( + "reprostim.qr.bids_inject.get_audio_video_info_ffprobe", + side_effect=RuntimeError("ffprobe not found"), + ): + ret, _ = _run( + [str(scans_tsv)], + videos_tsv, + match=_MATCH_TASK_REST, + overwrite="skip", + dry_run=False, + ) + + assert ret == 0 + assert "sidecar_metadata" in captured + assert "VideoCodecRFC6381" not in captured["sidecar_metadata"] + assert "AudioCodecRFC6381" not in captured["sidecar_metadata"] + + # =========================================================================== # _parse_bids_float # =========================================================================== diff --git a/tests/qr/test_split_video.py b/tests/qr/test_split_video.py index bc5ec529..de607de3 100644 --- a/tests/qr/test_split_video.py +++ b/tests/qr/test_split_video.py @@ -680,6 +680,28 @@ def test_to_bids_model_video_codec_absent_when_na(): assert "VideoCodec" not in data +def test_to_bids_model_rfc6381_from_sidecar_metadata(): + """VideoCodecRFC6381 and AudioCodecRFC6381 are taken from sidecar_metadata.""" + sr = SplitResult(video_codec="h264", audio_codec="aac") + data = _to_bids_model( + sr, + sidecar_metadata={ + "VideoCodecRFC6381": "avc1.640028", + "AudioCodecRFC6381": "mp4a.40.2", + }, + ) + assert data["VideoCodecRFC6381"] == "avc1.640028" + assert data["AudioCodecRFC6381"] == "mp4a.40.2" + + +def test_to_bids_model_rfc6381_defaults_to_na_without_sidecar_metadata(): + """VideoCodecRFC6381 and AudioCodecRFC6381 default to 'n/a' when absent.""" + sr = SplitResult(video_codec="h264", audio_codec="aac") + data = _to_bids_model(sr) + assert data["VideoCodecRFC6381"] == "n/a" + assert data["AudioCodecRFC6381"] == "n/a" + + def test_to_bids_model_no_raw_fields(): """BIDS output contains no raw SplitResult field names.""" sr = _make_split_result() diff --git a/tests/qr/test_video_audit.py b/tests/qr/test_video_audit.py index a662547c..b0069def 100644 --- a/tests/qr/test_video_audit.py +++ b/tests/qr/test_video_audit.py @@ -11,10 +11,13 @@ import csv import json import os +import pathlib import subprocess from datetime import datetime from unittest.mock import MagicMock, patch +_DATA_DIR = pathlib.Path(__file__).parent.parent / "data" / "video_audit" + import click.testing import pytest from filelock import Timeout @@ -25,6 +28,7 @@ VaMode, VaRecord, VaSource, + VideoInfo, _build_dated_path, _compare_rec_ts, _get_tsv_records, @@ -35,6 +39,7 @@ _parse_rec_datetime, _save_tsv, _tsv_cache, + audio_codec_to_rfc6381, check_coherent, check_ffprobe, do_audit, @@ -48,12 +53,13 @@ format_date, format_duration, format_time, - get_audio_info_ffprobe, + get_audio_video_info_ffprobe, get_file_video_audit, iter_metadata_json, run_ext_all, run_ext_nosignal, run_ext_qr, + video_codec_to_rfc6381, ) runner = click.testing.CliRunner() @@ -821,11 +827,13 @@ def test_do_audit_file_happy_path(tmp_path): bits_per_sample=16, duration_sec=60.0, ) + mock_vi2 = MagicMock(spec=VideoInfo) sb = {"frameRate": "30.0", "cx": 1920, "cy": 1080} with patch("reprostim.qr.video_audit.find_metadata_json", return_value=sb), patch( "reprostim.qr.video_audit.do_info_file", return_value=(mock_vi, mock_vti) ), patch("reprostim.qr.video_audit.do_parse", return_value=iter([mock_ps])), patch( - "reprostim.qr.video_audit.get_audio_info_ffprobe", return_value=mock_ai + "reprostim.qr.video_audit.get_audio_video_info_ffprobe", + return_value=(mock_ai, mock_vi2), ): records = list(do_audit_file(_ctx(), str(mkv))) assert len(records) == 1 @@ -1032,38 +1040,98 @@ def test_iter_metadata_json_invalid_json(tmp_path): # =========================================================================== -# get_audio_info_ffprobe +# audio_codec_to_rfc6381 / video_codec_to_rfc6381 # =========================================================================== -def test_get_audio_info_ffprobe_success(): +def test_audio_codec_to_rfc6381_aac_lc(): + assert audio_codec_to_rfc6381("aac", "LC") == "mp4a.40.2" - ffprobe_output = json.dumps( - { - "streams": [ - { - "codec_type": "audio", - "codec_name": "pcm_s16le", - "sample_rate": "44100", - "channels": 2, - "bits_per_sample": 16, - "tags": {"DURATION": "00:01:00.000000000"}, - } - ] - } + +def test_audio_codec_to_rfc6381_aac_he(): + assert audio_codec_to_rfc6381("aac", "HE-AAC") == "mp4a.40.5" + + +def test_audio_codec_to_rfc6381_aac_unknown_profile(): + assert audio_codec_to_rfc6381("aac", None) == "mp4a.40.2" + + +def test_audio_codec_to_rfc6381_mp3(): + assert audio_codec_to_rfc6381("mp3", None) == "mp4a.69" + + +def test_audio_codec_to_rfc6381_opus(): + assert audio_codec_to_rfc6381("opus", None) == "opus" + + +def test_audio_codec_to_rfc6381_unknown(): + assert audio_codec_to_rfc6381("pcm_s16le", None) is None + + +def test_video_codec_to_rfc6381_h264_high(): + assert video_codec_to_rfc6381("h264", "High", 42) == "avc1.640002A".replace( + "0002A", "002A" ) - mock_result = MagicMock(stdout=ffprobe_output, returncode=0) + assert video_codec_to_rfc6381("h264", "High", 42) == "avc1.64002A" + + +def test_video_codec_to_rfc6381_h264_baseline(): + assert video_codec_to_rfc6381("h264", "Baseline", 30) == "avc1.42001E" + + +def test_video_codec_to_rfc6381_h264_no_level(): + result = video_codec_to_rfc6381("h264", "Main", None) + assert result == "avc1.4D0000" + + +def test_video_codec_to_rfc6381_unknown(): + assert video_codec_to_rfc6381("vp9", None, None) is None + + +# =========================================================================== +# get_audio_video_info_ffprobe +# =========================================================================== + +_FULL_FFPROBE_OUTPUT = (_DATA_DIR / "ffprobe_h264_aac.json").read_text() + + +def test_get_audio_video_info_ffprobe_audio_fields(): + mock_result = MagicMock(stdout=_FULL_FFPROBE_OUTPUT, returncode=0) with patch("reprostim.qr.video_audit.subprocess.run", return_value=mock_result): - ai = get_audio_info_ffprobe("/fake/test.mkv") - assert ai.codec == "pcm_s16le" - assert ai.sample_rate == 44100 + ai, vi = get_audio_video_info_ffprobe("/fake/test.mkv") + assert ai.codec == "aac" + assert ai.codec_long == "AAC (Advanced Audio Coding)" + assert ai.profile == "LC" + assert ai.sample_rate == 48000 assert ai.channels == 2 - assert ai.bits_per_sample == 16 - assert abs(ai.duration_sec - 60.0) < 0.01 + assert ai.bits_per_sample is None # 0 is treated as absent + assert ai.codec_rfc6381 == "mp4a.40.2" + assert abs(ai.duration_sec - (2 * 3600 + 50 * 60 + 16.420)) < 0.01 + assert ai.start_time == 0.0 + assert ai.tag_str == "[0][0][0][0]" -def test_get_audio_info_ffprobe_duration_from_stream(): - ffprobe_output = json.dumps( +def test_get_audio_video_info_ffprobe_video_fields(): + mock_result = MagicMock(stdout=_FULL_FFPROBE_OUTPUT, returncode=0) + with patch("reprostim.qr.video_audit.subprocess.run", return_value=mock_result): + ai, vi = get_audio_video_info_ffprobe("/fake/test.mkv") + assert vi.codec == "h264" + assert vi.codec_long == "H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10" + assert vi.profile == "High" + assert vi.level == 42 + assert vi.width == 1920 + assert vi.height == 1080 + assert vi.pix_fmt == "yuv420p" + assert vi.bit_depth == 8 + assert abs(vi.fps - 60.0) < 0.01 + assert vi.codec_rfc6381 == "avc1.64002A" + assert abs(vi.duration_sec - (2 * 3600 + 50 * 60 + 16.438)) < 0.01 + assert vi.start_time == pytest.approx(0.021) + assert vi.tag_str == "[0][0][0][0]" + + +def test_get_audio_video_info_ffprobe_duration_from_stream_field(): + output = json.dumps( { "streams": [ { @@ -1076,33 +1144,51 @@ def test_get_audio_info_ffprobe_duration_from_stream(): ] } ) - mock_result = MagicMock(stdout=ffprobe_output, returncode=0) + mock_result = MagicMock(stdout=output, returncode=0) with patch("reprostim.qr.video_audit.subprocess.run", return_value=mock_result): - ai = get_audio_info_ffprobe("/fake/test.mkv") + ai, vi = get_audio_video_info_ffprobe("/fake/test.mkv") assert abs(ai.duration_sec - 30.5) < 0.01 + assert vi.codec is None -def test_get_audio_info_ffprobe_no_audio_streams(): - ffprobe_output = json.dumps({"streams": [{"codec_type": "video"}]}) - mock_result = MagicMock(stdout=ffprobe_output, returncode=0) +def test_get_audio_video_info_ffprobe_no_audio_streams(): + output = json.dumps( + { + "streams": [ + { + "codec_type": "video", + "codec_name": "h264", + "profile": "High", + "level": 40, + "width": 1280, + "height": 720, + "avg_frame_rate": "30/1", + } + ] + } + ) + mock_result = MagicMock(stdout=output, returncode=0) with patch("reprostim.qr.video_audit.subprocess.run", return_value=mock_result): - ai = get_audio_info_ffprobe("/fake/test.mkv") + ai, vi = get_audio_video_info_ffprobe("/fake/test.mkv") assert ai.codec is None + assert vi.codec == "h264" -def test_get_audio_info_ffprobe_not_found(): +def test_get_audio_video_info_ffprobe_not_found(): with patch( "reprostim.qr.video_audit.subprocess.run", side_effect=FileNotFoundError ): - ai = get_audio_info_ffprobe("/fake/test.mkv") + ai, vi = get_audio_video_info_ffprobe("/fake/test.mkv") assert ai.codec is None + assert vi.codec is None -def test_get_audio_info_ffprobe_called_process_error(): +def test_get_audio_video_info_ffprobe_called_process_error(): err = subprocess.CalledProcessError(1, "ffprobe", output="", stderr="error") with patch("reprostim.qr.video_audit.subprocess.run", side_effect=err): - ai = get_audio_info_ffprobe("/fake/test.mkv") + ai, vi = get_audio_video_info_ffprobe("/fake/test.mkv") assert ai.codec is None + assert vi.codec is None # =========================================================================== From 9bf2e84906031734d533e6f8d79ce44255a46545 Mon Sep 17 00:00:00 2001 From: Vadim Melnik Date: Sun, 24 May 2026 00:00:59 +0300 Subject: [PATCH 4/4] Add BitDepth and PixelFormat to BIDS sidecar JSON --- .ai/task-bids-inject.md | 6 +++++ src/reprostim/qr/bids_inject.py | 11 ++++++---- src/reprostim/qr/split_video.py | 11 +++++++++- tests/qr/test_bids_inject.py | 39 +++++++++++++++++++++++++++++++-- tests/qr/test_split_video.py | 20 +++++++++++++++++ 5 files changed, 80 insertions(+), 7 deletions(-) diff --git a/.ai/task-bids-inject.md b/.ai/task-bids-inject.md index 8c907b56..7a757da9 100644 --- a/.ai/task-bids-inject.md +++ b/.ai/task-bids-inject.md @@ -80,6 +80,8 @@ Tracks implementation progress against [spec-bids-inject.md](spec-bids-inject.md - [x] Delegate to `split-video` Python API (`do_main`) - [x] Build `sidecar_metadata` dict from `record.metadata.TaskName` and pass to `do_main` - [x] Populate `VideoCodecRFC6381` / `AudioCodecRFC6381` in `sidecar_metadata` via `get_audio_video_info_ffprobe` (TODO: future — read from `videos.tsv` columns instead) +- [x] Populate `BitDepth` / `PixelFormat` in `sidecar_metadata` from `VideoInfo.bit_depth` / `VideoInfo.pix_fmt` +- [x] ffprobe errors logged and injection continues (RFC6381 / BitDepth / PixelFormat silently omitted) ### Dry-run mode - [x] Skip `split-video` call and file writes when `--dry-run` @@ -216,8 +218,12 @@ Test file location: `tests/qr/test_bids_inject.py` (mirrors `tests/audio/test_au - [x] `_call_split_video` passes `sidecar_metadata` with `TaskName` to `split-video` - [x] `_call_split_video` passes empty `sidecar_metadata` when `TaskName` absent - [x] `_call_split_video` adds `VideoCodecRFC6381` / `AudioCodecRFC6381` to `sidecar_metadata` via ffprobe +- [x] `_call_split_video` adds `BitDepth` / `PixelFormat` to `sidecar_metadata` via ffprobe +- [x] ffprobe failure logs error and injection continues; RFC6381/BitDepth/PixelFormat silently omitted - [x] `_to_bids_model` uses `VideoCodecRFC6381` / `AudioCodecRFC6381` from `sidecar_metadata` when provided - [x] `_to_bids_model` defaults `VideoCodecRFC6381` / `AudioCodecRFC6381` to `"n/a"` when absent +- [x] `_to_bids_model` writes `BitDepth` (int) / `PixelFormat` (str) from `sidecar_metadata` when present +- [x] `_to_bids_model` omits `BitDepth` / `PixelFormat` when absent from `sidecar_metadata` ### Integration tests (with synthetic BIDS fixture) diff --git a/src/reprostim/qr/bids_inject.py b/src/reprostim/qr/bids_inject.py index 744ec66e..f036789a 100644 --- a/src/reprostim/qr/bids_inject.py +++ b/src/reprostim/qr/bids_inject.py @@ -895,17 +895,20 @@ def _capturing_out_func(msg: str) -> None: if record.metadata and record.metadata.TaskName: sidecar_metadata["TaskName"] = record.metadata.TaskName - # TODO: in the future, read VideoCodecRFC6381 / AudioCodecRFC6381 from - # dedicated columns in videos.tsv populated by video-audit, avoiding - # the extra ffprobe call here. + # TODO: in the future, read these fields from dedicated columns in videos.tsv + # populated by video-audit, avoiding the extra ffprobe call here. try: ai, vi = get_audio_video_info_ffprobe(input_path) if vi.codec_rfc6381: sidecar_metadata["VideoCodecRFC6381"] = vi.codec_rfc6381 if ai.codec_rfc6381: sidecar_metadata["AudioCodecRFC6381"] = ai.codec_rfc6381 + if vi.bit_depth is not None: + sidecar_metadata["BitDepth"] = vi.bit_depth + if vi.pix_fmt: + sidecar_metadata["PixelFormat"] = vi.pix_fmt except Exception as e: - logger.error(f"Failed to get codec RFC6381 info for {input_path}: {e}") + logger.error(f"Failed to get video info for {input_path}: {e}") ret, split_results = split_video_main( input_path=input_path, diff --git a/src/reprostim/qr/split_video.py b/src/reprostim/qr/split_video.py index 50989fb6..d98018fe 100644 --- a/src/reprostim/qr/split_video.py +++ b/src/reprostim/qr/split_video.py @@ -114,7 +114,8 @@ def _to_bids_model(sr: "SplitResult", sidecar_metadata: dict | None = None) -> d :param sr: SplitResult to convert :param sidecar_metadata: Optional dict with extra BIDS fields to inject. Supports ``TaskName`` (written as the first field when present), - ``VideoCodecRFC6381``, and ``AudioCodecRFC6381``. + ``VideoCodecRFC6381``, ``AudioCodecRFC6381``, ``BitDepth``, and + ``PixelFormat``. :return: Dict with BIDS-compliant metadata field names """ result = {} @@ -154,6 +155,14 @@ def _to_bids_model(sr: "SplitResult", sidecar_metadata: dict | None = None) -> d except (ValueError, TypeError): pass + if sidecar_metadata: + bit_depth = sidecar_metadata.get("BitDepth") + if bit_depth is not None: + result["BitDepth"] = int(bit_depth) + pix_fmt = sidecar_metadata.get("PixelFormat") + if pix_fmt: + result["PixelFormat"] = pix_fmt + if sr.audio_codec != "n/a": result["AudioCodec"] = sr.audio_codec result["AudioCodecRFC6381"] = (sidecar_metadata or {}).get( diff --git a/tests/qr/test_bids_inject.py b/tests/qr/test_bids_inject.py index 1666ca9c..29669902 100644 --- a/tests/qr/test_bids_inject.py +++ b/tests/qr/test_bids_inject.py @@ -48,6 +48,7 @@ dt_utc_to_reprostim, ) from reprostim.qr.split_video import SplitResult +from reprostim.qr.video_audit import AudioInfo, VideoInfo # --------------------------------------------------------------------------- # Shared fixtures / helpers @@ -1046,8 +1047,6 @@ def _fake_split_video_main(**kwargs): captured.update(kwargs) return 0, [] - from reprostim.qr.video_audit import AudioInfo, VideoInfo - fake_ai = AudioInfo(codec="aac", codec_rfc6381="mp4a.40.2") fake_vi = VideoInfo(codec="h264", codec_rfc6381="avc1.640028") @@ -1070,6 +1069,42 @@ def _fake_split_video_main(**kwargs): assert captured["sidecar_metadata"].get("AudioCodecRFC6381") == "mp4a.40.2" +def test_call_split_video_passes_bit_depth_and_pixel_format_in_sidecar_metadata( + tmp_path, +): + """_call_split_video adds BitDepth and PixelFormat to sidecar_metadata + via ffprobe.""" + scans_tsv = _copy_bids_fixture(tmp_path) + videos_tsv = _write_videos_tsv(tmp_path, _VA_V1) + + captured = {} + + def _fake_split_video_main(**kwargs): + captured.update(kwargs) + return 0, [] + + fake_ai = AudioInfo() + fake_vi = VideoInfo(codec="h264", bit_depth=10, pix_fmt="yuv420p10le") + + with patch("reprostim.qr.split_video.do_main", side_effect=_fake_split_video_main): + with patch( + "reprostim.qr.bids_inject.get_audio_video_info_ffprobe", + return_value=(fake_ai, fake_vi), + ): + ret, _ = _run( + [str(scans_tsv)], + videos_tsv, + match=_MATCH_TASK_REST, + overwrite="skip", + dry_run=False, + ) + + assert ret == 0 + assert "sidecar_metadata" in captured + assert captured["sidecar_metadata"].get("BitDepth") == 10 + assert captured["sidecar_metadata"].get("PixelFormat") == "yuv420p10le" + + def test_call_split_video_rfc6381_ffprobe_error_continues(tmp_path): """ffprobe failure logs an error but injection still proceeds.""" scans_tsv = _copy_bids_fixture(tmp_path) diff --git a/tests/qr/test_split_video.py b/tests/qr/test_split_video.py index de607de3..0a6847dd 100644 --- a/tests/qr/test_split_video.py +++ b/tests/qr/test_split_video.py @@ -702,6 +702,26 @@ def test_to_bids_model_rfc6381_defaults_to_na_without_sidecar_metadata(): assert data["AudioCodecRFC6381"] == "n/a" +def test_to_bids_model_bit_depth_and_pixel_format_from_sidecar_metadata(): + """BitDepth and PixelFormat are written when present in sidecar_metadata.""" + sr = SplitResult(video_codec="h264") + data = _to_bids_model( + sr, + sidecar_metadata={"BitDepth": 10, "PixelFormat": "yuv420p"}, + ) + assert data["BitDepth"] == 10 + assert isinstance(data["BitDepth"], int) + assert data["PixelFormat"] == "yuv420p" + + +def test_to_bids_model_bit_depth_and_pixel_format_absent_when_not_in_sidecar(): + """BitDepth and PixelFormat are omitted when absent from sidecar_metadata.""" + sr = SplitResult(video_codec="h264") + data = _to_bids_model(sr) + assert "BitDepth" not in data + assert "PixelFormat" not in data + + def test_to_bids_model_no_raw_fields(): """BIDS output contains no raw SplitResult field names.""" sr = _make_split_result()