fix(go2rtc): cut snapshot cache from 30s to 1s so detection sees fresh frames

[johnchia]

go2rtc_snapshot.c requested snapshots with cache=30s. That parameter does not do what the comment beside it claimed, and the consequence is that API detection runs on frames up to 30 seconds old.

What cache= actually does

The comment asserted the cache only comes into play when the producer is unavailable:

We add cache=30s to allow go2rtc to return a cached frame if the stream is temporarily unavailable (e.g., video doorbell in sleep mode).

go2rtc implements it as an unconditional TTL cache, checked before the stream is touched at all — handlerKeyframe in internal/mjpeg/mjpeg.go:

if s := query.Get("cache"); s != "" {
    if timeout, err := time.ParseDuration(s); err == nil {
        ...
        if found && time.Since(entry.timestamp) < timeout {
            writeJPEGResponse(w, entry.payload)
            return          // the stream is never consulted
        }

There is no stream-health condition anywhere on that path. A healthy, actively producing stream is served a stale frame exactly as a stalled one is.

Impact

run_detection_on_frame() calls detect_objects_api_snapshot() ahead of any frame decode, so with go2rtc enabled every API detection uses this cached JPEG. A stream at detection_interval=1 therefore re-runs detection once a second against one frozen image, and an object entering the scene stays invisible until the cache expires — up to 30 s later.

Observed on a live 1 Hz 2560×1440 stream: 28 consecutive person rows carrying a single distinct confidence value and a single distinct bounding box.

In effect the 30 s cache silently defeats any detection_interval shorter than 30 s.

Measurement

Six requests with cache=30s interleaved with six without, over the same 12 s window, with the stream demonstrably healthy throughout — the uncached requests interleaved between the cached ones returned a fresh frame every time:

	distinct frames	latency
cache=30s	1 of 6	~0.001 s
no cache param	6 of 6	~1 s

With cache=1s, polling at 1 Hz for 10 requests: 10 of 10 distinct frames. The staleness is gone.

Side effects of cache=1s

This is not free, and the cost is worth stating plainly.

Fetch latency rises. On a miss go2rtc waits for the stream's next keyframe, so a fetch costs up to one GOP. Measured at 1 Hz polling against a 1 s GOP, latency alternates cleanly between a hit and a miss:

	mean	max	frames
cache=30s	0.006 s	0.052 s	1 of 12
cache=1s	0.496 s	1.080 s	10 of 10

That wait is paid on the UDT main loop. The loop is av_read_frame() (line 1726) → process_packet() (line 1746) → run_detection_on_frame(), single-threaded, so a slow snapshot fetch stalls packet reading for that stream. This repo already treats that as a hazard — the ONVIF path was moved to its own thread for exactly this reason, and the comment introducing it notes that "SOD and API detection paths are completely unaffected", i.e. still inline.

So on a 1 s GOP at detection_interval=1, the reader goes from stalling ~170 ms per cycle (the detect API POST alone) to roughly 170 ms + up to 1 s on the cycles that miss. I have not measured the downstream effect on recording continuity or on the effective detection rate; both could plausibly degrade, and the rate is partly self-limiting since a longer cycle means fewer detections. Flagging it rather than claiming it is benign.

What is not affected:

• go2rtc CPU is unchanged — 6.5% → 6.6% of a core over the same workload. The per-miss H264→JPEG transcode is cheap relative to what go2rtc is already doing.
• No producer churn. The per-request AddConsumer/RemoveConsumer does not reconnect to the camera: go2rtc holds one persistent RTSP producer shared across consumers (confirmed via /api/streams — a single producer with 374 MB received), so snapshots attach to the existing feed.

Alternatives, if the stall is judged too expensive

Both are larger changes than this PR and I have not implemented either:

1. Skip the fetch entirely. detect_objects_api_snapshot() runs before the decode path, but the caller already holds the keyframe packet. Using the decoded frame costs a decode plus a JPEG encode locally and involves no network round trip and no keyframe wait.
2. Move the fetch off the UDT loop, as was done for ONVIF.

The value could also be made configurable rather than hardcoded, so deployments can pick their own point on the freshness/stall trade.

The change

cache=30s → cache=1s, plus the comment rewritten to describe the actual TTL behaviour and to record the keyframe-wait cost it shifts onto the caller.

1 s bounds staleness at roughly one detection interval, which is as fine as lightNVR can express — both detection gates compare whole seconds in unified_detection_thread.c, so detection_interval cannot go below 1.

Draft because the fix is verified against go2rtc's source and by direct measurement of the live endpoint, but I have not yet run a rebuilt image end to end, and the packet-reader stall above deserves a maintainer's judgement.

[johnchia]

Confirmed on bench cam running light-object-detect with tiled detection (openvino backend igpu execution)

[Copilot]

Pull request overview

This PR updates the go2rtc snapshot request behavior to reduce stale-frame reuse so API-based detection can operate on fresher frames when snapshots are polled frequently.

Changes:

• Reduce go2rtc snapshot TTL cache from cache=30s to cache=1s.
• Rewrite the in-code comment to describe go2rtc’s cache= semantics and the keyframe-wait latency tradeoff.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.