Use the new `RTP::Packet` class #1689

ibc · 2026-01-14T17:37:54Z

Details

Remove former RTC::RtpPacket class.
Use the new RTC::RTP::Packet class.

NOTE: WIP

# Details - Remove former `RTC::RtpPacket` class. - Use the new `RTC::RTP::Packet` class. _NOTE:_ WIP # TODO * [ ] In `RtpProbationGenerator` we cannot use `Parse()` and then `packet->SetPayloadType()` because it's frozen. So check all usages of former `RtpPacket::Parse()` and new `RTP::Packet::Parse()`. And if needed, use the new `Packet::ParseFromApplicationBuffer()` static method. But is it ok? * [ ] `RtpProbationGenerator.cpp`: Remove the `packet->Dump()` and `MS_DUMP()` (added to see if the new padding variable mechanism works as pexpected). * [ ] `RTP::Packet`: Test the new `Packet::ParseFromApplicationBuffer()` static method.

…ize_t bufferLength)

ibc · 2026-01-16T10:29:15Z

TODO 1: `TestRtpStreamSend.cpp` fails

Notice that, for simplicity, only a SECTION is enabled in that file:

SECTION("receive NACK and get retransmitted packets")

MS_TEST_LOG_LEVEL=warn MS_TEST_LOG_TAGS=rtp MEDIASOUP_TEST_TAGS="[rtpstreamsend]" make test

Temporal added extra logs show that something is indeed wrong because the buffer (packet->GetBuffer()) of almost every RTX generated packet is the same!!!

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16d8c2140:
<RTP::Packet>
  length: 12 (buffer length: 12)
  sequence number: 21006
  timestamp: 1533790901
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 0
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16d8c1580:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21008
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16d8c1b60:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21007
  timestamp: 1533790901
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16d8c1580:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21008
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16d8c0fa0:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21009
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16d8c09c0:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21010
  timestamp: 1533796931
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16d8c09c0:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21010
  timestamp: 1533796931
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x6000027ca4c0:
<RTP::Packet>
  length: 12 (buffer length: 112)
  sequence number: 21006
  timestamp: 1533790901
  marker: false
  payload type: 36
  ssrc: 1111
  csrcs: false
  payload length: 0
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x158015c00:
<RTP::Packet>
  length: 1500 (buffer length: 1600)
  sequence number: 21007
  timestamp: 1533790901
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x158015c00:
<RTP::Packet>
  length: 1500 (buffer length: 1600)
  sequence number: 21008
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x158015c00:
<RTP::Packet>
  length: 1500 (buffer length: 1600)
  sequence number: 21009
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x158015c00:
<RTP::Packet>
  length: 1500 (buffer length: 1600)
  sequence number: 21010
  timestamp: 1533796931
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mediasoup-worker-test is a Catch2 v3.12.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
Scenario: NACK and RTP packets retransmission
  receive NACK and get retransmitted packets
-------------------------------------------------------------------------------
../../../test/src/RTC/TestRtpStreamSend.cpp:125
...............................................................................

../../../test/src/RTC/TestRtpStreamSend.cpp:66: FAILED:
  REQUIRE( rtxPacket->GetSequenceNumber() == origPacket->GetSequenceNumber() )
with expansion:
  22651 == 21006

ibc · 2026-01-16T10:46:46Z

Update TODO 1

The tests pass if I comment out the destructor body of the RTP::SharedPacket:

SharedPacket::~SharedPacket()
{
	MS_TRACE();

	// If we hold a Packet we must delete its internal buffer (the one we
	// passed to it via Clone() method).
	// if (HasPacket())
	// {
	// 	delete[] GetPacket()->GetBuffer();
	// }
}

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16b136140:
<RTP::Packet>
  length: 12 (buffer length: 12)
  sequence number: 21006
  timestamp: 1533790901
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 0
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16b135580:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21008
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16b135b60:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21007
  timestamp: 1533790901
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16b135580:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21008
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16b134fa0:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21009
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16b1349c0:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21010
  timestamp: 1533796931
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

------ stream->ReceivePacket() | packet->GetBuffer(): 0x16b1349c0:
<RTP::Packet>
  length: 1500 (buffer length: 1500)
  sequence number: 21010
  timestamp: 1533796931
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x600003fc8460:
<RTP::Packet>
  length: 12 (buffer length: 112)
  sequence number: 21006
  timestamp: 1533790901
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 0
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x13b80de00:
<RTP::Packet>
  length: 1500 (buffer length: 1600)
  sequence number: 21007
  timestamp: 1533790901
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x13b80d600:
<RTP::Packet>
  length: 1500 (buffer length: 1600)
  sequence number: 21008
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x13b80e600:
<RTP::Packet>
  length: 1500 (buffer length: 1600)
  sequence number: 21009
  timestamp: 1533793871
  marker: false
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

++++++ OnRtpStreamRetransmitRtpPacket() | RTX packet->GetBuffer(): 0x13b80ee00:
<RTP::Packet>
  length: 1500 (buffer length: 1600)
  sequence number: 21010
  timestamp: 1533796931
  marker: true
  payload type: 123
  ssrc: 1111
  csrcs: false
  payload length: 1488
  padding length: 0
</RTP::Packet>

===============================================================================
All tests passed (28 assertions in 1 test case)

But this is not good because it means that we NEVER deallocated the buffer where RTP::SharedPacket cloned the RTP Packet.

ibc added 6 commits January 14, 2026 18:34

fix CHANGELOG

4aade67

more

3244d4f

Add test for RTP::Parse(const uint8_t* buffer, size_t packetLength, s…

49e3dbb

…ize_t bufferLength)

add tmp logs in failing test

95e9c79

more

f40e01f

temp invalid solution

348d4d5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Use the new `RTP::Packet` class #1689

Use the new `RTP::Packet` class #1689

Uh oh!

ibc commented Jan 14, 2026 •

edited

Loading

Uh oh!

ibc commented Jan 16, 2026 •

edited

Loading

Uh oh!

ibc commented Jan 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Use the new RTP::Packet class #1689

Are you sure you want to change the base?

Use the new RTP::Packet class #1689

Uh oh!

Conversation

ibc commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Uh oh!

ibc commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO 1: TestRtpStreamSend.cpp fails

Uh oh!

ibc commented Jan 16, 2026

Update TODO 1

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Use the new `RTP::Packet` class #1689

Use the new `RTP::Packet` class #1689

ibc commented Jan 14, 2026 •

edited

Loading

ibc commented Jan 16, 2026 •

edited

Loading

TODO 1: `TestRtpStreamSend.cpp` fails