diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h index da82b65e10..235ca8498c 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -67,6 +67,7 @@ class RtpRtcp : public Module { RtpAudioFeedback* audio_messages; RemoteBitrateEstimator* remote_bitrate_estimator; PacedSender* paced_sender; + BitrateStatisticsObserver* send_bitrate_observer; }; /* @@ -308,13 +309,6 @@ class RtpRtcp : public Module { uint32_t* fecRate, uint32_t* nackRate) const = 0; - /* - * Called on any new send bitrate estimate. - */ - virtual void RegisterVideoBitrateObserver( - BitrateStatisticsObserver* observer) = 0; - virtual BitrateStatisticsObserver* GetVideoBitrateObserver() const = 0; - /* * Used by the codec module to deliver a video or audio frame for * packetization. diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index aff4234259..855d51b797 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -37,7 +37,8 @@ RtpRtcp::Configuration::Configuration() rtt_stats(NULL), audio_messages(NullObjectRtpAudioFeedback()), remote_bitrate_estimator(NULL), - paced_sender(NULL) { + paced_sender(NULL), + send_bitrate_observer(NULL) { } RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) { @@ -60,7 +61,8 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) configuration.clock, configuration.outgoing_transport, configuration.audio_messages, - configuration.paced_sender), + configuration.paced_sender, + configuration.send_bitrate_observer), rtcp_sender_(configuration.id, configuration.audio, configuration.clock, @@ -1234,16 +1236,6 @@ void ModuleRtpRtcpImpl::BitrateSent(uint32_t* total_rate, *nack_rate = rtp_sender_.NackOverheadRate(); } -void ModuleRtpRtcpImpl::RegisterVideoBitrateObserver( - BitrateStatisticsObserver* observer) { - assert(!IsDefaultModule()); - rtp_sender_.RegisterBitrateObserver(observer); -} - -BitrateStatisticsObserver* ModuleRtpRtcpImpl::GetVideoBitrateObserver() const { - return rtp_sender_.GetBitrateObserver(); -} - void ModuleRtpRtcpImpl::OnRequestIntraFrame() { RequestKeyFrame(); } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index c1694078ca..b65131fb2c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -344,11 +344,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp { uint32_t* fec_rate, uint32_t* nackRate) const OVERRIDE; - virtual void RegisterVideoBitrateObserver(BitrateStatisticsObserver* observer) - OVERRIDE; - - virtual BitrateStatisticsObserver* GetVideoBitrateObserver() const OVERRIDE; - virtual uint32_t SendTimeOfSendReport(const uint32_t send_report); virtual bool SendTimeOfXrRrReport(uint32_t mid_ntp, int64_t* time_ms) const; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index dc9f4b1d19..858fc42a24 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -45,7 +45,8 @@ RTPSender::RTPSender(const int32_t id, Clock* clock, Transport* transport, RtpAudioFeedback* audio_feedback, - PacedSender* paced_sender) + PacedSender* paced_sender, + BitrateStatisticsObserver* bitrate_callback) : clock_(clock), bitrate_sent_(clock, this), id_(id), @@ -72,7 +73,7 @@ RTPSender::RTPSender(const int32_t id, statistics_crit_(CriticalSectionWrapper::CreateCriticalSection()), frame_count_observer_(NULL), rtp_stats_callback_(NULL), - bitrate_callback_(NULL), + bitrate_callback_(bitrate_callback), // RTP variables start_timestamp_forced_(false), start_timestamp_(0), @@ -1684,16 +1685,6 @@ StreamDataCountersCallback* RTPSender::GetRtpStatisticsCallback() const { return rtp_stats_callback_; } -void RTPSender::RegisterBitrateObserver(BitrateStatisticsObserver* observer) { - CriticalSectionScoped cs(statistics_crit_.get()); - bitrate_callback_ = observer; -} - -BitrateStatisticsObserver* RTPSender::GetBitrateObserver() const { - CriticalSectionScoped cs(statistics_crit_.get()); - return bitrate_callback_; -} - uint32_t RTPSender::BitrateSent() const { return bitrate_sent_.BitrateLast(); } void RTPSender::BitrateUpdated(const BitrateStatistics& stats) { @@ -1702,7 +1693,6 @@ void RTPSender::BitrateUpdated(const BitrateStatistics& stats) { CriticalSectionScoped ssrc_lock(send_critsect_); ssrc = ssrc_; } - CriticalSectionScoped cs(statistics_crit_.get()); if (bitrate_callback_) { bitrate_callback_->Notify(stats, ssrc); } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index d7ebc7ddd1..0cc35cf4c1 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -69,7 +69,8 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer { public: RTPSender(const int32_t id, const bool audio, Clock *clock, Transport *transport, RtpAudioFeedback *audio_feedback, - PacedSender *paced_sender); + PacedSender *paced_sender, + BitrateStatisticsObserver* bitrate_callback); virtual ~RTPSender(); void ProcessBitrate(); @@ -275,10 +276,6 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer { void RegisterRtpStatisticsCallback(StreamDataCountersCallback* callback); StreamDataCountersCallback* GetRtpStatisticsCallback() const; - // Called on new send bitrate estimate. - void RegisterBitrateObserver(BitrateStatisticsObserver* observer); - BitrateStatisticsObserver* GetBitrateObserver() const; - uint32_t BitrateSent() const; virtual void BitrateUpdated(const BitrateStatistics& stats) OVERRIDE; @@ -380,7 +377,7 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer { StreamDataCounters rtp_stats_ GUARDED_BY(statistics_crit_); StreamDataCounters rtx_rtp_stats_ GUARDED_BY(statistics_crit_); StreamDataCountersCallback* rtp_stats_callback_ GUARDED_BY(statistics_crit_); - BitrateStatisticsObserver* bitrate_callback_ GUARDED_BY(statistics_crit_); + BitrateStatisticsObserver* const bitrate_callback_; // RTP variables bool start_timestamp_forced_ GUARDED_BY(send_critsect_); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 9ba60b1bf1..e08aa202e2 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -94,7 +94,7 @@ class RtpSenderTest : public ::testing::Test { virtual void SetUp() { rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport_, NULL, - &mock_paced_sender_)); + &mock_paced_sender_, NULL)); rtp_sender_->SetSequenceNumber(kSeqNum); } @@ -672,7 +672,7 @@ TEST_F(RtpSenderTest, SendPadding) { TEST_F(RtpSenderTest, SendRedundantPayloads) { MockTransport transport; rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport, NULL, - &mock_paced_sender_)); + &mock_paced_sender_, NULL)); rtp_sender_->SetSequenceNumber(kSeqNum); // Make all packets go through the pacer. EXPECT_CALL(mock_paced_sender_, @@ -865,6 +865,8 @@ TEST_F(RtpSenderTest, BitrateCallbacks) { uint32_t ssrc_; BitrateStatistics bitrate_; } callback; + rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport_, NULL, + &mock_paced_sender_, &callback)); // Simulate kNumPackets sent with kPacketInterval ms intervals. const uint32_t kNumPackets = 15; @@ -881,8 +883,6 @@ TEST_F(RtpSenderTest, BitrateCallbacks) { rtp_sender_->SetStorePacketsStatus(true, 1); uint32_t ssrc = rtp_sender_->SSRC(); - rtp_sender_->RegisterBitrateObserver(&callback); - // Initial process call so we get a new time window. rtp_sender_->ProcessBitrate(); uint64_t start_time = fake_clock_.CurrentNtpInMilliseconds(); @@ -912,7 +912,7 @@ TEST_F(RtpSenderTest, BitrateCallbacks) { EXPECT_EQ((kPacketOverhead + sizeof(payload)) * 8 * expected_packet_rate, callback.bitrate_.bitrate_bps); - rtp_sender_->RegisterBitrateObserver(NULL); + rtp_sender_.reset(); } class RtpSenderAudioTest : public RtpSenderTest { @@ -922,7 +922,7 @@ class RtpSenderAudioTest : public RtpSenderTest { virtual void SetUp() { payload_ = kAudioPayload; rtp_sender_.reset(new RTPSender(0, true, &fake_clock_, &transport_, NULL, - &mock_paced_sender_)); + &mock_paced_sender_, NULL)); rtp_sender_->SetSequenceNumber(kSeqNum); } }; diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index 5eb36076cd..46622518f6 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -116,6 +116,7 @@ ViEChannel::ViEChannel(int32_t channel_id, configuration.remote_bitrate_estimator = remote_bitrate_estimator; configuration.paced_sender = paced_sender; configuration.receive_statistics = vie_receiver_.GetReceiveStatistics(); + configuration.send_bitrate_observer = &send_bitrate_observer_; rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(configuration)); vie_receiver_.SetRtpRtcpModule(rtp_rtcp_.get()); @@ -300,7 +301,6 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec, rtp_rtcp->RegisterSendFrameCountObserver(NULL); rtp_rtcp->RegisterSendChannelRtcpStatisticsCallback(NULL); rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(NULL); - rtp_rtcp->RegisterVideoBitrateObserver(NULL); simulcast_rtp_rtcp_.pop_back(); removed_rtp_rtcp_.push_front(rtp_rtcp); } @@ -352,8 +352,6 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec, rtp_rtcp_->GetSendChannelRtcpStatisticsCallback()); rtp_rtcp->RegisterSendChannelRtpStatisticsCallback( rtp_rtcp_->GetSendChannelRtpStatisticsCallback()); - rtp_rtcp->RegisterVideoBitrateObserver( - rtp_rtcp_->GetVideoBitrateObserver()); } // |RegisterSimulcastRtpRtcpModules| resets all old weak pointers and old // modules can be deleted after this step. @@ -367,7 +365,6 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec, rtp_rtcp->RegisterSendFrameCountObserver(NULL); rtp_rtcp->RegisterSendChannelRtcpStatisticsCallback(NULL); rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(NULL); - rtp_rtcp->RegisterVideoBitrateObserver(NULL); simulcast_rtp_rtcp_.pop_back(); removed_rtp_rtcp_.push_front(rtp_rtcp); } @@ -1212,13 +1209,7 @@ bool ViEChannel::GetSendSideDelay(int* avg_send_delay, void ViEChannel::RegisterSendBitrateObserver( BitrateStatisticsObserver* observer) { - rtp_rtcp_->RegisterVideoBitrateObserver(observer); - CriticalSectionScoped cs(rtp_rtcp_cs_.get()); - for (std::list::const_iterator it = simulcast_rtp_rtcp_.begin(); - it != simulcast_rtp_rtcp_.end(); - it++) { - (*it)->RegisterVideoBitrateObserver(observer); - } + send_bitrate_observer_.Set(observer); } void ViEChannel::GetReceiveBandwidthEstimatorStats( @@ -1536,7 +1527,6 @@ void ViEChannel::ReserveRtpRtcpModules(size_t num_modules) { rtp_rtcp->RegisterSendFrameCountObserver(NULL); rtp_rtcp->RegisterSendChannelRtcpStatisticsCallback(NULL); rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(NULL); - rtp_rtcp->RegisterVideoBitrateObserver(NULL); removed_rtp_rtcp_.push_back(rtp_rtcp); } } diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h index 0ec5043705..39f9b75cef 100644 --- a/webrtc/video_engine/vie_channel.h +++ b/webrtc/video_engine/vie_channel.h @@ -16,6 +16,7 @@ #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h" #include "webrtc/modules/video_coding/main/interface/video_coding_defines.h" +#include "webrtc/system_wrappers/interface/critical_section_wrapper.h" #include "webrtc/system_wrappers/interface/scoped_ptr.h" #include "webrtc/system_wrappers/interface/tick_util.h" #include "webrtc/typedefs.h" @@ -382,6 +383,43 @@ class ViEChannel int GetRequiredNackListSize(int target_delay_ms); void SetRtxSendStatus(bool enable); + // ViEChannel exposes methods that allow to modify observers and callbacks + // to be modified. Such an API-style is cumbersome to implement and maintain + // at all the levels when comparing to only setting them at construction. As + // so this class instantiates its children with a wrapper that can be modified + // at a later time. + template + class RegisterableCallback : public T { + public: + RegisterableCallback() + : critsect_(CriticalSectionWrapper::CreateCriticalSection()), + callback_(NULL) {} + + void Set(T* callback) { + CriticalSectionScoped cs(critsect_.get()); + callback_ = callback; + } + + protected: + // Note: this should be implemented with a RW-lock to allow simultaneous + // calls into the callback. However that doesn't seem to be needed for the + // current type of callbacks covered by this class. + scoped_ptr critsect_; + T* callback_ GUARDED_BY(critsect_); + + private: + DISALLOW_COPY_AND_ASSIGN(RegisterableCallback); + }; + + class : public RegisterableCallback { + virtual void Notify(const BitrateStatistics& stats, uint32_t ssrc) { + CriticalSectionScoped cs(critsect_.get()); + if (callback_) + callback_->Notify(stats, ssrc); + } + } + send_bitrate_observer_; + int32_t channel_id_; int32_t engine_id_; uint32_t number_of_cores_;