Skip to content

Encode capture time in rtp timestamp - #164

Merged
lkang-nuro merged 5 commits into
mainfrom
feat/encode-capture-time-in-rtp-timestamp
Jul 1, 2026
Merged

Encode capture time in rtp timestamp#164
lkang-nuro merged 5 commits into
mainfrom
feat/encode-capture-time-in-rtp-timestamp

Conversation

@lkang-nuro

@lkang-nuro lkang-nuro commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Description

Encodes each video frame's capture time into the outgoing RTP timestamp so the
capture instant survives an SFU that re-forwards RTP timestamps but strips header
extensions on egress (e.g. LiveKit). The capture time is written as 90 kHz ticks
((captureUs*9/100) mod 2^32); the receiver recovers it via
getSynchronizationSources()[].rtpTimestamp * 100/9. abs-capture-time is also
stamped on the header extension for receivers that keep it.

Along the way this branch reworks the sender encode pipeline and per-track stats
that the feature builds on.

Capture-time encoding

  • New captureTimestampInterceptor (sender/capture_timestamp.go): stores a
    per-SSRC capture time supplied out-of-band via SetCaptureTSUs immediately
    before each WriteSample, and encodes it into the RTP timestamp. Keying by
    SSRC keeps concurrent per-track encode goroutines from stamping each other's
    frames.
  • stampCaptureTime records the frame's capture timestamp on the interceptor
    right before WriteSample; packetization and the interceptor write run
    synchronously in the same goroutine, so the value is correct for the packets
    that sample produces.

Encode pipeline

  • Event-driven, per-track encode goroutine replacing the previous poll loop;
    poll-free single-slot encode loop with recreateEncoder serialized against the
    encode goroutine.
  • FrameBuffer capacity reduced to 2 with hardened lifecycle handling; various
    race/rollback fixes (AddTrack rollback, Close map reset, bitrate race).

Stats & options

  • RTCSender.GetTrackStats per-track stats API, extended with FIR, bitrate, and
    quality-limitation reason.
  • Max bitrate support on the GCC option.
  • OnFrameSent callback with dequeue + encode-done timing stamps.

Perf / cleanup

  • SSRC is resolved once and cached in an atomic field, so the per-frame encode
    path and GetTrackStats no longer call RTPSender.GetParameters (and its
    per-call allocations) on every invocation.
  • Shared newGCCFactory helper removes duplicated gcc.Option setup.
  • Dependency bump (pion/webrtc/v4 v4.2.11), CI config updates to v0.12.2, and
    lint/modernize passes.

Reference issue

Fixes #...

@lkang-nuro
lkang-nuro marked this pull request as ready for review July 1, 2026 01:19
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.54430% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.20%. Comparing base (64a421c) to head (da184cb).

Files with missing lines Patch % Lines
sender/rtc_sender.go 66.66% 6 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #164      +/-   ##
==========================================
+ Coverage   46.63%   48.20%   +1.56%     
==========================================
  Files          18       19       +1     
  Lines        1829     1894      +65     
