From 53b6cc3832f72db8d7aaf293723f8d2d1bef757d Mon Sep 17 00:00:00 2001 From: stefan Date: Fri, 3 Feb 2017 08:13:57 -0800 Subject: [PATCH] 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} --- webrtc/modules/pacing/packet_router.cc | 2 +- .../modules/pacing/packet_router_unittest.cc | 17 ++++++++++ webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 2 ++ webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 1 + .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 9 +++++ .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 2 ++ webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 34 ++++++++++++++----- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 2 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 25 ++++++++++++++ 9 files changed, 83 insertions(+), 11 deletions(-) diff --git a/webrtc/modules/pacing/packet_router.cc b/webrtc/modules/pacing/packet_router.cc index 65d6db7cb7..3deb3a8faf 100644 --- a/webrtc/modules/pacing/packet_router.cc +++ b/webrtc/modules/pacing/packet_router.cc @@ -72,7 +72,7 @@ size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send, 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 9011f81b36..a0688ffbbe 100644 --- a/webrtc/modules/pacing/packet_router_unittest.cc +++ b/webrtc/modules/pacing/packet_router_unittest.cc @@ -122,10 +122,12 @@ TEST_F(PacketRouterTest, TimeToSendPadding) { 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 @@ TEST_F(PacketRouterTest, TimeToSendPadding) { 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 @@ TEST_F(PacketRouterTest, TimeToSendPadding) { 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 325d77ad2d..5ebb252c25 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -155,6 +155,8 @@ class RtpRtcp : public Module { 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 fc93655aa5..a2e14f9a3c 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 @@ class MockRtpRtcp : public RtpRtcp { 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 e0bcebda15..d1f70ee6f0 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 @@ int32_t ModuleRtpRtcpImpl::DeregisterSendRtpHeaderExtension( 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 836f62cebe..188bd9b743 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 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { 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 add7c21e9b..9ceebd99e0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -36,6 +36,7 @@ namespace webrtc { 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 @@ int32_t RTPSender::RegisterRtpHeaderExtension(RTPExtensionType type, 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::TrySendRedundantPayloads(size_t bytes_to_send, } 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 @@ size_t RTPSender::SendPadData(size_t bytes, int probe_cluster_id) { 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 @@ bool RTPSender::IsFecPacket(const RtpPacketToSend& packet) const { } 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 87326daa00..09884b374d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -120,7 +120,7 @@ class RTPSender { // 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 1b73b6553f..a6a886be94 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 @@ TEST_F(RtpSenderTest, AddOverheadToTransportFeedbackObserver) { 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