From 4f417860382a94db3d25f1d65dccbc3ae44b9d96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Verstraeten?= Date: Wed, 5 Aug 2026 14:36:40 +0200 Subject: [PATCH] Store measured FPS in upload marker Instead of snapshotting the configured IP camera FPS, derive the average frame rate from the samples actually committed to the finalized MP4. Adds MP4.AverageFPS(), backed by a SampleCount that is now incremented when a video sample is successfully written, and passes that value to queueRecordingForUpload. Validation (0 < fps <= 240, finite) moves to the numeric value, and unknown FPS still produces an empty, backwards-compatible marker. --- machinery/src/capture/main.go | 20 ++++++++------------ machinery/src/capture/main_test.go | 17 ++++++----------- machinery/src/video/mp4.go | 11 +++++++++++ machinery/src/video/mp4_duration_test.go | 7 +++++++ 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/machinery/src/capture/main.go b/machinery/src/capture/main.go index 44c704c..0a3dfd3 100644 --- a/machinery/src/capture/main.go +++ b/machinery/src/capture/main.go @@ -54,16 +54,12 @@ func publishRecordingState(mqttClient mqtt.Client, hubKey string, configuration } // queueRecordingForUpload creates the marker consumed by the upload worker and -// snapshots the main-stream FPS into it. Keeping the value with the recording -// prevents a delayed upload from using the FPS of a later camera configuration. -// Empty markers remain valid for recordings whose FPS is not yet known. -func queueRecordingForUpload(configDirectory, name string, configuration *models.Configuration) { +// stores the average FPS of the finalized recording in it. Empty markers remain +// valid for recordings whose FPS cannot be determined. +func queueRecordingForUpload(configDirectory, name string, value float64) { fps := "" - if configuration != nil { - candidate := strings.TrimSpace(configuration.Config.Capture.IPCamera.FPS) - if parsed, err := strconv.ParseFloat(candidate, 64); err == nil && parsed > 0 && parsed <= 240 && !math.IsInf(parsed, 0) && !math.IsNaN(parsed) { - fps = candidate - } + if value > 0 && value <= 240 && !math.IsInf(value, 0) && !math.IsNaN(value) { + fps = strings.TrimRight(strings.TrimRight(strconv.FormatFloat(value, 'f', 2, 64), "0"), ".") } // Publish the marker with a same-filesystem rename. Writing directly to the @@ -477,7 +473,7 @@ func HandleRecordStream(queue *packets.Queue, configDirectory string, configurat } } - queueRecordingForUpload(configDirectory, name, configuration) + queueRecordingForUpload(configDirectory, name, mp4Video.AverageFPS()) recordingStatus = "idle" @@ -634,7 +630,7 @@ func HandleRecordStream(queue *packets.Queue, configDirectory string, configurat } } - queueRecordingForUpload(configDirectory, name, configuration) + queueRecordingForUpload(configDirectory, name, mp4Video.AverageFPS()) recordingStatus = "idle" @@ -904,7 +900,7 @@ func HandleRecordStream(queue *packets.Queue, configDirectory string, configurat } } - queueRecordingForUpload(configDirectory, name, configuration) + queueRecordingForUpload(configDirectory, name, mp4Video.AverageFPS()) // Clean up the recording directory if necessary. CleanupRecordingDirectory(configDirectory, configuration) diff --git a/machinery/src/capture/main_test.go b/machinery/src/capture/main_test.go index 6347e43..349ec0b 100644 --- a/machinery/src/capture/main_test.go +++ b/machinery/src/capture/main_test.go @@ -1,22 +1,19 @@ package capture import ( + "math" "os" "path/filepath" "testing" - - "github.com/kerberos-io/agent/machinery/src/models" ) -func TestQueueRecordingForUploadSnapshotsFPS(t *testing.T) { +func TestQueueRecordingForUploadStoresFinalizedFPS(t *testing.T) { configDirectory := t.TempDir() if err := os.MkdirAll(filepath.Join(configDirectory, "data", "cloud"), 0o755); err != nil { t.Fatalf("mkdir cloud queue: %v", err) } - configuration := &models.Configuration{} - configuration.Config.Capture.IPCamera.FPS = "29.97" - queueRecordingForUpload(configDirectory, "recording.mp4", configuration) + queueRecordingForUpload(configDirectory, "recording.mp4", 29.970029) got, err := os.ReadFile(filepath.Join(configDirectory, "data", "cloud", "recording.mp4")) if err != nil { @@ -28,16 +25,14 @@ func TestQueueRecordingForUploadSnapshotsFPS(t *testing.T) { } func TestQueueRecordingForUploadKeepsUnknownFPSCompatible(t *testing.T) { - for _, fps := range []string{"", "invalid", "0", "NaN", "241"} { - t.Run(fps, func(t *testing.T) { + for _, fps := range []float64{0, -1, math.NaN(), math.Inf(1), 241} { + t.Run("invalid FPS", func(t *testing.T) { configDirectory := t.TempDir() if err := os.MkdirAll(filepath.Join(configDirectory, "data", "cloud"), 0o755); err != nil { t.Fatalf("mkdir cloud queue: %v", err) } - configuration := &models.Configuration{} - configuration.Config.Capture.IPCamera.FPS = fps - queueRecordingForUpload(configDirectory, "recording.mp4", configuration) + queueRecordingForUpload(configDirectory, "recording.mp4", fps) got, err := os.ReadFile(filepath.Join(configDirectory, "data", "cloud", "recording.mp4")) if err != nil { diff --git a/machinery/src/video/mp4.go b/machinery/src/video/mp4.go index 4d5eb66..6bc4e8c 100644 --- a/machinery/src/video/mp4.go +++ b/machinery/src/video/mp4.go @@ -283,6 +283,8 @@ func (mp4 *MP4) flushPendingVideoSample(nextPTS uint64) bool { err := mp4.MultiTrackFragment.AddFullSampleToTrack(*mp4.VideoFullSample, uint32(mp4.VideoTrack)) if err != nil { log.Log.Error("mp4.flushPendingVideoSample(): error adding sample: " + err.Error()) + } else { + mp4.SampleCount++ } if isKF { mp4.TotalKeyframesWritten++ @@ -296,6 +298,15 @@ func (mp4 *MP4) flushPendingVideoSample(nextPTS uint64) bool { return true } +// AverageFPS returns the average frame rate of the video samples actually +// committed to this recording. +func (mp4 *MP4) AverageFPS() float64 { + if mp4.SampleCount == 0 || mp4.VideoTotalDuration == 0 { + return 0 + } + return float64(mp4.SampleCount) * 1000 / float64(mp4.VideoTotalDuration) +} + // AddSampleToTrack appends a sample to the given track. // // For video, pts is the decode timestamp (DTS, in milliseconds) and diff --git a/machinery/src/video/mp4_duration_test.go b/machinery/src/video/mp4_duration_test.go index c1b3ab7..c651cb4 100644 --- a/machinery/src/video/mp4_duration_test.go +++ b/machinery/src/video/mp4_duration_test.go @@ -2,6 +2,7 @@ package video import ( "fmt" + "math" "os" "testing" @@ -173,4 +174,10 @@ func TestMP4Duration(t *testing.T) { t.Errorf("MISMATCH: mdhd.Duration should be 0 for fragmented MP4, got %d", parsedFile.Moov.Traks[0].Mdia.Mdhd.Duration) } + if mp4Video.SampleCount != sampleCount { + t.Errorf("SampleCount = %d, finalized MP4 contains %d video samples", mp4Video.SampleCount, sampleCount) + } + if fps := mp4Video.AverageFPS(); math.Abs(fps-25) > 0.001 { + t.Errorf("AverageFPS() = %.3f, want 25", fps) + } }