fix(api): flag the _mainv repair from the SERVED SDP, not the producer - #592
Merged
Conversation
#591's _mainv detection reused sdp_video_lacks_fmtp on the MAIN producer SDP. The reference LPR camera (Uniview H.265) publishes a producer fmtp with sprop-sps+sprop-pps but no sprop-vps, so the producer verdict is Some(false) ("has fmtp") and the camera is never flagged -- yet go2rtc, unable to build a complete HEVC parameter set, serves consumers an SDP with no a=fmtp at all, which is what makes Media3 throw 'missing attribute fmtp'. The merged repair was therefore inert for exactly the camera it targeted. Detect on the SDP go2rtc SERVES to RTSP consumers instead: add stream_served_video_lacks_fmtp + StreamIndex::video_lacks_fmtp_served, used for _mainv only. _subv keeps the producer side (a sub is often un-consumed, so it has no served SDP); the main is always consumed by the recorder, so its served SDP is reliably present. Verified against the live prod go2rtc: LPR served SDP lacks fmtp while frontyard/driveway/garage (H.265, playable) serve a complete fmtp with sprop-vps. Unit tests use the real captured SDPs. DECISIONS.md corrected. Signed-off-by: badbread <badbread@users.noreply.github.com>
badbread
added a commit
that referenced
this pull request
Aug 8, 2026
…no-verdict debug log Addresses the Fable review of #592: - config.rs / dto.rs doc comments still described PRODUCER-side detection (sdp_video_lacks_fmtp); point them at stream_served_video_lacks_fmtp / the served SDP (same-change doc-sync). - 'the main is always consumed by the recorder' is only true for the default record-from-main policy; soften in the go2rtc.rs docstring, the reconcile comment, and the DECISIONS correction (a record-from-sub main has no RTSP consumer -> served verdict None -> unrepaired, by design). - add a reconcile debug log when the repair is enabled but a main has no served-SDP verdict, so 'enabled but silently does nothing' is observable. - extra unit cases: webrtc-before-rtsp ordering, and a mid-handshake rtsp consumer (no sdp) skipped for the next one. record-from-sub main-repair blindness and the sub-side incomplete-fmtp class are tracked as follow-ups, not this PR. Signed-off-by: badbread <badbread@users.noreply.github.com>
Owner
Author
|
Fable adversarial review: SHIP-WITH-FIXES, applied in the second commit.
|
…no-verdict debug log Addresses the Fable review of #592: - config.rs / dto.rs doc comments still described PRODUCER-side detection (sdp_video_lacks_fmtp); point them at stream_served_video_lacks_fmtp / the served SDP (same-change doc-sync). - 'the main is always consumed by the recorder' is only true for the default record-from-main policy; soften in the go2rtc.rs docstring, the reconcile comment, and the DECISIONS correction (a record-from-sub main has no RTSP consumer -> served verdict None -> unrepaired, by design). - add a reconcile debug log when the repair is enabled but a main has no served-SDP verdict, so 'enabled but silently does nothing' is observable. - extra unit cases: webrtc-before-rtsp ordering, and a mid-handshake rtsp consumer (no sdp) skipped for the next one. record-from-sub main-repair blindness and the sub-side incomplete-fmtp class are tracked as follow-ups, not this PR. Signed-off-by: badbread <badbread@users.noreply.github.com>
badbread
force-pushed
the
fix/mainv-consumer-sdp-detection
branch
from
August 8, 2026 21:47
114a7d6 to
5ed4f24
Compare
This was referenced Aug 9, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #591. The opt-in
_mainvH.265→H.264 main repair was inert for the exact camera it targeted (the Uniview H.265 LPR main).Root cause
#591 detects a broken main by running
sdp_video_lacks_fmtpon the producer SDP. But the LPR camera's producer SDP does carry ana=fmtpline — it just hassprop-sps+sprop-ppsand nosprop-vps. HEVC needs VPS+SPS+PPS, so go2rtc can't assemble a complete parameter set and serves consumers an SDP with noa=fmtpat all — which is what makes Media3 throwmissing attribute fmtp.So the producer verdict is
Some(false)("has fmtp") and the camera is never flagged, while the served SDP is the one that actually breaks clients.Verified against a live go2rtc:
a=fmtp?sprop-vps→ playsFix
Detect on the SDP go2rtc serves to RTSP consumers:
stream_served_video_lacks_fmtp+StreamIndex::video_lacks_fmtp_served_mainvuses the served verdict;_subvkeeps the producer verdict (a sub is often un-consumed, so it has no served SDP — the main is always consumed by the recorder, so its served SDP is reliably present)resolve_needs_subv's sticky-across-unknown semantics unchangedplayback.rs'srtsp_mainv_urlgate is unchanged (stillstate.mainv_needed); only the comment is correctedUnit tests use the real captured SDPs (IPs genericised to RFC 5737).
docs/DECISIONS.mdgets a dated correction to the #591 entry.The transcode approach itself (H.265→H.264 re-encode, opt-in, default off) is unchanged — only the trigger is corrected.