Remove TimeToSendPacket and TimeToSendPadding from the default module.
Thie CL moves the default RTP module logic for TimeToSendPacket and
TimeToSendPadding to PayloadRouter class and asserts on usage of the
default module.
BUG=769
TEST=New unittest.
R=stefan@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/33319004
Cr-Commit-Position: refs/heads/master@{#8383}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8383 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index f821c02..89ac9a0 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -501,42 +501,18 @@
uint16_t sequence_number,
int64_t capture_time_ms,
bool retransmission) {
- if (!IsDefaultModule()) {
- // Don't send from default module.
- if (SendingMedia() && ssrc == rtp_sender_.SSRC()) {
- return rtp_sender_.TimeToSendPacket(sequence_number, capture_time_ms,
- retransmission);
- }
- } else {
- CriticalSectionScoped lock(critical_section_module_ptrs_.get());
- std::vector<ModuleRtpRtcpImpl*>::iterator it = child_modules_.begin();
- while (it != child_modules_.end()) {
- if ((*it)->SendingMedia() && ssrc == (*it)->rtp_sender_.SSRC()) {
- return (*it)->rtp_sender_.TimeToSendPacket(sequence_number,
- capture_time_ms,
- retransmission);
- }
- ++it;
- }
+ assert(!IsDefaultModule());
+ if (SendingMedia() && ssrc == rtp_sender_.SSRC()) {
+ return rtp_sender_.TimeToSendPacket(
+ sequence_number, capture_time_ms, retransmission);
}
// No RTP sender is interested in sending this packet.
return true;
}
size_t ModuleRtpRtcpImpl::TimeToSendPadding(size_t bytes) {
- if (!IsDefaultModule()) {
- // Don't send from default module.
- return rtp_sender_.TimeToSendPadding(bytes);
- } else {
- CriticalSectionScoped lock(critical_section_module_ptrs_.get());
- for (size_t i = 0; i < child_modules_.size(); ++i) {
- // Send padding on one of the modules sending media.
- if (child_modules_[i]->SendingMedia()) {
- return child_modules_[i]->rtp_sender_.TimeToSendPadding(bytes);
- }
- }
- }
- return 0;
+ assert(!IsDefaultModule());
+ return rtp_sender_.TimeToSendPadding(bytes);
}
bool ModuleRtpRtcpImpl::GetSendSideDelay(int* avg_send_delay_ms,
diff --git a/webrtc/video_engine/payload_router.cc b/webrtc/video_engine/payload_router.cc
index 00c64ad..58a2fb4 100644
--- a/webrtc/video_engine/payload_router.cc
+++ b/webrtc/video_engine/payload_router.cc
@@ -71,6 +71,29 @@
payload_length, fragmentation, rtp_video_hdr) == 0 ? true : false;
}
+bool PayloadRouter::TimeToSendPacket(uint32_t ssrc,
+ uint16_t sequence_number,
+ int64_t capture_timestamp,
+ bool retransmission) {
+ CriticalSectionScoped cs(crit_.get());
+ for (auto* rtp_module : rtp_modules_) {
+ if (rtp_module->SendingMedia() && ssrc == rtp_module->SSRC()) {
+ return rtp_module->TimeToSendPacket(ssrc, sequence_number,
+ capture_timestamp, retransmission);
+ }
+ }
+ return true;
+}
+
+size_t PayloadRouter::TimeToSendPadding(size_t bytes) {
+ CriticalSectionScoped cs(crit_.get());
+ for(auto* rtp_module : rtp_modules_) {
+ if (rtp_module->SendingMedia())
+ return rtp_module->TimeToSendPadding(bytes);
+ }
+ return 0;
+}
+
size_t PayloadRouter::MaxPayloadLength() const {
size_t min_payload_length = DefaultMaxPayloadLength();
CriticalSectionScoped cs(crit_.get());
diff --git a/webrtc/video_engine/payload_router.h b/webrtc/video_engine/payload_router.h
index baad38f..60f4747 100644
--- a/webrtc/video_engine/payload_router.h
+++ b/webrtc/video_engine/payload_router.h
@@ -54,11 +54,23 @@
const RTPFragmentationHeader* fragmentation,
const RTPVideoHeader* rtp_video_hdr);
+ // Called when it's time to send a stored packet.
+ bool TimeToSendPacket(uint32_t ssrc,
+ uint16_t sequence_number,
+ int64_t capture_timestamp,
+ bool retransmission);
+
+ // Called when it's time to send padding, returns the number of bytes actually
+ // sent.
+ size_t TimeToSendPadding(size_t bytes);
+
// Returns the maximum allowed data payload length, given the configured MTU
// and RTP headers.
size_t MaxPayloadLength() const;
private:
+ // TODO(mflodman): When the new video API has launched, remove crit_ and
+ // assume rtp_modules_ will never change during a call.
scoped_ptr<CriticalSectionWrapper> crit_;
// Active sending RTP modules, in layer order.
diff --git a/webrtc/video_engine/payload_router_unittest.cc b/webrtc/video_engine/payload_router_unittest.cc
index abab965..ff4f9b3 100644
--- a/webrtc/video_engine/payload_router_unittest.cc
+++ b/webrtc/video_engine/payload_router_unittest.cc
@@ -161,4 +161,145 @@
EXPECT_EQ(kTestMinPayloadLength, payload_router_->MaxPayloadLength());
}
+TEST_F(PayloadRouterTest, TimeToSendPacket) {
+ MockRtpRtcp rtp_1;
+ MockRtpRtcp rtp_2;
+ std::list<RtpRtcp*> modules;
+ modules.push_back(&rtp_1);
+ modules.push_back(&rtp_2);
+ payload_router_->SetSendingRtpModules(modules);
+
+ const uint16_t kSsrc1 = 1234;
+ uint16_t sequence_number = 17;
+ uint64_t timestamp = 7890;
+ bool retransmission = false;
+
+ // Send on the first module by letting rtp_1 be sending with correct ssrc.
+ EXPECT_CALL(rtp_1, SendingMedia())
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_CALL(rtp_1, SSRC())
+ .Times(1)
+ .WillOnce(Return(kSsrc1));
+ EXPECT_CALL(rtp_1, TimeToSendPacket(kSsrc1, sequence_number, timestamp,
+ retransmission))
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _))
+ .Times(0);
+ EXPECT_TRUE(payload_router_->TimeToSendPacket(
+ kSsrc1, sequence_number, timestamp, retransmission));
+
+
+ // Send on the second module by letting rtp_2 be sending, but not rtp_1.
+ ++sequence_number;
+ timestamp += 30;
+ retransmission = true;
+ const uint16_t kSsrc2 = 4567;
+ EXPECT_CALL(rtp_1, SendingMedia())
+ .Times(1)
+ .WillOnce(Return(false));
+ EXPECT_CALL(rtp_2, SendingMedia())
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_CALL(rtp_2, SSRC())
+ .Times(1)
+ .WillOnce(Return(kSsrc2));
+ EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _))
+ .Times(0);
+ EXPECT_CALL(rtp_2, TimeToSendPacket(kSsrc2, sequence_number, timestamp,
+ retransmission))
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_TRUE(payload_router_->TimeToSendPacket(
+ kSsrc2, sequence_number, timestamp, retransmission));
+
+ // No module is sending, hence no packet should be sent.
+ EXPECT_CALL(rtp_1, SendingMedia())
+ .Times(1)
+ .WillOnce(Return(false));
+ EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _,_))
+ .Times(0);
+ EXPECT_CALL(rtp_2, SendingMedia())
+ .Times(1)
+ .WillOnce(Return(false));
+ EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _))
+ .Times(0);
+ EXPECT_TRUE(payload_router_->TimeToSendPacket(
+ kSsrc1, sequence_number, timestamp, retransmission));
+
+ // Add a packet with incorrect ssrc and test it's dropped in the router.
+ EXPECT_CALL(rtp_1, SendingMedia())
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_CALL(rtp_1, SSRC())
+ .Times(1)
+ .WillOnce(Return(kSsrc1));
+ EXPECT_CALL(rtp_2, SendingMedia())
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_CALL(rtp_2, SSRC())
+ .Times(1)
+ .WillOnce(Return(kSsrc2));
+ EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _,_))
+ .Times(0);
+ EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _))
+ .Times(0);
+ EXPECT_TRUE(payload_router_->TimeToSendPacket(
+ kSsrc1 + kSsrc2, sequence_number, timestamp, retransmission));
+}
+
+TEST_F(PayloadRouterTest, TimeToSendPadding) {
+ MockRtpRtcp rtp_1;
+ MockRtpRtcp rtp_2;
+ std::list<RtpRtcp*> modules;
+ modules.push_back(&rtp_1);
+ modules.push_back(&rtp_2);
+ payload_router_->SetSendingRtpModules(modules);
+
+
+ // Default configuration, sending padding on the first sending module.
+ 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))
+ .Times(1)
+ .WillOnce(Return(sent_padding_bytes));
+ EXPECT_CALL(rtp_2, TimeToSendPadding(_))
+ .Times(0);
+ EXPECT_EQ(sent_padding_bytes,
+ payload_router_->TimeToSendPadding(requested_padding_bytes));
+
+ // 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(_))
+ .Times(1)
+ .WillOnce(Return(sent_padding_bytes));
+ EXPECT_EQ(sent_padding_bytes,
+ payload_router_->TimeToSendPadding(requested_padding_bytes));
+
+ // No sending module at all.
+ 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(false));
+ EXPECT_CALL(rtp_2, TimeToSendPadding(_))
+ .Times(0);
+ EXPECT_EQ(static_cast<size_t>(0),
+ payload_router_->TimeToSendPadding(requested_padding_bytes));
+}
} // namespace webrtc
diff --git a/webrtc/video_engine/vie_channel_manager.cc b/webrtc/video_engine/vie_channel_manager.cc
index f2788cf..c9f0f4e 100644
--- a/webrtc/video_engine/vie_channel_manager.cc
+++ b/webrtc/video_engine/vie_channel_manager.cc
@@ -117,7 +117,7 @@
return -1;
}
// Connect the encoder with the send packet router, to enable sending.
- vie_encoder->SetSendPayloadRouter(
+ vie_encoder->StartThreadsAndSetSendPayloadRouter(
channel_map_[new_channel_id]->send_payload_router());
// Add ViEEncoder to EncoderFeedBackObserver.
@@ -183,7 +183,7 @@
vie_encoder = NULL;
}
// Connect the encoder with the send packet router, to enable sending.
- vie_encoder->SetSendPayloadRouter(
+ vie_encoder->StartThreadsAndSetSendPayloadRouter(
channel_map_[new_channel_id]->send_payload_router());
// Register the ViEEncoder to get key frame requests for this channel.
@@ -249,9 +249,11 @@
vie_channel->GetStatsObserver());
group->SetChannelRembStatus(channel_id, false, false, vie_channel);
- // Remove the feedback if we're owning the encoder.
+ // If we're owning the encoder, remove the feedback and stop all encoding
+ // threads and processing. This must be done before deleting the channel.
if (vie_encoder->channel_id() == channel_id) {
group->GetEncoderStateFeedback()->RemoveEncoder(vie_encoder);
+ vie_encoder->StopThreadsAndRemovePayloadRouter();
}
unsigned int remote_ssrc = 0;
diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc
index e2947aa..7e010b1 100644
--- a/webrtc/video_engine/vie_encoder.cc
+++ b/webrtc/video_engine/vie_encoder.cc
@@ -107,6 +107,7 @@
ViEEncoder* owner_;
};
+// TODO(mflodman): Move this observer to PayloadRouter class.
class ViEPacedSenderCallback : public PacedSender::Callback {
public:
explicit ViEPacedSenderCallback(ViEEncoder* owner)
@@ -191,14 +192,6 @@
// Enable/disable content analysis: off by default for now.
vpm_.EnableContentAnalysis(false);
- if (module_process_thread_.RegisterModule(&vcm_) != 0 ||
- module_process_thread_.RegisterModule(default_rtp_rtcp_.get()) != 0) {
- return false;
- }
- if (pacer_thread_->RegisterModule(paced_sender_.get()) != 0 ||
- pacer_thread_->Start() != 0) {
- return false;
- }
if (qm_callback_) {
delete qm_callback_;
}
@@ -235,9 +228,24 @@
return true;
}
-void ViEEncoder::SetSendPayloadRouter(PayloadRouter* send_payload_router) {
+void ViEEncoder::StartThreadsAndSetSendPayloadRouter(
+ PayloadRouter* send_payload_router) {
DCHECK(send_payload_router_ == NULL);
send_payload_router_ = send_payload_router;
+
+ module_process_thread_.RegisterModule(&vcm_);
+ module_process_thread_.RegisterModule(default_rtp_rtcp_.get());
+ pacer_thread_->RegisterModule(paced_sender_.get());
+ pacer_thread_->Start();
+}
+
+void ViEEncoder::StopThreadsAndRemovePayloadRouter() {
+ pacer_thread_->Stop();
+ pacer_thread_->DeRegisterModule(paced_sender_.get());
+ module_process_thread_.DeRegisterModule(&vcm_);
+ module_process_thread_.DeRegisterModule(&vpm_);
+ module_process_thread_.DeRegisterModule(default_rtp_rtcp_.get());
+ send_payload_router_ = nullptr;
}
ViEEncoder::~ViEEncoder() {
@@ -245,11 +253,6 @@
if (bitrate_controller_) {
bitrate_controller_->RemoveBitrateObserver(bitrate_observer_.get());
}
- pacer_thread_->Stop();
- pacer_thread_->DeRegisterModule(paced_sender_.get());
- module_process_thread_.DeRegisterModule(&vcm_);
- module_process_thread_.DeRegisterModule(&vpm_);
- module_process_thread_.DeRegisterModule(default_rtp_rtcp_.get());
VideoCodingModule::Destroy(&vcm_);
VideoProcessingModule::Destroy(&vpm_);
delete qm_callback_;
@@ -453,8 +456,8 @@
uint16_t sequence_number,
int64_t capture_time_ms,
bool retransmission) {
- return default_rtp_rtcp_->TimeToSendPacket(ssrc, sequence_number,
- capture_time_ms, retransmission);
+ return send_payload_router_->TimeToSendPacket(
+ ssrc, sequence_number, capture_time_ms, retransmission);
}
size_t ViEEncoder::TimeToSendPadding(size_t bytes) {
@@ -465,7 +468,7 @@
send_padding_ || video_suspended_ || min_transmit_bitrate_kbps_ > 0;
}
if (send_padding) {
- return default_rtp_rtcp_->TimeToSendPadding(bytes);
+ return send_payload_router_->TimeToSendPadding(bytes);
}
return 0;
}
diff --git a/webrtc/video_engine/vie_encoder.h b/webrtc/video_engine/vie_encoder.h
index 3777adb..d9c6847 100644
--- a/webrtc/video_engine/vie_encoder.h
+++ b/webrtc/video_engine/vie_encoder.h
@@ -65,8 +65,15 @@
bool Init();
- // This function is assumed to be called before any frames are delivered.
- void SetSendPayloadRouter(PayloadRouter* send_payload_router);
+ // This function is assumed to be called before any frames are delivered and
+ // only once.
+ // Ideally this would be done in Init, but the dependencies between ViEEncoder
+ // and ViEChannel makes it really hard to do in a good way.
+ void StartThreadsAndSetSendPayloadRouter(PayloadRouter* send_payload_router);
+
+ // This function must be called before the corresponding ViEChannel is
+ // deleted.
+ void StopThreadsAndRemovePayloadRouter();
void SetNetworkTransmissionState(bool is_transmitting);