sender: make capture-time RTP rewrite opt-in - #174
Open
hanguyen-nuro wants to merge 1 commit into
Open
Conversation
captureTimestampInterceptor overwrote every frame's RTP timestamp with the supplied capture time (captureUs*9/100). That is unsafe for any track whose sending is gated outside the RTP stack: while such a track is idle it emits no packets, but the capture clock keeps advancing, so when it starts sending it enters mid-session with a base derived from wall time and receivers never render it. Tracks that send continuously from session start are unaffected, which is why this went unnoticed. Observed with 14 outbound camera tracks where only the first few are allocated non-zero bitrate at startup and the rest are switched on later by the operator: every late-started track stayed black while funded, encoding and sending. Disabling the rewrite restores all of them, with no other change. Gate the overwrite behind sender.CaptureTimestampRewrite(), default off. Capture times passed to SetCaptureTSUs are still recorded, so callers using them only for telemetry keep working. Note there is no way to both preserve the packetizer's timeline and carry an absolute capture instant in the same field; callers whose tracks may start or stop should carry capture time out-of-band (e.g. a data channel keyed by the RTP timestamp), which also survives an SFU that strips header extensions.
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
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.
Problem
captureTimestampInterceptoroverwrites every frame's RTP timestamp with the supplied capture time (captureUs*9/100). That is unsafe for any track whose sending is gated outside the RTP stack: while such a track is idle it emits no packets, but the capture clock keeps advancing, so when it starts sending it enters mid-session with a base derived from wall time and receivers never render it. Tracks that send continuously from session start are unaffected, which is why this went unnoticed.Reproduction
14 outbound camera tracks, where only the first 5 are allocated non-zero bitrate at startup and the remaining 9 are switched on later by an operator request. Every late-started track stayed black despite being funded, encoding and sending; the 5 continuous ones were fine throughout.
Isolated by building the same consumer against a matrix of variants (one variable at a time, each run validated by logging the funding transition and the per-track encode counters):
FrameBuffercapacity 2 instead of 1So the rewrite is the trigger, independent of the frame-buffer change.
Change
Gate the overwrite behind
sender.CaptureTimestampRewrite(), default off. Capture times passed toSetCaptureTSUsare still recorded, so callers that use them only for telemetry are unaffected.Note there is no way to both preserve the packetizer's timeline and carry an absolute capture instant in the same field. Callers whose tracks may start or stop should carry capture time out-of-band (e.g. a data channel keyed by the RTP timestamp), which also survives an SFU that strips header extensions on egress.
Tests
EncodesCaptureTimenow opts in explicitly (behavior change is intentional).DisabledByDefault— packetizer timestamp preserved, capture time still recorded.RewriteToggle— the gate is honored per frame.Full
sendersuite passes;gofmtandgo vetclean.I have not established the receiver-side reason a wall-clock-derived base is rejected for a mid-session start, so this PR gates the hazard rather than claiming to explain it.