Skip to content

rtpmanager/rtptwccstats: track recovery per means of repair (RTX/FEC) - #65

Open
MishaBaranov wants to merge 1 commit into
mainfrom
misha/twcc-split-recovery
Open

rtpmanager/rtptwccstats: track recovery per means of repair (RTX/FEC)#65
MishaBaranov wants to merge 1 commit into
mainfrom
misha/twcc-split-recovery

Conversation

@MishaBaranov

Copy link
Copy Markdown
Contributor

What

Track TWCC delivery/recovery per means of repair (RTX vs FEC) independently, so
each recovery mechanism can be credited on its own rather than lumped into a single
aggregate recovery figure.

Details

  • Each sent packet carries an array of repair-instrument IDs ({ssrc, pt} pairs,
    TWCCRepairId), populated from RTPRepairMeta. A packet's RECOVERED state is
    derived from status == LOST && recovered_by_count > 0 rather than stored.
  • Per-window stats now expose two structures:
    • RTPTWCCRepairStats — per-repair recovered count, recovery %, and bitrate-sent
      (bitrate spent on RTX/FEC redundant traffic).
    • RTPTWCCRecoveryOverlap — share of losses recoverable by both RTX and FEC
      (pairwise overlap), useful to detect redundant repair spend.
  • Separate accounting of redundant (repair) vs primary (non-recovery) bitrate.
  • Fix redundant-packet detection so RTX and FEC blocks are keyed correctly when they
    coexist (RedBlockKey = protected seqnums + repair id).

Tests

  • tests/check/elements/rtpsession.c: cooperation tests feeding overlapping RTX + FEC
    and asserting each means of repair is credited independently.

Scope

2 files (gst/rtpmanager/rtptwccstats.c, tests/check/elements/rtpsession.c),
single commit on top of main.

Consumed downstream by the Media stack (per-repair recovery reported on PmxStatsTX).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extends rtptwccstats to attribute TWCC loss recovery per means of repair (e.g., RTX vs FEC) instead of a single aggregate, and adds tests to validate independent crediting and overlap reporting.

Changes:

  • Track per-packet recovery attribution via {ssrc, pt} repair IDs and derive “recovered” state from status == LOST && recovered_by_count > 0.
  • Compute and expose per-window recovery-by-repair stats (recovered count/pct + redundant bitrate) and recovery-overlap stats (pairwise overlap cells).
  • Update rtpsession tests to assert per-repair recovery, overlap behavior, and repair bitrate accounting.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
subprojects/gst-plugins-good/gst/rtpmanager/rtptwccstats.c Adds per-repair attribution, overlap accounting, and exposes new stats arrays in the TWCC stats structure.
subprojects/gst-plugins-good/tests/check/elements/rtpsession.c Expands TWCC tests to validate per-repair recovery/overlap and repair bitrate reporting.

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

Comment thread subprojects/gst-plugins-good/tests/check/elements/rtpsession.c
Comment thread subprojects/gst-plugins-good/tests/check/elements/rtpsession.c Outdated
Comment thread subprojects/gst-plugins-good/gst/rtpmanager/rtptwccstats.c

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

subprojects/gst-plugins-good/tests/check/elements/rtpsession.c:210

  • generate_rtx_buffer() no longer sets the RTX packet's RTP header timestamp (previously copied from the original packet). gst_buffer_copy_into(..., GST_BUFFER_COPY_TIMESTAMPS, ...) only copies the GstBuffer PTS/DTS, not the RTP header timestamp field, so the RTX packet will likely carry a zero/incorrect RTP timestamp.
  gst_rtp_buffer_map (new_buffer, GST_MAP_WRITE, &new_rtp);
  gst_rtp_buffer_set_payload_type (&new_rtp, TEST_RTX_BUF_PT);
  gst_rtp_buffer_set_ssrc (&new_rtp, TEST_RTX_BUF_SSRC);
  gst_rtp_buffer_set_seq (&new_rtp, rtx_seqnum);
  gst_rtp_buffer_unmap (&new_rtp);

  /* Copy over timestamps */
  gst_buffer_copy_into (new_buffer, buffer, GST_BUFFER_COPY_TIMESTAMPS, 0, -1);
  gst_rtp_repair_meta_add (new_buffer, 0, 1, orig_ssrc, &orig_seqnum,
      &orig_timestamp, 1);

When multiple means of packet recovery operate simultaneously
it might be useull to account each of them independently,
e.g. to credit RTX for a recovery where FEC wasn't capable
to tolerate number of losses in a block.

To do so each packet has an array of repair instrument IDs:
pairs {ssrc, pt}. Then statistics reports structs
"RTPTWCCRepairStats" and "RTPTWCCRecoveryOverlap" for
independent perfromance overview and overlap of any means of
resilience pairwise.

In RTPTWCCRepairStats in addition to an old bits_sent_non_recovery field,
a field bitrate-sent is added. It carries RTX/FEC bitrate.
@MishaBaranov
MishaBaranov force-pushed the misha/twcc-split-recovery branch from 5fe657d to 274504f Compare July 28, 2026 14:08
@MishaBaranov
MishaBaranov marked this pull request as ready for review July 30, 2026 13:54
@MishaBaranov
MishaBaranov requested a review from havardgraff July 31, 2026 08:39
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.

2 participants