From 0ed556155394c9d56fd2939032444b4f215f0aaa Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:37:28 -0500 Subject: [PATCH] skip vod clips that start after their audio ends When a camera's audio dropped out, a recording segment could end its audio early. If a playback range started past that point in the segment, nginx-vod saw no audio track in the first clip, failed the whole range with a track count mismatch, and left the player spinning for up to an hour of footage. Recording now stores each segment's audio length alongside its keyframe index, from the same ffprobe pass, and plan_clip skips a clip whose window starts at or after its audio ends when the range has other clips. A range made of only that clip still plays video-only, and legacy rows without the new column are unchanged. --- frigate/api/media.py | 6 +- frigate/models.py | 1 + frigate/record/maintainer.py | 13 ++- frigate/test/test_maintainer.py | 8 +- frigate/test/test_record_sub_maintainer.py | 15 +-- frigate/test/test_recording_coverage.py | 107 ++++++++++++++++++ frigate/util/media.py | 54 ++++++--- frigate/util/recording_coverage.py | 28 ++++- .../040_add_recordings_audio_duration.py | 14 +++ 9 files changed, 208 insertions(+), 38 deletions(-) create mode 100644 migrations/040_add_recordings_audio_duration.py diff --git a/frigate/api/media.py b/frigate/api/media.py index 8ca13d8860..fb22d40906 100644 --- a/frigate/api/media.py +++ b/frigate/api/media.py @@ -624,14 +624,14 @@ async def recording_clip( def _build_vod_clip( - row: Any, start: float, end: float + row: Any, start: float, end: float, shared: bool ) -> tuple[dict[str, Any], int] | None: """Build one nginx-vod clip dict + duration (ms) for a recording row trimmed to [start, end). Realization comes entirely from the shared plan_clip, so the coverage endpoint's realized timelines match this manifest by construction. """ - plan = plan_clip(row, start, end) + plan = plan_clip(row, start, end, shared) if plan.skipped: return None @@ -707,7 +707,7 @@ async def _vod_response( row.end_time, row.duration, ) - built = _build_vod_clip(row, span_start, span_end) + built = _build_vod_clip(row, span_start, span_end, len(spans) > 1) if built is None: continue diff --git a/frigate/models.py b/frigate/models.py index f3b5346858..8d813d3a66 100644 --- a/frigate/models.py +++ b/frigate/models.py @@ -85,6 +85,7 @@ class Recordings(Model): audio_rate = IntegerField(null=True) # Hz; NULL = unknown (legacy rows) audio_codec = CharField(null=True, max_length=20) # NULL = unknown (legacy rows) video_codec = CharField(null=True, max_length=20) # NULL = unknown (legacy rows) + audio_duration = FloatField(null=True) # seconds of audio samples; NULL = unknown class ExportCase(Model): diff --git a/frigate/record/maintainer.py b/frigate/record/maintainer.py index e75efe2003..dc41b02d50 100644 --- a/frigate/record/maintainer.py +++ b/frigate/record/maintainer.py @@ -42,7 +42,7 @@ from frigate.const import ( ) from frigate.models import Recordings, ReviewSegment from frigate.review.types import SeverityEnum -from frigate.util.media import get_keyframe_offsets +from frigate.util.media import get_playback_index from frigate.util.ownership import chown_to_runtime from frigate.util.services import get_video_properties @@ -188,7 +188,7 @@ class RecordingMaintainer(threading.Thread): self.object_recordings_info: dict[str, list] = defaultdict(list) self.audio_recordings_info: dict[str, list] = defaultdict(list) # cache_path -> (end_time, duration, has_audio, audio_rate, - # audio_codec, video_codec, keyframes) + # audio_codec, video_codec, keyframes, audio_duration) self.end_time_cache: dict[ str, tuple[ @@ -199,6 +199,7 @@ class RecordingMaintainer(threading.Thread): str | None, str | None, list[int] | None, + float | None, ], ] = {} # last known capture end per (camera, stream_type); 0.0 marks a key @@ -619,6 +620,7 @@ class RecordingMaintainer(threading.Thread): audio_codec, video_codec, keyframes, + audio_duration, ) = self.end_time_cache[cache_path] # recover the resolved start rather than reusing the truncated # filename timestamp @@ -652,7 +654,7 @@ class RecordingMaintainer(threading.Thread): # playback snaps mid-file entry points against these offsets # instead of probing files on demand async with self.probe_semaphore: - keyframes = await get_keyframe_offsets(cache_path) + keyframes, audio_duration = await get_playback_index(cache_path) if previous_start is not None: await previous_start.wait() @@ -669,6 +671,7 @@ class RecordingMaintainer(threading.Thread): audio_codec, video_codec, keyframes, + audio_duration, ) # segments later discarded by retention still advance the # chain for the next kept segment @@ -763,6 +766,7 @@ class RecordingMaintainer(threading.Thread): audio_codec, video_codec, keyframes, + audio_duration, ) # we fell through the continuous / motion check, so we need to check the review items @@ -815,6 +819,7 @@ class RecordingMaintainer(threading.Thread): audio_codec, video_codec, keyframes, + audio_duration, ) else: self.drop_segment(cache_path) @@ -960,6 +965,7 @@ class RecordingMaintainer(threading.Thread): audio_codec: str | None = None, video_codec: str | None = None, keyframes: list[int] | None = None, + audio_duration: float | None = None, ) -> dict[str, Any] | None: path_time = segment_path_time(cache_path) or start_time @@ -1053,6 +1059,7 @@ class RecordingMaintainer(threading.Thread): Recordings.audio_codec.name: audio_codec, Recordings.video_codec.name: video_codec, Recordings.keyframes.name: keyframes, + Recordings.audio_duration.name: audio_duration, } except Exception: logger.exception(f"Unable to store recording segment {cache_path}") diff --git a/frigate/test/test_maintainer.py b/frigate/test/test_maintainer.py index 43202b29d9..f4fafc7ab6 100644 --- a/frigate/test/test_maintainer.py +++ b/frigate/test/test_maintainer.py @@ -155,7 +155,7 @@ class TestMaintainer(unittest.IsolatedAsyncioTestCase): cache_path = "/tmp/cache/test_cam@20260417150000+0000.mp4" maintainer.end_time_cache = { - cache_path: (end_time, 10.0, None, None, None, None, None) + cache_path: (end_time, 10.0, None, None, None, None, None, None) } # Single processed frame well past end_time with no motion/objects. maintainer.object_recordings_info["test_cam"] = [(now.timestamp(), [], [], [])] @@ -199,7 +199,7 @@ class TestMaintainer(unittest.IsolatedAsyncioTestCase): cache_path = "/tmp/cache/test_cam@20260417150000+0000.mp4" maintainer.end_time_cache = { - cache_path: (end_time, 10.0, False, None, None, None, []) + cache_path: (end_time, 10.0, False, None, None, None, [], None) } maintainer.object_recordings_info["test_cam"] = [(now.timestamp(), [], [], [])] maintainer.audio_recordings_info["test_cam"] = [] @@ -255,7 +255,7 @@ class TestMaintainer(unittest.IsolatedAsyncioTestCase): cache_path = "/tmp/cache/test_cam@20260417150000+0000.mp4" maintainer.end_time_cache = { - cache_path: (end_time, 10.0, False, None, None, None, []) + cache_path: (end_time, 10.0, False, None, None, None, [], None) } maintainer.object_recordings_info["test_cam"] = [(now.timestamp(), [], [], [])] maintainer.audio_recordings_info["test_cam"] = [] @@ -313,7 +313,7 @@ class TestMaintainer(unittest.IsolatedAsyncioTestCase): cache_path = "/tmp/cache/test_cam@20260417150000+0000.mp4" maintainer.end_time_cache = { - cache_path: (end_time, 10.0, False, None, None, None, []) + cache_path: (end_time, 10.0, False, None, None, None, [], None) } # Metadata has only reached partway into the segment. maintainer.object_recordings_info["test_cam"] = [ diff --git a/frigate/test/test_record_sub_maintainer.py b/frigate/test/test_record_sub_maintainer.py index 17649e1ee7..836590b8ff 100644 --- a/frigate/test/test_record_sub_maintainer.py +++ b/frigate/test/test_record_sub_maintainer.py @@ -77,8 +77,8 @@ async def _validate_segment( with ( patch("frigate.record.maintainer.get_video_properties", probe), patch( - "frigate.record.maintainer.get_keyframe_offsets", - AsyncMock(return_value=[0]), + "frigate.record.maintainer.get_playback_index", + AsyncMock(return_value=([0], 10.0)), ), patch("frigate.record.maintainer.os.path.getmtime", getmtime), ): @@ -178,7 +178,7 @@ class TestValidateAndMoveSubSegment(unittest.IsolatedAsyncioTestCase): # pre-fill the end time cache so no ffprobe is attempted; the # audio and codec fields stay unknown without one maintainer.end_time_cache = { - cache_path: (end_time, 10.0, None, None, None, None, None) + cache_path: (end_time, 10.0, None, None, None, None, None, None) } # a processed frame past end_time so the segment is considered ready maintainer.object_recordings_info["test_cam"] = [(now.timestamp(), [], [], [])] @@ -264,8 +264,8 @@ class TestSegmentAudioPresence(unittest.IsolatedAsyncioTestCase): with ( patch("frigate.record.maintainer.get_video_properties", probe), patch( - "frigate.record.maintainer.get_keyframe_offsets", - AsyncMock(return_value=[0, 2000]), + "frigate.record.maintainer.get_playback_index", + AsyncMock(return_value=([0, 2000], 9.5)), ), ): await maintainer.validate_and_move_segment( @@ -284,6 +284,7 @@ class TestSegmentAudioPresence(unittest.IsolatedAsyncioTestCase): self.assertEqual(call_args[8], audio_rate) self.assertEqual(call_args[9], audio_codec) self.assertEqual(call_args[11], [0, 2000]) + self.assertEqual(call_args[12], 9.5) self.assertEqual(call_args[10], video_codec) # the probe result is cached alongside the end time so the # cached path stays as informed as the probed path @@ -558,8 +559,8 @@ class TestSegmentStartChaining(unittest.IsolatedAsyncioTestCase): with ( patch("frigate.record.maintainer.get_video_properties", probe), patch( - "frigate.record.maintainer.get_keyframe_offsets", - AsyncMock(return_value=[0]), + "frigate.record.maintainer.get_playback_index", + AsyncMock(return_value=([0], 10.0)), ), patch( "frigate.record.maintainer.os.path.getmtime", diff --git a/frigate/test/test_recording_coverage.py b/frigate/test/test_recording_coverage.py index c559ec4997..45545493b1 100644 --- a/frigate/test/test_recording_coverage.py +++ b/frigate/test/test_recording_coverage.py @@ -10,6 +10,7 @@ from peewee import SqliteDatabase from frigate.api.media import _vod_response from frigate.const import MAX_SEGMENT_DURATION from frigate.models import Recordings +from frigate.util.media import parse_playback_index from frigate.util.recording_coverage import ( _rows_query, coverage_spans, @@ -47,6 +48,7 @@ class CoverageDbTestCase(unittest.TestCase): has_audio=None, segment_size=0, keyframes=None, + audio_duration=None, ): Recordings.create( id=id, @@ -62,6 +64,7 @@ class CoverageDbTestCase(unittest.TestCase): has_audio=has_audio, segment_size=segment_size, keyframes=keyframes, + audio_duration=audio_duration, ) @@ -419,3 +422,107 @@ class TestVodManifestKeyFrames(CoverageDbTestCase): self._insert("m1", 1000.0, 1010.0, "main") clips = self._clips(1000.0, 1010.0) assert clips[0]["keyFrameDurations"] == [10000] + + +class TestVodAudioCutShort(CoverageDbTestCase): + """A clip whose audio ends before its clipFrom has no audio track to nginx-vod.""" + + KEYFRAMES = [0, 1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000] + + def _insert_pair(self, first_audio_duration): + # the first row's audio dropped out; the range starts 5s into it + for id, start, audio_duration in ( + ("m1", 1000.0, first_audio_duration), + ("m2", 1010.0, 10.0), + ): + self._insert( + id, + start, + start + 10.0, + "main", + has_audio=True, + audio_codec="aac", + audio_rate=16000, + keyframes=self.KEYFRAMES, + audio_duration=audio_duration, + ) + + def _clips(self, start, end): + response = asyncio.run(_vod_response("front_door", start, end)) + return json.loads(response.body)["sequences"][0]["clips"] + + def test_clip_past_audio_end_is_skipped(self): + self._insert_pair(2.112) + clips = self._clips(1005.0, 1020.0) + assert [c["path"] for c in clips] == ["/tmp/m2.mp4"] + timeline = realized_timeline( + resolve_coverage("front_door", 1005.0, 1020.0), None + ) + assert [t["duration"] for t in timeline] == [0, 10000] + + def test_clip_with_audio_left_is_kept(self): + # nginx-vod keeps the track while any sample lies past clipFrom + self._insert_pair(5.001) + clips = self._clips(1005.0, 1020.0) + assert clips[0]["clipFrom"] == 5000 + assert len(clips) == 2 + + def test_clip_starting_on_audio_end_is_skipped(self): + self._insert_pair(5.0) + clips = self._clips(1005.0, 1020.0) + assert len(clips) == 1 + + def test_legacy_row_is_kept(self): + self._insert_pair(None) + clips = self._clips(1005.0, 1020.0) + assert len(clips) == 2 + + def test_sole_clip_is_kept(self): + # alone in its sequence the clip plays video-only, so keep it + self._insert_pair(2.112) + clips = self._clips(1005.0, 1009.0) + assert clips[0]["path"] == "/tmp/m1.mp4" + timeline = realized_timeline( + resolve_coverage("front_door", 1005.0, 1009.0), None + ) + assert timeline[0]["duration"] > 0 + + +class TestParsePlaybackIndex(unittest.TestCase): + def test_splits_video_keyframes_from_audio_duration(self): + output = "\n".join( + [ + "video,0,0.000000,0.040000,K__", + "audio,1,0.000000,0.064000,K__", + "video,0,0.040000,0.040000,___", + "audio,1,0.064000,0.064000,K__", + "video,0,1.000000,0.040000,K__", + "audio,1,0.128000,N/A,K__", + ] + ) + keyframes, audio_duration = parse_playback_index(output) + assert keyframes == [0, 1000] + assert audio_duration == 0.128 + + def test_no_audio_packets_reports_zero(self): + keyframes, audio_duration = parse_playback_index( + "video,0,0.000000,0.040000,K__" + ) + assert keyframes == [0] + assert audio_duration == 0.0 + + def test_only_first_video_and_audio_streams_count(self): + output = "\n".join( + [ + "video,1,0.000000,0.100000,K__", + "video,0,0.000000,0.040000,K__", + "audio,3,0.000000,0.064000,K__", + "audio,2,0.000000,0.064000,K__", + "video,1,2.500000,0.100000,K__", + "video,0,1.000000,0.040000,K__", + "audio,3,0.064000,0.064000,K__", + ] + ) + keyframes, audio_duration = parse_playback_index(output) + assert keyframes == [0, 1000] + assert audio_duration == 0.064 diff --git a/frigate/util/media.py b/frigate/util/media.py index 5810a75ee4..e87b8ec2dc 100644 --- a/frigate/util/media.py +++ b/frigate/util/media.py @@ -938,21 +938,22 @@ def sync_all_media( return results -async def get_keyframe_offsets(path: str) -> list[int] | None: - """Get every video keyframe offset (ms from segment start) in an mp4. +async def get_playback_index(path: str) -> tuple[list[int] | None, float | None]: + """Get the video keyframe offsets (ms from segment start) and audio length of an mp4. - Runs at record time so playback never needs to probe. Returns None if - ffprobe fails, so the caller stores NULL and playback falls back to - serving whole files. + Runs at record time so playback never needs to probe. The audio length + is the summed duration of the audio samples. nginx-vod ignores edit + lists, so it plays late-starting audio from zero and also counts the + AAC priming frame ffprobe drops, which puts this sum at or one frame + below its measure. Returns (None, None) if ffprobe fails, so the caller + stores NULL and playback falls back to serving whole files. """ proc = None try: proc = await asyncio.create_subprocess_exec( FFPROBE_PATH, - "-select_streams", - "v:0", "-show_entries", - "packet=pts_time,flags", + "packet=codec_type,stream_index,pts_time,duration_time,flags", "-of", "csv=p=0", "-loglevel", @@ -968,22 +969,39 @@ async def get_keyframe_offsets(path: str) -> list[int] | None: proc.kill() with contextlib.suppress(TimeoutError): await asyncio.wait_for(proc.communicate(), timeout=2) - return None + return None, None if proc.returncode != 0: - return None + return None, None - offsets: list[int] = [] - for line in stdout.decode().strip().splitlines(): + return parse_playback_index(stdout.decode()) + + +def parse_playback_index(output: str) -> tuple[list[int], float]: + """Parse ffprobe packet CSV (codec_type,stream_index,pts_time,duration_time,flags). + + Only the first video and first audio stream count, matching the tracks + nginx-vod serves. + """ + keyframes: dict[int, list[int]] = {} + audio: dict[int, float] = {} + for line in output.strip().splitlines(): parts = line.strip().split(",") - if len(parts) != 2: - continue - ts_str, flags = parts - if "K" not in flags: + if len(parts) != 5: continue + codec_type, index_str, ts_str, dur_str, flags = parts try: - offsets.append(int(float(ts_str) * 1000)) + index = int(index_str) + if codec_type == "video": + stream = keyframes.setdefault(index, []) + if "K" in flags: + stream.append(int(float(ts_str) * 1000)) + elif codec_type == "audio": + audio[index] = audio.get(index, 0.0) + float(dur_str) except ValueError: continue - return offsets + return ( + keyframes[min(keyframes)] if keyframes else [], + audio[min(audio)] if audio else 0.0, + ) diff --git a/frigate/util/recording_coverage.py b/frigate/util/recording_coverage.py index 5c059811e1..305d231d1d 100644 --- a/frigate/util/recording_coverage.py +++ b/frigate/util/recording_coverage.py @@ -49,6 +49,7 @@ def _rows_query(camera: str, after: float, before: float, stream_type: str) -> A Recordings.video_codec, Recordings.segment_size, Recordings.keyframes, + Recordings.audio_duration, ) .where( (Recordings.camera == camera) @@ -340,13 +341,15 @@ class ClipPlan: first_key_frame_offset_ms: int = 0 -def plan_clip(row: Any, start: float, end: float) -> ClipPlan: +def plan_clip(row: Any, start: float, end: float, shared: bool = False) -> ClipPlan: """Plan one nginx-vod clip for a recording row trimmed to [start, end). The single source of truth for clip realization: the vod mapping builder emits exactly this plan and the coverage timelines report it to the frontend, so the playhead model matches the playlist by construction rather than accumulating drift at each hand-off. + + shared marks a clip that sits in a sequence with other clips. """ min_duration_ms = 100 # Minimum 100ms to ensure at least one video frame max_duration_ms = MAX_SEGMENT_DURATION * 1000 @@ -397,6 +400,22 @@ def plan_clip(row: Any, start: float, end: float) -> ClipPlan: ) return ClipPlan(None, 0, True) + # audio cut short by a camera dropout can end before the clip starts. + # nginx-vod then counts no audio track in this clip and fails the whole + # sequence on the track count mismatch + if ( + shared + and row.has_audio + and row.audio_duration is not None + and (clip_from or 0) >= int(row.audio_duration * 1000) + ): + logger.debug( + "VOD: skipping recording %s - no audio after %sms", + row.path, + clip_from or 0, + ) + return ClipPlan(None, 0, True) + if duration >= max_duration_ms: logger.warning(f"Recording clip is missing or empty: {row.path}") return ClipPlan(None, 0, True) @@ -445,14 +464,17 @@ def realized_timeline( so summing durations reproduces playlist time exactly. A span whose clip is skipped reports duration 0. """ + spans = build_spans(intervals, stream) + shared = len(spans) > 1 + return [ { "start_time": span_start, "end_time": span_end, "duration": plan.duration_ms, } - for row, span_start, span_end, _ in build_spans(intervals, stream) - for plan in (plan_clip(row, span_start, span_end),) + for row, span_start, span_end, _ in spans + for plan in (plan_clip(row, span_start, span_end, shared),) ] diff --git a/migrations/040_add_recordings_audio_duration.py b/migrations/040_add_recordings_audio_duration.py new file mode 100644 index 0000000000..cdfe15c777 --- /dev/null +++ b/migrations/040_add_recordings_audio_duration.py @@ -0,0 +1,14 @@ +"""Peewee migrations -- 040_add_recordings_audio_duration.py.""" + +import peewee as pw + +SQL = pw.SQL + + +def migrate(migrator, database, fake=False, **kwargs): + # nullable so legacy rows stay NULL, meaning unknown + migrator.sql('ALTER TABLE "recordings" ADD COLUMN "audio_duration" REAL') + + +def rollback(migrator, database, fake=False, **kwargs): + pass