From d3d3ba5159311296d1a190aa84d20cf1c785ba3d Mon Sep 17 00:00:00 2001 From: deadbeef Date: Wed, 1 Feb 2017 15:45:53 -0800 Subject: [PATCH] Revert of Enable audio streams to send padding. (patchset #4 id:60001 of https://codereview.webrtc.org/2652893004/ ) Reason for revert: Speculatively reverting, since Android end-to-end tests (such as https://build.chromium.org/p/client.webrtc/builders/Android64%20%28M%20Nexus5X%29) started failing. 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 TBR=mflodman@webrtc.org,danilchap@webrtc.org,stefan@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:7043 Review-Url: https://codereview.webrtc.org/2669033003 Cr-Commit-Position: refs/heads/master@{#16407} --- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 31 +++++-------------- .../rtp_rtcp/source/rtp_sender_unittest.cc | 25 --------------- 2 files changed, 7 insertions(+), 49 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index c28420cfbe..add7c21e9b 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -36,7 +36,6 @@ 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. @@ -482,21 +481,11 @@ size_t RTPSender::TrySendRedundantPayloads(size_t bytes_to_send, } size_t RTPSender::SendPadData(size_t bytes, int probe_cluster_id) { - size_t padding_bytes_in_packet; - if (audio_configured_) { - // Allow smaller padding packets for audio. - padding_bytes_in_packet = std::max(std::min(bytes, MaxPayloadSize()), - kMinAudioPaddingLength); - 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); - } + // 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 bytes_sent = 0; while (bytes_sent < bytes) { int64_t now_ms = clock_->TimeInMilliseconds(); @@ -513,15 +502,9 @@ size_t RTPSender::SendPadData(size_t bytes, int probe_cluster_id) { timestamp = last_rtp_timestamp_; capture_time_ms = capture_time_ms_; if (rtx_ == kRtxOff) { - 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_) { + if (!last_packet_marker_bit_) break; - } ssrc = ssrc_; sequence_number = sequence_number_; ++sequence_number_; @@ -813,7 +796,7 @@ bool RTPSender::IsFecPacket(const RtpPacketToSend& packet) const { } size_t RTPSender::TimeToSendPadding(size_t bytes, int probe_cluster_id) { - if (bytes == 0) + if (audio_configured_ || 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_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index a6a886be94..1b73b6553f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -1491,29 +1491,4 @@ 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