Reland: Fix race / crash in OnNetworkRouteChanged().
To achieve this some refactoring was done to make it possible to synchronize
access to the DelayBasedBwe in TransportFeedbackAdapter:
- The callback was removed from DelayBasedBwe, it now instead returns its
result.
- TransportFeedbackAdapter was moved to modules/congestion_controller to avoid
unnecessary dependencies.
Reenables previously disabled flaky test. Can no longer reproduce flakiness with gtest-parallel and asan/tsan builds.
BUG=webrtc:6427, webrtc:6422
R=terelius@webrtc.org
Review URL: https://codereview.webrtc.org/2378103005 .
Cr-Commit-Position: refs/heads/master@{#14452}
diff --git a/webrtc/modules/BUILD.gn b/webrtc/modules/BUILD.gn
index ccc681c..b3bfbac 100644
--- a/webrtc/modules/BUILD.gn
+++ b/webrtc/modules/BUILD.gn
@@ -375,6 +375,7 @@
"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 @@
"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 bc2f1f6..b336367 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
@@ -186,22 +186,18 @@
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<uint32_t>& 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 c8bb102..7ee6b19 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
@@ -61,10 +61,7 @@
uint8_t* fraction_loss,
int64_t* rtt) override;
- // RemoteBitrateObserver overrides.
- void OnReceiveBitrateChanged(const std::vector<uint32_t>& 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 7da947b..4b298cc 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
@@ -169,7 +169,8 @@
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 90b6471..7400d7d 100644
--- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h
+++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h
@@ -17,9 +17,9 @@
#include <map>
+#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 @@
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 @@
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 758fba5..1069d62 100644
--- a/webrtc/modules/congestion_controller/BUILD.gn
+++ b/webrtc/modules/congestion_controller/BUILD.gn
@@ -18,6 +18,8 @@
"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 @@
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 c0ec3da..f7e7e56 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 @@
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 @@
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() {}
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 @@
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 @@
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 5c7ddf2..3fcddfb 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 87dc502..34b8242 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 @@
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 @@
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<PacketInfo>& packet_feedback_vector) {
RTC_DCHECK(network_thread_.CalledOnValidThread());
if (!uma_recorded_) {
@@ -63,87 +60,87 @@
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) {
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<uint32_t>(
+ ((static_cast<uint64_t>(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<uint32_t>(((static_cast<uint64_t>(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<uint32_t> 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<uint32_t> 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<uint32_t> 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 +157,10 @@
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<uint32_t>* ssrcs,
uint32_t* bitrate_bps) const {
// Currently accessed from both the process thread (see
@@ -182,7 +169,6 @@
// thread.
RTC_DCHECK(ssrcs);
RTC_DCHECK(bitrate_bps);
- rtc::CritScope lock(&crit_);
if (!remote_rate_.ValidEstimate())
return false;
@@ -194,7 +180,6 @@
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 3e0a014..7e2c8bd 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<PacketInfo>& packet_feedback_vector) override;
- void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
+ Result IncomingPacketFeedbackVector(
+ const std::vector<PacketInfo>& packet_feedback_vector);
+ void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms);
bool LatestEstimate(std::vector<uint32_t>* 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<InterArrival> inter_arrival_;
std::unique_ptr<OveruseEstimator> estimator_;
OveruseDetector detector_;
@@ -74,10 +69,8 @@
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 7220967..e751013 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 3469251..a3a1893 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 @@
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 @@
sequence_number, payload_size, probe_cluster_id);
std::vector<PacketInfo> 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 @@
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 @@
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 @@
int64_t send_time_ms = 0;
uint16_t sequence_number = 0;
std::vector<uint32_t> 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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
sequence_number++, 1000);
clock_.AdvanceTimeMilliseconds(kFrameIntervalMs);
send_time_ms += kFrameIntervalMs;
- bitrate_estimator_->Process();
}
uint32_t bitrate_before = 0;
std::vector<uint32_t> 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 f97cd52..add9fb3 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 @@
static const uint32_t kDefaultSsrc;
SimulatedClock clock_; // Time at the receiver.
- std::unique_ptr<test::TestBitrateObserver> bitrate_observer_;
- std::unique_ptr<RemoteBitrateEstimator> bitrate_estimator_;
+ test::TestBitrateObserver bitrate_observer_;
+ DelayBasedBwe bitrate_estimator_;
std::unique_ptr<test::StreamGenerator> 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 862f2cd..eb6db33 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 66ef7f0..4663906 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 <algorithm>
#include <limits>
#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 @@
};
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 @@
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<PacketInfo> TransportFeedbackAdapter::GetPacketFeedbackVector(
const rtcp::TransportFeedback& feedback) {
int64_t timestamp_us = feedback.GetBaseTimeUs();
@@ -129,9 +134,14 @@
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_);
+ result = delay_based_bwe_->IncomingPacketFeedbackVector(
last_packet_feedback_vector_);
+ }
+ if (result.updated)
+ bitrate_controller_->OnDelayBasedBweResult(result);
}
std::vector<PacketInfo> TransportFeedbackAdapter::GetTransportFeedbackVector()
@@ -141,8 +151,8 @@
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 2db6603..6422736 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 <memory>
#include <vector>
#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<PacketInfo> 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<PacketInfo> GetPacketFeedbackVector(
const rtcp::TransportFeedback& feedback);
rtc::CriticalSection lock_;
+ rtc::CriticalSection bwe_lock_;
SendTimeHistory send_time_history_ GUARDED_BY(&lock_);
- std::unique_ptr<RemoteBitrateEstimator> bitrate_estimator_;
+ std::unique_ptr<DelayBasedBwe> 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<PacketInfo> 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 b312f96..0c77fe9 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 @@
class TransportFeedbackAdapterTest : public ::testing::Test {
public:
TransportFeedbackAdapterTest()
- : clock_(0),
- bitrate_estimator_(nullptr),
- bitrate_controller_(this),
- receiver_estimated_bitrate_(0) {}
+ : clock_(0), bitrate_controller_(this), target_bitrate_bps_(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 @@
~MockBitrateControllerAdapter() override {}
- void OnReceiveBitrateChanged(const std::vector<uint32_t>& ssrcs,
- uint32_t bitrate_bps) override {
- owner_->receiver_estimated_bitrate_ = bitrate_bps;
+ void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override {
+ owner_->target_bitrate_bps_ = result.target_bitrate_bps;
}
TransportFeedbackAdapterTest* const owner_;
@@ -106,11 +97,10 @@
}
SimulatedClock clock_;
- MockRemoteBitrateEstimator* bitrate_estimator_;
MockBitrateControllerAdapter bitrate_controller_;
std::unique_ptr<TransportFeedbackAdapter> adapter_;
- uint32_t receiver_estimated_bitrate_;
+ uint32_t target_bitrate_bps_;
};
TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) {
@@ -135,13 +125,8 @@
feedback.Build();
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke(
- [packets, this](const std::vector<PacketInfo>& feedback_vector) {
- ComparePacketVectors(packets, feedback_vector);
- }));
adapter_->OnTransportFeedback(feedback);
+ ComparePacketVectors(packets, adapter_->GetTransportFeedbackVector());
}
TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) {
@@ -177,13 +162,9 @@
packets.begin() + kSendSideDropBefore,
packets.begin() + kReceiveSideDropAfter + 1);
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke([expected_packets,
- this](const std::vector<PacketInfo>& feedback_vector) {
- ComparePacketVectors(expected_packets, feedback_vector);
- }));
adapter_->OnTransportFeedback(feedback);
+ ComparePacketVectors(expected_packets,
+ adapter_->GetTransportFeedbackVector());
}
TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) {
@@ -217,13 +198,9 @@
std::vector<PacketInfo> expected_packets;
expected_packets.push_back(packets[i]);
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke([expected_packets, this](
- const std::vector<PacketInfo>& feedback_vector) {
- ComparePacketVectors(expected_packets, feedback_vector);
- }));
adapter_->OnTransportFeedback(*feedback.get());
+ ComparePacketVectors(expected_packets,
+ adapter_->GetTransportFeedbackVector());
}
}
@@ -251,13 +228,9 @@
feedback.Build();
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke([expected_packets,
- this](const std::vector<PacketInfo>& feedback_vector) {
- ComparePacketVectors(expected_packets, feedback_vector);
- }));
adapter_->OnTransportFeedback(feedback);
+ ComparePacketVectors(expected_packets,
+ adapter_->GetTransportFeedbackVector());
}
TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
@@ -294,14 +267,6 @@
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<PacketInfo> 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 @@
std::vector<PacketInfo> received_feedback;
EXPECT_TRUE(feedback.get() != nullptr);
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke([expected_packets, &received_feedback](
- const std::vector<PacketInfo>& 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<PacketInfo> 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,41 @@
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<PacketInfo>& feedback_vector) {
- EXPECT_EQ(1u, feedback_vector.size());
- received_feedback.push_back(feedback_vector[0]);
- }));
adapter_->OnTransportFeedback(*feedback.get());
+ {
+ std::vector<PacketInfo> expected_packets;
+ expected_packets.push_back(info);
+ ComparePacketVectors(expected_packets,
+ adapter_->GetTransportFeedbackVector());
+ }
+}
- expected_packets.push_back(info);
-
- ComparePacketVectors(expected_packets, received_feedback);
+TEST_F(TransportFeedbackAdapterTest, UpdatesDelayBasedEstimate) {
+ uint16_t seq_num = 0;
+ size_t kPayloadSize = 1000;
+ // The test must run and insert packets/feedback long enough that the
+ // BWE computes a valid estimate.
+ const int64_t kRunTimeMs = 6000;
+ int64_t start_time_ms = clock_.TimeInMilliseconds();
+ while (clock_.TimeInMilliseconds() - start_time_ms < kRunTimeMs) {
+ PacketInfo packet(clock_.TimeInMilliseconds(), clock_.TimeInMilliseconds(),
+ seq_num, kPayloadSize, PacketInfo::kNotAProbe);
+ OnSentPacket(packet);
+ // Create expected feedback and send into adapter.
+ std::unique_ptr<rtcp::TransportFeedback> feedback(
+ new rtcp::TransportFeedback());
+ feedback->SetBase(packet.sequence_number, packet.arrival_time_ms * 1000);
+ EXPECT_TRUE(feedback->AddReceivedPacket(packet.sequence_number,
+ packet.arrival_time_ms * 1000));
+ rtc::Buffer raw_packet = feedback->Build();
+ feedback = rtcp::TransportFeedback::ParseFrom(raw_packet.data(),
+ raw_packet.size());
+ EXPECT_TRUE(feedback.get() != nullptr);
+ adapter_->OnTransportFeedback(*feedback.get());
+ clock_.AdvanceTimeMilliseconds(50);
+ ++seq_num;
+ }
+ EXPECT_GT(target_bitrate_bps_, 0u);
}
} // namespace test
diff --git a/webrtc/modules/remote_bitrate_estimator/BUILD.gn b/webrtc/modules/remote_bitrate_estimator/BUILD.gn
index ea0e214..2a7c68f 100644
--- a/webrtc/modules/remote_bitrate_estimator/BUILD.gn
+++ b/webrtc/modules/remote_bitrate_estimator/BUILD.gn
@@ -30,8 +30,6 @@
"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 3a2e9ea..e719f2f 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 cc33de1..507106a 100644
--- a/webrtc/tools/DEPS
+++ b/webrtc/tools/DEPS
@@ -4,6 +4,7 @@
"+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 260975b..d1dec22 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 @@
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<uint64_t, const LoggedRtpPacket*> outgoing_rtp;
std::map<uint64_t, const LoggedRtcpPacket*> incoming_rtcp;
@@ -1073,7 +1102,8 @@
}
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 656aadc..cce2955 100644
--- a/webrtc/video/video_send_stream_tests.cc
+++ b/webrtc/video/video_send_stream_tests.cc
@@ -1135,20 +1135,44 @@
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<VideoReceiveStream::Config>* 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<AudioReceiveStream::Config>* 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();
