From adaec45f3654ce066734d1c5c71b4a61d089aca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 13 May 2020 15:04:35 +0200 Subject: [PATCH] Removes RepairedRtpStreamId from overhead calculation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In https://webrtc-review.googlesource.com/c/src/+/173704 the overhead calculations were made more static, so that "volatile" extensions (those that are not set on every packet) are ignored. The intent, as the comments specify, was to ignore RepairedRtpStreamId since that is only used on RTX packets. This CL makes us actually count that extension as volatile. Bug: webrtc:10809 Change-Id: If42ae84e4c09ff9112e93f8d872ee890c6253a23 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175010 Reviewed-by: Sebastian Jansson Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#31246} --- modules/rtp_rtcp/source/rtp_sender.cc | 2 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 3d60552e9b..3023e59557 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -110,7 +110,6 @@ bool IsNonVolatile(RTPExtensionType type) { case kRtpExtensionTransportSequenceNumber02: case kRtpExtensionFrameMarking: case kRtpExtensionRtpStreamId: - case kRtpExtensionRepairedRtpStreamId: case kRtpExtensionMid: case kRtpExtensionGenericFrameDescriptor00: case kRtpExtensionGenericFrameDescriptor02: @@ -121,6 +120,7 @@ bool IsNonVolatile(RTPExtensionType type) { case kRtpExtensionPlayoutDelay: case kRtpExtensionVideoContentType: case kRtpExtensionVideoTiming: + case kRtpExtensionRepairedRtpStreamId: case kRtpExtensionColorSpace: return false; case kRtpExtensionNone: diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 474810a88a..65e2e04ef4 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -2032,6 +2032,30 @@ TEST_P(RtpSenderTest, CountMidOnlyUntilAcked) { EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 12u); } +TEST_P(RtpSenderTest, DontCountVolatileExtensionsIntoOverhead) { + RtpRtcp::Configuration config; + config.clock = &fake_clock_; + config.outgoing_transport = &transport_; + config.local_media_ssrc = kSsrc; + config.retransmission_rate_limiter = &retransmission_rate_limiter_; + rtp_sender_context_ = std::make_unique(config); + + // Base RTP overhead is 12B. + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 12u); + + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionInbandComfortNoise, 1); + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionAbsoluteCaptureTime, 2); + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionVideoRotation, 3); + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionPlayoutDelay, 4); + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionVideoContentType, 5); + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionVideoTiming, 6); + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionRepairedRtpStreamId, 7); + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionColorSpace, 8); + + // Still only 12B counted since can't count on above being sent. + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 12u); +} + TEST_P(RtpSenderTest, SendPacketMatchesVideo) { std::unique_ptr packet = BuildRtpPacket(kPayload, true, 0, fake_clock_.TimeInMilliseconds());