diff --git a/webrtc/modules/BUILD.gn b/webrtc/modules/BUILD.gn index ccc681c02f..b3bfbac46f 100644 --- a/webrtc/modules/BUILD.gn +++ b/webrtc/modules/BUILD.gn @@ -375,6 +375,7 @@ if (rtc_include_tests) { "congestion_controller/delay_based_bwe_unittest_helper.h", "congestion_controller/probe_bitrate_estimator_unittest.cc", "congestion_controller/probe_controller_unittest.cc", + "congestion_controller/transport_feedback_adapter_unittest.cc", "media_file/media_file_unittest.cc", "module_common_types_unittest.cc", "pacing/bitrate_prober_unittest.cc", @@ -394,7 +395,6 @@ if (rtc_include_tests) { "remote_bitrate_estimator/test/bwe_unittest.cc", "remote_bitrate_estimator/test/estimators/nada_unittest.cc", "remote_bitrate_estimator/test/metric_recorder_unittest.cc", - "remote_bitrate_estimator/transport_feedback_adapter_unittest.cc", "rtp_rtcp/source/byte_io_unittest.cc", "rtp_rtcp/source/fec_receiver_unittest.cc", "rtp_rtcp/source/fec_test_helper.cc", diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc index bc2f1f63e6..b3363679bd 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -186,22 +186,18 @@ void BitrateControllerImpl::OnReceiverEstimatedBitrate(uint32_t bitrate) { MaybeTriggerOnNetworkChanged(); } -void BitrateControllerImpl::OnProbeBitrate(uint32_t bitrate_bps) { +void BitrateControllerImpl::OnDelayBasedBweResult( + const DelayBasedBwe::Result& result) { + if (!result.updated) + return; { rtc::CritScope cs(&critsect_); - bandwidth_estimation_.SetSendBitrate(bitrate_bps); - } - MaybeTriggerOnNetworkChanged(); -} - -// TODO(isheriff): Perhaps need new interface for invocation from DelayBasedBwe. -void BitrateControllerImpl::OnReceiveBitrateChanged( - const std::vector& ssrcs, - uint32_t bitrate_bps) { - { - rtc::CritScope cs(&critsect_); - bandwidth_estimation_.UpdateDelayBasedEstimate(clock_->TimeInMilliseconds(), - bitrate_bps); + if (result.probe) { + bandwidth_estimation_.SetSendBitrate(result.target_bitrate_bps); + } else { + bandwidth_estimation_.UpdateDelayBasedEstimate( + clock_->TimeInMilliseconds(), result.target_bitrate_bps); + } } MaybeTriggerOnNetworkChanged(); } diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h index c8bb10282b..7ee6b19380 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h @@ -61,10 +61,7 @@ class BitrateControllerImpl : public BitrateController { uint8_t* fraction_loss, int64_t* rtt) override; - // RemoteBitrateObserver overrides. - void OnReceiveBitrateChanged(const std::vector& ssrcs, - uint32_t bitrate_bps) override; - void OnProbeBitrate(uint32_t bitrate_bps) override; + void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override; int64_t TimeUntilNextProcess() override; void Process() override; diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc index 7da947b7b2..4b298cc152 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc @@ -169,7 +169,8 @@ TEST_F(BitrateControllerTest, OneBitrateObserverOneRtcpObserver) { EXPECT_EQ(300000, bitrate_observer_.last_bitrate_); // Test that a low delay-based estimate limits the combined estimate. - controller_->OnReceiveBitrateChanged({0}, 280000); + webrtc::DelayBasedBwe::Result result(false, 280000); + controller_->OnDelayBasedBweResult(result); EXPECT_EQ(280000, bitrate_observer_.last_bitrate_); // Test that a low REMB limits the combined estimate. diff --git a/webrtc/modules/bitrate_controller/include/bitrate_controller.h b/webrtc/modules/bitrate_controller/include/bitrate_controller.h index 90b6471390..7400d7d032 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h @@ -17,9 +17,9 @@ #include +#include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/include/module.h" #include "webrtc/modules/pacing/paced_sender.h" -#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" namespace webrtc { @@ -44,7 +44,7 @@ class BitrateObserver { virtual ~BitrateObserver() {} }; -class BitrateController : public Module, public RemoteBitrateObserver { +class BitrateController : public Module { // This class collects feedback from all streams sent to a peer (via // RTCPBandwidthObservers). It does one aggregated send side bandwidth // estimation and divide the available bitrate between all its registered @@ -78,6 +78,8 @@ class BitrateController : public Module, public RemoteBitrateObserver { int min_bitrate_bps, int max_bitrate_bps) = 0; + virtual void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) = 0; + // Gets the available payload bandwidth in bits per second. Note that // this bandwidth excludes packet headers. virtual bool AvailableBandwidth(uint32_t* bandwidth) const = 0; diff --git a/webrtc/modules/congestion_controller/BUILD.gn b/webrtc/modules/congestion_controller/BUILD.gn index 758fba59d8..1069d624ff 100644 --- a/webrtc/modules/congestion_controller/BUILD.gn +++ b/webrtc/modules/congestion_controller/BUILD.gn @@ -18,6 +18,8 @@ rtc_static_library("congestion_controller") { "probe_bitrate_estimator.h", "probe_controller.cc", "probe_controller.h", + "transport_feedback_adapter.cc", + "transport_feedback_adapter.h", ] # TODO(jschuh): Bug 1348: fix this warning. @@ -32,6 +34,5 @@ rtc_static_library("congestion_controller") { deps = [ "../bitrate_controller", "../pacing", - "../remote_bitrate_estimator", ] } diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index c0ec3da0f9..f7e7e5625a 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -21,7 +21,6 @@ #include "webrtc/base/socket.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" -#include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/congestion_controller/probe_controller.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h" @@ -171,7 +170,7 @@ CongestionController::CongestionController( retransmission_rate_limiter_( new RateLimiter(clock, kRetransmitWindowSizeMs)), remote_estimator_proxy_(clock_, packet_router_.get()), - transport_feedback_adapter_(clock_), + transport_feedback_adapter_(clock_, bitrate_controller_.get()), min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), max_bitrate_bps_(0), last_reported_bitrate_bps_(0), @@ -202,7 +201,7 @@ CongestionController::CongestionController( retransmission_rate_limiter_( new RateLimiter(clock, kRetransmitWindowSizeMs)), remote_estimator_proxy_(clock_, packet_router_.get()), - transport_feedback_adapter_(clock_), + transport_feedback_adapter_(clock_, bitrate_controller_.get()), min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), max_bitrate_bps_(0), last_reported_bitrate_bps_(0), @@ -215,10 +214,8 @@ CongestionController::CongestionController( CongestionController::~CongestionController() {} void CongestionController::Init() { - transport_feedback_adapter_.SetBitrateEstimator( - new DelayBasedBwe(bitrate_controller_.get(), clock_)); - transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate( - min_bitrate_bps_); + transport_feedback_adapter_.InitBwe(); + transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps_); } void CongestionController::SetBweBitrates(int min_bitrate_bps, @@ -236,8 +233,7 @@ void CongestionController::SetBweBitrates(int min_bitrate_bps, if (remote_bitrate_estimator_) remote_bitrate_estimator_->SetMinBitrate(min_bitrate_bps); min_bitrate_bps_ = min_bitrate_bps; - transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate( - min_bitrate_bps_); + transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps_); MaybeTriggerOnNetworkChanged(); } @@ -256,10 +252,8 @@ void CongestionController::ResetBweAndBitrates(int bitrate_bps, if (remote_bitrate_estimator_) remote_bitrate_estimator_->SetMinBitrate(min_bitrate_bps); - RemoteBitrateEstimator* rbe = - new DelayBasedBwe(bitrate_controller_.get(), clock_); - transport_feedback_adapter_.SetBitrateEstimator(rbe); - rbe->SetMinBitrate(min_bitrate_bps); + transport_feedback_adapter_.InitBwe(); + transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps); // TODO(holmer): Trigger a new probe once mid-call probing is implemented. MaybeTriggerOnNetworkChanged(); } diff --git a/webrtc/modules/congestion_controller/congestion_controller.gypi b/webrtc/modules/congestion_controller/congestion_controller.gypi index 5c7ddf2784..3fcddfb248 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.gypi +++ b/webrtc/modules/congestion_controller/congestion_controller.gypi @@ -14,7 +14,6 @@ 'dependencies': [ '<(webrtc_root)/modules/modules.gyp:bitrate_controller', '<(webrtc_root)/modules/modules.gyp:paced_sender', - '<(webrtc_root)/modules/modules.gyp:remote_bitrate_estimator', ], 'sources': [ 'congestion_controller.cc', @@ -25,6 +24,8 @@ 'probe_bitrate_estimator.h', 'probe_controller.cc', 'probe_controller.h', + 'transport_feedback_adapter.cc', + 'transport_feedback_adapter.h', ], # TODO(jschuh): Bug 1348: fix size_t to int truncations. 'msvs_disabled_warnings': [ 4267, ], diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index 87dc502b28..db3ae582f1 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -21,7 +21,6 @@ #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/pacing/paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" -#include "webrtc/system_wrappers/include/critical_section_wrapper.h" #include "webrtc/system_wrappers/include/metrics.h" #include "webrtc/typedefs.h" @@ -40,9 +39,8 @@ constexpr uint32_t kFixedSsrc = 0; namespace webrtc { -DelayBasedBwe::DelayBasedBwe(RemoteBitrateObserver* observer, Clock* clock) +DelayBasedBwe::DelayBasedBwe(Clock* clock) : clock_(clock), - observer_(observer), inter_arrival_(), estimator_(), detector_(OverUseDetectorOptions()), @@ -50,11 +48,10 @@ DelayBasedBwe::DelayBasedBwe(RemoteBitrateObserver* observer, Clock* clock) last_update_ms_(-1), last_seen_packet_ms_(-1), uma_recorded_(false) { - RTC_DCHECK(observer_); network_thread_.DetachFromThread(); } -void DelayBasedBwe::IncomingPacketFeedbackVector( +DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( const std::vector& packet_feedback_vector) { RTC_DCHECK(network_thread_.CalledOnValidThread()); if (!uma_recorded_) { @@ -63,87 +60,89 @@ void DelayBasedBwe::IncomingPacketFeedbackVector( BweNames::kBweNamesMax); uma_recorded_ = true; } + Result aggregated_result; for (const auto& packet_info : packet_feedback_vector) { - IncomingPacketInfo(packet_info); + Result result = IncomingPacketInfo(packet_info); + if (result.updated) + aggregated_result = result; } + return aggregated_result; } -void DelayBasedBwe::IncomingPacketInfo(const PacketInfo& info) { +DelayBasedBwe::Result DelayBasedBwe::IncomingPacketInfo( + const PacketInfo& info) { + // printf("Acked: %ld\n", info.payload_size); int64_t now_ms = clock_->TimeInMilliseconds(); incoming_bitrate_.Update(info.payload_size, info.arrival_time_ms); - bool delay_based_bwe_changed = false; - uint32_t target_bitrate_bps = 0; - { - rtc::CritScope lock(&crit_); + Result result; + // Reset if the stream has timed out. + if (last_seen_packet_ms_ == -1 || + now_ms - last_seen_packet_ms_ > kStreamTimeOutMs) { + inter_arrival_.reset(new InterArrival( + (kTimestampGroupLengthMs << kInterArrivalShift) / 1000, + kTimestampToMs, true)); + estimator_.reset(new OveruseEstimator(OverUseDetectorOptions())); + } + last_seen_packet_ms_ = now_ms; - // Reset if the stream has timed out. - if (last_seen_packet_ms_ == -1 || - now_ms - last_seen_packet_ms_ > kStreamTimeOutMs) { - inter_arrival_.reset(new InterArrival( - (kTimestampGroupLengthMs << kInterArrivalShift) / 1000, - kTimestampToMs, true)); - estimator_.reset(new OveruseEstimator(OverUseDetectorOptions())); - } - last_seen_packet_ms_ = now_ms; + uint32_t send_time_24bits = + static_cast(((static_cast(info.send_time_ms) + << kAbsSendTimeFraction) + + 500) / + 1000) & + 0x00FFFFFF; + // Shift up send time to use the full 32 bits that inter_arrival works with, + // so wrapping works properly. + uint32_t timestamp = send_time_24bits << kAbsSendTimeInterArrivalUpshift; - uint32_t send_time_24bits = - static_cast(((static_cast(info.send_time_ms) - << kAbsSendTimeFraction) + - 500) / - 1000) & - 0x00FFFFFF; - // Shift up send time to use the full 32 bits that inter_arrival works with, - // so wrapping works properly. - uint32_t timestamp = send_time_24bits << kAbsSendTimeInterArrivalUpshift; - - uint32_t ts_delta = 0; - int64_t t_delta = 0; - int size_delta = 0; - if (inter_arrival_->ComputeDeltas(timestamp, info.arrival_time_ms, now_ms, - info.payload_size, &ts_delta, &t_delta, - &size_delta)) { - double ts_delta_ms = (1000.0 * ts_delta) / (1 << kInterArrivalShift); - estimator_->Update(t_delta, ts_delta_ms, size_delta, detector_.State(), - info.arrival_time_ms); - detector_.Detect(estimator_->offset(), ts_delta_ms, - estimator_->num_of_deltas(), info.arrival_time_ms); - } - - int probing_bps = 0; - if (info.probe_cluster_id != PacketInfo::kNotAProbe) { - probing_bps = - probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); - } - - // Currently overusing the bandwidth. - if (detector_.State() == kBwOverusing) { - rtc::Optional incoming_rate = - incoming_bitrate_.Rate(info.arrival_time_ms); - if (incoming_rate && - remote_rate_.TimeToReduceFurther(now_ms, *incoming_rate)) { - delay_based_bwe_changed = - UpdateEstimate(info.arrival_time_ms, now_ms, &target_bitrate_bps); - } - } else if (probing_bps > 0) { - // No overuse, but probing measured a bitrate. - remote_rate_.SetEstimate(probing_bps, info.arrival_time_ms); - observer_->OnProbeBitrate(probing_bps); - delay_based_bwe_changed = - UpdateEstimate(info.arrival_time_ms, now_ms, &target_bitrate_bps); - } - if (!delay_based_bwe_changed && - (last_update_ms_ == -1 || - now_ms - last_update_ms_ > remote_rate_.GetFeedbackInterval())) { - delay_based_bwe_changed = - UpdateEstimate(info.arrival_time_ms, now_ms, &target_bitrate_bps); - } + uint32_t ts_delta = 0; + int64_t t_delta = 0; + int size_delta = 0; + if (inter_arrival_->ComputeDeltas(timestamp, info.arrival_time_ms, now_ms, + info.payload_size, &ts_delta, &t_delta, + &size_delta)) { + double ts_delta_ms = (1000.0 * ts_delta) / (1 << kInterArrivalShift); + estimator_->Update(t_delta, ts_delta_ms, size_delta, detector_.State(), + info.arrival_time_ms); + detector_.Detect(estimator_->offset(), ts_delta_ms, + estimator_->num_of_deltas(), info.arrival_time_ms); } - if (delay_based_bwe_changed) { + int probing_bps = 0; + if (info.probe_cluster_id != PacketInfo::kNotAProbe) { + probing_bps = + probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); + } + + // Currently overusing the bandwidth. + if (detector_.State() == kBwOverusing) { + rtc::Optional incoming_rate = + incoming_bitrate_.Rate(info.arrival_time_ms); + if (incoming_rate && + remote_rate_.TimeToReduceFurther(now_ms, *incoming_rate)) { + result.updated = UpdateEstimate(info.arrival_time_ms, now_ms, + &result.target_bitrate_bps); + } + } else if (probing_bps > 0) { + // No overuse, but probing measured a bitrate. + remote_rate_.SetEstimate(probing_bps, info.arrival_time_ms); + result.probe = true; + result.updated = UpdateEstimate(info.arrival_time_ms, now_ms, + &result.target_bitrate_bps); + } + rtc::Optional incoming_rate = + incoming_bitrate_.Rate(info.arrival_time_ms); + if (!result.updated && + (last_update_ms_ == -1 || + now_ms - last_update_ms_ > remote_rate_.GetFeedbackInterval())) { + result.updated = UpdateEstimate(info.arrival_time_ms, now_ms, + &result.target_bitrate_bps); + } + if (result.updated) last_update_ms_ = now_ms; - observer_->OnReceiveBitrateChanged({kFixedSsrc}, target_bitrate_bps); - } + + return result; } bool DelayBasedBwe::UpdateEstimate(int64_t arrival_time_ms, @@ -160,20 +159,10 @@ bool DelayBasedBwe::UpdateEstimate(int64_t arrival_time_ms, return remote_rate_.ValidEstimate(); } -void DelayBasedBwe::Process() {} - -int64_t DelayBasedBwe::TimeUntilNextProcess() { - const int64_t kDisabledModuleTime = 1000; - return kDisabledModuleTime; -} - void DelayBasedBwe::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) { - rtc::CritScope lock(&crit_); remote_rate_.SetRtt(avg_rtt_ms); } -void DelayBasedBwe::RemoveStream(uint32_t ssrc) {} - bool DelayBasedBwe::LatestEstimate(std::vector* ssrcs, uint32_t* bitrate_bps) const { // Currently accessed from both the process thread (see @@ -182,7 +171,6 @@ bool DelayBasedBwe::LatestEstimate(std::vector* ssrcs, // thread. RTC_DCHECK(ssrcs); RTC_DCHECK(bitrate_bps); - rtc::CritScope lock(&crit_); if (!remote_rate_.ValidEstimate()) return false; @@ -194,7 +182,6 @@ bool DelayBasedBwe::LatestEstimate(std::vector* ssrcs, void DelayBasedBwe::SetMinBitrate(int min_bitrate_bps) { // Called from both the configuration thread and the network thread. Shouldn't // be called from the network thread in the future. - rtc::CritScope lock(&crit_); remote_rate_.SetMinBitrate(min_bitrate_bps); } } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index 3e0a0149a4..7e2c8bdc52 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -18,7 +18,6 @@ #include "webrtc/base/checks.h" #include "webrtc/base/constructormagic.h" -#include "webrtc/base/criticalsection.h" #include "webrtc/base/rate_statistics.h" #include "webrtc/base/thread_checker.h" #include "webrtc/modules/congestion_controller/probe_bitrate_estimator.h" @@ -27,46 +26,42 @@ #include "webrtc/modules/remote_bitrate_estimator/inter_arrival.h" #include "webrtc/modules/remote_bitrate_estimator/overuse_detector.h" #include "webrtc/modules/remote_bitrate_estimator/overuse_estimator.h" -#include "webrtc/system_wrappers/include/critical_section_wrapper.h" namespace webrtc { -class DelayBasedBwe : public RemoteBitrateEstimator { +class DelayBasedBwe { public: - DelayBasedBwe(RemoteBitrateObserver* observer, Clock* clock); + static const int64_t kStreamTimeOutMs = 2000; + + struct Result { + Result() : updated(false), probe(false), target_bitrate_bps(0) {} + Result(bool probe, uint32_t target_bitrate_bps) + : updated(true), probe(probe), target_bitrate_bps(target_bitrate_bps) {} + bool updated; + bool probe; + uint32_t target_bitrate_bps; + }; + + explicit DelayBasedBwe(Clock* clock); virtual ~DelayBasedBwe() {} - void IncomingPacketFeedbackVector( - const std::vector& packet_feedback_vector) override; - void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; + Result IncomingPacketFeedbackVector( + const std::vector& packet_feedback_vector); + void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms); bool LatestEstimate(std::vector* ssrcs, - uint32_t* bitrate_bps) const override; - void SetMinBitrate(int min_bitrate_bps) override; - - // Required by RemoteBitrateEstimator but does nothing. - void Process() override; - // Required by RemoteBitrateEstimator but does nothing. - int64_t TimeUntilNextProcess() override; - // Required by RemoteBitrateEstimator but does nothing. - void RemoveStream(uint32_t ssrc) override; - void IncomingPacket(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header) override { - RTC_NOTREACHED(); - } + uint32_t* bitrate_bps) const; + void SetMinBitrate(int min_bitrate_bps); private: - void IncomingPacketInfo(const PacketInfo& info); + Result IncomingPacketInfo(const PacketInfo& info); // Updates the current remote rate estimate and returns true if a valid // estimate exists. bool UpdateEstimate(int64_t packet_arrival_time_ms, int64_t now_ms, - uint32_t* target_bitrate_bps) - EXCLUSIVE_LOCKS_REQUIRED(crit_); + uint32_t* target_bitrate_bps); rtc::ThreadChecker network_thread_; Clock* const clock_; - RemoteBitrateObserver* const observer_; std::unique_ptr inter_arrival_; std::unique_ptr estimator_; OveruseDetector detector_; @@ -74,10 +69,8 @@ class DelayBasedBwe : public RemoteBitrateEstimator { int64_t last_update_ms_; int64_t last_seen_packet_ms_; bool uma_recorded_; - - rtc::CriticalSection crit_; - AimdRateControl remote_rate_ GUARDED_BY(&crit_); - ProbeBitrateEstimator probe_bitrate_estimator_ GUARDED_BY(&crit_); + AimdRateControl remote_rate_; + ProbeBitrateEstimator probe_bitrate_estimator_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(DelayBasedBwe); }; diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc index 7220967be2..e751013a5d 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -32,7 +32,7 @@ TEST_F(DelayBasedBweTest, ProbeDetection) { now_ms = clock_.TimeInMilliseconds(); IncomingFeedback(now_ms, now_ms, seq_num++, 1000, 0); } - EXPECT_TRUE(bitrate_observer_->updated()); + EXPECT_TRUE(bitrate_observer_.updated()); // Second burst sent at 8 * 1000 / 5 = 1600 kbps. for (int i = 0; i < kNumProbes; ++i) { @@ -41,8 +41,8 @@ TEST_F(DelayBasedBweTest, ProbeDetection) { IncomingFeedback(now_ms, now_ms, seq_num++, 1000, 1); } - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_GT(bitrate_observer_->latest_bitrate(), 1500000u); + EXPECT_TRUE(bitrate_observer_.updated()); + EXPECT_GT(bitrate_observer_.latest_bitrate(), 1500000u); } TEST_F(DelayBasedBweTest, ProbeDetectionNonPacedPackets) { @@ -61,8 +61,8 @@ TEST_F(DelayBasedBweTest, ProbeDetectionNonPacedPackets) { PacketInfo::kNotAProbe); } - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_GT(bitrate_observer_->latest_bitrate(), 800000u); + EXPECT_TRUE(bitrate_observer_.updated()); + EXPECT_GT(bitrate_observer_.latest_bitrate(), 800000u); } TEST_F(DelayBasedBweTest, ProbeDetectionFasterArrival) { @@ -78,7 +78,7 @@ TEST_F(DelayBasedBweTest, ProbeDetectionFasterArrival) { IncomingFeedback(now_ms, send_time_ms, seq_num++, 1000, 0); } - EXPECT_FALSE(bitrate_observer_->updated()); + EXPECT_FALSE(bitrate_observer_.updated()); } TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrival) { @@ -94,8 +94,8 @@ TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrival) { IncomingFeedback(now_ms, send_time_ms, seq_num++, 1000, 1); } - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_NEAR(bitrate_observer_->latest_bitrate(), 1140000u, 10000u); + EXPECT_TRUE(bitrate_observer_.updated()); + EXPECT_NEAR(bitrate_observer_.latest_bitrate(), 1140000u, 10000u); } TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) { @@ -111,8 +111,8 @@ TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) { IncomingFeedback(now_ms, send_time_ms, seq_num++, 1000, 1); } - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_NEAR(bitrate_observer_->latest_bitrate(), 4000000u, 10000u); + EXPECT_TRUE(bitrate_observer_.updated()); + EXPECT_NEAR(bitrate_observer_.latest_bitrate(), 4000000u, 10000u); } TEST_F(DelayBasedBweTest, InitialBehavior) { diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc index 34692511b9..a3a1893cbe 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc @@ -150,8 +150,7 @@ int64_t StreamGenerator::GenerateFrame(std::vector* packets, DelayBasedBweTest::DelayBasedBweTest() : clock_(100000000), - bitrate_observer_(new test::TestBitrateObserver), - bitrate_estimator_(new DelayBasedBwe(bitrate_observer_.get(), &clock_)), + bitrate_estimator_(&clock_), stream_generator_(new test::StreamGenerator(1e6, // Capacity. clock_.TimeInMicroseconds())), arrival_time_offset_ms_(0) {} @@ -182,7 +181,13 @@ void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, sequence_number, payload_size, probe_cluster_id); std::vector packets; packets.push_back(packet); - bitrate_estimator_->IncomingPacketFeedbackVector(packets); + DelayBasedBwe::Result result = + bitrate_estimator_.IncomingPacketFeedbackVector(packets); + const uint32_t kDummySsrc = 0; + if (result.updated) { + bitrate_observer_.OnReceiveBitrateChanged({kDummySsrc}, + result.target_bitrate_bps); + } } // Generates a frame of packets belonging to a stream at a given bitrate and @@ -201,17 +206,20 @@ bool DelayBasedBweTest::GenerateAndProcessFrame(uint32_t ssrc, return false; bool overuse = false; - bitrate_observer_->Reset(); + bitrate_observer_.Reset(); clock_.AdvanceTimeMicroseconds(1000 * packets.back().arrival_time_ms - clock_.TimeInMicroseconds()); for (auto& packet : packets) { RTC_CHECK_GE(packet.arrival_time_ms + arrival_time_offset_ms_, 0); packet.arrival_time_ms += arrival_time_offset_ms_; } - bitrate_estimator_->IncomingPacketFeedbackVector(packets); - - if (bitrate_observer_->updated()) { - if (bitrate_observer_->latest_bitrate() < bitrate_bps) + DelayBasedBwe::Result result = + bitrate_estimator_.IncomingPacketFeedbackVector(packets); + const uint32_t kDummySsrc = 0; + if (result.updated) { + bitrate_observer_.OnReceiveBitrateChanged({kDummySsrc}, + result.target_bitrate_bps); + if (result.target_bitrate_bps < bitrate_bps) overuse = true; } @@ -235,13 +243,13 @@ uint32_t DelayBasedBweTest::SteadyStateRun(uint32_t ssrc, 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); - EXPECT_GT(bitrate_observer_->latest_bitrate(), min_bitrate); - bitrate_bps = bitrate_observer_->latest_bitrate(); + EXPECT_LT(bitrate_observer_.latest_bitrate(), max_bitrate); + EXPECT_GT(bitrate_observer_.latest_bitrate(), min_bitrate); + bitrate_bps = bitrate_observer_.latest_bitrate(); bitrate_update_seen = true; - } else if (bitrate_observer_->updated()) { - bitrate_bps = bitrate_observer_->latest_bitrate(); - bitrate_observer_->Reset(); + } else if (bitrate_observer_.updated()) { + bitrate_bps = bitrate_observer_.latest_bitrate(); + bitrate_observer_.Reset(); } if (bitrate_update_seen && bitrate_bps > target_bitrate) { break; @@ -259,13 +267,12 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( int64_t send_time_ms = 0; uint16_t sequence_number = 0; std::vector ssrcs; - EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); + EXPECT_FALSE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps)); EXPECT_EQ(0u, ssrcs.size()); clock_.AdvanceTimeMilliseconds(1000); - bitrate_estimator_->Process(); - EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); - EXPECT_FALSE(bitrate_observer_->updated()); - bitrate_observer_->Reset(); + EXPECT_FALSE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps)); + EXPECT_FALSE(bitrate_observer_.updated()); + bitrate_observer_.Reset(); clock_.AdvanceTimeMilliseconds(1000); // Inserting packets for 5 seconds to get a valid estimate. for (int i = 0; i < 5 * kFramerate + 1 + kNumInitialPackets; ++i) { @@ -274,25 +281,23 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( int cluster_id = i < kInitialProbingPackets ? 0 : PacketInfo::kNotAProbe; if (i == kNumInitialPackets) { - bitrate_estimator_->Process(); - EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); + EXPECT_FALSE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps)); EXPECT_EQ(0u, ssrcs.size()); - EXPECT_FALSE(bitrate_observer_->updated()); - bitrate_observer_->Reset(); + EXPECT_FALSE(bitrate_observer_.updated()); + bitrate_observer_.Reset(); } IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, sequence_number++, kMtu, cluster_id); clock_.AdvanceTimeMilliseconds(1000 / kFramerate); send_time_ms += kFrameIntervalMs; } - bitrate_estimator_->Process(); - EXPECT_TRUE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); + EXPECT_TRUE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps)); ASSERT_EQ(1u, ssrcs.size()); EXPECT_EQ(kDefaultSsrc, ssrcs.front()); EXPECT_NEAR(expected_converge_bitrate, bitrate_bps, kAcceptedBitrateErrorBps); - EXPECT_TRUE(bitrate_observer_->updated()); - bitrate_observer_->Reset(); - EXPECT_EQ(bitrate_observer_->latest_bitrate(), bitrate_bps); + EXPECT_TRUE(bitrate_observer_.updated()); + bitrate_observer_.Reset(); + EXPECT_EQ(bitrate_observer_.latest_bitrate(), bitrate_bps); } void DelayBasedBweTest::RateIncreaseReorderingTestHelper( @@ -311,17 +316,16 @@ void DelayBasedBweTest::RateIncreaseReorderingTestHelper( // as it doesn't do anything in Process(). if (i == kNumInitialPackets) { // Process after we have enough frames to get a valid input rate estimate. - bitrate_estimator_->Process(); - EXPECT_FALSE(bitrate_observer_->updated()); // No valid estimate. + + EXPECT_FALSE(bitrate_observer_.updated()); // No valid estimate. } IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, sequence_number++, kMtu, cluster_id); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); send_time_ms += kFrameIntervalMs; } - bitrate_estimator_->Process(); - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_NEAR(expected_bitrate_bps, bitrate_observer_->latest_bitrate(), + EXPECT_TRUE(bitrate_observer_.updated()); + EXPECT_NEAR(expected_bitrate_bps, bitrate_observer_.latest_bitrate(), kAcceptedBitrateErrorBps); for (int i = 0; i < 10; ++i) { clock_.AdvanceTimeMilliseconds(2 * kFrameIntervalMs); @@ -333,9 +337,8 @@ void DelayBasedBweTest::RateIncreaseReorderingTestHelper( 1000); sequence_number += 2; } - bitrate_estimator_->Process(); - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_NEAR(expected_bitrate_bps, bitrate_observer_->latest_bitrate(), + EXPECT_TRUE(bitrate_observer_.updated()); + EXPECT_NEAR(expected_bitrate_bps, bitrate_observer_.latest_bitrate(), kAcceptedBitrateErrorBps); } @@ -353,15 +356,15 @@ void DelayBasedBweTest::RateIncreaseRtpTimestampsTestHelper( while (bitrate_bps < 5e5) { bool overuse = GenerateAndProcessFrame(kDefaultSsrc, bitrate_bps); if (overuse) { - EXPECT_GT(bitrate_observer_->latest_bitrate(), bitrate_bps); - bitrate_bps = bitrate_observer_->latest_bitrate(); - bitrate_observer_->Reset(); - } else if (bitrate_observer_->updated()) { - bitrate_bps = bitrate_observer_->latest_bitrate(); - bitrate_observer_->Reset(); + EXPECT_GT(bitrate_observer_.latest_bitrate(), bitrate_bps); + bitrate_bps = bitrate_observer_.latest_bitrate(); + bitrate_observer_.Reset(); + } else if (bitrate_observer_.updated()) { + bitrate_bps = bitrate_observer_.latest_bitrate(); + bitrate_observer_.Reset(); } ++iterations; - ASSERT_LE(iterations, expected_iterations); + // ASSERT_LE(iterations, expected_iterations); } ASSERT_EQ(expected_iterations, iterations); } @@ -405,7 +408,7 @@ void DelayBasedBweTest::CapacityDropTestHelper( kDefaultSsrc, steady_state_time * kFramerate, kStartBitrate, kMinExpectedBitrate, kMaxExpectedBitrate, kInitialCapacityBps); EXPECT_NEAR(kInitialCapacityBps, bitrate_bps, 130000u); - bitrate_observer_->Reset(); + bitrate_observer_.Reset(); // Add an offset to make sure the BWE can handle it. arrival_time_offset_ms_ += receiver_clock_offset_change_ms; @@ -417,11 +420,11 @@ void DelayBasedBweTest::CapacityDropTestHelper( for (int i = 0; i < 100 * number_of_streams; ++i) { GenerateAndProcessFrame(kDefaultSsrc, bitrate_bps); if (bitrate_drop_time == -1 && - bitrate_observer_->latest_bitrate() <= kReducedCapacityBps) { + bitrate_observer_.latest_bitrate() <= kReducedCapacityBps) { bitrate_drop_time = clock_.TimeInMilliseconds(); } - if (bitrate_observer_->updated()) - bitrate_bps = bitrate_observer_->latest_bitrate(); + if (bitrate_observer_.updated()) + bitrate_bps = bitrate_observer_.latest_bitrate(); } EXPECT_NEAR(expected_bitrate_drop_delta, @@ -439,12 +442,11 @@ void DelayBasedBweTest::TestTimestampGroupingTestHelper() { IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, sequence_number++, 1000); - bitrate_estimator_->Process(); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); send_time_ms += kFrameIntervalMs; } - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_GE(bitrate_observer_->latest_bitrate(), 400000u); + EXPECT_TRUE(bitrate_observer_.updated()); + EXPECT_GE(bitrate_observer_.latest_bitrate(), 400000u); // Insert batches of frames which were sent very close in time. Also simulate // capacity over-use to see that we back off correctly. @@ -461,11 +463,10 @@ void DelayBasedBweTest::TestTimestampGroupingTestHelper() { // Increase time until next batch to simulate over-use. clock_.AdvanceTimeMilliseconds(10); send_time_ms += kFrameIntervalMs - kTimestampGroupLength; - bitrate_estimator_->Process(); } - EXPECT_TRUE(bitrate_observer_->updated()); + EXPECT_TRUE(bitrate_observer_.updated()); // Should have reduced the estimate. - EXPECT_LT(bitrate_observer_->latest_bitrate(), 400000u); + EXPECT_LT(bitrate_observer_.latest_bitrate(), 400000u); } void DelayBasedBweTest::TestWrappingHelper(int silence_time_s) { @@ -479,25 +480,22 @@ void DelayBasedBweTest::TestWrappingHelper(int silence_time_s) { sequence_number++, 1000); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); send_time_ms += kFrameIntervalMs; - bitrate_estimator_->Process(); } uint32_t bitrate_before = 0; std::vector ssrcs; - bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_before); + bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_before); clock_.AdvanceTimeMilliseconds(silence_time_s * 1000); send_time_ms += silence_time_s * 1000; - bitrate_estimator_->Process(); for (size_t i = 0; i < 21; ++i) { IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, sequence_number++, 1000); clock_.AdvanceTimeMilliseconds(2 * kFrameIntervalMs); send_time_ms += kFrameIntervalMs; - bitrate_estimator_->Process(); } uint32_t bitrate_after = 0; - bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_after); + bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_after); EXPECT_LT(bitrate_after, bitrate_before); } } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h index f97cd5272d..add9fb3277 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h @@ -19,6 +19,7 @@ #include "webrtc/test/gtest.h" #include "webrtc/base/constructormagic.h" +#include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/system_wrappers/include/clock.h" @@ -160,8 +161,8 @@ class DelayBasedBweTest : public ::testing::Test { static const uint32_t kDefaultSsrc; SimulatedClock clock_; // Time at the receiver. - std::unique_ptr bitrate_observer_; - std::unique_ptr bitrate_estimator_; + test::TestBitrateObserver bitrate_observer_; + DelayBasedBwe bitrate_estimator_; std::unique_ptr stream_generator_; int64_t arrival_time_offset_ms_; diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index 862f2cd6be..eb6db33712 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -15,12 +15,12 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/common_types.h" +#include "webrtc/modules/congestion_controller/transport_feedback_adapter.h" #include "webrtc/modules/include/module.h" #include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/pacing/packet_router.h" #include "webrtc/modules/pacing/paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h" -#include "webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h" namespace rtc { struct SentPacket; diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc similarity index 82% rename from webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc rename to webrtc/modules/congestion_controller/transport_feedback_adapter.cc index 66ef7f0495..8875806e3e 100644 --- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc @@ -8,14 +8,15 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h" +#include "webrtc/modules/congestion_controller/transport_feedback_adapter.h" #include #include #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" -#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" +#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" +#include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "webrtc/modules/utility/include/process_thread.h" @@ -39,20 +40,19 @@ class PacketInfoComparator { }; TransportFeedbackAdapter::TransportFeedbackAdapter( - Clock* clock) + Clock* clock, + BitrateController* bitrate_controller) : send_time_history_(clock, kSendTimeHistoryWindowMs), clock_(clock), current_offset_ms_(kNoTimestamp), - last_timestamp_us_(kNoTimestamp) {} + last_timestamp_us_(kNoTimestamp), + bitrate_controller_(bitrate_controller) {} -TransportFeedbackAdapter::~TransportFeedbackAdapter() { -} +TransportFeedbackAdapter::~TransportFeedbackAdapter() {} -void TransportFeedbackAdapter::SetBitrateEstimator( - RemoteBitrateEstimator* rbe) { - if (bitrate_estimator_.get() != rbe) { - bitrate_estimator_.reset(rbe); - } +void TransportFeedbackAdapter::InitBwe() { + rtc::CritScope cs(&bwe_lock_); + delay_based_bwe_.reset(new DelayBasedBwe(clock_)); } void TransportFeedbackAdapter::AddPacket(uint16_t sequence_number, @@ -68,6 +68,11 @@ void TransportFeedbackAdapter::OnSentPacket(uint16_t sequence_number, send_time_history_.OnSentPacket(sequence_number, send_time_ms); } +void TransportFeedbackAdapter::SetMinBitrate(int min_bitrate_bps) { + rtc::CritScope cs(&bwe_lock_); + delay_based_bwe_->SetMinBitrate(min_bitrate_bps); +} + std::vector TransportFeedbackAdapter::GetPacketFeedbackVector( const rtcp::TransportFeedback& feedback) { int64_t timestamp_us = feedback.GetBaseTimeUs(); @@ -129,9 +134,14 @@ std::vector TransportFeedbackAdapter::GetPacketFeedbackVector( void TransportFeedbackAdapter::OnTransportFeedback( const rtcp::TransportFeedback& feedback) { last_packet_feedback_vector_ = GetPacketFeedbackVector(feedback); - if (bitrate_estimator_.get()) - bitrate_estimator_->IncomingPacketFeedbackVector( + DelayBasedBwe::Result result; + { + rtc::CritScope cs(&bwe_lock_); + delay_based_bwe_->IncomingPacketFeedbackVector( last_packet_feedback_vector_); + } + if (result.updated) + bitrate_controller_->OnDelayBasedBweResult(result); } std::vector TransportFeedbackAdapter::GetTransportFeedbackVector() @@ -141,8 +151,8 @@ std::vector TransportFeedbackAdapter::GetTransportFeedbackVector() void TransportFeedbackAdapter::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) { - RTC_DCHECK(bitrate_estimator_.get() != nullptr); - bitrate_estimator_->OnRttUpdate(avg_rtt_ms, max_rtt_ms); + rtc::CritScope cs(&bwe_lock_); + delay_based_bwe_->OnRttUpdate(avg_rtt_ms, max_rtt_ms); } } // namespace webrtc diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h b/webrtc/modules/congestion_controller/transport_feedback_adapter.h similarity index 67% rename from webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h rename to webrtc/modules/congestion_controller/transport_feedback_adapter.h index 2db6603984..6422736c5c 100644 --- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.h @@ -8,58 +8,63 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TRANSPORT_FEEDBACK_ADAPTER_H_ -#define WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TRANSPORT_FEEDBACK_ADAPTER_H_ +#ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_ +#define WEBRTC_MODULES_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_ #include #include #include "webrtc/base/criticalsection.h" #include "webrtc/base/thread_annotations.h" +#include "webrtc/base/thread_checker.h" +#include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/include/module_common_types.h" -#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" namespace webrtc { +class BitrateController; class ProcessThread; class TransportFeedbackAdapter : public TransportFeedbackObserver, public CallStatsObserver { public: - explicit TransportFeedbackAdapter(Clock* clock); + TransportFeedbackAdapter(Clock* clock, BitrateController* bitrate_controller); virtual ~TransportFeedbackAdapter(); - void SetBitrateEstimator(RemoteBitrateEstimator* rbe); - RemoteBitrateEstimator* GetBitrateEstimator() const { - return bitrate_estimator_.get(); - } - + void InitBwe(); // Implements TransportFeedbackObserver. void AddPacket(uint16_t sequence_number, size_t length, int probe_cluster_id) override; void OnSentPacket(uint16_t sequence_number, int64_t send_time_ms); + // TODO(holmer): This method should return DelayBasedBwe::Result so that we + // can get rid of the dependency on BitrateController. Requires changes + // to the CongestionController interface. void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override; std::vector GetTransportFeedbackVector() const override; // Implements CallStatsObserver. void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; + void SetMinBitrate(int min_bitrate_bps); + private: std::vector GetPacketFeedbackVector( const rtcp::TransportFeedback& feedback); rtc::CriticalSection lock_; + rtc::CriticalSection bwe_lock_; SendTimeHistory send_time_history_ GUARDED_BY(&lock_); - std::unique_ptr bitrate_estimator_; + std::unique_ptr delay_based_bwe_ GUARDED_BY(&bwe_lock_); Clock* const clock_; int64_t current_offset_ms_; int64_t last_timestamp_us_; + BitrateController* const bitrate_controller_; std::vector last_packet_feedback_vector_; }; } // namespace webrtc -#endif // WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TRANSPORT_FEEDBACK_ADAPTER_H_ +#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_ diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc similarity index 78% rename from webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc rename to webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc index b312f96f10..60c851442a 100644 --- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc @@ -17,8 +17,7 @@ #include "webrtc/base/checks.h" #include "webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h" -#include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_estimator.h" -#include "webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h" +#include "webrtc/modules/congestion_controller/transport_feedback_adapter.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "webrtc/system_wrappers/include/clock.h" @@ -32,23 +31,16 @@ namespace test { class TransportFeedbackAdapterTest : public ::testing::Test { public: TransportFeedbackAdapterTest() - : clock_(0), - bitrate_estimator_(nullptr), - bitrate_controller_(this), - receiver_estimated_bitrate_(0) {} + : clock_(0), bitrate_controller_(this), receiver_estimated_bitrate_(0) {} virtual ~TransportFeedbackAdapterTest() {} virtual void SetUp() { - adapter_.reset(new TransportFeedbackAdapter(&clock_)); - - bitrate_estimator_ = new MockRemoteBitrateEstimator(); - adapter_->SetBitrateEstimator(bitrate_estimator_); + adapter_.reset(new TransportFeedbackAdapter(&clock_, &bitrate_controller_)); + adapter_->InitBwe(); } - virtual void TearDown() { - adapter_.reset(); - } + virtual void TearDown() { adapter_.reset(); } protected: // Proxy class used since TransportFeedbackAdapter will own the instance @@ -60,9 +52,8 @@ class TransportFeedbackAdapterTest : public ::testing::Test { ~MockBitrateControllerAdapter() override {} - void OnReceiveBitrateChanged(const std::vector& ssrcs, - uint32_t bitrate_bps) override { - owner_->receiver_estimated_bitrate_ = bitrate_bps; + void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override { + owner_->receiver_estimated_bitrate_ = result.target_bitrate_bps; } TransportFeedbackAdapterTest* const owner_; @@ -106,7 +97,6 @@ class TransportFeedbackAdapterTest : public ::testing::Test { } SimulatedClock clock_; - MockRemoteBitrateEstimator* bitrate_estimator_; MockBitrateControllerAdapter bitrate_controller_; std::unique_ptr adapter_; @@ -135,13 +125,8 @@ TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) { feedback.Build(); - EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_)) - .Times(1) - .WillOnce(Invoke( - [packets, this](const std::vector& feedback_vector) { - ComparePacketVectors(packets, feedback_vector); - })); adapter_->OnTransportFeedback(feedback); + ComparePacketVectors(packets, adapter_->GetTransportFeedbackVector()); } TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) { @@ -177,13 +162,9 @@ TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) { packets.begin() + kSendSideDropBefore, packets.begin() + kReceiveSideDropAfter + 1); - EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_)) - .Times(1) - .WillOnce(Invoke([expected_packets, - this](const std::vector& feedback_vector) { - ComparePacketVectors(expected_packets, feedback_vector); - })); adapter_->OnTransportFeedback(feedback); + ComparePacketVectors(expected_packets, + adapter_->GetTransportFeedbackVector()); } TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) { @@ -217,13 +198,9 @@ TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) { std::vector expected_packets; expected_packets.push_back(packets[i]); - EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_)) - .Times(1) - .WillOnce(Invoke([expected_packets, this]( - const std::vector& feedback_vector) { - ComparePacketVectors(expected_packets, feedback_vector); - })); adapter_->OnTransportFeedback(*feedback.get()); + ComparePacketVectors(expected_packets, + adapter_->GetTransportFeedbackVector()); } } @@ -251,13 +228,9 @@ TEST_F(TransportFeedbackAdapterTest, HandlesReordering) { feedback.Build(); - EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_)) - .Times(1) - .WillOnce(Invoke([expected_packets, - this](const std::vector& feedback_vector) { - ComparePacketVectors(expected_packets, feedback_vector); - })); adapter_->OnTransportFeedback(feedback); + ComparePacketVectors(expected_packets, + adapter_->GetTransportFeedbackVector()); } TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { @@ -294,14 +267,6 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { info.arrival_time_ms += (kLargePositiveDeltaUs + 1000) / 1000; ++info.sequence_number; - // Expected to be ordered on arrival time when the feedback message has been - // parsed. - std::vector expected_packets; - expected_packets.push_back(sent_packets[0]); - expected_packets.push_back(sent_packets[3]); - expected_packets.push_back(sent_packets[1]); - expected_packets.push_back(sent_packets[2]); - // Packets will be added to send history. for (const PacketInfo& packet : sent_packets) OnSentPacket(packet); @@ -327,14 +292,18 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { std::vector received_feedback; EXPECT_TRUE(feedback.get() != nullptr); - EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_)) - .Times(1) - .WillOnce(Invoke([expected_packets, &received_feedback]( - const std::vector& feedback_vector) { - EXPECT_EQ(expected_packets.size(), feedback_vector.size()); - received_feedback = feedback_vector; - })); adapter_->OnTransportFeedback(*feedback.get()); + { + // Expected to be ordered on arrival time when the feedback message has been + // parsed. + std::vector expected_packets; + expected_packets.push_back(sent_packets[0]); + expected_packets.push_back(sent_packets[3]); + expected_packets.push_back(sent_packets[1]); + expected_packets.push_back(sent_packets[2]); + ComparePacketVectors(expected_packets, + adapter_->GetTransportFeedbackVector()); + } // Create a new feedback message and add the trailing item. feedback.reset(new rtcp::TransportFeedback()); @@ -346,18 +315,13 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { rtcp::TransportFeedback::ParseFrom(raw_packet.data(), raw_packet.size()); EXPECT_TRUE(feedback.get() != nullptr); - EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_)) - .Times(1) - .WillOnce(Invoke( - [&received_feedback](const std::vector& feedback_vector) { - EXPECT_EQ(1u, feedback_vector.size()); - received_feedback.push_back(feedback_vector[0]); - })); adapter_->OnTransportFeedback(*feedback.get()); - - expected_packets.push_back(info); - - ComparePacketVectors(expected_packets, received_feedback); + { + std::vector expected_packets; + expected_packets.push_back(info); + ComparePacketVectors(expected_packets, + adapter_->GetTransportFeedbackVector()); + } } } // namespace test diff --git a/webrtc/modules/remote_bitrate_estimator/BUILD.gn b/webrtc/modules/remote_bitrate_estimator/BUILD.gn index ea0e214bdc..2a7c68f773 100644 --- a/webrtc/modules/remote_bitrate_estimator/BUILD.gn +++ b/webrtc/modules/remote_bitrate_estimator/BUILD.gn @@ -30,8 +30,6 @@ rtc_static_library("remote_bitrate_estimator") { "remote_estimator_proxy.h", "send_time_history.cc", "test/bwe_test_logging.h", - "transport_feedback_adapter.cc", - "transport_feedback_adapter.h", ] if (!rtc_include_tests) { diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi index 3a2e9ea915..e719f2ff9d 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi @@ -38,8 +38,6 @@ 'remote_estimator_proxy.cc', 'remote_estimator_proxy.h', 'send_time_history.cc', - 'transport_feedback_adapter.cc', - 'transport_feedback_adapter.h', 'test/bwe_test_logging.h', ], # source 'conditions': [ diff --git a/webrtc/tools/DEPS b/webrtc/tools/DEPS index cc33de1f96..507106a063 100644 --- a/webrtc/tools/DEPS +++ b/webrtc/tools/DEPS @@ -4,6 +4,7 @@ include_rules = [ "+webrtc/common_video", "+webrtc/modules/audio_device", "+webrtc/modules/audio_processing", + "+webrtc/modules/bitrate_controller", "+webrtc/modules/congestion_controller", "+webrtc/modules/rtp_rtcp", "+webrtc/system_wrappers", diff --git a/webrtc/tools/event_log_visualizer/analyzer.cc b/webrtc/tools/event_log_visualizer/analyzer.cc index 260975b4b5..d1dec22f0a 100644 --- a/webrtc/tools/event_log_visualizer/analyzer.cc +++ b/webrtc/tools/event_log_visualizer/analyzer.cc @@ -24,6 +24,7 @@ #include "webrtc/base/rate_statistics.h" #include "webrtc/call.h" #include "webrtc/common_types.h" +#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -1053,6 +1054,34 @@ void EventLogAnalyzer::CreateBweSimulationGraph(Plot* plot) { plot->SetTitle("Simulated BWE behavior"); } +// TODO(holmer): Remove once TransportFeedbackAdapter no longer needs a +// BitrateController. +class NullBitrateController : public BitrateController { + public: + ~NullBitrateController() override {} + RtcpBandwidthObserver* CreateRtcpBandwidthObserver() override { + return nullptr; + } + void SetStartBitrate(int start_bitrate_bps) override {} + void SetMinMaxBitrate(int min_bitrate_bps, int max_bitrate_bps) override {} + void SetBitrates(int start_bitrate_bps, + int min_bitrate_bps, + int max_bitrate_bps) override {} + void ResetBitrates(int bitrate_bps, + int min_bitrate_bps, + int max_bitrate_bps) override {} + void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override {} + bool AvailableBandwidth(uint32_t* bandwidth) const override { return false; } + void SetReservedBitrate(uint32_t reserved_bitrate_bps) override {} + bool GetNetworkParameters(uint32_t* bitrate, + uint8_t* fraction_loss, + int64_t* rtt) override { + return false; + } + int64_t TimeUntilNextProcess() override { return 0; } + void Process() override {} +}; + void EventLogAnalyzer::CreateNetworkDelayFeedbackGraph(Plot* plot) { std::map outgoing_rtp; std::map incoming_rtcp; @@ -1073,7 +1102,8 @@ void EventLogAnalyzer::CreateNetworkDelayFeedbackGraph(Plot* plot) { } SimulatedClock clock(0); - TransportFeedbackAdapter feedback_adapter(&clock); + NullBitrateController null_controller; + TransportFeedbackAdapter feedback_adapter(&clock, &null_controller); TimeSeries time_series; time_series.label = "Network Delay Change"; diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 656aadce37..cce29558f1 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -1135,20 +1135,44 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { RunBaseTest(&test); } -TEST_F(VideoSendStreamTest, DISABLED_ChangingNetworkRoute) { +TEST_F(VideoSendStreamTest, ChangingNetworkRoute) { + static const int kStartBitrateBps = 300000; + static const int kNewMaxBitrateBps = 1234567; + static const uint8_t kExtensionId = 13; class ChangingNetworkRouteTest : public test::EndToEndTest { public: - const int kStartBitrateBps = 300000; - const int kNewMaxBitrateBps = 1234567; - ChangingNetworkRouteTest() - : EndToEndTest(test::CallTest::kDefaultTimeoutMs), - call_(nullptr) {} + : EndToEndTest(test::CallTest::kDefaultTimeoutMs), call_(nullptr) { + EXPECT_TRUE(parser_->RegisterRtpHeaderExtension( + kRtpExtensionTransportSequenceNumber, kExtensionId)); + } void OnCallsCreated(Call* sender_call, Call* receiver_call) override { call_ = sender_call; } + void ModifyVideoConfigs( + VideoSendStream::Config* send_config, + std::vector* receive_configs, + VideoEncoderConfig* encoder_config) override { + send_config->rtp.extensions.clear(); + send_config->rtp.extensions.push_back(RtpExtension( + RtpExtension::kTransportSequenceNumberUri, kExtensionId)); + (*receive_configs)[0].rtp.extensions = send_config->rtp.extensions; + (*receive_configs)[0].rtp.transport_cc = true; + } + + void ModifyAudioConfigs( + AudioSendStream::Config* send_config, + std::vector* receive_configs) override { + send_config->rtp.extensions.clear(); + send_config->rtp.extensions.push_back(RtpExtension( + RtpExtension::kTransportSequenceNumberUri, kExtensionId)); + (*receive_configs)[0].rtp.extensions.clear(); + (*receive_configs)[0].rtp.extensions = send_config->rtp.extensions; + (*receive_configs)[0].rtp.transport_cc = true; + } + Action OnSendRtp(const uint8_t* packet, size_t length) override { if (call_->GetStats().send_bandwidth_bps > kStartBitrateBps) { observation_complete_.Set();