From 0ab8ba313b7920134d503c63e4e995949d99e039 Mon Sep 17 00:00:00 2001 From: "mflodman@webrtc.org" Date: Mon, 9 Jan 2012 16:16:49 +0000 Subject: [PATCH] We now require a manually set sender to send REMB packets. BUG= TEST=video_engine_unittests Review URL: http://webrtc-codereview.appspot.com/348003 git-svn-id: http://webrtc.googlecode.com/svn/trunk@1358 4adac7df-926f-26a2-2b94-8c16560cd09d --- src/video_engine/vie_channel_manager.cc | 5 ++- src/video_engine/vie_remb.cc | 30 +++++++++-------- src/video_engine/vie_remb_unittest.cc | 44 +++++++++---------------- 3 files changed, 34 insertions(+), 45 deletions(-) diff --git a/src/video_engine/vie_channel_manager.cc b/src/video_engine/vie_channel_manager.cc index f88efe1d52..16b8b31074 100644 --- a/src/video_engine/vie_channel_manager.cc +++ b/src/video_engine/vie_channel_manager.cc @@ -369,11 +369,10 @@ bool ViEChannelManager::SetRembStatus(int channel_id, bool sender, channel->EnableRemb(false); } RtpRtcp* rtp_module = channel->rtp_rtcp(); - // TODO(mflodman) Add when implemented. if (sender) { - // remb_->AddSendChannel(rtp_module); + remb_->AddSendChannel(rtp_module); } else { - // remb_->RemoveSendChannel(rtp_module); + remb_->RemoveSendChannel(rtp_module); } if (receiver) { remb_->AddReceiveChannel(rtp_module); diff --git a/src/video_engine/vie_remb.cc b/src/video_engine/vie_remb.cc index 837dcc4f27..2ebde551f5 100644 --- a/src/video_engine/vie_remb.cc +++ b/src/video_engine/vie_remb.cc @@ -61,7 +61,7 @@ void VieRemb::RemoveReceiveChannel(RtpRtcp* rtp_rtcp) { unsigned int ssrc = rtp_rtcp->RemoteSSRC(); for (RtpModules::iterator it = receive_modules_.begin(); it != receive_modules_.end(); ++it) { - if ((*it)->RemoteSSRC() == ssrc) { + if ((*it) == rtp_rtcp) { receive_modules_.erase(it); break; } @@ -73,16 +73,28 @@ void VieRemb::AddSendChannel(RtpRtcp* rtp_rtcp) { WEBRTC_TRACE(kTraceStateInfo, kTraceVideo, engine_id_, "VieRemb::AddSendChannel"); assert(rtp_rtcp); - assert(false); - return; + + CriticalSectionScoped cs(list_crit_.get()); + + // TODO(mflodman) Allow multiple senders. + assert(send_modules_.empty()); + + send_modules_.push_back(rtp_rtcp); } void VieRemb::RemoveSendChannel(RtpRtcp* rtp_rtcp) { WEBRTC_TRACE(kTraceStateInfo, kTraceVideo, engine_id_, "VieRemb::AddSendChannel"); assert(rtp_rtcp); - assert(false); - return; + + CriticalSectionScoped cs(list_crit_.get()); + for (RtpModules::iterator it = send_modules_.begin(); + it != send_modules_.end(); ++it) { + if ((*it) == rtp_rtcp) { + send_modules_.erase(it); + return; + } + } } void VieRemb::OnReceiveBitrateChanged(unsigned int ssrc, unsigned int bitrate) { @@ -151,14 +163,6 @@ WebRtc_Word32 VieRemb::Process() { RtpRtcp* sender = NULL; if (!send_modules_.empty()) { sender = send_modules_.front(); - } else { - for (RtpModules::iterator it = receive_modules_.begin(); - it != receive_modules_.end(); ++it) { - if ((*it)->Sending()) { - sender = *it; - break; - } - } } last_send_bitrate_ = total_bitrate; list_crit_->Leave(); diff --git a/src/video_engine/vie_remb_unittest.cc b/src/video_engine/vie_remb_unittest.cc index afbdb0b0e8..b6cb083647 100644 --- a/src/video_engine/vie_remb_unittest.cc +++ b/src/video_engine/vie_remb_unittest.cc @@ -47,10 +47,8 @@ class ViERembTest : public ::testing::Test { TEST_F(ViERembTest, OneModuleTestForSendingRemb) { MockRtpRtcp rtp; - EXPECT_CALL(rtp, Sending()) - .WillRepeatedly(Return(true)); - vie_remb_->AddReceiveChannel(&rtp); + vie_remb_->AddSendChannel(&rtp); const unsigned int bitrate_estimate = 456; unsigned int ssrc[] = { 1234 }; @@ -72,15 +70,14 @@ TEST_F(ViERembTest, OneModuleTestForSendingRemb) vie_remb_->Process(); vie_remb_->RemoveReceiveChannel(&rtp); + vie_remb_->RemoveSendChannel(&rtp); } TEST_F(ViERembTest, LowerEstimateToSendRemb) { MockRtpRtcp rtp; - EXPECT_CALL(rtp, Sending()) - .WillRepeatedly(Return(true)); - vie_remb_->AddReceiveChannel(&rtp); + vie_remb_->AddSendChannel(&rtp); unsigned int bitrate_estimate = 456; unsigned int ssrc[] = { 1234 }; @@ -101,13 +98,9 @@ TEST_F(ViERembTest, LowerEstimateToSendRemb) TEST_F(ViERembTest, VerifyCombinedBitrateEstimate) { MockRtpRtcp rtp_0; - EXPECT_CALL(rtp_0, Sending()) - .WillRepeatedly(Return(true)); MockRtpRtcp rtp_1; - EXPECT_CALL(rtp_1, Sending()) - .WillRepeatedly(Return(true)); - vie_remb_->AddReceiveChannel(&rtp_0); + vie_remb_->AddSendChannel(&rtp_0); vie_remb_->AddReceiveChannel(&rtp_1); unsigned int bitrate_estimate[] = { 456, 789 }; @@ -131,19 +124,16 @@ TEST_F(ViERembTest, VerifyCombinedBitrateEstimate) vie_remb_->Process(); vie_remb_->RemoveReceiveChannel(&rtp_0); + vie_remb_->RemoveSendChannel(&rtp_0); vie_remb_->RemoveReceiveChannel(&rtp_1); } TEST_F(ViERembTest, NoRembForIncreasedBitrate) { MockRtpRtcp rtp_0; - EXPECT_CALL(rtp_0, Sending()) - .WillRepeatedly(Return(true)); MockRtpRtcp rtp_1; - EXPECT_CALL(rtp_1, Sending()) - .WillRepeatedly(Return(true)); - vie_remb_->AddReceiveChannel(&rtp_0); + vie_remb_->AddSendChannel(&rtp_0); vie_remb_->AddReceiveChannel(&rtp_1); unsigned int bitrate_estimate[] = { 456, 789 }; @@ -181,18 +171,15 @@ TEST_F(ViERembTest, NoRembForIncreasedBitrate) vie_remb_->Process(); vie_remb_->RemoveReceiveChannel(&rtp_1); vie_remb_->RemoveReceiveChannel(&rtp_0); + vie_remb_->RemoveSendChannel(&rtp_0); } TEST_F(ViERembTest, ChangeSendRtpModule) { MockRtpRtcp rtp_0; - EXPECT_CALL(rtp_0, Sending()) - .WillRepeatedly(Return(true)); MockRtpRtcp rtp_1; - EXPECT_CALL(rtp_1, Sending()) - .WillRepeatedly(Return(true)); - vie_remb_->AddReceiveChannel(&rtp_0); + vie_remb_->AddSendChannel(&rtp_0); vie_remb_->AddReceiveChannel(&rtp_1); unsigned int bitrate_estimate[] = { 456, 789 }; @@ -218,8 +205,8 @@ TEST_F(ViERembTest, ChangeSendRtpModule) // Remove the sending module, add it again -> should get remb on the second // module. - vie_remb_->RemoveReceiveChannel(&rtp_0); - vie_remb_->AddReceiveChannel(&rtp_0); + vie_remb_->RemoveSendChannel(&rtp_0); + vie_remb_->AddSendChannel(&rtp_1); vie_remb_->OnReceiveBitrateChanged(ssrc[0], bitrate_estimate[0]); bitrate_estimate[1] = bitrate_estimate[1] - 100; @@ -231,18 +218,17 @@ TEST_F(ViERembTest, ChangeSendRtpModule) vie_remb_->RemoveReceiveChannel(&rtp_0); vie_remb_->RemoveReceiveChannel(&rtp_1); + vie_remb_->RemoveSendChannel(&rtp_1); } TEST_F(ViERembTest, OnlyOneRembForDoubleProcess) { MockRtpRtcp rtp; - EXPECT_CALL(rtp, Sending()) - .WillRepeatedly(Return(true)); - unsigned int bitrate_estimate = 456; unsigned int ssrc[] = { 1234 }; vie_remb_->AddReceiveChannel(&rtp); + vie_remb_->AddSendChannel(&rtp); vie_remb_->OnReceiveBitrateChanged(ssrc[0], bitrate_estimate); EXPECT_CALL(rtp, RemoteSSRC()) .WillRepeatedly(Return(ssrc[0])); @@ -259,6 +245,7 @@ TEST_F(ViERembTest, OnlyOneRembForDoubleProcess) .Times(0); vie_remb_->Process(); vie_remb_->RemoveReceiveChannel(&rtp); + vie_remb_->RemoveSendChannel(&rtp); } TEST_F(ViERembTest, NoOnReceivedBitrateChangedCall) @@ -268,6 +255,7 @@ TEST_F(ViERembTest, NoOnReceivedBitrateChangedCall) .WillRepeatedly(Return(1234)); vie_remb_->AddReceiveChannel(&rtp); + vie_remb_->AddSendChannel(&rtp); // TODO(mflodman) Add fake clock. TestSleep(1010); // No bitrate estimate given, no callback expected. @@ -276,14 +264,12 @@ TEST_F(ViERembTest, NoOnReceivedBitrateChangedCall) vie_remb_->Process(); vie_remb_->RemoveReceiveChannel(&rtp); + vie_remb_->RemoveSendChannel(&rtp); } TEST_F(ViERembTest, NoSendingRtpModule) { MockRtpRtcp rtp; - EXPECT_CALL(rtp, Sending()) - .WillRepeatedly(Return(false)); - vie_remb_->AddReceiveChannel(&rtp); unsigned int bitrate_estimate = 456;