==========================================
+ Hits          853      913      +60     
- Misses        907      908       +1     
- Partials       69       73       +4     
Flag Coverage Δ
go 48.20% <83.54%> (+1.56%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lkang-nuro
lkang-nuro force-pushed the feat/encode-capture-time-in-rtp-timestamp branch from cbfcab1 to 24f5162 Compare July 1, 2026 03:20
lkang-nuro and others added 4 commits June 30, 2026 20:23
Carry each video frame's capture timestamp onto the wire as the standard
abs-capture-time RTP header extension, so receivers (and a forwarding
LiveKit SFU) can recover the original capture instant for end-to-end
latency measurement / sync.

The encode pipeline already records per-frame capture time
(FrameBuffer.LastCaptureTSUs, surfaced in encodeAndSendTrack). This adds
the missing piece: getting it onto RTP. WriteSample ignores
Sample.Timestamp/RTPHeaders in pion v4.2.9, so a send interceptor is the
only way to stamp the extension.

- abs_capture_time.go: interceptor (embeds interceptor.NoOp) that stamps
  the first packet of each frame with the negotiated abs-capture-time
  extension. Capture time is held per-SSRC so concurrent per-track
  encode goroutines don't stamp each other's frames; no-op when the
  extension is not negotiated or the capture time is unset.
- rtc_sender.go: register the extension + interceptor in
  setupAbsCaptureTime (called unconditionally from NewRTCSender); set
  the SSRC's capture time right before WriteSample in
  encodeAndSendTrack.

Correct because WriteSample -> packetize -> interceptor runs
synchronously in the encode goroutine, and the value is keyed by SSRC.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the abs-capture-time header-extension interceptor with one that
encodes each frame's capture timestamp into the outgoing RTP timestamp,
so it survives an SFU that strips header extensions on egress.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve each track's primary SSRC once and cache it in an atomic field
so the per-frame encode path (stampCaptureTime) and GetTrackStats no
longer call RTPSender.GetParameters on every invocation, avoiding its
per-call allocations. The SSRC is fixed once the sender is bound, so
caching is safe.

Also extract the send-side BWE estimator construction into a shared
newGCCFactory helper, removing the duplicated gcc.Option setup between
setupGCC and the GCC option's fallback path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- capture_timestamp_test: replace t.Fatalf with testify assert.Equal
  (forbidigo requires testify over t.Fatalf).
- setupCaptureTimestamp: drop the always-nil error return and its check
  at the call site (unparam).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lkang-nuro
lkang-nuro force-pushed the feat/encode-capture-time-in-rtp-timestamp branch from 24f5162 to 2761ab8 Compare July 1, 2026 03:24

@jayli-nuro jayli-nuro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCC factory extraction and interceptor ordering look good — because the capture interceptor is added after senderReports, the SR reports the on-wire timestamp, so A/V sync stays consistent.

Blocking: the new EncodedTrack.ssrc cache is never invalidated, but SetupPeerConnection is re-entrant (TestRTCSender_SetupPeerConnection_CancelsRTCP). After a reconnect, AddTrack yields a new SSRC while the cache holds the old one, silently breaking both capture-time encoding and GetTrackStats. Details inline; both share one fix.

Comment thread sender/rtc_sender.go
// repeated callers (the per-frame encode path and GetTrackStats) avoid the
// allocations RTPSender.GetParameters makes on every call. The SSRC is fixed
// once the sender is bound, so caching is safe.
func (t *EncodedTrack) resolveSSRC() (uint32, bool) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetupPeerConnection is re-entrant (it cancels the prior RTCP goroutines; TestRTCSender_SetupPeerConnection_CancelsRTCP calls it 3x). Each call runs AddTrack → new RTPSender with a new SSRC and reassigns track.rtpSender (line 730), but this cache is written once and never reset, so resolveSSRC keeps returning the pre-reconnect SSRC.

Fix: reset the cache when rtpSender is replaced — add track.ssrc.Store(0) right after track.rtpSender = rtpSender in SetupPeerConnection. The next resolveSSRC re-resolves against the new sender.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in af7ea3b. SetupPeerConnection now calls invalidateSSRCCache(track) right before swapping in the new rtpSender, so resolveSSRC re-resolves against the new sender after a reconnect. Thanks!

Comment thread sender/rtc_sender.go
// WriteSample → packetize → interceptor runs synchronously in this
// goroutine, and the value is keyed by SSRC, so it is correct for the
// packets WriteSample produces even with concurrent tracks.
s.stampCaptureTime(track, captureTSUs)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symptom of the stale cache: after a reconnect this stores the capture time under the old SSRC, while the interceptor binds the new stream under the new SSRC and reads an always-empty slot — capture-time encoding silently stops and RTP falls back to the packetizer clock. GetTrackStats breaks the same way (queries the old SSRC → zeros), a regression since it previously read GetParameters() fresh on each call.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by the same cache invalidation — after a reconnect both the capture-time stamping and GetTrackStats now resolve the current SSRC instead of the stale one. Thanks for tracing the symptom.

interceptor.NoOp

mu sync.Mutex
slots map[uint32]*atomic.Int64 // ssrc -> capture time (unix microseconds), 0 = none

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: slots grows unbounded with no cleanup (NoOp.Close is a no-op). Since the SSRC changes on each reconnect, every reconnect orphans an entry. Low impact for a test harness, but a Close/RemoveSSRC that drops stale entries would keep it bounded.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: added captureTimestampInterceptor.RemoveSSRC, and invalidateSSRCCache drops the old SSRC's slot on each reconnect, so the map stays bounded. Thanks!

@lkang-nuro

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review @jayli-nuro — all three comments were spot on. Pushed af7ea3b:

  • SSRC cache never invalidated (blocking): SetupPeerConnection now calls a new invalidateSSRCCache(track) immediately before reassigning track.rtpSender. It Swap(0)s the cached SSRC so resolveSSRC re-resolves against the freshly bound sender on the next call — fixing both the silently-broken capture-time encoding and the GetTrackStats regression after a reconnect.
  • Unbounded slots growth: added captureTimestampInterceptor.RemoveSSRC, which invalidateSSRCCache calls with the old SSRC so orphaned entries are dropped on each reconnect and the map stays bounded.

Also added TestCaptureTimestampInterceptor_RemoveSSRC for the new path. Existing tests (incl. TestRTCSender_SetupPeerConnection_CancelsRTCP) still pass. PTAL 🙏

@lkang-nuro
lkang-nuro requested a review from jayli-nuro July 1, 2026 05:49
SetupPeerConnection is re-entrant; on reconnect AddTrack yields a new
RTPSender with a fresh SSRC, but EncodedTrack.ssrc was cached once and
never reset. resolveSSRC kept returning the stale pre-reconnect SSRC,
silently breaking capture-time encoding (stamped under the old SSRC
that the interceptor no longer binds) and GetTrackStats (queried the
old SSRC, returning zeros).

Reset the cache before swapping in the new sender via
invalidateSSRCCache (extracted to keep SetupPeerConnection under the
cyclomatic-complexity limit), and drop the orphaned capture-time slot
via a new RemoveSSRC so the interceptor map stays bounded across
reconnects.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lkang-nuro
lkang-nuro force-pushed the feat/encode-capture-time-in-rtp-timestamp branch from af7ea3b to da184cb Compare July 1, 2026 05:50
@lkang-nuro
lkang-nuro merged commit cd845a9 into main Jul 1, 2026
27 of 28 checks passed
@lkang-nuro
lkang-nuro deleted the feat/encode-capture-time-in-rtp-timestamp branch July 1, 2026 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants