Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Ignore RTP packets with empty payload
Browse files Browse the repository at this point in the history
### Details

- This PR replaces PR #793 (by @ggarber) with code updated to current v3 and addresses feedback.
- All `XxxxConsumer` classes drop empty RTP packets.
- `RtpStreamRecv` no longer considers empty RTP packets for counter increase and stream activation.
- A substancial change with original PR is that `RtpStreamRecv` accepts those empty packets (otherwise we sould generate NACKs), it's just that those don't account anymore for counter increase and stream activation.
ibc committed May 19, 2024
1 parent 536ac74 commit 0bd2960
Showing 8 changed files with 65 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@
### NEXT

- `SimulcastConsumer`: Fix increase layer when current layer has not receive SR ([PR #1098](https://github.com/versatica/mediasoup/pull/1098) by @penguinol).
- Ignore RTP packets with empty payload ([PR #XXXX](https://github.com/versatica/mediasoup/pull/XXXX), credits to @ggarber).

### 3.14.6

1 change: 1 addition & 0 deletions worker/include/RTC/RtcLogger.hpp
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ namespace RTC
INVALID_TARGET_LAYER,
UNSUPPORTED_PAYLOAD_TYPE,
NOT_A_KEYFRAME,
EMPTY_PAYLOAD,
SPATIAL_LAYER_MISMATCH,
TOO_HIGH_TIMESTAMP_EXTRA_NEEDED,
PACKET_PREVIOUS_TO_SPATIAL_LAYER_SWITCH,
12 changes: 12 additions & 0 deletions worker/src/RTC/PipeConsumer.cpp
Original file line number Diff line number Diff line change
@@ -250,6 +250,18 @@ namespace RTC
auto& syncRequired = this->mapRtpStreamSyncRequired.at(rtpStream);
auto& rtpSeqManager = this->mapRtpStreamRtpSeqManager.at(rtpStream);

// Packets with only padding are not forwarded.
if (packet->GetPayloadLength() == 0)
{
rtpSeqManager.Drop(packet->GetSequenceNumber());

#ifdef MS_RTC_LOGGER_RTP
packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD);
#endif

return;
}

// If we need to sync, support key frames and this is not a key frame, ignore
// the packet.
if (syncRequired && this->keyFrameSupported && !packet->IsKeyFrame())
1 change: 1 addition & 0 deletions worker/src/RTC/RtcLogger.cpp
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ namespace RTC
{ RtpPacket::DropReason::INVALID_TARGET_LAYER, "InvalidTargetLayer" },
{ RtpPacket::DropReason::UNSUPPORTED_PAYLOAD_TYPE, "UnsupportedPayloadType" },
{ RtpPacket::DropReason::NOT_A_KEYFRAME, "NotAKeyframe" },
{ RtpPacket::DropReason::EMPTY_PAYLOAD, "EmptyPayload" },
{ RtpPacket::DropReason::SPATIAL_LAYER_MISMATCH, "SpatialLayerMismatch" },
{ RtpPacket::DropReason::TOO_HIGH_TIMESTAMP_EXTRA_NEEDED, "TooHighTimestampExtraNeeded" },
{ RtpPacket::DropReason::PACKET_PREVIOUS_TO_SPATIAL_LAYER_SWITCH, "PacketPreviousToSpatialLayerSwitch" },
14 changes: 14 additions & 0 deletions worker/src/RTC/RtpStreamRecv.cpp
Original file line number Diff line number Diff line change
@@ -303,6 +303,13 @@ namespace RTC
// Calculate Jitter.
CalculateJitter(packet->GetTimestamp());

// Padding only packet, do not consider it for counter increase nor
// stream activation.
if (packet->GetPayloadLength() == 0)
{
return true;
}

// Increase transmission counter.
this->transmissionCounter.Update(packet);

@@ -413,6 +420,13 @@ namespace RTC
// NACKed packet.
if (this->nackGenerator->ReceivePacket(packet, /*isRecovered*/ true))
{
// Padding only packet, do not consider it for counter increase nor
// stream activation.
if (packet->GetPayloadLength() == 0)
{
return true;
}

// Mark the packet as repaired.
RTC::RtpStream::PacketRepaired(packet);

12 changes: 12 additions & 0 deletions worker/src/RTC/SimpleConsumer.cpp
Original file line number Diff line number Diff line change
@@ -314,6 +314,18 @@ namespace RTC
return;
}

// Packets with only padding are not forwarded.
if (packet->GetPayloadLength() == 0)
{
this->rtpSeqManager.Drop(packet->GetSequenceNumber());

#ifdef MS_RTC_LOGGER_RTP
packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD);
#endif

return;
}

auto payloadType = packet->GetPayloadType();

// NOTE: This may happen if this Consumer supports just some codecs of those
12 changes: 12 additions & 0 deletions worker/src/RTC/SimulcastConsumer.cpp
Original file line number Diff line number Diff line change
@@ -724,6 +724,18 @@ namespace RTC
return;
}

// Packets with only padding are not forwarded.
if (packet->GetPayloadLength() == 0)
{
this->rtpSeqManager.Drop(packet->GetSequenceNumber());

#ifdef MS_RTC_LOGGER_RTP
packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD);
#endif

return;
}

if (this->targetTemporalLayer == -1)
{
#ifdef MS_RTC_LOGGER_RTP
12 changes: 12 additions & 0 deletions worker/src/RTC/SvcConsumer.cpp
Original file line number Diff line number Diff line change
@@ -639,6 +639,18 @@ namespace RTC
return;
}

// Packets with only padding are not forwarded.
if (packet->GetPayloadLength() == 0)
{
this->rtpSeqManager.Drop(packet->GetSequenceNumber());

#ifdef MS_RTC_LOGGER_RTP
packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD);
#endif

return;
}

// clang-format off
if (
this->encodingContext->GetTargetSpatialLayer() == -1 ||

0 comments on commit 0bd2960

Please sign in to comment.