From 16b02211a938c2320ef458425bcd4f657328ba52 Mon Sep 17 00:00:00 2001 From: stefan Date: Fri, 27 Jan 2017 07:12:16 -0800 Subject: [PATCH] Prioritize video packets when sending padding or preemptive retransmits. Video modules are added in reverse order to ensure that the padding order is the same as before, prioritizing high resolution streams. BUG=webrtc:7043 Review-Url: https://codereview.webrtc.org/2655033002 Cr-Commit-Position: refs/heads/master@{#16329} --- webrtc/modules/pacing/packet_router.cc | 9 ++++++- .../modules/pacing/packet_router_unittest.cc | 25 +++++++++++-------- webrtc/video/video_send_stream.cc | 7 ++++-- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/webrtc/modules/pacing/packet_router.cc b/webrtc/modules/pacing/packet_router.cc index af1d5595be..65d6db7cb7 100644 --- a/webrtc/modules/pacing/packet_router.cc +++ b/webrtc/modules/pacing/packet_router.cc @@ -30,7 +30,13 @@ void PacketRouter::AddRtpModule(RtpRtcp* rtp_module) { rtc::CritScope cs(&modules_crit_); RTC_DCHECK(std::find(rtp_modules_.begin(), rtp_modules_.end(), rtp_module) == rtp_modules_.end()); - rtp_modules_.push_back(rtp_module); + // Put modules which can use regular payload packets (over rtx) instead of + // padding first as it's less of a waste + if ((rtp_module->RtxSendStatus() & kRtxRedundantPayloads) > 0) { + rtp_modules_.push_front(rtp_module); + } else { + rtp_modules_.push_back(rtp_module); + } } void PacketRouter::RemoveRtpModule(RtpRtcp* rtp_module) { @@ -64,6 +70,7 @@ size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send, RTC_DCHECK(pacer_thread_checker_.CalledOnValidThread()); size_t total_bytes_sent = 0; 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()) { size_t bytes_sent = module->TimeToSendPadding( diff --git a/webrtc/modules/pacing/packet_router_unittest.cc b/webrtc/modules/pacing/packet_router_unittest.cc index 456e1def4f..9011f81b36 100644 --- a/webrtc/modules/pacing/packet_router_unittest.cc +++ b/webrtc/modules/pacing/packet_router_unittest.cc @@ -108,34 +108,37 @@ TEST_F(PacketRouterTest, TimeToSendPadding) { const uint16_t kSsrc2 = 4567; MockRtpRtcp rtp_1; + EXPECT_CALL(rtp_1, RtxSendStatus()).WillOnce(Return(kRtxOff)); EXPECT_CALL(rtp_1, SSRC()).WillRepeatedly(Return(kSsrc1)); MockRtpRtcp rtp_2; + // rtp_2 will be prioritized for padding. + EXPECT_CALL(rtp_2, RtxSendStatus()).WillOnce(Return(kRtxRedundantPayloads)); EXPECT_CALL(rtp_2, SSRC()).WillRepeatedly(Return(kSsrc2)); packet_router_->AddRtpModule(&rtp_1); packet_router_->AddRtpModule(&rtp_2); // Default configuration, sending padding on all modules sending media, - // ordered by SSRC. + // ordered by priority (based on rtx mode). const size_t requested_padding_bytes = 1000; const size_t sent_padding_bytes = 890; - EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes, 111)) + EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, 111)) .Times(1) .WillOnce(Return(sent_padding_bytes)); - EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_2, TimeToSendPadding( + EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_1, TimeToSendPadding( requested_padding_bytes - sent_padding_bytes, 111)) .Times(1) .WillOnce(Return(requested_padding_bytes - sent_padding_bytes)); EXPECT_EQ(requested_padding_bytes, packet_router_->TimeToSendPadding(requested_padding_bytes, 111)); - // Let only the second module be sending and verify the padding request is - // routed there. - EXPECT_CALL(rtp_1, SendingMedia()).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, TimeToSendPadding(_, _)) + // Let only the lower priority module be sending and verify the padding + // request is routed there. + 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, TimeToSendPadding(_, _)) .Times(1) .WillOnce(Return(sent_padding_bytes)); EXPECT_EQ(sent_padding_bytes, diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index aa67532046..9189b98e5a 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -815,9 +815,12 @@ VideoSendStreamImpl::VideoSendStreamImpl( config_->periodic_alr_bandwidth_probing); // RTP/RTCP initialization. - for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) { + + // We add the highest spatial layer first to ensure it'll be prioritized + // when sending padding, with the hope that the packet rate will be smaller, + // and that it's more important to protect than the lower layers. + for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) packet_router_->AddRtpModule(rtp_rtcp); - } for (size_t i = 0; i < config_->rtp.extensions.size(); ++i) { const std::string& extension = config_->rtp.extensions[i].uri;