From b5865079868c4dec49571e7aef0aa52124b50c64 Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Fri, 1 Feb 2013 14:33:42 +0000 Subject: [PATCH] Break out RemoteBitrateEstimator from RtpRtcp module and make RemoteBitrateEstimator::Process trigger new REMB messages. Also make sure RTT is computed independently of whether it's time to send RTCP messages or not. BUG=1298 Review URL: https://webrtc-codereview.appspot.com/1060005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3455 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../modules/interface/module_common_types.h | 10 + .../include/bwe_defines.h | 6 +- .../include/remote_bitrate_estimator.h | 28 +-- .../overuse_detector.cc | 8 +- .../overuse_detector.h | 4 + .../remote_bitrate_estimator_multi_stream.cc | 70 +++++-- .../remote_bitrate_estimator_multi_stream.h | 23 ++- .../remote_bitrate_estimator_single_stream.cc | 87 +++++++-- .../remote_bitrate_estimator_single_stream.h | 41 +++-- .../remote_bitrate_estimator_unittest.cc | 174 ++++++++++-------- ...emote_bitrate_estimator_unittest_helper.cc | 98 +++++----- ...remote_bitrate_estimator_unittest_helper.h | 35 ++-- .../remote_rate_control.cc | 19 +- .../remote_rate_control.h | 6 + webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h | 6 - .../source/rtcp_format_remb_unittest.cc | 7 +- .../modules/rtp_rtcp/source/rtcp_receiver.cc | 13 ++ .../modules/rtp_rtcp/source/rtcp_receiver.h | 6 +- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 5 +- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 7 +- .../modules/rtp_rtcp/source/rtp_rtcp_config.h | 3 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 75 +++----- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 4 +- .../rtp_rtcp/test/testAPI/test_api_rtcp.cc | 2 +- webrtc/video_engine/call_stats.h | 10 +- webrtc/video_engine/vie_channel.cc | 4 +- webrtc/video_engine/vie_channel_group.cc | 10 +- webrtc/video_engine/vie_receiver.cc | 14 ++ webrtc/video_engine/vie_receiver.h | 2 + webrtc/video_engine/vie_remb.cc | 64 ++----- webrtc/video_engine/vie_remb.h | 11 +- webrtc/video_engine/vie_remb_unittest.cc | 64 ++----- 32 files changed, 523 insertions(+), 393 deletions(-) diff --git a/webrtc/modules/interface/module_common_types.h b/webrtc/modules/interface/module_common_types.h index a33e7efb0..5815996b5 100644 --- a/webrtc/modules/interface/module_common_types.h +++ b/webrtc/modules/interface/module_common_types.h @@ -320,6 +320,16 @@ struct FecProtectionParams { FecMaskType fec_mask_type; }; +// Interface used by the CallStats class to distribute call statistics. +// Callbacks will be triggered as soon as the class has been registered to a +// CallStats object using RegisterStatsObserver. +class StatsObserver { + public: + virtual void OnRttUpdate(uint32_t rtt_ms) = 0; + + virtual ~StatsObserver() {} +}; + // class describing a complete, or parts of an encoded frame. class EncodedVideoData { diff --git a/webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h b/webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h index 395ea7ebd..0ad64ddcc 100644 --- a/webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h +++ b/webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h @@ -19,9 +19,9 @@ namespace webrtc { enum BandwidthUsage { - kBwNormal, - kBwOverusing, - kBwUnderusing + kBwNormal = 0, + kBwUnderusing = 1, + kBwOverusing = 2, }; enum RateControlState diff --git a/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h b/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h index 0fa964112..0a1a92cb4 100644 --- a/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h +++ b/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h @@ -16,11 +16,15 @@ #include #include -#include "common_types.h" -#include "typedefs.h" +#include "webrtc/common_types.h" +#include "webrtc/modules/interface/module.h" +#include "webrtc/modules/interface/module_common_types.h" +#include "webrtc/typedefs.h" namespace webrtc { +class Clock; + // RemoteBitrateObserver is used to signal changes in bitrate estimates for // the incoming streams. class RemoteBitrateObserver { @@ -33,7 +37,7 @@ class RemoteBitrateObserver { virtual ~RemoteBitrateObserver() {} }; -class RemoteBitrateEstimator { +class RemoteBitrateEstimator : public StatsObserver, public Module { public: enum EstimationMode { kMultiStreamEstimation, @@ -42,9 +46,10 @@ class RemoteBitrateEstimator { virtual ~RemoteBitrateEstimator() {} - static RemoteBitrateEstimator* Create(RemoteBitrateObserver* observer, - const OverUseDetectorOptions& options, - EstimationMode mode); + static RemoteBitrateEstimator* Create(const OverUseDetectorOptions& options, + EstimationMode mode, + RemoteBitrateObserver* observer, + Clock* clock); // Stores an RTCP SR (NTP, RTP timestamp) tuple for a specific SSRC to be used // in future RTP timestamp to NTP time conversions. As soon as any SSRC has @@ -61,13 +66,6 @@ class RemoteBitrateEstimator { int64_t arrival_time, uint32_t rtp_timestamp) = 0; - // Triggers a new estimate calculation. - virtual void UpdateEstimate(unsigned int ssrc, int64_t time_now) = 0; - - // Set the current round-trip time experienced by the streams going into this - // estimator. - virtual void SetRtt(unsigned int rtt) = 0; - // Removes all data for |ssrc|. virtual void RemoveStream(unsigned int ssrc) = 0; @@ -76,6 +74,10 @@ class RemoteBitrateEstimator { // currently being received and of which the bitrate estimate is based upon. virtual bool LatestEstimate(std::vector* ssrcs, unsigned int* bitrate_bps) const = 0; + + protected: + static const int kProcessIntervalMs = 1000; + static const int kStreamTimeOutMs = 2000; }; } // namespace webrtc diff --git a/webrtc/modules/remote_bitrate_estimator/overuse_detector.cc b/webrtc/modules/remote_bitrate_estimator/overuse_detector.cc index dfd6b906a..0fbf6f15f 100644 --- a/webrtc/modules/remote_bitrate_estimator/overuse_detector.cc +++ b/webrtc/modules/remote_bitrate_estimator/overuse_detector.cc @@ -43,7 +43,8 @@ OveruseDetector::OveruseDetector(const OverUseDetectorOptions& options) prev_offset_(0.0), time_over_using_(-1), over_use_counter_(0), - hypothesis_(kBwNormal) + hypothesis_(kBwNormal), + time_of_last_received_packet_(-1) #ifdef WEBRTC_BWE_MATLAB , plots_() #endif @@ -80,6 +81,7 @@ void OveruseDetector::Update(uint16_t packet_size, int64_t timestamp_ms, uint32_t timestamp, const int64_t now_ms) { + time_of_last_received_packet_ = now_ms; #ifdef WEBRTC_BWE_MATLAB // Create plots const int64_t startTimeMs = nowMS; @@ -166,6 +168,10 @@ void OveruseDetector::SetRateControlRegion(RateControlRegion region) { } } +int64_t OveruseDetector::time_of_last_received_packet() const { + return time_of_last_received_packet_; +} + void OveruseDetector::SwitchTimeBase() { current_frame_.size = 0; current_frame_.complete_time_ms = -1; diff --git a/webrtc/modules/remote_bitrate_estimator/overuse_detector.h b/webrtc/modules/remote_bitrate_estimator/overuse_detector.h index 67b998dd8..a8a038cc8 100644 --- a/webrtc/modules/remote_bitrate_estimator/overuse_detector.h +++ b/webrtc/modules/remote_bitrate_estimator/overuse_detector.h @@ -23,6 +23,8 @@ namespace webrtc { enum RateControlRegion; +// This class is assumed to be protected by the owner if used by multiple +// threads. class OveruseDetector { public: explicit OveruseDetector(const OverUseDetectorOptions& options); @@ -34,6 +36,7 @@ class OveruseDetector { BandwidthUsage State() const; double NoiseVar() const; void SetRateControlRegion(RateControlRegion region); + int64_t time_of_last_received_packet() const; private: struct FrameSample { @@ -100,6 +103,7 @@ class OveruseDetector { double time_over_using_; uint16_t over_use_counter_; BandwidthUsage hypothesis_; + int64_t time_of_last_received_packet_; #ifdef WEBRTC_BWE_MATLAB DebugPlots plots_; #endif diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.cc index 710c41203..a69a899d6 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.cc @@ -8,38 +8,43 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.h" +#include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.h" -#include "modules/remote_bitrate_estimator/include/rtp_to_ntp.h" -#include "modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" -#include "system_wrappers/interface/tick_util.h" +#include "webrtc/modules/remote_bitrate_estimator/include/rtp_to_ntp.h" +#include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" +#include "webrtc/system_wrappers/interface/clock.h" +#include "webrtc/system_wrappers/interface/tick_util.h" namespace webrtc { RemoteBitrateEstimator* RemoteBitrateEstimator::Create( - RemoteBitrateObserver* observer, const OverUseDetectorOptions& options, - EstimationMode mode) { + EstimationMode mode, + RemoteBitrateObserver* observer, + Clock* clock) { switch (mode) { case kMultiStreamEstimation: - return new RemoteBitrateEstimatorMultiStream(observer, options); + return new RemoteBitrateEstimatorMultiStream(options, observer, clock); case kSingleStreamEstimation: - return new RemoteBitrateEstimatorSingleStream(observer, options); + return new RemoteBitrateEstimatorSingleStream(options, observer, clock); } return NULL; } RemoteBitrateEstimatorMultiStream::RemoteBitrateEstimatorMultiStream( + const OverUseDetectorOptions& options, RemoteBitrateObserver* observer, - const OverUseDetectorOptions& options) - : remote_rate_(), + Clock* clock) + : clock_(clock), + remote_rate_(), overuse_detector_(options), incoming_bitrate_(), observer_(observer), streams_(), crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), initial_ssrc_(0), - multi_stream_(false) { + multi_stream_(false), + last_process_time_(-1) { assert(observer_); } @@ -108,16 +113,45 @@ void RemoteBitrateEstimatorMultiStream::IncomingPacket(unsigned int ssrc, } overuse_detector_.Update(payload_size, timestamp_in_ms, rtp_timestamp, arrival_time); - if (prior_state != kBwOverusing && - overuse_detector_.State() == kBwOverusing) { - // The first overuse should immediately trigger a new estimate. - UpdateEstimate(1, arrival_time); + if (overuse_detector_.State() == kBwOverusing) { + unsigned int incoming_bitrate = incoming_bitrate_.BitRate(arrival_time); + if (prior_state != kBwOverusing || + remote_rate_.TimeToReduceFurther(arrival_time, incoming_bitrate)) { + // The first overuse should immediately trigger a new estimate. + // We also have to update the estimate immediately if we are overusing + // and the target bitrate is too high compared to what we are receiving. + UpdateEstimate(arrival_time); + } } } -void RemoteBitrateEstimatorMultiStream::UpdateEstimate(unsigned int ssrc, - int64_t time_now) { +int32_t RemoteBitrateEstimatorMultiStream::Process() { + if (TimeUntilNextProcess() > 0) { + return 0; + } + UpdateEstimate(clock_->TimeInMilliseconds()); + last_process_time_ = clock_->TimeInMilliseconds(); + return 0; +} + +int32_t RemoteBitrateEstimatorMultiStream::TimeUntilNextProcess() { + if (last_process_time_ < 0) { + return 0; + } + return last_process_time_ + kProcessIntervalMs - clock_->TimeInMilliseconds(); +} + +void RemoteBitrateEstimatorMultiStream::UpdateEstimate(int64_t time_now) { CriticalSectionScoped cs(crit_sect_.get()); + const int64_t time_of_last_received_packet = + overuse_detector_.time_of_last_received_packet(); + if (time_of_last_received_packet >= 0 && + time_now - time_of_last_received_packet > kStreamTimeOutMs) { + // This over-use detector hasn't received packets for |kStreamTimeOutMs| + // milliseconds and is considered stale. + remote_rate_.Reset(); + return; + } const RateControlInput input(overuse_detector_.State(), incoming_bitrate_.BitRate(time_now), overuse_detector_.NoiseVar()); @@ -133,7 +167,7 @@ void RemoteBitrateEstimatorMultiStream::UpdateEstimate(unsigned int ssrc, overuse_detector_.SetRateControlRegion(region); } -void RemoteBitrateEstimatorMultiStream::SetRtt(unsigned int rtt) { +void RemoteBitrateEstimatorMultiStream::OnRttUpdate(uint32_t rtt) { CriticalSectionScoped cs(crit_sect_.get()); remote_rate_.SetRtt(rtt); } diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.h b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.h index 088ef01d1..713b961b5 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.h +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.h @@ -27,10 +27,13 @@ namespace webrtc { +class Clock; + class RemoteBitrateEstimatorMultiStream : public RemoteBitrateEstimator { public: - RemoteBitrateEstimatorMultiStream(RemoteBitrateObserver* observer, - const OverUseDetectorOptions& options); + RemoteBitrateEstimatorMultiStream(const OverUseDetectorOptions& options, + RemoteBitrateObserver* observer, + Clock* clock); ~RemoteBitrateEstimatorMultiStream() {} @@ -52,11 +55,12 @@ class RemoteBitrateEstimatorMultiStream : public RemoteBitrateEstimator { uint32_t rtp_timestamp); // Triggers a new estimate calculation. - void UpdateEstimate(unsigned int ssrc, int64_t time_now); - - // Set the current round-trip time experienced by the streams going into this - // estimator. - void SetRtt(unsigned int rtt); + // Implements the Module interface. + virtual int32_t Process(); + virtual int32_t TimeUntilNextProcess(); + // Set the current round-trip time experienced by the stream. + // Implements the StatsObserver interface. + virtual void OnRttUpdate(uint32_t rtt); // Removes all data for |ssrc|. void RemoveStream(unsigned int ssrc); @@ -70,8 +74,12 @@ class RemoteBitrateEstimatorMultiStream : public RemoteBitrateEstimator { private: typedef std::map StreamMap; + // Triggers a new estimate calculation. + void UpdateEstimate(int64_t time_now); + void GetSsrcs(std::vector* ssrcs) const; + Clock* clock_; RemoteRateControl remote_rate_; OveruseDetector overuse_detector_; BitRateStats incoming_bitrate_; @@ -80,6 +88,7 @@ class RemoteBitrateEstimatorMultiStream : public RemoteBitrateEstimator { scoped_ptr crit_sect_; unsigned int initial_ssrc_; bool multi_stream_; + int32_t last_process_time_; DISALLOW_COPY_AND_ASSIGN(RemoteBitrateEstimatorMultiStream); }; diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc index a9d1ece6b..6098b2f0c 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc @@ -8,17 +8,21 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" +#include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" -#include "system_wrappers/interface/tick_util.h" +#include "webrtc/system_wrappers/interface/clock.h" namespace webrtc { RemoteBitrateEstimatorSingleStream::RemoteBitrateEstimatorSingleStream( - RemoteBitrateObserver* observer, const OverUseDetectorOptions& options) + const OverUseDetectorOptions& options, + RemoteBitrateObserver* observer, + Clock* clock) : options_(options), + clock_(clock), observer_(observer), - crit_sect_(CriticalSectionWrapper::CreateCriticalSection()) { + crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), + last_process_time_(-1) { assert(observer_); } @@ -45,37 +49,80 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket( incoming_bitrate_.Update(payload_size, arrival_time); const BandwidthUsage prior_state = overuse_detector->State(); overuse_detector->Update(payload_size, -1, rtp_timestamp, arrival_time); - if (prior_state != overuse_detector->State() && - overuse_detector->State() == kBwOverusing) { - // The first overuse should immediately trigger a new estimate. - UpdateEstimate(ssrc, arrival_time); + if (overuse_detector->State() == kBwOverusing) { + unsigned int incoming_bitrate = incoming_bitrate_.BitRate(arrival_time); + if (prior_state != kBwOverusing || + remote_rate_.TimeToReduceFurther(arrival_time, incoming_bitrate)) { + // The first overuse should immediately trigger a new estimate. + // We also have to update the estimate immediately if we are overusing + // and the target bitrate is too high compared to what we are receiving. + UpdateEstimate(arrival_time); + } } } -void RemoteBitrateEstimatorSingleStream::UpdateEstimate(unsigned int ssrc, - int64_t time_now) { +int32_t RemoteBitrateEstimatorSingleStream::Process() { + if (TimeUntilNextProcess() > 0) { + return 0; + } + UpdateEstimate(clock_->TimeInMilliseconds()); + last_process_time_ = clock_->TimeInMilliseconds(); + return 0; +} + +int32_t RemoteBitrateEstimatorSingleStream::TimeUntilNextProcess() { + if (last_process_time_ < 0) { + return 0; + } + return last_process_time_ + kProcessIntervalMs - clock_->TimeInMilliseconds(); +} + +void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t time_now) { CriticalSectionScoped cs(crit_sect_.get()); - SsrcOveruseDetectorMap::iterator it = overuse_detectors_.find(ssrc); - if (it == overuse_detectors_.end()) { + BandwidthUsage bw_state = kBwNormal; + double sum_noise_var = 0.0; + SsrcOveruseDetectorMap::iterator it = overuse_detectors_.begin(); + while (it != overuse_detectors_.end()) { + const int64_t time_of_last_received_packet = + it->second.time_of_last_received_packet(); + if (time_of_last_received_packet >= 0 && + time_now - time_of_last_received_packet > kStreamTimeOutMs) { + // This over-use detector hasn't received packets for |kStreamTimeOutMs| + // milliseconds and is considered stale. + overuse_detectors_.erase(it++); + } else { + sum_noise_var += it->second.NoiseVar(); + // Make sure that we trigger an over-use if any of the over-use detectors + // is detecting over-use. + if (it->second.State() > bw_state) { + bw_state = it->second.State(); + } + ++it; + } + } + // We can't update the estimate if we don't have any active streams. + if (overuse_detectors_.empty()) { + remote_rate_.Reset(); return; } - OveruseDetector* overuse_detector = &it->second; - const RateControlInput input(overuse_detector->State(), + double mean_noise_var = sum_noise_var / + static_cast(overuse_detectors_.size()); + const RateControlInput input(bw_state, incoming_bitrate_.BitRate(time_now), - overuse_detector->NoiseVar()); + mean_noise_var); const RateControlRegion region = remote_rate_.Update(&input, time_now); unsigned int target_bitrate = remote_rate_.UpdateBandwidthEstimate(time_now); if (remote_rate_.ValidEstimate()) { std::vector ssrcs; GetSsrcs(&ssrcs); - if (!ssrcs.empty()) { - observer_->OnReceiveBitrateChanged(&ssrcs, target_bitrate); - } + observer_->OnReceiveBitrateChanged(&ssrcs, target_bitrate); + } + for (it = overuse_detectors_.begin(); it != overuse_detectors_.end(); ++it) { + it->second.SetRateControlRegion(region); } - overuse_detector->SetRateControlRegion(region); } -void RemoteBitrateEstimatorSingleStream::SetRtt(unsigned int rtt) { +void RemoteBitrateEstimatorSingleStream::OnRttUpdate(uint32_t rtt) { CriticalSectionScoped cs(crit_sect_.get()); remote_rate_.SetRtt(rtt); } diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h index 46819db4b..575ed09cd 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h @@ -15,20 +15,25 @@ #include -#include "modules/remote_bitrate_estimator/bitrate_estimator.h" -#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" -#include "modules/remote_bitrate_estimator/overuse_detector.h" -#include "modules/remote_bitrate_estimator/remote_rate_control.h" -#include "system_wrappers/interface/critical_section_wrapper.h" -#include "system_wrappers/interface/scoped_ptr.h" -#include "typedefs.h" +#include "webrtc/modules/remote_bitrate_estimator/bitrate_estimator.h" +#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" +#include "webrtc/modules/remote_bitrate_estimator/overuse_detector.h" +#include "webrtc/modules/remote_bitrate_estimator/remote_rate_control.h" +#include "webrtc/system_wrappers/interface/critical_section_wrapper.h" +#include "webrtc/system_wrappers/interface/scoped_ptr.h" +#include "webrtc/typedefs.h" namespace webrtc { +class Clock; + class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator { public: - RemoteBitrateEstimatorSingleStream(RemoteBitrateObserver* observer, - const OverUseDetectorOptions& options); + RemoteBitrateEstimatorSingleStream(const OverUseDetectorOptions& options, + RemoteBitrateObserver* observer, + Clock* clock); + + virtual ~RemoteBitrateEstimatorSingleStream() {} void IncomingRtcp(unsigned int ssrc, uint32_t ntp_secs, uint32_t ntp_frac, uint32_t rtp_timestamp) {} @@ -43,12 +48,13 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator { int64_t arrival_time, uint32_t rtp_timestamp); - // Triggers a new estimate calculation for the stream identified by |ssrc|. - void UpdateEstimate(unsigned int ssrc, int64_t time_now); - - // Set the current round-trip time experienced by the stream identified by - // |ssrc|. - void SetRtt(unsigned int ssrc); + // Triggers a new estimate calculation. + // Implements the Module interface. + virtual int32_t Process(); + virtual int32_t TimeUntilNextProcess(); + // Set the current round-trip time experienced by the stream. + // Implements the StatsObserver interface. + virtual void OnRttUpdate(uint32_t rtt); // Removes all data for |ssrc|. void RemoveStream(unsigned int ssrc); @@ -62,14 +68,19 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator { private: typedef std::map SsrcOveruseDetectorMap; + // Triggers a new estimate calculation. + void UpdateEstimate(int64_t time_now); + void GetSsrcs(std::vector* ssrcs) const; const OverUseDetectorOptions& options_; + Clock* clock_; SsrcOveruseDetectorMap overuse_detectors_; BitRateStats incoming_bitrate_; RemoteRateControl remote_rate_; RemoteBitrateObserver* observer_; scoped_ptr crit_sect_; + int64_t last_process_time_; }; } // namespace webrtc diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest.cc index 83d5142f2..e60fdde80 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest.cc @@ -23,73 +23,84 @@ namespace webrtc { TEST_F(RemoteBitrateEstimatorTest, TestInitialBehavior) { + const int kFramerate = 50; // 50 fps to avoid rounding errors. + const int kFrameIntervalMs = 1000 / kFramerate; unsigned int bitrate_bps = 0; - int64_t time_now = 0; uint32_t timestamp = 0; std::vector ssrcs; EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); EXPECT_EQ(0u, ssrcs.size()); - bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now); + clock_.AdvanceTimeMilliseconds(1000); + bitrate_estimator_->Process(); EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); EXPECT_FALSE(bitrate_observer_->updated()); bitrate_observer_->Reset(); + clock_.AdvanceTimeMilliseconds(1000); // Inserting a packet. Still no valid estimate. We need to wait 1 second. - bitrate_estimator_->IncomingPacket(kDefaultSsrc, kMtu, time_now, - timestamp); - bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now); + bitrate_estimator_->IncomingPacket(kDefaultSsrc, kMtu, + clock_.TimeInMilliseconds(), timestamp); + bitrate_estimator_->Process(); EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); EXPECT_EQ(0u, ssrcs.size()); EXPECT_FALSE(bitrate_observer_->updated()); bitrate_observer_->Reset(); - // Waiting more than one second gives us a valid estimate. - // We need at least two packets for the incoming bitrate to be > 0 since the - // window is 500 ms. - time_now += 499; - bitrate_estimator_->IncomingPacket(kDefaultSsrc, kMtu, time_now, - timestamp); - time_now += 2; - bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now); + // Inserting packets for one second to get a valid estimate. + for (int i = 0; i < kFramerate; ++i) { + bitrate_estimator_->IncomingPacket(kDefaultSsrc, kMtu, + clock_.TimeInMilliseconds(), timestamp); + clock_.AdvanceTimeMilliseconds(1000 / kFramerate); + timestamp += 90 * kFrameIntervalMs; + } + bitrate_estimator_->Process(); EXPECT_TRUE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); - EXPECT_EQ(1u, ssrcs.size()); + ASSERT_EQ(1u, ssrcs.size()); EXPECT_EQ(kDefaultSsrc, ssrcs.front()); - EXPECT_EQ(20607u, bitrate_bps); + EXPECT_EQ(498075u, bitrate_bps); EXPECT_TRUE(bitrate_observer_->updated()); bitrate_observer_->Reset(); EXPECT_EQ(bitrate_observer_->latest_bitrate(), bitrate_bps); } TEST_F(RemoteBitrateEstimatorTest, TestRateIncreaseReordering) { - int64_t time_now = 0; uint32_t timestamp = 0; - const int framerate = 50; // 50 fps to avoid rounding errors. - const int frame_interval_ms = 1000 / framerate; - bitrate_estimator_->IncomingPacket(kDefaultSsrc, 1000, time_now, timestamp); - bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now); + const int kFramerate = 50; // 50 fps to avoid rounding errors. + const int kFrameIntervalMs = 1000 / kFramerate; + bitrate_estimator_->IncomingPacket(kDefaultSsrc, 1000, + clock_.TimeInMilliseconds(), timestamp); + bitrate_estimator_->Process(); EXPECT_FALSE(bitrate_observer_->updated()); // No valid estimate. - // Increase time with 1 second to get a valid estimate. - time_now += 1000; - timestamp += 90 * 1000; - bitrate_estimator_->IncomingPacket(kDefaultSsrc, 1000, time_now, timestamp); - bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now); + // Inserting packets for one second to get a valid estimate. + for (int i = 0; i < kFramerate; ++i) { + bitrate_estimator_->IncomingPacket(kDefaultSsrc, kMtu, + clock_.TimeInMilliseconds(), timestamp); + clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); + timestamp += 90 * kFrameIntervalMs; + } + bitrate_estimator_->Process(); EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_EQ(17645u, bitrate_observer_->latest_bitrate()); + EXPECT_EQ(498136u, bitrate_observer_->latest_bitrate()); for (int i = 0; i < 10; ++i) { - time_now += 2 * frame_interval_ms; - timestamp += 2 * 90 * frame_interval_ms; - bitrate_estimator_->IncomingPacket(kDefaultSsrc, 1000, time_now, timestamp); + clock_.AdvanceTimeMilliseconds(2 * kFrameIntervalMs); + timestamp += 2 * 90 * kFrameIntervalMs; + bitrate_estimator_->IncomingPacket(kDefaultSsrc, 1000, + clock_.TimeInMilliseconds(), timestamp); bitrate_estimator_->IncomingPacket(kDefaultSsrc, 1000, - time_now - frame_interval_ms, - timestamp - 90 * frame_interval_ms); + clock_.TimeInMilliseconds() - + kFrameIntervalMs, + timestamp - 90 * kFrameIntervalMs); } - bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now); + bitrate_estimator_->Process(); EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_EQ(18985u, bitrate_observer_->latest_bitrate()); + EXPECT_EQ(498136u, bitrate_observer_->latest_bitrate()); } // Make sure we initially increase the bitrate as expected. TEST_F(RemoteBitrateEstimatorTest, TestRateIncreaseRtpTimestamps) { - const int kExpectedIterations = 276; + // This threshold corresponds approximately to increasing linearly with + // bitrate(i) = 1.04 * bitrate(i-1) + 1000 + // until bitrate(i) > 500000, with bitrate(1) ~= 30000. + const int kExpectedIterations = 1621; unsigned int bitrate_bps = 30000; int iterations = 0; AddDefaultStream(); @@ -114,32 +125,35 @@ TEST_F(RemoteBitrateEstimatorTest, TestRateIncreaseRtpTimestamps) { // Verify that the time it takes for the estimator to reduce the bitrate when // the capacity is tightened stays the same. TEST_F(RemoteBitrateEstimatorTest, TestCapacityDropRtpTimestamps) { - const int kNumberOfFrames= 300; + const int kNumberOfFrames = 300; const int kStartBitrate = 900e3; const int kMinExpectedBitrate = 800e3; const int kMaxExpectedBitrate = 1100e3; AddDefaultStream(); // Run in steady state to make the estimator converge. + unsigned int capacity_bps = 1000e3; stream_generator_->set_capacity_bps(1000e3); unsigned int bitrate_bps = SteadyStateRun(kDefaultSsrc, kNumberOfFrames, kStartBitrate, kMinExpectedBitrate, - kMaxExpectedBitrate); + kMaxExpectedBitrate, capacity_bps); // Reduce the capacity and verify the decrease time. - stream_generator_->set_capacity_bps(500e3); + capacity_bps = 500e3; + stream_generator_->set_capacity_bps(capacity_bps); + int64_t overuse_start_time = clock_.TimeInMilliseconds(); int64_t bitrate_drop_time = -1; for (int i = 0; i < 200; ++i) { GenerateAndProcessFrame(kDefaultSsrc, bitrate_bps); // Check for either increase or decrease. if (bitrate_observer_->updated()) { if (bitrate_drop_time == -1 && - bitrate_observer_->latest_bitrate() <= 500e3) { - bitrate_drop_time = time_now_; + bitrate_observer_->latest_bitrate() <= capacity_bps) { + bitrate_drop_time = clock_.TimeInMilliseconds(); } bitrate_bps = bitrate_observer_->latest_bitrate(); bitrate_observer_->Reset(); } } - EXPECT_EQ(10333, bitrate_drop_time); + EXPECT_EQ(367, bitrate_drop_time - overuse_start_time); } // Verify that the time it takes for the estimator to reduce the bitrate when @@ -156,29 +170,33 @@ TEST_F(RemoteBitrateEstimatorTest, TestCapacityDropRtpTimestampsWrap) { stream_generator_->set_rtp_timestamp_offset(kDefaultSsrc, std::numeric_limits::max() - kSteadyStateTime * 90000); // Run in steady state to make the estimator converge. + unsigned int capacity_bps = 1000e3; stream_generator_->set_capacity_bps(1000e3); unsigned int bitrate_bps = SteadyStateRun(kDefaultSsrc, kSteadyStateTime * kFramerate, kStartBitrate, kMinExpectedBitrate, - kMaxExpectedBitrate); + kMaxExpectedBitrate, + capacity_bps); bitrate_observer_->Reset(); // Reduce the capacity and verify the decrease time. - stream_generator_->set_capacity_bps(500e3); + capacity_bps = 500e3; + stream_generator_->set_capacity_bps(capacity_bps); + int64_t overuse_start_time = clock_.TimeInMilliseconds(); int64_t bitrate_drop_time = -1; for (int i = 0; i < 200; ++i) { GenerateAndProcessFrame(kDefaultSsrc, bitrate_bps); // Check for either increase or decrease. if (bitrate_observer_->updated()) { if (bitrate_drop_time == -1 && - bitrate_observer_->latest_bitrate() <= 500e3) { - bitrate_drop_time = time_now_; + bitrate_observer_->latest_bitrate() <= capacity_bps) { + bitrate_drop_time = clock_.TimeInMilliseconds(); } bitrate_bps = bitrate_observer_->latest_bitrate(); bitrate_observer_->Reset(); } } - EXPECT_EQ(8299, bitrate_drop_time); + EXPECT_EQ(367, bitrate_drop_time - overuse_start_time); } // Verify that the time it takes for the estimator to reduce the bitrate when @@ -196,29 +214,33 @@ TEST_F(RemoteBitrateEstimatorTestAlign, TestCapacityDropRtpTimestampsWrap) { stream_generator_->set_rtp_timestamp_offset(kDefaultSsrc, std::numeric_limits::max() - kSteadyStateTime * 90000); // Run in steady state to make the estimator converge. - stream_generator_->set_capacity_bps(1000e3); + unsigned int capacity_bps = 1000e3; + stream_generator_->set_capacity_bps(capacity_bps); unsigned int bitrate_bps = SteadyStateRun(kDefaultSsrc, kSteadyStateTime * kFramerate, kStartBitrate, kMinExpectedBitrate, - kMaxExpectedBitrate); + kMaxExpectedBitrate, + capacity_bps); bitrate_observer_->Reset(); // Reduce the capacity and verify the decrease time. - stream_generator_->set_capacity_bps(500e3); + capacity_bps = 500e3; + stream_generator_->set_capacity_bps(capacity_bps); + int64_t overuse_start_time = clock_.TimeInMilliseconds(); int64_t bitrate_drop_time = -1; for (int i = 0; i < 200; ++i) { GenerateAndProcessFrame(kDefaultSsrc, bitrate_bps); // Check for either increase or decrease. if (bitrate_observer_->updated()) { if (bitrate_drop_time == -1 && - bitrate_observer_->latest_bitrate() <= 500e3) { - bitrate_drop_time = time_now_; + bitrate_observer_->latest_bitrate() <= capacity_bps) { + bitrate_drop_time = clock_.TimeInMilliseconds(); } bitrate_bps = bitrate_observer_->latest_bitrate(); bitrate_observer_->Reset(); } } - EXPECT_EQ(8299, bitrate_drop_time); + EXPECT_EQ(367, bitrate_drop_time - overuse_start_time); } // Verify that the time it takes for the estimator to reduce the bitrate when @@ -229,7 +251,7 @@ TEST_F(RemoteBitrateEstimatorTestAlign, TwoStreamsCapacityDropWithWrap) { const int kStartBitrate = 900e3; const int kMinExpectedBitrate = 800e3; const int kMaxExpectedBitrate = 1100e3; - const int kSteadyStateTime = 7; // Seconds. + const int kSteadyStateFrames = 9 * kFramerate; stream_generator_->AddStream(new testing::RtpStream( 30, // Frames per second. kStartBitrate/2, // Bitrate. @@ -247,31 +269,35 @@ TEST_F(RemoteBitrateEstimatorTestAlign, TwoStreamsCapacityDropWithWrap) { 0)); // RTCP receive time. // Trigger wrap right after the steady state run. stream_generator_->set_rtp_timestamp_offset(kDefaultSsrc, - std::numeric_limits::max() - kSteadyStateTime * 90000); + std::numeric_limits::max() - kSteadyStateFrames * 90000); // Run in steady state to make the estimator converge. - stream_generator_->set_capacity_bps(1000e3); + unsigned int capacity_bps = 1000e3; + stream_generator_->set_capacity_bps(capacity_bps); unsigned int bitrate_bps = SteadyStateRun(kDefaultSsrc, - kSteadyStateTime * kFramerate, + kSteadyStateFrames, kStartBitrate, kMinExpectedBitrate, - kMaxExpectedBitrate); + kMaxExpectedBitrate, + capacity_bps); bitrate_observer_->Reset(); // Reduce the capacity and verify the decrease time. - stream_generator_->set_capacity_bps(500e3); + capacity_bps = 500e3; + stream_generator_->set_capacity_bps(capacity_bps); + int64_t overuse_start_time = clock_.TimeInMilliseconds(); int64_t bitrate_drop_time = -1; for (int i = 0; i < 200; ++i) { GenerateAndProcessFrame(kDefaultSsrc, bitrate_bps); // Check for either increase or decrease. if (bitrate_observer_->updated()) { if (bitrate_drop_time == -1 && - bitrate_observer_->latest_bitrate() <= 500e3) { - bitrate_drop_time = time_now_; + bitrate_observer_->latest_bitrate() <= capacity_bps) { + bitrate_drop_time = clock_.TimeInMilliseconds(); } bitrate_bps = bitrate_observer_->latest_bitrate(); bitrate_observer_->Reset(); } } - EXPECT_EQ(4933, bitrate_drop_time); + EXPECT_EQ(567, bitrate_drop_time - overuse_start_time); } // Verify that the time it takes for the estimator to reduce the bitrate when @@ -282,9 +308,9 @@ TEST_F(RemoteBitrateEstimatorTestAlign, ThreeStreams) { const int kStartBitrate = 900e3; const int kMinExpectedBitrate = 800e3; const int kMaxExpectedBitrate = 1100e3; - const int kSteadyStateTime = 11; // Seconds. + const int kSteadyStateFrames = 12 * kFramerate; stream_generator_->AddStream(new testing::RtpStream( - 30, // Frames per second. + kFramerate, // Frames per second. kStartBitrate/2, // Bitrate. 1, // SSRC. 90000, // RTP frequency. @@ -292,7 +318,7 @@ TEST_F(RemoteBitrateEstimatorTestAlign, ThreeStreams) { 0)); // RTCP receive time. stream_generator_->AddStream(new testing::RtpStream( - 30, // Frames per second. + kFramerate, // Frames per second. kStartBitrate/3, // Bitrate. 2, // SSRC. 90000, // RTP frequency. @@ -300,7 +326,7 @@ TEST_F(RemoteBitrateEstimatorTestAlign, ThreeStreams) { 0)); // RTCP receive time. stream_generator_->AddStream(new testing::RtpStream( - 30, // Frames per second. + kFramerate, // Frames per second. kStartBitrate/6, // Bitrate. 3, // SSRC. 90000, // RTP frequency. @@ -308,31 +334,35 @@ TEST_F(RemoteBitrateEstimatorTestAlign, ThreeStreams) { 0)); // RTCP receive time. // Trigger wrap right after the steady state run. stream_generator_->set_rtp_timestamp_offset(kDefaultSsrc, - std::numeric_limits::max() - kSteadyStateTime * 90000); + std::numeric_limits::max() - kSteadyStateFrames * 90000); // Run in steady state to make the estimator converge. - stream_generator_->set_capacity_bps(1000e3); + unsigned int capacity_bps = 1000e3; + stream_generator_->set_capacity_bps(capacity_bps); unsigned int bitrate_bps = SteadyStateRun(kDefaultSsrc, - kSteadyStateTime * kFramerate, + kSteadyStateFrames, kStartBitrate, kMinExpectedBitrate, - kMaxExpectedBitrate); + kMaxExpectedBitrate, + capacity_bps); bitrate_observer_->Reset(); // Reduce the capacity and verify the decrease time. - stream_generator_->set_capacity_bps(500e3); + capacity_bps = 500e3; + stream_generator_->set_capacity_bps(capacity_bps); + int64_t overuse_start_time = clock_.TimeInMilliseconds(); int64_t bitrate_drop_time = -1; for (int i = 0; i < 200; ++i) { GenerateAndProcessFrame(kDefaultSsrc, bitrate_bps); // Check for either increase or decrease. if (bitrate_observer_->updated()) { if (bitrate_drop_time == -1 && - bitrate_observer_->latest_bitrate() <= 500e3) { - bitrate_drop_time = time_now_; + bitrate_observer_->latest_bitrate() <= capacity_bps) { + bitrate_drop_time = clock_.TimeInMilliseconds(); } bitrate_bps = bitrate_observer_->latest_bitrate(); bitrate_observer_->Reset(); } } - EXPECT_EQ(3966, bitrate_drop_time); + EXPECT_EQ(433, bitrate_drop_time - overuse_start_time); } } // namespace webrtc diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc index 23fc8cf59..7aa68f0ce 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc @@ -39,25 +39,25 @@ void RtpStream::set_rtp_timestamp_offset(uint32_t offset) { // Generates a new frame for this stream. If called too soon after the // previous frame, no frame will be generated. The frame is split into // packets. -int64_t RtpStream::GenerateFrame(double time_now, PacketList* packets) { - if (time_now < next_rtp_time_) { +int64_t RtpStream::GenerateFrame(int64_t time_now_us, PacketList* packets) { + if (time_now_us < next_rtp_time_) { return next_rtp_time_; } assert(packets != NULL); int bits_per_frame = (bitrate_bps_ + fps_ / 2) / fps_; - int n_packets = std::max((bits_per_frame + 8 * kMtu) / (8 * kMtu), 1); + int n_packets = std::max((bits_per_frame + 4 * kMtu) / (8 * kMtu), 1); int packet_size = (bits_per_frame + 4 * n_packets) / (8 * n_packets); assert(n_packets >= 0); for (int i = 0; i < n_packets; ++i) { RtpPacket* packet = new RtpPacket; - packet->send_time = time_now + kSendSideOffsetMs + 0.5f; + packet->send_time = time_now_us + kSendSideOffsetUs; packet->size = packet_size; packet->rtp_timestamp = rtp_timestamp_offset_ + static_cast( - (frequency_ / 1000.0) * packet->send_time + 0.5); + ((frequency_ / 1000) * packet->send_time + 500) / 1000); packet->ssrc = ssrc_; packets->push_back(packet); } - next_rtp_time_ = time_now + 1000.0 / static_cast(fps_); + next_rtp_time_ = time_now_us + (1000000 + fps_ / 2) / fps_; return next_rtp_time_; } @@ -67,18 +67,18 @@ double RtpStream::next_rtp_time() const { } // Generates an RTCP packet. -RtpStream::RtcpPacket* RtpStream::Rtcp(double time_now) { - if (time_now < next_rtcp_time_) { +RtpStream::RtcpPacket* RtpStream::Rtcp(int64_t time_now_us) { + if (time_now_us < next_rtcp_time_) { return NULL; } RtcpPacket* rtcp = new RtcpPacket; - int64_t send_time = RtpStream::kSendSideOffsetMs + time_now + 0.5; + int64_t send_time_us = RtpStream::kSendSideOffsetUs + time_now_us; rtcp->timestamp = rtp_timestamp_offset_ + static_cast( - (frequency_ / 1000.0) * send_time + 0.5); - rtcp->ntp_secs = send_time / 1000; - rtcp->ntp_frac = (send_time % 1000) * kNtpFracPerMs; + ((frequency_ / 1000) * send_time_us + 500) / 1000); + rtcp->ntp_secs = send_time_us / 1000000; + rtcp->ntp_frac = (send_time_us % 1000000) * kNtpFracPerMs; rtcp->ssrc = ssrc_; - next_rtcp_time_ = time_now + kRtcpIntervalMs; + next_rtcp_time_ = time_now_us + kRtcpIntervalUs; return rtcp; } @@ -102,7 +102,7 @@ bool RtpStream::Compare(const std::pair& left, StreamGenerator::StreamGenerator(int capacity, double time_now) : capacity_(capacity), - prev_arrival_time_(time_now) {} + prev_arrival_time_us_(time_now) {} StreamGenerator::~StreamGenerator() { for (StreamMap::iterator it = streams_.begin(); it != streams_.end(); @@ -150,30 +150,33 @@ void StreamGenerator::set_rtp_timestamp_offset(unsigned int ssrc, // TODO(holmer): Break out the channel simulation part from this class to make // it possible to simulate different types of channels. -double StreamGenerator::GenerateFrame(RtpStream::PacketList* packets, - double time_now) { +int64_t StreamGenerator::GenerateFrame(RtpStream::PacketList* packets, + int64_t time_now_us) { assert(packets != NULL); assert(packets->empty()); assert(capacity_ > 0); StreamMap::iterator it = std::min_element(streams_.begin(), streams_.end(), RtpStream::Compare); - (*it).second->GenerateFrame(time_now, packets); + (*it).second->GenerateFrame(time_now_us, packets); + int i = 0; for (RtpStream::PacketList::iterator packet_it = packets->begin(); packet_it != packets->end(); ++packet_it) { - int required_network_time = - (8 * 1000 * (*packet_it)->size + capacity_ / 2) / capacity_; - prev_arrival_time_ = std::max(time_now + required_network_time, - prev_arrival_time_ + required_network_time); - (*packet_it)->arrival_time = prev_arrival_time_ + 0.5; + int capacity_bpus = capacity_ / 1000; + int64_t required_network_time_us = + (8 * 1000 * (*packet_it)->size + capacity_bpus / 2) / capacity_bpus; + prev_arrival_time_us_ = std::max(time_now_us + required_network_time_us, + prev_arrival_time_us_ + required_network_time_us); + (*packet_it)->arrival_time = prev_arrival_time_us_; + ++i; } it = std::min_element(streams_.begin(), streams_.end(), RtpStream::Compare); return (*it).second->next_rtp_time(); } -void StreamGenerator::Rtcps(RtcpList* rtcps, double time_now) const { +void StreamGenerator::Rtcps(RtcpList* rtcps, int64_t time_now_us) const { for (StreamMap::const_iterator it = streams_.begin(); it != streams_.end(); ++it) { - RtpStream::RtcpPacket* rtcp = it->second->Rtcp(time_now); + RtpStream::RtcpPacket* rtcp = it->second->Rtcp(time_now_us); if (rtcp) { rtcps->push_front(rtcp); } @@ -182,22 +185,24 @@ void StreamGenerator::Rtcps(RtcpList* rtcps, double time_now) const { } // namespace testing RemoteBitrateEstimatorTest::RemoteBitrateEstimatorTest() - : time_now_(0.0), + : clock_(0), align_streams_(false) {} RemoteBitrateEstimatorTest::RemoteBitrateEstimatorTest(bool align_streams) - : time_now_(0.0), + : clock_(0), align_streams_(align_streams) {} void RemoteBitrateEstimatorTest::SetUp() { bitrate_observer_.reset(new testing::TestBitrateObserver); bitrate_estimator_.reset( RemoteBitrateEstimator::Create( + overuse_detector_options_, + RemoteBitrateEstimator::kSingleStreamEstimation, bitrate_observer_.get(), - over_use_detector_options_, - RemoteBitrateEstimator::kSingleStreamEstimation)); - stream_generator_.reset(new testing::StreamGenerator(1e6, // Capacity. - time_now_)); + &clock_)); + stream_generator_.reset(new testing::StreamGenerator( + 1e6, // Capacity. + clock_.TimeInMicroseconds())); } void RemoteBitrateEstimatorTest::AddDefaultStream() { @@ -220,15 +225,14 @@ bool RemoteBitrateEstimatorTest::GenerateAndProcessFrame(unsigned int ssrc, unsigned int bitrate_bps) { stream_generator_->set_bitrate_bps(bitrate_bps); testing::RtpStream::PacketList packets; - time_now_ = stream_generator_->GenerateFrame(&packets, time_now_); - int64_t last_arrival_time = -1; - bool prev_was_decrease = false; + int64_t next_time_us = stream_generator_->GenerateFrame( + &packets, clock_.TimeInMicroseconds()); bool overuse = false; while (!packets.empty()) { testing::RtpStream::RtpPacket* packet = packets.front(); if (align_streams_) { testing::StreamGenerator::RtcpList rtcps; - stream_generator_->Rtcps(&rtcps, time_now_); + stream_generator_->Rtcps(&rtcps, clock_.TimeInMicroseconds()); for (testing::StreamGenerator::RtcpList::iterator it = rtcps.begin(); it != rtcps.end(); ++it) { bitrate_estimator_->IncomingRtcp((*it)->ssrc, @@ -241,40 +245,39 @@ bool RemoteBitrateEstimatorTest::GenerateAndProcessFrame(unsigned int ssrc, bitrate_observer_->Reset(); bitrate_estimator_->IncomingPacket(packet->ssrc, packet->size, - packet->arrival_time, + (packet->arrival_time + 500) / 1000, packet->rtp_timestamp); if (bitrate_observer_->updated()) { // Verify that new estimates only are triggered by an overuse and a // rate decrease. overuse = true; EXPECT_LE(bitrate_observer_->latest_bitrate(), bitrate_bps); - EXPECT_FALSE(prev_was_decrease); - prev_was_decrease = true; - } else { - prev_was_decrease = false; } - last_arrival_time = packet->arrival_time; + clock_.AdvanceTimeMicroseconds(packet->arrival_time - + clock_.TimeInMicroseconds()); delete packet; packets.pop_front(); } - EXPECT_GT(last_arrival_time, -1); - bitrate_estimator_->UpdateEstimate(ssrc, last_arrival_time); + bitrate_estimator_->Process(); + clock_.AdvanceTimeMicroseconds(next_time_us - clock_.TimeInMicroseconds()); return overuse; } -// Run the bandwidth estimator with a stream of |number_of_frames| frames. +// Run the bandwidth estimator with a stream of |number_of_frames| frames, or +// until it reaches |target_bitrate|. // Can for instance be used to run the estimator for some time to get it // into a steady state. unsigned int RemoteBitrateEstimatorTest::SteadyStateRun( unsigned int ssrc, - int number_of_frames, + int max_number_of_frames, unsigned int start_bitrate, unsigned int min_bitrate, - unsigned int max_bitrate) { + unsigned int max_bitrate, + unsigned int target_bitrate) { unsigned int bitrate_bps = start_bitrate; bool bitrate_update_seen = false; // Produce |number_of_frames| frames and give them to the estimator. - for (int i = 0; i < number_of_frames; ++i) { + for (int i = 0; i < max_number_of_frames; ++i) { bool overuse = GenerateAndProcessFrame(ssrc, bitrate_bps); if (overuse) { EXPECT_LT(bitrate_observer_->latest_bitrate(), max_bitrate); @@ -285,6 +288,9 @@ unsigned int RemoteBitrateEstimatorTest::SteadyStateRun( bitrate_bps = bitrate_observer_->latest_bitrate(); bitrate_observer_->Reset(); } + if (bitrate_update_seen && bitrate_bps > target_bitrate) { + break; + } } EXPECT_TRUE(bitrate_update_seen); return bitrate_bps; diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h index 0eb4b224c..34e4fab53 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h @@ -17,9 +17,10 @@ #include #include -#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" -#include "system_wrappers/interface/constructor_magic.h" -#include "system_wrappers/interface/scoped_ptr.h" +#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" +#include "webrtc/system_wrappers/interface/clock.h" +#include "webrtc/system_wrappers/interface/constructor_magic.h" +#include "webrtc/system_wrappers/interface/scoped_ptr.h" namespace webrtc { @@ -74,7 +75,7 @@ class RtpStream { typedef std::list PacketList; - enum { kSendSideOffsetMs = 1000 }; + enum { kSendSideOffsetUs = 1000000 }; RtpStream(int fps, int bitrate_bps, unsigned int ssrc, unsigned int frequency, uint32_t timestamp_offset, int64_t rtcp_receive_time); @@ -83,13 +84,13 @@ class RtpStream { // Generates a new frame for this stream. If called too soon after the // previous frame, no frame will be generated. The frame is split into // packets. - int64_t GenerateFrame(double time_now, PacketList* packets); + int64_t GenerateFrame(int64_t time_now_us, PacketList* packets); // The send-side time when the next frame can be generated. double next_rtp_time() const; // Generates an RTCP packet. - RtcpPacket* Rtcp(double time_now); + RtcpPacket* Rtcp(int64_t time_now_us); void set_bitrate_bps(int bitrate_bps); @@ -101,14 +102,14 @@ class RtpStream { const std::pair& right); private: - enum { kRtcpIntervalMs = 1000 }; + enum { kRtcpIntervalUs = 1000000 }; int fps_; int bitrate_bps_; unsigned int ssrc_; unsigned int frequency_; - double next_rtp_time_; - double next_rtcp_time_; + int64_t next_rtp_time_; + int64_t next_rtcp_time_; uint32_t rtp_timestamp_offset_; const double kNtpFracPerMs; @@ -138,9 +139,9 @@ class StreamGenerator { // TODO(holmer): Break out the channel simulation part from this class to make // it possible to simulate different types of channels. - double GenerateFrame(RtpStream::PacketList* packets, double time_now); + int64_t GenerateFrame(RtpStream::PacketList* packets, int64_t time_now_us); - void Rtcps(RtcpList* rtcps, double time_now) const; + void Rtcps(RtcpList* rtcps, int64_t time_now_us) const; private: typedef std::map StreamMap; @@ -148,7 +149,7 @@ class StreamGenerator { // Capacity of the simulated channel in bits per second. int capacity_; // The time when the last packet arrived. - double prev_arrival_time_; + int64_t prev_arrival_time_us_; // All streams being transmitted on this simulated channel. StreamMap streams_; @@ -174,19 +175,21 @@ class RemoteBitrateEstimatorTest : public ::testing::Test { // target bitrate after the call to this function. bool GenerateAndProcessFrame(unsigned int ssrc, unsigned int bitrate_bps); - // Run the bandwidth estimator with a stream of |number_of_frames| frames. + // Run the bandwidth estimator with a stream of |number_of_frames| frames, or + // until it reaches |target_bitrate|. // Can for instance be used to run the estimator for some time to get it // into a steady state. unsigned int SteadyStateRun(unsigned int ssrc, int number_of_frames, unsigned int start_bitrate, unsigned int min_bitrate, - unsigned int max_bitrate); + unsigned int max_bitrate, + unsigned int target_bitrate); enum { kDefaultSsrc = 1 }; - double time_now_; // Current time at the receiver. - OverUseDetectorOptions over_use_detector_options_; + SimulatedClock clock_; // Time at the receiver. + OverUseDetectorOptions overuse_detector_options_; scoped_ptr bitrate_estimator_; scoped_ptr bitrate_observer_; scoped_ptr stream_generator_; diff --git a/webrtc/modules/remote_bitrate_estimator/remote_rate_control.cc b/webrtc/modules/remote_bitrate_estimator/remote_rate_control.cc index ae48cc239..eb293bb3b 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_rate_control.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_rate_control.cc @@ -24,6 +24,9 @@ extern MatlabEngine eng; // global variable defined elsewhere #endif namespace webrtc { + +const unsigned int kDefaultRttMs = 200; + RemoteRateControl::RemoteRateControl() : _minConfiguredBitRate(30000), @@ -43,7 +46,7 @@ _initializedBitRate(false), _avgChangePeriod(1000.0f), _lastChangeMs(-1), _beta(0.9f), -_rtt(0) +_rtt(kDefaultRttMs) #ifdef MATLAB ,_plot1(NULL), _plot2(NULL) @@ -86,6 +89,20 @@ bool RemoteRateControl::ValidEstimate() const { return _initializedBitRate; } +bool RemoteRateControl::TimeToReduceFurther( + int64_t time_now, unsigned int incoming_bitrate) const { + const int bitrate_reduction_interval = BWE_MAX(BWE_MIN(_rtt, 200), 10); + if (time_now - _lastBitRateChange >= bitrate_reduction_interval) { + return true; + } + if (ValidEstimate()) { + const int threshold = static_cast(1.05 * incoming_bitrate); + const int bitrate_difference = LatestEstimate() - incoming_bitrate; + return bitrate_difference > threshold; + } + return false; +} + WebRtc_Word32 RemoteRateControl::SetConfiguredBitRates( WebRtc_UWord32 minBitRateBps, WebRtc_UWord32 maxBitRateBps) { diff --git a/webrtc/modules/remote_bitrate_estimator/remote_rate_control.h b/webrtc/modules/remote_bitrate_estimator/remote_rate_control.h index 6c9d11658..51391988b 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_rate_control.h +++ b/webrtc/modules/remote_bitrate_estimator/remote_rate_control.h @@ -36,6 +36,12 @@ public: // Returns true if there is a valid estimate of the incoming bitrate, false // otherwise. bool ValidEstimate() const; + // Returns true if the bitrate estimate hasn't been changed for more than + // an RTT, or if the incoming_bitrate is more than 5% above the current + // estimate. Should be used to decide if we should reduce the rate further + // when over-using. + bool TimeToReduceFurther(int64_t time_now, + unsigned int incoming_bitrate) const; private: WebRtc_UWord32 ChangeBitRate(WebRtc_UWord32 currentBitRate, diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h index 540d9ef69..3a0105d15 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -462,12 +462,6 @@ class RtpRtcp : public Module { WebRtc_UWord32* fecRate, WebRtc_UWord32* nackRate) const = 0; - /* - * Get the receive-side estimate of the available bandwidth. - */ - virtual int EstimatedReceiveBandwidth( - WebRtc_UWord32* available_bandwidth) const = 0; - /* * Used by the codec module to deliver a video or audio frame for * packetization. diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc index 0438510c8..5340ed2c8 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc @@ -60,11 +60,13 @@ class RtcpFormatRembTest : public ::testing::Test { protected: RtcpFormatRembTest() : over_use_detector_options_(), + system_clock_(Clock::GetRealTimeClock()), remote_bitrate_observer_(), remote_bitrate_estimator_(RemoteBitrateEstimator::Create( - &remote_bitrate_observer_, over_use_detector_options_, - RemoteBitrateEstimator::kMultiStreamEstimation)) {} + RemoteBitrateEstimator::kMultiStreamEstimation, + &remote_bitrate_observer_, + system_clock_)) {} virtual void SetUp(); virtual void TearDown(); @@ -79,7 +81,6 @@ class RtcpFormatRembTest : public ::testing::Test { }; void RtcpFormatRembTest::SetUp() { - system_clock_ = Clock::GetRealTimeClock(); RtpRtcp::Configuration configuration; configuration.id = 0; configuration.audio = false; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index e30043983..d05dd2d5c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -113,6 +113,19 @@ RTCPReceiver::LastReceived() return _lastReceived; } +WebRtc_Word64 +RTCPReceiver::LastReceivedReceiverReport() const { + CriticalSectionScoped lock(_criticalSectionRTCPReceiver); + WebRtc_Word64 last_received_rr = -1; + for (ReceivedInfoMap::const_iterator it = _receivedInfoMap.begin(); + it != _receivedInfoMap.end(); ++it) { + if (it->second->lastTimeReceived > last_received_rr) { + last_received_rr = it->second->lastTimeReceived; + } + } + return last_received_rr; +} + WebRtc_Word32 RTCPReceiver::SetRemoteSSRC( const WebRtc_UWord32 ssrc) { diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index 08ff37ba9..befe2dfd8 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h @@ -37,6 +37,7 @@ public: WebRtc_Word32 SetRTCPStatus(const RTCPMethod method); WebRtc_Word64 LastReceived(); + WebRtc_Word64 LastReceivedReceiverReport() const; void SetSSRC( const WebRtc_UWord32 ssrc); void SetRelaySSRC( const WebRtc_UWord32 ssrc); @@ -197,6 +198,8 @@ protected: RTCPHelp::RTCPPacketInformation& rtcpPacketInformation); private: + typedef std::map + ReceivedInfoMap; WebRtc_Word32 _id; Clock* _clock; RTCPMethod _method; @@ -221,8 +224,7 @@ protected: // Received report blocks. std::map _receivedReportBlockMap; - std::map - _receivedInfoMap; + ReceivedInfoMap _receivedInfoMap; std::map _receivedCnameMap; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 783b13a34..11d158acc 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -172,9 +172,10 @@ class RtcpReceiverTest : public ::testing::Test { remote_bitrate_observer_(), remote_bitrate_estimator_( RemoteBitrateEstimator::Create( - &remote_bitrate_observer_, over_use_detector_options_, - RemoteBitrateEstimator::kMultiStreamEstimation)) { + RemoteBitrateEstimator::kMultiStreamEstimation, + &remote_bitrate_observer_, + &system_clock_)) { test_transport_ = new TestTransport(); RtpRtcp::Configuration configuration; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 7bd293123..e3cadbd5f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -99,13 +99,14 @@ class RtcpSenderTest : public ::testing::Test { protected: RtcpSenderTest() : over_use_detector_options_(), + system_clock_(Clock::GetRealTimeClock()), remote_bitrate_observer_(), remote_bitrate_estimator_( RemoteBitrateEstimator::Create( - &remote_bitrate_observer_, over_use_detector_options_, - RemoteBitrateEstimator::kMultiStreamEstimation)) { - system_clock_ = Clock::GetRealTimeClock(); + RemoteBitrateEstimator::kMultiStreamEstimation, + &remote_bitrate_observer_, + system_clock_)) { test_transport_ = new TestTransport(); RtpRtcp::Configuration configuration; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h index 066b3c26d..e128a10ff 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h @@ -15,7 +15,8 @@ namespace webrtc { enum { kRtpRtcpMaxIdleTimeProcess = 5, kRtpRtcpBitrateProcessTimeMs = 10, - kRtpRtcpPacketTimeoutProcessTimeMs = 100 }; + kRtpRtcpPacketTimeoutProcessTimeMs = 100, + kRtpRtcpRttProcessTimeMs = 1000 }; enum { NACK_PACKETS_MAX_SIZE = 256 }; // in packets enum { NACK_BYTECOUNT_SIZE = 60}; // size of our NACK history diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 2cc17a3cc..3cb799482 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -11,7 +11,6 @@ #include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h" #include - #include #include "webrtc/common_types.h" @@ -39,8 +38,6 @@ const float kFracMs = 4.294967296E6f; namespace webrtc { -const WebRtc_UWord16 kDefaultRtt = 200; - static RtpData* NullObjectRtpData() { static NullRtpData null_rtp_data; return &null_rtp_data; @@ -107,6 +104,7 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) last_bitrate_process_time_(configuration.clock->TimeInMilliseconds()), last_packet_timeout_process_time_( configuration.clock->TimeInMilliseconds()), + last_rtt_process_time_(configuration.clock->TimeInMilliseconds()), packet_overhead_(28), // IPV4 UDP. critical_section_module_ptrs_( CriticalSectionWrapper::CreateCriticalSection()), @@ -258,33 +256,30 @@ WebRtc_Word32 ModuleRtpRtcpImpl::Process() { ProcessDeadOrAliveTimer(); const bool default_instance(child_modules_.empty() ? false : true); - if (!default_instance && rtcp_sender_.TimeToSendRTCPReport()) { - WebRtc_UWord16 max_rtt = 0; + if (!default_instance) { if (rtcp_sender_.Sending()) { - std::vector receive_blocks; - rtcp_receiver_.StatisticsReceived(&receive_blocks); - for (std::vector::iterator it = receive_blocks.begin(); - it != receive_blocks.end(); ++it) { - WebRtc_UWord16 rtt = 0; - rtcp_receiver_.RTT(it->remoteSSRC, &rtt, NULL, NULL, NULL); - max_rtt = (rtt > max_rtt) ? rtt : max_rtt; + // Process RTT if we have received a receiver report and we haven't + // processed RTT for at least |kRtpRtcpRttProcessTimeMs| milliseconds. + if (rtcp_receiver_.LastReceivedReceiverReport() > + last_rtt_process_time_ && now >= last_rtt_process_time_ + + kRtpRtcpRttProcessTimeMs) { + last_rtt_process_time_ = now; + std::vector receive_blocks; + rtcp_receiver_.StatisticsReceived(&receive_blocks); + uint16_t max_rtt = 0; + for (std::vector::iterator it = receive_blocks.begin(); + it != receive_blocks.end(); ++it) { + uint16_t rtt = 0; + rtcp_receiver_.RTT(it->remoteSSRC, &rtt, NULL, NULL, NULL); + max_rtt = (rtt > max_rtt) ? rtt : max_rtt; + } + // Report the rtt. + if (rtt_observer_ && max_rtt != 0) + rtt_observer_->OnRttUpdate(max_rtt); } - // Report the rtt. - if (rtt_observer_ && max_rtt != 0) - rtt_observer_->OnRttUpdate(max_rtt); - } else { - // No valid RTT estimate, probably since this is a receive only channel. - // Use an estimate set by a send module. - max_rtt = rtcp_receiver_.RTT(); - } - if (max_rtt == 0) { - // No own rtt calculation or set rtt, use default value. - max_rtt = kDefaultRtt; - } - // Verify receiver reports are delivered and the reported sequence number is - // increasing. - if (rtcp_sender_.Sending()) { + // Verify receiver reports are delivered and the reported sequence number + // is increasing. int64_t rtcp_interval = RtcpReportInterval(); if (rtcp_receiver_.RtcpRrTimeout(rtcp_interval)) { LOG_F(LS_WARNING) << "Timeout: No RTCP RR received."; @@ -292,13 +287,8 @@ WebRtc_Word32 ModuleRtpRtcpImpl::Process() { LOG_F(LS_WARNING) << "Timeout: No increase in RTCP RR extended highest sequence number."; } - } - if (remote_bitrate_) { - // TODO(mflodman) Remove this and let this be propagated by CallStats. - remote_bitrate_->SetRtt(max_rtt); - remote_bitrate_->UpdateEstimate(rtp_receiver_->SSRC(), now); - if (TMMBR()) { + if (remote_bitrate_ && TMMBR()) { unsigned int target_bitrate = 0; std::vector ssrcs; if (remote_bitrate_->LatestEstimate(&ssrcs, &target_bitrate)) { @@ -309,7 +299,8 @@ WebRtc_Word32 ModuleRtpRtcpImpl::Process() { } } } - rtcp_sender_.SendRTCP(kRtcpReport); + if (rtcp_sender_.TimeToSendRTCPReport()) + rtcp_sender_.SendRTCP(kRtcpReport); } if (UpdateRTCPReceiveInformationTimers()) { @@ -1995,22 +1986,6 @@ void ModuleRtpRtcpImpl::BitrateSent(WebRtc_UWord32* total_rate, *nack_rate = rtp_sender_.NackOverheadRate(); } -int ModuleRtpRtcpImpl::EstimatedReceiveBandwidth( - WebRtc_UWord32* available_bandwidth) const { - if (remote_bitrate_) { - std::vector ssrcs; - if (!remote_bitrate_->LatestEstimate(&ssrcs, available_bandwidth)) { - return -1; - } - if (!ssrcs.empty()) { - *available_bandwidth /= ssrcs.size(); - } - return 0; - } - // No bandwidth receive-side bandwidth estimation is connected to this module. - return -1; -} - // Bad state of RTP receiver request a keyframe. 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 5883a1152..666612bda 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -427,9 +427,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp { WebRtc_UWord32* fec_rate, WebRtc_UWord32* nackRate) const; - virtual int EstimatedReceiveBandwidth( - WebRtc_UWord32* available_bandwidth) const; - virtual void SetRemoteSSRC(const WebRtc_UWord32 ssrc); virtual WebRtc_UWord32 SendTimeOfSendReport(const WebRtc_UWord32 send_report); @@ -493,6 +490,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp { WebRtc_Word64 last_process_time_; WebRtc_Word64 last_bitrate_process_time_; WebRtc_Word64 last_packet_timeout_process_time_; + WebRtc_Word64 last_rtt_process_time_; WebRtc_UWord16 packet_overhead_; scoped_ptr critical_section_module_ptrs_; diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc index d2ea53212..d15dcc85d 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc @@ -271,7 +271,7 @@ TEST_F(RtpRtcpRtcpTest, RTCP) { std::vector report_blocks; EXPECT_EQ(-1, module1->RemoteRTCPStat(NULL)); EXPECT_EQ(0, module1->RemoteRTCPStat(&report_blocks)); - EXPECT_EQ(1u, report_blocks.size()); + ASSERT_EQ(1u, report_blocks.size()); const RTCPReportBlock& reportBlockReceived = report_blocks[0]; float secSinceLastReport = diff --git a/webrtc/video_engine/call_stats.h b/webrtc/video_engine/call_stats.h index 14619f1e6..4a6c447b0 100644 --- a/webrtc/video_engine/call_stats.h +++ b/webrtc/video_engine/call_stats.h @@ -21,15 +21,7 @@ namespace webrtc { class CriticalSectionWrapper; class RtcpRttObserver; - -// Interface used to distribute call statistics. Callbacks will be triggered as -// soon as the class has been registered using RegisterStatsObserver. -class StatsObserver { - public: - virtual void OnRttUpdate(uint32_t rtt) = 0; - - virtual ~StatsObserver() {} -}; +class StatsObserver; // CallStats keeps track of statistics for a call. class CallStats : public Module { diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index f22a3503a..eeee2f283 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -1158,7 +1158,9 @@ void ViEChannel::GetBandwidthUsage(uint32_t* total_bitrate_sent, int ViEChannel::GetEstimatedReceiveBandwidth( uint32_t* estimated_bandwidth) const { - return rtp_rtcp_->EstimatedReceiveBandwidth(estimated_bandwidth); + if (!vie_receiver_.EstimatedReceiveBandwidth(estimated_bandwidth)) + return -1; + return 0; } WebRtc_Word32 ViEChannel::StartRTPDump(const char file_nameUTF8[1024], diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc index be8c6f855..4fa060ffd 100644 --- a/webrtc/video_engine/vie_channel_group.cc +++ b/webrtc/video_engine/vie_channel_group.cc @@ -25,18 +25,22 @@ namespace webrtc { ChannelGroup::ChannelGroup(ProcessThread* process_thread, const OverUseDetectorOptions& options, RemoteBitrateEstimator::EstimationMode mode) - : remb_(new VieRemb(process_thread)), + : remb_(new VieRemb()), bitrate_controller_(BitrateController::CreateBitrateController()), call_stats_(new CallStats()), - remote_bitrate_estimator_(RemoteBitrateEstimator::Create(remb_.get(), - options, mode)), + remote_bitrate_estimator_(RemoteBitrateEstimator::Create( + options, mode, remb_.get(), Clock::GetRealTimeClock())), encoder_state_feedback_(new EncoderStateFeedback()), process_thread_(process_thread) { + call_stats_->RegisterStatsObserver(remote_bitrate_estimator_.get()); process_thread->RegisterModule(call_stats_.get()); + process_thread->RegisterModule(remote_bitrate_estimator_.get()); } ChannelGroup::~ChannelGroup() { + call_stats_->DeregisterStatsObserver(remote_bitrate_estimator_.get()); process_thread_->DeRegisterModule(call_stats_.get()); + process_thread_->DeRegisterModule(remote_bitrate_estimator_.get()); assert(channels_.empty()); assert(!remb_->InUse()); } diff --git a/webrtc/video_engine/vie_receiver.cc b/webrtc/video_engine/vie_receiver.cc index 490731cf7..89ad655da 100644 --- a/webrtc/video_engine/vie_receiver.cc +++ b/webrtc/video_engine/vie_receiver.cc @@ -284,4 +284,18 @@ int ViEReceiver::StopRTPDump() { return 0; } +// TODO(holmer): To be moved to ViEChannelGroup. +bool ViEReceiver::EstimatedReceiveBandwidth( + unsigned int* available_bandwidth) const { + std::vector ssrcs; + if (!remote_bitrate_estimator_->LatestEstimate(&ssrcs, + available_bandwidth)) { + return false; + } + if (!ssrcs.empty()) { + *available_bandwidth /= ssrcs.size(); + } + return true; +} + } // namespace webrtc diff --git a/webrtc/video_engine/vie_receiver.h b/webrtc/video_engine/vie_receiver.h index 40510ff4e..5eecfb3c2 100644 --- a/webrtc/video_engine/vie_receiver.h +++ b/webrtc/video_engine/vie_receiver.h @@ -74,6 +74,8 @@ class ViEReceiver : public UdpTransportData, public RtpData { uint32_t ntp_frac, uint32_t timestamp); + bool EstimatedReceiveBandwidth(unsigned int* available_bandwidth) const; + private: int InsertRTPPacket(const WebRtc_Word8* rtp_packet, int rtp_packet_length); int InsertRTCPPacket(const WebRtc_Word8* rtcp_packet, int rtcp_packet_length); diff --git a/webrtc/video_engine/vie_remb.cc b/webrtc/video_engine/vie_remb.cc index ea795ac18..8cc473263 100644 --- a/webrtc/video_engine/vie_remb.cc +++ b/webrtc/video_engine/vie_remb.cc @@ -28,19 +28,13 @@ const unsigned int kRembMinimumBitrateKbps = 50; // % threshold for if we should send a new REMB asap. const unsigned int kSendThresholdPercent = 97; -VieRemb::VieRemb(ProcessThread* process_thread) - : process_thread_(process_thread), - list_crit_(CriticalSectionWrapper::CreateCriticalSection()), +VieRemb::VieRemb() + : list_crit_(CriticalSectionWrapper::CreateCriticalSection()), last_remb_time_(TickTime::MillisecondTimestamp()), last_send_bitrate_(0), - bitrate_(0), - bitrate_update_time_ms_(-1) { - process_thread->RegisterModule(this); -} + bitrate_(0) {} -VieRemb::~VieRemb() { - process_thread_->DeRegisterModule(this); -} +VieRemb::~VieRemb() {} void VieRemb::AddReceiveChannel(RtpRtcp* rtp_rtcp) { assert(rtp_rtcp); @@ -115,7 +109,7 @@ void VieRemb::OnReceiveBitrateChanged(std::vector* ssrcs, WEBRTC_TRACE(kTraceStream, kTraceVideo, -1, "VieRemb::UpdateBitrateEstimate(bitrate: %u)", bitrate); assert(ssrcs); - CriticalSectionScoped cs(list_crit_.get()); + list_crit_->Enter(); // If we already have an estimate, check if the new total estimate is below // kSendThresholdPercent of the previous estimate. if (last_send_bitrate_ > 0) { @@ -128,40 +122,23 @@ void VieRemb::OnReceiveBitrateChanged(std::vector* ssrcs, } } bitrate_ = bitrate; + // TODO(holmer): Remove |ssrcs_| from this class as the REMB is produced + // immediately upon a call to this function. ssrcs_.resize(ssrcs->size()); std::copy(ssrcs->begin(), ssrcs->end(), ssrcs_.begin()); - bitrate_update_time_ms_ = TickTime::MillisecondTimestamp(); -} - -WebRtc_Word32 VieRemb::ChangeUniqueId(const WebRtc_Word32 id) { - return 0; -} - -WebRtc_Word32 VieRemb::TimeUntilNextProcess() { - return kRembSendIntervallMs - - (TickTime::MillisecondTimestamp() - last_remb_time_); -} - -WebRtc_Word32 VieRemb::Process() { - int64_t now = TickTime::MillisecondTimestamp(); - if (now - last_remb_time_ < kRembSendIntervallMs) - return 0; - - last_remb_time_ = now; // Calculate total receive bitrate estimate. - list_crit_->Enter(); + int64_t now = TickTime::MillisecondTimestamp(); - // Reset the estimate if it has timed out. - if (TickTime::MillisecondTimestamp() - bitrate_update_time_ms_ > - kRembTimeOutThresholdMs) { - bitrate_ = 0; - bitrate_update_time_ms_ = -1; - } - if (bitrate_update_time_ms_ == -1 || ssrcs_.empty() || - receive_modules_.empty()) { + if (now - last_remb_time_ < kRembSendIntervallMs) { list_crit_->Leave(); - return 0; + return; + } + last_remb_time_ = now; + + if (ssrcs_.empty() || receive_modules_.empty()) { + list_crit_->Leave(); + return; } // Send a REMB packet. @@ -179,18 +156,17 @@ WebRtc_Word32 VieRemb::Process() { } // Copy SSRCs to avoid race conditions. int ssrcs_length = ssrcs_.size(); - unsigned int* ssrcs = new unsigned int[ssrcs_length]; + unsigned int* ssrcs_copy = new unsigned int[ssrcs_length]; for (int i = 0; i < ssrcs_length; ++i) { - ssrcs[i] = ssrcs_[i]; + ssrcs_copy[i] = ssrcs_[i]; } list_crit_->Leave(); if (sender) { // TODO(holmer): Change RTP module API to take a vector pointer. - sender->SetREMBData(bitrate_, ssrcs_length, ssrcs); + sender->SetREMBData(bitrate_, ssrcs_length, ssrcs_copy); } - delete [] ssrcs; - return 0; + delete [] ssrcs_copy; } } // namespace webrtc diff --git a/webrtc/video_engine/vie_remb.h b/webrtc/video_engine/vie_remb.h index 0ad36ca48..5365b7fd0 100644 --- a/webrtc/video_engine/vie_remb.h +++ b/webrtc/video_engine/vie_remb.h @@ -26,9 +26,9 @@ class CriticalSectionWrapper; class ProcessThread; class RtpRtcp; -class VieRemb : public RemoteBitrateObserver, public Module { +class VieRemb : public RemoteBitrateObserver { public: - explicit VieRemb(ProcessThread* process_thread); + VieRemb(); ~VieRemb(); // Called to add a receive channel to include in the REMB packet. @@ -54,15 +54,9 @@ class VieRemb : public RemoteBitrateObserver, public Module { virtual void OnReceiveBitrateChanged(std::vector* ssrcs, unsigned int bitrate); - // Implements Module. - virtual WebRtc_Word32 ChangeUniqueId(const WebRtc_Word32 id); - virtual WebRtc_Word32 TimeUntilNextProcess(); - virtual WebRtc_Word32 Process(); - private: typedef std::list RtpModules; - ProcessThread* process_thread_; scoped_ptr list_crit_; // The last time a REMB was sent. @@ -78,7 +72,6 @@ class VieRemb : public RemoteBitrateObserver, public Module { // The last bitrate update. unsigned int bitrate_; std::vector ssrcs_; - int64_t bitrate_update_time_ms_; }; } // namespace webrtc diff --git a/webrtc/video_engine/vie_remb_unittest.cc b/webrtc/video_engine/vie_remb_unittest.cc index 4cd1cc126..65ad1a07b 100644 --- a/webrtc/video_engine/vie_remb_unittest.cc +++ b/webrtc/video_engine/vie_remb_unittest.cc @@ -44,7 +44,7 @@ class ViERembTest : public ::testing::Test { virtual void SetUp() { TickTime::UseFakeClock(12345); process_thread_.reset(new TestProcessThread); - vie_remb_.reset(new VieRemb(process_thread_.get())); + vie_remb_.reset(new VieRemb()); } scoped_ptr process_thread_; scoped_ptr vie_remb_; @@ -64,13 +64,12 @@ TEST_F(ViERembTest, OneModuleTestForSendingRemb) { TickTime::AdvanceFakeClock(1000); EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, 1, _)) .Times(1); - vie_remb_->Process(); + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); // Lower bitrate to send another REMB packet. - vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate - 100); EXPECT_CALL(rtp, SetREMBData(bitrate_estimate - 100, 1, _)) .Times(1); - vie_remb_->Process(); + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate - 100); vie_remb_->RemoveReceiveChannel(&rtp); vie_remb_->RemoveRembSender(&rtp); @@ -86,11 +85,11 @@ TEST_F(ViERembTest, LowerEstimateToSendRemb) { std::vector ssrcs(&ssrc, &ssrc + 1); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - // Call process to get a first estimate. + // Call OnReceiveBitrateChanged twice to get a first estimate. TickTime::AdvanceFakeClock(1000); EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, 1, _)) .Times(1); - vie_remb_->Process(); + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); // Lower the estimate with more than 3% to trigger a call to SetREMBData right // away. @@ -98,7 +97,6 @@ TEST_F(ViERembTest, LowerEstimateToSendRemb) { EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, 1, _)) .Times(1); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - vie_remb_->Process(); } TEST_F(ViERembTest, VerifyIncreasingAndDecreasing) { @@ -114,11 +112,11 @@ TEST_F(ViERembTest, VerifyIncreasingAndDecreasing) { vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate[0]); - // Call process to get a first estimate. + // Call OnReceiveBitrateChanged twice to get a first estimate. EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate[0], 2, _)) .Times(1); TickTime::AdvanceFakeClock(1000); - vie_remb_->Process(); + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate[0]); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate[1] + 100); @@ -126,7 +124,6 @@ TEST_F(ViERembTest, VerifyIncreasingAndDecreasing) { EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate[1], 2, _)) .Times(1); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate[1]); - vie_remb_->Process(); vie_remb_->RemoveReceiveChannel(&rtp_0); vie_remb_->RemoveRembSender(&rtp_0); @@ -145,24 +142,23 @@ TEST_F(ViERembTest, NoRembForIncreasedBitrate) { std::vector ssrcs(ssrc, ssrc + sizeof(ssrc) / sizeof(ssrc[0])); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - // Trigger a first call to have a running state. + // Call OnReceiveBitrateChanged twice to get a first estimate. TickTime::AdvanceFakeClock(1000); EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate, 2, _)) .Times(1); - vie_remb_->Process(); + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); // Increased estimate shouldn't trigger a callback right away. - vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate + 1); EXPECT_CALL(rtp_0, SetREMBData(_, _, _)) .Times(0); + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate + 1); // Decreasing the estimate less than 3% shouldn't trigger a new callback. - int lower_estimate = bitrate_estimate * 98 / 100; - vie_remb_->OnReceiveBitrateChanged(&ssrcs, lower_estimate); EXPECT_CALL(rtp_0, SetREMBData(_, _, _)) .Times(0); + int lower_estimate = bitrate_estimate * 98 / 100; + vie_remb_->OnReceiveBitrateChanged(&ssrcs, lower_estimate); - vie_remb_->Process(); vie_remb_->RemoveReceiveChannel(&rtp_1); vie_remb_->RemoveReceiveChannel(&rtp_0); vie_remb_->RemoveRembSender(&rtp_0); @@ -180,18 +176,17 @@ TEST_F(ViERembTest, ChangeSendRtpModule) { std::vector ssrcs(ssrc, ssrc + sizeof(ssrc) / sizeof(ssrc[0])); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - // Call process to get a first estimate. + // Call OnReceiveBitrateChanged twice to get a first estimate. TickTime::AdvanceFakeClock(1000); EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate, 2, _)) .Times(1); - vie_remb_->Process(); + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); // Decrease estimate to trigger a REMB. bitrate_estimate = bitrate_estimate - 100; EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate, 2, _)) .Times(1); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - vie_remb_->Process(); // Remove the sending module, add it again -> should get remb on the second // module. @@ -203,7 +198,6 @@ TEST_F(ViERembTest, ChangeSendRtpModule) { EXPECT_CALL(rtp_1, SetREMBData(bitrate_estimate, 2, _)) .Times(1); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - vie_remb_->Process(); vie_remb_->RemoveReceiveChannel(&rtp_0); vie_remb_->RemoveReceiveChannel(&rtp_1); @@ -218,39 +212,22 @@ TEST_F(ViERembTest, OnlyOneRembForDoubleProcess) { vie_remb_->AddReceiveChannel(&rtp); vie_remb_->AddRembSender(&rtp); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - // Call process to get a first estimate. + // Call OnReceiveBitrateChanged twice to get a first estimate. TickTime::AdvanceFakeClock(1000); EXPECT_CALL(rtp, SetREMBData(_, _, _)) .Times(1); - vie_remb_->Process(); + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); // Lower the estimate, should trigger a call to SetREMBData right away. bitrate_estimate = bitrate_estimate - 100; EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, 1, _)) .Times(1); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - vie_remb_->Process(); - // Call Process again, this should not trigger a new callback. + // Call OnReceiveBitrateChanged again, this should not trigger a new callback. EXPECT_CALL(rtp, SetREMBData(_, _, _)) .Times(0); - vie_remb_->Process(); - vie_remb_->RemoveReceiveChannel(&rtp); - vie_remb_->RemoveRembSender(&rtp); -} - -TEST_F(ViERembTest, NoOnReceivedBitrateChangedCall) { - MockRtpRtcp rtp; - - vie_remb_->AddReceiveChannel(&rtp); - vie_remb_->AddRembSender(&rtp); - // TODO(mflodman) Add fake clock. - TickTime::AdvanceFakeClock(1000); - // No bitrate estimate given, no callback expected. - EXPECT_CALL(rtp, SetREMBData(_, _, _)) - .Times(0); - vie_remb_->Process(); - + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); vie_remb_->RemoveReceiveChannel(&rtp); vie_remb_->RemoveRembSender(&rtp); } @@ -267,18 +244,17 @@ TEST_F(ViERembTest, NoSendingRtpModule) { vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - // Call process to get a first estimate. + // Call OnReceiveBitrateChanged twice to get a first estimate. TickTime::AdvanceFakeClock(1000); EXPECT_CALL(rtp, SetREMBData(_, _, _)) .Times(1); - vie_remb_->Process(); + vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); // Lower the estimate to trigger a new packet REMB packet. bitrate_estimate = bitrate_estimate - 100; EXPECT_CALL(rtp, SetREMBData(_, _, _)) .Times(1); vie_remb_->OnReceiveBitrateChanged(&ssrcs, bitrate_estimate); - vie_remb_->Process(); } } // namespace webrtc