Reland of Enable audio streams to send padding. (patchset #4 id:60001 of https://codereview.webrtc.org/2652893004/ )
Original issue's description:
> Enable audio streams to send padding.
>
> Useful if bitrate probing is to be used with audio streams.
>
> BUG=webrtc:7043
>
> Review-Url: https://codereview.webrtc.org/2652893004
> Cr-Commit-Position: refs/heads/master@{#16404}
> Committed: https://chromium.googlesource.com/external/webrtc/+/e35f89a484ca376d5c187d166714eba578dfadc3
BUG=webrtc:7043
Review-Url: https://codereview.webrtc.org/2675703002
Cr-Commit-Position: refs/heads/master@{#16433}
diff --git a/webrtc/modules/pacing/packet_router.cc b/webrtc/modules/pacing/packet_router.cc
index 65d6db7..3deb3a8 100644
--- a/webrtc/modules/pacing/packet_router.cc
+++ b/webrtc/modules/pacing/packet_router.cc
@@ -72,7 +72,7 @@
rtc::CritScope cs(&modules_crit_);
// Rtp modules are ordered by which stream can most benefit from padding.
for (RtpRtcp* module : rtp_modules_) {
- if (module->SendingMedia()) {
+ if (module->SendingMedia() && module->HasBweExtensions()) {
size_t bytes_sent = module->TimeToSendPadding(
bytes_to_send - total_bytes_sent, probe_cluster_id);
total_bytes_sent += bytes_sent;
diff --git a/webrtc/modules/pacing/packet_router_unittest.cc b/webrtc/modules/pacing/packet_router_unittest.cc
index 9011f81..a0688ff 100644
--- a/webrtc/modules/pacing/packet_router_unittest.cc
+++ b/webrtc/modules/pacing/packet_router_unittest.cc
@@ -122,10 +122,12 @@
const size_t requested_padding_bytes = 1000;
const size_t sent_padding_bytes = 890;
EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true));
EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, 111))
.Times(1)
.WillOnce(Return(sent_padding_bytes));
EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(true));
EXPECT_CALL(rtp_1, TimeToSendPadding(
requested_padding_bytes - sent_padding_bytes, 111))
.Times(1)
@@ -138,6 +140,7 @@
EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(false));
EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _)).Times(0);
EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(true));
EXPECT_CALL(rtp_1, TimeToSendPadding(_, _))
.Times(1)
.WillOnce(Return(sent_padding_bytes));
@@ -153,11 +156,25 @@
EXPECT_EQ(0u, packet_router_->TimeToSendPadding(requested_padding_bytes,
PacketInfo::kNotAProbe));
+ // Only one module has BWE extensions.
+ EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(false));
+ EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes, _)).Times(0);
+ EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _))
+ .Times(1)
+ .WillOnce(Return(sent_padding_bytes));
+ EXPECT_EQ(sent_padding_bytes,
+ packet_router_->TimeToSendPadding(requested_padding_bytes,
+ PacketInfo::kNotAProbe));
+
packet_router_->RemoveRtpModule(&rtp_1);
// rtp_1 has been removed, try sending padding and make sure rtp_1 isn't asked
// to send by not expecting any calls. Instead verify rtp_2 is called.
EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true));
+ EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true));
EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _)).Times(1);
EXPECT_EQ(0u, packet_router_->TimeToSendPadding(requested_padding_bytes,
PacketInfo::kNotAProbe));
diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h
index 325d77a..5ebb252 100644
--- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -155,6 +155,8 @@
virtual int32_t DeregisterSendRtpHeaderExtension(RTPExtensionType type) = 0;
+ virtual bool HasBweExtensions() const = 0;
+
// Returns start timestamp.
virtual uint32_t StartTimestamp() const = 0;
diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
index fc93655..a2e14f9 100644
--- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
+++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
@@ -66,6 +66,7 @@
int32_t(RTPExtensionType type, uint8_t id));
MOCK_METHOD1(DeregisterSendRtpHeaderExtension,
int32_t(RTPExtensionType type));
+ MOCK_CONST_METHOD0(HasBweExtensions, bool());
MOCK_CONST_METHOD0(StartTimestamp, uint32_t());
MOCK_METHOD1(SetStartTimestamp, void(uint32_t timestamp));
MOCK_CONST_METHOD0(SequenceNumber, uint16_t());
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index e0bcebd..d1f70ee 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -617,6 +617,15 @@
return rtp_sender_.DeregisterRtpHeaderExtension(type);
}
+bool ModuleRtpRtcpImpl::HasBweExtensions() const {
+ return rtp_sender_.IsRtpHeaderExtensionRegistered(
+ kRtpExtensionTransportSequenceNumber) ||
+ rtp_sender_.IsRtpHeaderExtensionRegistered(
+ kRtpExtensionAbsoluteSendTime) ||
+ rtp_sender_.IsRtpHeaderExtensionRegistered(
+ kRtpExtensionTransmissionTimeOffset);
+}
+
// (TMMBR) Temporary Max Media Bit Rate.
bool ModuleRtpRtcpImpl::TMMBR() const {
return rtcp_sender_.TMMBR();
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index 836f62c..188bd9b 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -65,6 +65,8 @@
int32_t DeregisterSendRtpHeaderExtension(RTPExtensionType type) override;
+ bool HasBweExtensions() const override;
+
// Get start timestamp.
uint32_t StartTimestamp() const override;
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
index add7c21..9ceebd9 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
@@ -36,6 +36,7 @@
namespace {
// Max in the RFC 3550 is 255 bytes, we limit it to be modulus 32 for SRTP.
constexpr size_t kMaxPaddingLength = 224;
+constexpr size_t kMinAudioPaddingLength = 50;
constexpr int kSendSideDelayWindowMs = 1000;
constexpr size_t kRtpHeaderLength = 12;
constexpr uint16_t kMaxInitRtpSeqNumber = 32767; // 2^15 -1.
@@ -215,7 +216,7 @@
return -1;
}
-bool RTPSender::IsRtpHeaderExtensionRegistered(RTPExtensionType type) {
+bool RTPSender::IsRtpHeaderExtensionRegistered(RTPExtensionType type) const {
rtc::CritScope lock(&send_critsect_);
return rtp_header_extension_map_.IsRegistered(type);
}
@@ -481,11 +482,20 @@
}
size_t RTPSender::SendPadData(size_t bytes, int probe_cluster_id) {
- // Always send full padding packets. This is accounted for by the
- // RtpPacketSender, which will make sure we don't send too much padding even
- // if a single packet is larger than requested.
- size_t padding_bytes_in_packet =
- std::min(MaxPayloadSize(), kMaxPaddingLength);
+ size_t padding_bytes_in_packet;
+ if (audio_configured_) {
+ // Allow smaller padding packets for audio.
+ padding_bytes_in_packet =
+ std::min(std::max(bytes, kMinAudioPaddingLength), MaxPayloadSize());
+ if (padding_bytes_in_packet > kMaxPaddingLength)
+ padding_bytes_in_packet = kMaxPaddingLength;
+ } else {
+ // Always send full padding packets. This is accounted for by the
+ // RtpPacketSender, which will make sure we don't send too much padding even
+ // if a single packet is larger than requested.
+ // We do this to avoid frequently sending small packets on higher bitrates.
+ padding_bytes_in_packet = std::min(MaxPayloadSize(), kMaxPaddingLength);
+ }
size_t bytes_sent = 0;
while (bytes_sent < bytes) {
int64_t now_ms = clock_->TimeInMilliseconds();
@@ -502,9 +512,15 @@
timestamp = last_rtp_timestamp_;
capture_time_ms = capture_time_ms_;
if (rtx_ == kRtxOff) {
- // Without RTX we can't send padding in the middle of frames.
- if (!last_packet_marker_bit_)
+ if (payload_type_ == -1)
break;
+ // Without RTX we can't send padding in the middle of frames.
+ // For audio marker bits doesn't mark the end of a frame and frames
+ // are usually a single packet, so for now we don't apply this rule
+ // for audio.
+ if (!audio_configured_ && !last_packet_marker_bit_) {
+ break;
+ }
ssrc = ssrc_;
sequence_number = sequence_number_;
++sequence_number_;
@@ -796,7 +812,7 @@
}
size_t RTPSender::TimeToSendPadding(size_t bytes, int probe_cluster_id) {
- if (audio_configured_ || bytes == 0)
+ if (bytes == 0)
return 0;
size_t bytes_sent = TrySendRedundantPayloads(bytes, probe_cluster_id);
if (bytes_sent < bytes)
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h
index 87326da..09884b3 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h
@@ -120,7 +120,7 @@
// RTP header extension
int32_t RegisterRtpHeaderExtension(RTPExtensionType type, uint8_t id);
- bool IsRtpHeaderExtensionRegistered(RTPExtensionType type);
+ bool IsRtpHeaderExtensionRegistered(RTPExtensionType type) const;
int32_t DeregisterRtpHeaderExtension(RTPExtensionType type);
bool TimeToSendPacket(uint32_t ssrc,
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 1b73b65..a6a886b 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -1491,4 +1491,29 @@
SendGenericPayload();
}
+TEST_F(RtpSenderTest, SendAudioPadding) {
+ MockTransport transport;
+ const bool kEnableAudio = true;
+ rtp_sender_.reset(new RTPSender(
+ kEnableAudio, &fake_clock_, &transport, &mock_paced_sender_, nullptr,
+ nullptr, nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_,
+ nullptr, &retransmission_rate_limiter_, nullptr));
+ rtp_sender_->SetSendPayloadType(kPayload);
+ rtp_sender_->SetSequenceNumber(kSeqNum);
+ rtp_sender_->SetTimestampOffset(0);
+ rtp_sender_->SetSSRC(kSsrc);
+
+ const size_t kPaddingSize = 59;
+ EXPECT_CALL(transport, SendRtp(_, kPaddingSize + kRtpHeaderSize, _))
+ .WillOnce(testing::Return(true));
+ EXPECT_EQ(kPaddingSize, rtp_sender_->TimeToSendPadding(
+ kPaddingSize, PacketInfo::kNotAProbe));
+
+ // Requested padding size is too small, will send a larger one.
+ const size_t kMinPaddingSize = 50;
+ EXPECT_CALL(transport, SendRtp(_, kMinPaddingSize + kRtpHeaderSize, _))
+ .WillOnce(testing::Return(true));
+ EXPECT_EQ(kMinPaddingSize, rtp_sender_->TimeToSendPadding(
+ kMinPaddingSize - 5, PacketInfo::kNotAProbe));
+}
} // namespace webrtc
