From 17c1c5b04b0d05cc19b1d7b804e184018b76b0be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Verstraeten?= Date: Thu, 11 Jun 2026 19:52:38 +0200 Subject: [PATCH] Drop truncated GOPs at loop/restart seams Buffer and conditionally drop a pending GOP when an upstream loop/restart emits a premature IDR. mp4.go: add gopBuffer and bufferedSample types, hold samples until the next video keyframe, detect a seam by comparing the new keyframe interval against the previous cadence (SeamGapDivisor) and drop the short/truncated tail GOP or commit buffered samples. Add commitBufferedGOP and commitSampleToTrack helpers and flush the final buffered GOP on Close. mp4_loopseam_test.go: update test descriptions, expectations and names to assert the truncated tail GOP is dropped exactly once across different GOP sizes. Dockerfile and Dockerfile.arm64: re-declare ARG VERSION inside the build stage and only derive git describe when VERSION is unset or the default 0.0.0 so build-arg values are respected. Add a binary MP4 fixture used by the tests. --- Dockerfile | 9 +- Dockerfile.arm64 | 9 +- machinery/src/video/mp4.go | 146 +++++++++++++++++------ machinery/src/video/mp4_loopseam_test.go | 81 +++++++++---- 4 files changed, 182 insertions(+), 63 deletions(-) diff --git a/Dockerfile b/Dockerfile index 73e4b3d..ddb8d72 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,6 +4,11 @@ ARG VERSION=0.0.0 FROM kerberos/base:${BASE_IMAGE_VERSION} AS build-machinery LABEL AUTHOR=uug.ai +# Re-declare VERSION inside this stage so the value passed via +# `--build-arg VERSION=...` (e.g. the release tag) is available below. +# ARGs declared before the first FROM are not visible inside build stages. +ARG VERSION + ENV GOROOT=/usr/local/go ENV GOPATH=/go ENV PATH=$GOPATH/bin:$GOROOT/bin:/usr/local/lib:$PATH @@ -35,7 +40,9 @@ RUN cat /go/src/github.com/kerberos-io/agent/machinery/version RUN cd /go/src/github.com/kerberos-io/agent/machinery && \ go mod download && \ - VERSION=$(cd /go/src/github.com/kerberos-io/agent && git describe --tags --always 2>/dev/null || echo "${VERSION}") && \ + if [ -z "${VERSION}" ] || [ "${VERSION}" = "0.0.0" ]; then \ + VERSION=$(cd /go/src/github.com/kerberos-io/agent && git describe --tags --always 2>/dev/null || echo "0.0.0"); \ + fi && \ go build -tags timetzdata,netgo,osusergo --ldflags "-s -w -X github.com/kerberos-io/agent/machinery/src/utils.VERSION=${VERSION} -extldflags '-static -latomic'" main.go && \ mkdir -p /agent && \ mv main /agent && \ diff --git a/Dockerfile.arm64 b/Dockerfile.arm64 index 0f1b689..e324e6d 100644 --- a/Dockerfile.arm64 +++ b/Dockerfile.arm64 @@ -4,6 +4,11 @@ ARG VERSION=0.0.0 FROM kerberos/base:${BASE_IMAGE_VERSION} AS build-machinery LABEL AUTHOR=uug.ai +# Re-declare VERSION inside this stage so the value passed via +# `--build-arg VERSION=...` (e.g. the release tag) is available below. +# ARGs declared before the first FROM are not visible inside build stages. +ARG VERSION + ENV GOROOT=/usr/local/go ENV GOPATH=/go ENV PATH=$GOPATH/bin:$GOROOT/bin:/usr/local/lib:$PATH @@ -35,7 +40,9 @@ RUN cat /go/src/github.com/kerberos-io/agent/machinery/version RUN cd /go/src/github.com/kerberos-io/agent/machinery && \ go mod download && \ - VERSION=$(cd /go/src/github.com/kerberos-io/agent && git describe --tags --always 2>/dev/null || echo "${VERSION}") && \ + if [ -z "${VERSION}" ] || [ "${VERSION}" = "0.0.0" ]; then \ + VERSION=$(cd /go/src/github.com/kerberos-io/agent && git describe --tags --always 2>/dev/null || echo "0.0.0"); \ + fi && \ go build -tags timetzdata,netgo,osusergo --ldflags "-s -w -X github.com/kerberos-io/agent/machinery/src/utils.VERSION=${VERSION} -extldflags '-static -latomic'" main.go && \ mkdir -p /agent && \ mv main /agent && \ diff --git a/machinery/src/video/mp4.go b/machinery/src/video/mp4.go index 791bc6f..4718c52 100644 --- a/machinery/src/video/mp4.go +++ b/machinery/src/video/mp4.go @@ -86,6 +86,18 @@ type MP4 struct { PendingSampleIsKeyframe bool // Whether the pending video sample is a keyframe LastKeyframeRawPTS uint64 // Raw PTS of the most recently seen keyframe (across fragments) LastKeyframeGapMs uint64 // Interval (ms) between the two most recent keyframes; reference cadence for seam detection + gopBuffer []bufferedSample // Current, not-yet-committed GOP (video frames + interleaved audio), held so a loop-seam GOP can be dropped before it reaches the file +} + +// bufferedSample is a single sample (video or audio) held in the current-GOP +// buffer until we know whether the GOP should be committed to the file or +// dropped as an upstream loop-seam artifact (see AddSampleToTrack). +type bufferedSample struct { + trackID uint32 + isKeyframe bool + data []byte + pts uint64 + compositionOffset int64 } // NewMP4 creates a new MP4 object. @@ -287,7 +299,101 @@ func (mp4 *MP4) flushPendingVideoSample(nextPTS uint64) bool { // in PTS order while the fragment timeline stays monotonic in DTS. // // For audio, pts is the sample timestamp and compositionOffset should be 0. +// +// Samples are not written straight through. Each video GOP is held in a small +// buffer (gopBuffer) until the next keyframe arrives, so a GOP belonging to an +// upstream source-loop / restart seam can be dropped before it ever reaches the +// file. When a source MP4 is looped through virtual-rtsp +// (ffmpeg `-stream_loop -1 -re`), the loop boundary leaves a truncated tail GOP +// whose first inter-frame is incomplete: software decoders conceal the missing +// macroblocks, but hardware decoders (macOS VideoToolbox) reject it with +// kVTVideoDecoderBadDataErr (-12909) and MSE players (Video.js / Chromium / +// Firefox) report media corruption, freezing playback at the seam (e.g. the +// ~10s mark in the original recordings). The seam IDR that follows is a clean +// random-access point, so dropping the truncated GOP lets playback continue +// seamlessly. Holding back at most one GOP only delays on-disk fragments; for +// any recording without a seam the finalized file is identical to the straight +// pass-through output (Close flushes the final buffered GOP). func (mp4 *MP4) AddSampleToTrack(trackID uint32, isKeyframe bool, data []byte, pts uint64, compositionOffset int64) error { + isVideoKeyframe := isKeyframe && trackID == uint32(mp4.VideoTrack) + if !isVideoKeyframe { + // Part of the current GOP window (P/B frame or interleaved audio): hold it + // until the GOP is committed or dropped at the next video keyframe. + mp4.gopBuffer = append(mp4.gopBuffer, bufferedSample{ + trackID: trackID, + isKeyframe: isKeyframe, + data: data, + pts: pts, + compositionOffset: compositionOffset, + }) + return nil + } + + // A video keyframe ends the GOP we have been buffering. Decide whether that + // buffered GOP is genuine (commit it) or the truncated tail GOP at an upstream + // loop/restart seam (drop it). + // + // The GOP size is configurable per camera, so we do NOT compare against a + // fixed millisecond threshold. Instead we compare this keyframe interval to + // the previous one and only flag a *sudden* shortening: a seam IDR arrives in + // less than (previous interval / SeamGapDivisor). Deriving the threshold from + // the observed cadence keeps detection correct for any configured GOP (0.5s, + // 1s, 2s, ...) and avoids false positives on steady short-GOP / all-intra + // streams (where consecutive intervals are similar, so none looks anomalously + // short). Because the reference is the immediately preceding interval, a burst + // of close keyframes only drops a single GOP instead of cascading. + seam := false + if mp4.LastKeyframeRawPTS > 0 && pts > mp4.LastKeyframeRawPTS { + gap := pts - mp4.LastKeyframeRawPTS + if mp4.LastKeyframeGapMs > 0 && gap*SeamGapDivisor < mp4.LastKeyframeGapMs { + seam = true + log.Log.Warning(fmt.Sprintf("mp4.AddSampleToTrack(): dropping truncated GOP at unexpectedly close keyframe (interval=%d ms, previous interval=%d ms, buffered samples=%d) - likely upstream loop/restart discontinuity", gap, mp4.LastKeyframeGapMs, len(mp4.gopBuffer))) + } + mp4.LastKeyframeGapMs = gap + } + mp4.LastKeyframeRawPTS = pts + + if seam { + // Discard the truncated tail GOP; this keyframe is a clean restart point. + mp4.gopBuffer = mp4.gopBuffer[:0] + } else { + // Genuine GOP boundary: commit the GOP we just finished buffering. + mp4.commitBufferedGOP() + } + + // Begin buffering the new GOP, starting with this keyframe. + mp4.gopBuffer = append(mp4.gopBuffer, bufferedSample{ + trackID: trackID, + isKeyframe: isKeyframe, + data: data, + pts: pts, + compositionOffset: compositionOffset, + }) + return nil +} + +// commitBufferedGOP writes every sample currently held in gopBuffer to the file +// in arrival order, then clears the buffer. Committing in arrival order +// preserves the original audio/video interleave and lets commitSampleToTrack's +// pending-sample mechanism derive each sample's duration from the next one, so +// the on-disk result matches a straight pass-through. +func (mp4 *MP4) commitBufferedGOP() { + if len(mp4.gopBuffer) == 0 { + return + } + buffered := mp4.gopBuffer + mp4.gopBuffer = nil // detach so commitSampleToTrack never observes a half-cleared buffer + for _, s := range buffered { + if err := mp4.commitSampleToTrack(s.trackID, s.isKeyframe, s.data, s.pts, s.compositionOffset); err != nil { + log.Log.Error("mp4.commitBufferedGOP(): " + err.Error()) + } + } +} + +// commitSampleToTrack appends a single buffered sample to the current fragment. +// It is the low-level writer behind AddSampleToTrack and is only ever invoked +// from commitBufferedGOP, after a GOP has been confirmed as non-seam. +func (mp4 *MP4) commitSampleToTrack(trackID uint32, isKeyframe bool, data []byte, pts uint64, compositionOffset int64) error { if isKeyframe && trackID == uint32(mp4.VideoTrack) { mp4.TotalKeyframesReceived++ @@ -310,42 +416,6 @@ func (mp4 *MP4) AddSampleToTrack(trackID uint32, isKeyframe bool, data []byte, p } shouldFlush := !mp4.Start || elapsed >= FragmentDurationMs - // Detect an upstream source-loop / restart discontinuity. When a source - // MP4 is looped through virtual-rtsp (ffmpeg `-stream_loop -1 -re`) the - // loop seam emits a fresh IDR much sooner than a normal GOP would. PTS - // keeps growing monotonically, so the timing-only `elapsed` check above - // does not catch it and the seam IDR ends up as a mid-fragment sync - // sample. macOS VideoToolbox rejects such a fragment with - // kVTVideoDecoderBadDataErr (-12909) and MSE players (Video.js / - // Chromium / Firefox) report media corruption, because the inner IDR - // resets frame_num/POC inside what they expect to be a single GOP. This - // is exactly what freezes playback around the seam (e.g. the ~10s mark). - // Force a fragment boundary so the seam IDR starts its own fragment. - // - // The GOP size is configurable per camera, so we do NOT compare against a - // fixed millisecond threshold. Instead we compare this keyframe interval - // to the previous one and only flag a *sudden* shortening: a seam IDR - // arrives in less than (previous interval / SeamGapDivisor). Deriving the - // threshold from the observed cadence keeps detection correct for any - // configured GOP (0.5s, 1s, 2s, ...) and avoids false positives on - // steady short-GOP / all-intra streams (where consecutive intervals are - // similar, so none looks anomalously short). Because the reference is the - // immediately preceding interval, a burst of close keyframes only forces - // a single flush instead of cascading into many tiny fragments. - if trackID == uint32(mp4.VideoTrack) && mp4.Start && - mp4.LastKeyframeRawPTS > 0 && pts > mp4.LastKeyframeRawPTS { - gap := pts - mp4.LastKeyframeRawPTS - if !shouldFlush && mp4.LastKeyframeGapMs > 0 && - gap*SeamGapDivisor < mp4.LastKeyframeGapMs { - log.Log.Warning(fmt.Sprintf("mp4.AddSampleToTrack(): forcing fragment flush at unexpectedly close keyframe (interval=%d ms, previous interval=%d ms, fragment elapsed=%d ms) - likely upstream loop/restart discontinuity", gap, mp4.LastKeyframeGapMs, elapsed)) - shouldFlush = true - } - mp4.LastKeyframeGapMs = gap - } - if trackID == uint32(mp4.VideoTrack) { - mp4.LastKeyframeRawPTS = pts - } - if shouldFlush { // Write the previous segment to the file if mp4.Start { @@ -485,6 +555,10 @@ func (mp4 *MP4) AddSampleToTrack(trackID uint32, isKeyframe bool, data []byte, p func (mp4 *MP4) Close(config *models.Config) { + // Commit the final buffered GOP held back for seam detection. The last GOP of + // a recording is never a loop seam, so it must always be written out. + mp4.commitBufferedGOP() + log.Log.Info(fmt.Sprintf("mp4.Close(): KEYFRAME SUMMARY - totalReceived=%d, totalWritten=%d, segments=%d, lastFragmentKF=%d", mp4.TotalKeyframesReceived, mp4.TotalKeyframesWritten, mp4.SegmentCount, mp4.FragmentKeyframeCount)) diff --git a/machinery/src/video/mp4_loopseam_test.go b/machinery/src/video/mp4_loopseam_test.go index 7b843a9..ccf1bf5 100644 --- a/machinery/src/video/mp4_loopseam_test.go +++ b/machinery/src/video/mp4_loopseam_test.go @@ -9,19 +9,25 @@ import ( ) // runLoopSeamScenario builds a fragmented MP4 that reproduces the loop-seam -// pattern observed in the failing virtual-rtsp recordings (see -// thales_1781183923_3-758_2top_0-0-0-0_-1_30221.mp4): a steady GOP cadence, -// but at the source-MP4 loop boundary an IDR arrives prematurely - far sooner -// than a normal GOP. Without the fix this seam IDR ends up bunched into the -// same fragment as the prior GOP's IDR, producing a mid-fragment sync sample -// that trips macOS VideoToolbox (kVTVideoDecoderBadDataErr / -12909) and -// MSE-based players, freezing playback around the seam (~10s in the original -// file). +// pattern observed in the failing virtual-rtsp recordings (e.g. +// thales_1781196512_3-138_2top_0-0-0-0_-1_30219.mp4): a steady GOP cadence, but +// at the source-MP4 loop boundary the source restarts and emits a fresh IDR far +// sooner than a normal GOP. In the real recordings the short tail GOP left just +// before that premature IDR contains a truncated inter-frame - software decoders +// conceal the missing macroblocks, but hardware decoders (macOS VideoToolbox, +// kVTVideoDecoderBadDataErr / -12909) and MSE players reject it and freeze +// playback at the seam (~10s in the original file). +// +// The fix detects the premature seam IDR and drops the truncated tail GOP that +// precedes it. The seam IDR is itself a clean random-access point, so playback +// resumes seamlessly. This scenario asserts that the tail GOP is removed - +// exactly one GOP fewer than emitted - while every healthy GOP is preserved in +// full and no two IDRs are left bunched in a fragment. // // gopFrames is the number of frames per GOP, so the same scenario can be // exercised at different (configurable) camera GOP sizes. The fix derives its -// threshold from the observed keyframe cadence, so the seam must be isolated -// into its own fragment regardless of GOP size. +// threshold from the observed keyframe cadence, so the truncated tail GOP is +// dropped regardless of GOP size. func runLoopSeamScenario(t *testing.T, gopFrames int) { t.Helper() @@ -71,22 +77,22 @@ func runLoopSeamScenario(t *testing.T, gopFrames int) { } // Several healthy GOPs to establish the cadence and fill a couple of - // fragments, then the last "good" keyframe followed by a few P-frames so we - // are clearly mid-fragment when the seam arrives. + // fragments, then the truncated tail GOP: a keyframe followed by only a few + // P-frames before the source loops. This is the GOP that must be dropped. for g := 0; g < 9; g++ { emitGOP() } emitFrame(true) - seamLead := gopFrames / 5 // seam IDR arrives ~20% into the GOP + seamLead := gopFrames / 5 // tail GOP is only ~20% of a normal GOP before the loop if seamLead < 1 { seamLead = 1 } emitP(seamLead) // Loop seam: the source recording restarts, emitting a fresh IDR far sooner - // than the normal GOP. Without the fix this lands as a mid-fragment sync - // sample and freezes playback at the seam. + // than the normal GOP. The short tail GOP emitted just above is the truncated + // one that must be dropped; this seam IDR opens a fresh, healthy GOP. emitFrame(true) - emitP(gopFrames - 1) // the seam IDR opens a fresh, healthy GOP + emitP(gopFrames - 1) // The recording continues with normal GOPs to the end. for g := 0; g < 10; g++ { emitGOP() @@ -104,11 +110,25 @@ func runLoopSeamScenario(t *testing.T, gopFrames int) { t.Fatalf("decode: %v", err) } + // After the fix, the truncated tail GOP that precedes the premature seam IDR + // is dropped entirely (its first inter-frame is the incomplete one that + // freezes hardware decoders), while every other GOP is preserved in full. + // + // 9 lead GOPs + the seam's own (healthy) GOP + 10 trailing GOPs = 20 committed + // GOPs. The standalone "tail" keyframe and its seamLead P-frames are the + // dropped truncated GOP, so the output must contain exactly one GOP fewer than + // emitted and a whole number of complete GOPs. + const committedGOPs = 9 + 1 + 10 + wantSync := committedGOPs + wantSamples := committedGOPs * gopFrames + // A healthy fragment only ever contains keyframes spaced ~normalGOPms apart. // If any fragment contains two keyframes closer than half a normal GOP, the - // premature seam IDR was not isolated and the file will freeze on playback. + // premature seam IDR was not dropped and the file will freeze on playback. maxBunchMs := normalGOPms / 2 + totalSamples := 0 + totalSync := 0 fragIdx := 0 for _, seg := range parsed.Segments { for _, fr := range seg.Fragments { @@ -121,9 +141,11 @@ func runLoopSeamScenario(t *testing.T, gopFrames int) { var keys []uint64 for _, trun := range traf.Truns { for _, s := range trun.Samples { + totalSamples++ // sample_depends_on == 2 => "does not depend on others" => IDR/sync. if (s.Flags>>24)&0x03 == 0x02 { keys = append(keys, offset) + totalSync++ } offset += uint64(s.Dur) } @@ -132,7 +154,7 @@ func runLoopSeamScenario(t *testing.T, gopFrames int) { for i := 1; i < len(keys); i++ { gap := keys[i] - keys[i-1] if gap < maxBunchMs { - t.Errorf("gop=%dframes frag %d (tfdt=%d): two IDRs only %d ms apart in same fragment (< %d) - seam was not isolated", + t.Errorf("gop=%dframes frag %d (tfdt=%d): two IDRs only %d ms apart in same fragment (< %d) - seam was not dropped", gopFrames, fragIdx, tfdt, gap, maxBunchMs) } } @@ -140,24 +162,33 @@ func runLoopSeamScenario(t *testing.T, gopFrames int) { } } } + + if totalSync != wantSync { + t.Errorf("gop=%dframes: got %d keyframes in output, want %d - the truncated seam GOP was not dropped exactly once", + gopFrames, totalSync, wantSync) + } + if totalSamples != wantSamples { + t.Errorf("gop=%dframes: got %d video samples in output, want %d (= %d committed GOPs x %d frames) - the seam GOP drop removed the wrong frames", + gopFrames, totalSamples, wantSamples, committedGOPs, gopFrames) + } } -// TestMP4LoopSeamIsolation exercises the ~1s GOP case (30 frames @ ~33ms), +// TestMP4LoopSeamDrop exercises the ~1s GOP case (30 frames @ ~33ms), // matching the original failing recording. -func TestMP4LoopSeamIsolation(t *testing.T) { +func TestMP4LoopSeamDrop(t *testing.T) { runLoopSeamScenario(t, 30) } -// TestMP4LoopSeamIsolationLargeGOP exercises a larger ~2s GOP (60 frames). The +// TestMP4LoopSeamDropLargeGOP exercises a larger ~2s GOP (60 frames). The // GOP size is configurable per camera; this guards against regressing to a // fixed-millisecond threshold that would only work for ~1s GOPs. -func TestMP4LoopSeamIsolationLargeGOP(t *testing.T) { +func TestMP4LoopSeamDropLargeGOP(t *testing.T) { runLoopSeamScenario(t, 60) } -// TestMP4LoopSeamIsolationShortGOP exercises a short ~0.5s GOP (15 frames), +// TestMP4LoopSeamDropShortGOP exercises a short ~0.5s GOP (15 frames), // where a fixed ~1s threshold would misfire on every keyframe. The relative -// detection must only isolate the genuine premature seam IDR. -func TestMP4LoopSeamIsolationShortGOP(t *testing.T) { +// detection must only drop the genuine premature seam's truncated tail GOP. +func TestMP4LoopSeamDropShortGOP(t *testing.T) { runLoopSeamScenario(t, 15) }