Revert of Fix race / crash in OnNetworkRouteChanged(). (patchset #5 id:80001 of https://codereview.webrtc.org/2366333003/ )
Reason for revert:
Caused issues with webrtc_perf_tests on build bots.
Original issue's description:
> 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
>
> Committed: https://crrev.com/fd0d42669204e6dd92a60736bca7ae0196663024
> Cr-Commit-Position: refs/heads/master@{#14430}
TBR=terelius@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6427, webrtc:6422
Review-Url: https://codereview.webrtc.org/2377303002
Cr-Commit-Position: refs/heads/master@{#14433}
diff --git a/webrtc/modules/BUILD.gn b/webrtc/modules/BUILD.gn
index b3bfbac..ccc681c 100644
--- a/webrtc/modules/BUILD.gn
+++ b/webrtc/modules/BUILD.gn
@@ -375,7 +375,6 @@
"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",
@@ -395,6 +394,7 @@
"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 b336367..bc2f1f6 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
@@ -186,18 +186,22 @@
MaybeTriggerOnNetworkChanged();
}
-void BitrateControllerImpl::OnDelayBasedBweResult(
- const DelayBasedBwe::Result& result) {
- if (!result.updated)
- return;
+void BitrateControllerImpl::OnProbeBitrate(uint32_t bitrate_bps) {
{
rtc::CritScope cs(&critsect_);
- if (result.probe) {
- bandwidth_estimation_.SetSendBitrate(result.target_bitrate_bps);
- } else {
- bandwidth_estimation_.UpdateDelayBasedEstimate(
- clock_->TimeInMilliseconds(), result.target_bitrate_bps);
- }
+ 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);
}
MaybeTriggerOnNetworkChanged();
}
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
index 7ee6b19..c8bb102 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
@@ -61,7 +61,10 @@
uint8_t* fraction_loss,
int64_t* rtt) override;
- void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override;
+ // RemoteBitrateObserver overrides.
+ void OnReceiveBitrateChanged(const std::vector<uint32_t>& ssrcs,
+ uint32_t bitrate_bps) override;
+ void OnProbeBitrate(uint32_t bitrate_bps) 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 4b298cc..7da947b 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
@@ -169,8 +169,7 @@
EXPECT_EQ(300000, bitrate_observer_.last_bitrate_);
// Test that a low delay-based estimate limits the combined estimate.
- webrtc::DelayBasedBwe::Result result(false, 280000);
- controller_->OnDelayBasedBweResult(result);
+ controller_->OnReceiveBitrateChanged({0}, 280000);
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 7400d7d..90b6471 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 {
+class BitrateController : public Module, public RemoteBitrateObserver {
// 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,8 +78,6 @@
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 1069d62..758fba5 100644
--- a/webrtc/modules/congestion_controller/BUILD.gn
+++ b/webrtc/modules/congestion_controller/BUILD.gn
@@ -18,8 +18,6 @@
"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.
@@ -34,5 +32,6 @@
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 f7e7e56..c0ec3da 100644
--- a/webrtc/modules/congestion_controller/congestion_controller.cc
+++ b/webrtc/modules/congestion_controller/congestion_controller.cc
@@ -21,6 +21,7 @@
#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"
@@ -170,7 +171,7 @@
retransmission_rate_limiter_(
new RateLimiter(clock, kRetransmitWindowSizeMs)),
remote_estimator_proxy_(clock_, packet_router_.get()),
- transport_feedback_adapter_(clock_, bitrate_controller_.get()),
+ transport_feedback_adapter_(clock_),
min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
max_bitrate_bps_(0),
last_reported_bitrate_bps_(0),
@@ -201,7 +202,7 @@
retransmission_rate_limiter_(
new RateLimiter(clock, kRetransmitWindowSizeMs)),
remote_estimator_proxy_(clock_, packet_router_.get()),
- transport_feedback_adapter_(clock_, bitrate_controller_.get()),
+ transport_feedback_adapter_(clock_),
min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
max_bitrate_bps_(0),
last_reported_bitrate_bps_(0),
@@ -214,8 +215,10 @@
CongestionController::~CongestionController() {}
void CongestionController::Init() {
- transport_feedback_adapter_.InitBwe();
- transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps_);
+ transport_feedback_adapter_.SetBitrateEstimator(
+ new DelayBasedBwe(bitrate_controller_.get(), clock_));
+ transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate(
+ min_bitrate_bps_);
}
void CongestionController::SetBweBitrates(int min_bitrate_bps,
@@ -233,7 +236,8 @@
if (remote_bitrate_estimator_)
remote_bitrate_estimator_->SetMinBitrate(min_bitrate_bps);
min_bitrate_bps_ = min_bitrate_bps;
- transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps_);
+ transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate(
+ min_bitrate_bps_);
MaybeTriggerOnNetworkChanged();
}
@@ -252,8 +256,10 @@
if (remote_bitrate_estimator_)
remote_bitrate_estimator_->SetMinBitrate(min_bitrate_bps);
- transport_feedback_adapter_.InitBwe();
- transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps);
+ RemoteBitrateEstimator* rbe =
+ new DelayBasedBwe(bitrate_controller_.get(), clock_);
+ transport_feedback_adapter_.SetBitrateEstimator(rbe);
+ rbe->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 3fcddfb..5c7ddf2 100644
--- a/webrtc/modules/congestion_controller/congestion_controller.gypi
+++ b/webrtc/modules/congestion_controller/congestion_controller.gypi
@@ -14,6 +14,7 @@
'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',
@@ -24,8 +25,6 @@
'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 db3ae58..87dc502 100644
--- a/webrtc/modules/congestion_controller/delay_based_bwe.cc
+++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc
@@ -21,6 +21,7 @@
#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"
@@ -39,8 +40,9 @@
namespace webrtc {
-DelayBasedBwe::DelayBasedBwe(Clock* clock)
+DelayBasedBwe::DelayBasedBwe(RemoteBitrateObserver* observer, Clock* clock)
: clock_(clock),
+ observer_(observer),
inter_arrival_(),
estimator_(),
detector_(OverUseDetectorOptions()),
@@ -48,10 +50,11 @@
last_update_ms_(-1),
last_seen_packet_ms_(-1),
uma_recorded_(false) {
+ RTC_DCHECK(observer_);
network_thread_.DetachFromThread();
}
-DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector(
+void DelayBasedBwe::IncomingPacketFeedbackVector(
const std::vector<PacketInfo>& packet_feedback_vector) {
RTC_DCHECK(network_thread_.CalledOnValidThread());
if (!uma_recorded_) {
@@ -60,89 +63,87 @@
BweNames::kBweNamesMax);
uma_recorded_ = true;
}
- Result aggregated_result;
for (const auto& packet_info : packet_feedback_vector) {
- Result result = IncomingPacketInfo(packet_info);
- if (result.updated)
- aggregated_result = result;
+ IncomingPacketInfo(packet_info);
}
- return aggregated_result;
}
-DelayBasedBwe::Result DelayBasedBwe::IncomingPacketInfo(
- const PacketInfo& info) {
- // printf("Acked: %ld\n", info.payload_size);
+void DelayBasedBwe::IncomingPacketInfo(const PacketInfo& info) {
int64_t now_ms = clock_->TimeInMilliseconds();
incoming_bitrate_.Update(info.payload_size, info.arrival_time_ms);
- 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;
+ bool delay_based_bwe_changed = false;
+ uint32_t target_bitrate_bps = 0;
+ {
+ rtc::CritScope lock(&crit_);
- 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)) {
- result.updated = UpdateEstimate(info.arrival_time_ms, now_ms,
- &result.target_bitrate_bps);
+ // 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()));
}
- } 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;
+ last_seen_packet_ms_ = now_ms;
- return result;
+ 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);
+ }
+ }
+
+ if (delay_based_bwe_changed) {
+ last_update_ms_ = now_ms;
+ observer_->OnReceiveBitrateChanged({kFixedSsrc}, target_bitrate_bps);
+ }
}
bool DelayBasedBwe::UpdateEstimate(int64_t arrival_time_ms,
@@ -159,10 +160,20 @@
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
@@ -171,6 +182,7 @@
// thread.
RTC_DCHECK(ssrcs);
RTC_DCHECK(bitrate_bps);
+ rtc::CritScope lock(&crit_);
if (!remote_rate_.ValidEstimate())
return false;
@@ -182,6 +194,7 @@
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 7e2c8bd..3e0a014 100644
--- a/webrtc/modules/congestion_controller/delay_based_bwe.h
+++ b/webrtc/modules/congestion_controller/delay_based_bwe.h
@@ -18,6 +18,7 @@
#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"
@@ -26,42 +27,46 @@
#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 {
+class DelayBasedBwe : public RemoteBitrateEstimator {
public:
- 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);
+ DelayBasedBwe(RemoteBitrateObserver* observer, Clock* clock);
virtual ~DelayBasedBwe() {}
- Result IncomingPacketFeedbackVector(
- const std::vector<PacketInfo>& packet_feedback_vector);
- void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms);
+ void IncomingPacketFeedbackVector(
+ const std::vector<PacketInfo>& packet_feedback_vector) override;
+ void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
bool LatestEstimate(std::vector<uint32_t>* ssrcs,
- uint32_t* bitrate_bps) const;
- void SetMinBitrate(int min_bitrate_bps);
+ 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();
+ }
private:
- Result IncomingPacketInfo(const PacketInfo& info);
+ void 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);
+ uint32_t* target_bitrate_bps)
+ EXCLUSIVE_LOCKS_REQUIRED(crit_);
rtc::ThreadChecker network_thread_;
Clock* const clock_;
+ RemoteBitrateObserver* const observer_;
std::unique_ptr<InterArrival> inter_arrival_;
std::unique_ptr<OveruseEstimator> estimator_;
OveruseDetector detector_;
@@ -69,8 +74,10 @@
int64_t last_update_ms_;
int64_t last_seen_packet_ms_;
bool uma_recorded_;
- AimdRateControl remote_rate_;
- ProbeBitrateEstimator probe_bitrate_estimator_;
+
+ rtc::CriticalSection crit_;
+ AimdRateControl remote_rate_ GUARDED_BY(&crit_);
+ ProbeBitrateEstimator probe_bitrate_estimator_ GUARDED_BY(&crit_);
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 e751013..7220967 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 a3a1893..3469251 100644
--- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc
+++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc
@@ -150,7 +150,8 @@
DelayBasedBweTest::DelayBasedBweTest()
: clock_(100000000),
- bitrate_estimator_(&clock_),
+ bitrate_observer_(new test::TestBitrateObserver),
+ bitrate_estimator_(new DelayBasedBwe(bitrate_observer_.get(), &clock_)),
stream_generator_(new test::StreamGenerator(1e6, // Capacity.
clock_.TimeInMicroseconds())),
arrival_time_offset_ms_(0) {}
@@ -181,13 +182,7 @@
sequence_number, payload_size, probe_cluster_id);
std::vector<PacketInfo> packets;
packets.push_back(packet);
- DelayBasedBwe::Result result =
- bitrate_estimator_.IncomingPacketFeedbackVector(packets);
- const uint32_t kDummySsrc = 0;
- if (result.updated) {
- bitrate_observer_.OnReceiveBitrateChanged({kDummySsrc},
- result.target_bitrate_bps);
- }
+ bitrate_estimator_->IncomingPacketFeedbackVector(packets);
}
// Generates a frame of packets belonging to a stream at a given bitrate and
@@ -206,20 +201,17 @@
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_;
}
- 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)
+ bitrate_estimator_->IncomingPacketFeedbackVector(packets);
+
+ if (bitrate_observer_->updated()) {
+ if (bitrate_observer_->latest_bitrate() < bitrate_bps)
overuse = true;
}
@@ -243,13 +235,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;
@@ -267,12 +259,13 @@
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);
- EXPECT_FALSE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps));
- EXPECT_FALSE(bitrate_observer_.updated());
- bitrate_observer_.Reset();
+ bitrate_estimator_->Process();
+ 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) {
@@ -281,23 +274,25 @@
int cluster_id = i < kInitialProbingPackets ? 0 : PacketInfo::kNotAProbe;
if (i == kNumInitialPackets) {
- EXPECT_FALSE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps));
+ bitrate_estimator_->Process();
+ 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;
}
- EXPECT_TRUE(bitrate_estimator_.LatestEstimate(&ssrcs, &bitrate_bps));
+ bitrate_estimator_->Process();
+ 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(
@@ -316,16 +311,17 @@
// as it doesn't do anything in Process().
if (i == kNumInitialPackets) {
// Process after we have enough frames to get a valid input rate estimate.
-
- EXPECT_FALSE(bitrate_observer_.updated()); // No valid estimate.
+ bitrate_estimator_->Process();
+ 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;
}
- EXPECT_TRUE(bitrate_observer_.updated());
- EXPECT_NEAR(expected_bitrate_bps, bitrate_observer_.latest_bitrate(),
+ bitrate_estimator_->Process();
+ 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);
@@ -337,8 +333,9 @@
1000);
sequence_number += 2;
}
- EXPECT_TRUE(bitrate_observer_.updated());
- EXPECT_NEAR(expected_bitrate_bps, bitrate_observer_.latest_bitrate(),
+ bitrate_estimator_->Process();
+ EXPECT_TRUE(bitrate_observer_->updated());
+ EXPECT_NEAR(expected_bitrate_bps, bitrate_observer_->latest_bitrate(),
kAcceptedBitrateErrorBps);
}
@@ -356,15 +353,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);
}
@@ -408,7 +405,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;
@@ -420,11 +417,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,
@@ -442,11 +439,12 @@
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.
@@ -463,10 +461,11 @@
// 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) {
@@ -480,22 +479,25 @@
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 add9fb3..f97cd52 100644
--- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h
+++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.h
@@ -19,7 +19,6 @@
#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"
@@ -161,8 +160,8 @@
static const uint32_t kDefaultSsrc;
SimulatedClock clock_; // Time at the receiver.
- test::TestBitrateObserver bitrate_observer_;
- DelayBasedBwe bitrate_estimator_;
+ std::unique_ptr<test::TestBitrateObserver> bitrate_observer_;
+ std::unique_ptr<RemoteBitrateEstimator> 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 eb6db33..862f2cd 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/BUILD.gn b/webrtc/modules/remote_bitrate_estimator/BUILD.gn
index 2a7c68f..ea0e214 100644
--- a/webrtc/modules/remote_bitrate_estimator/BUILD.gn
+++ b/webrtc/modules/remote_bitrate_estimator/BUILD.gn
@@ -30,6 +30,8 @@
"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 e719f2f..3a2e9ea 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi
@@ -38,6 +38,8 @@
'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/modules/congestion_controller/transport_feedback_adapter.cc b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc
similarity index 82%
rename from webrtc/modules/congestion_controller/transport_feedback_adapter.cc
rename to webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc
index 8875806..66ef7f0 100644
--- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc
+++ b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc
@@ -8,15 +8,14 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include "webrtc/modules/congestion_controller/transport_feedback_adapter.h"
+#include "webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h"
#include <algorithm>
#include <limits>
#include "webrtc/base/checks.h"
#include "webrtc/base/logging.h"
-#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
-#include "webrtc/modules/congestion_controller/delay_based_bwe.h"
+#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "webrtc/modules/utility/include/process_thread.h"
@@ -40,19 +39,20 @@
};
TransportFeedbackAdapter::TransportFeedbackAdapter(
- Clock* clock,
- BitrateController* bitrate_controller)
+ Clock* clock)
: send_time_history_(clock, kSendTimeHistoryWindowMs),
clock_(clock),
current_offset_ms_(kNoTimestamp),
- last_timestamp_us_(kNoTimestamp),
- bitrate_controller_(bitrate_controller) {}
+ last_timestamp_us_(kNoTimestamp) {}
-TransportFeedbackAdapter::~TransportFeedbackAdapter() {}
+TransportFeedbackAdapter::~TransportFeedbackAdapter() {
+}
-void TransportFeedbackAdapter::InitBwe() {
- rtc::CritScope cs(&bwe_lock_);
- delay_based_bwe_.reset(new DelayBasedBwe(clock_));
+void TransportFeedbackAdapter::SetBitrateEstimator(
+ RemoteBitrateEstimator* rbe) {
+ if (bitrate_estimator_.get() != rbe) {
+ bitrate_estimator_.reset(rbe);
+ }
}
void TransportFeedbackAdapter::AddPacket(uint16_t sequence_number,
@@ -68,11 +68,6 @@
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();
@@ -134,14 +129,9 @@
void TransportFeedbackAdapter::OnTransportFeedback(
const rtcp::TransportFeedback& feedback) {
last_packet_feedback_vector_ = GetPacketFeedbackVector(feedback);
- DelayBasedBwe::Result result;
- {
- rtc::CritScope cs(&bwe_lock_);
- delay_based_bwe_->IncomingPacketFeedbackVector(
+ if (bitrate_estimator_.get())
+ bitrate_estimator_->IncomingPacketFeedbackVector(
last_packet_feedback_vector_);
- }
- if (result.updated)
- bitrate_controller_->OnDelayBasedBweResult(result);
}
std::vector<PacketInfo> TransportFeedbackAdapter::GetTransportFeedbackVector()
@@ -151,8 +141,8 @@
void TransportFeedbackAdapter::OnRttUpdate(int64_t avg_rtt_ms,
int64_t max_rtt_ms) {
- rtc::CritScope cs(&bwe_lock_);
- delay_based_bwe_->OnRttUpdate(avg_rtt_ms, max_rtt_ms);
+ RTC_DCHECK(bitrate_estimator_.get() != nullptr);
+ bitrate_estimator_->OnRttUpdate(avg_rtt_ms, max_rtt_ms);
}
} // namespace webrtc
diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.h b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h
similarity index 67%
rename from webrtc/modules/congestion_controller/transport_feedback_adapter.h
rename to webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h
index 6422736..2db6603 100644
--- a/webrtc/modules/congestion_controller/transport_feedback_adapter.h
+++ b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h
@@ -8,63 +8,58 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_
-#define WEBRTC_MODULES_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_
+#ifndef WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TRANSPORT_FEEDBACK_ADAPTER_H_
+#define WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_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:
- TransportFeedbackAdapter(Clock* clock, BitrateController* bitrate_controller);
+ explicit TransportFeedbackAdapter(Clock* clock);
virtual ~TransportFeedbackAdapter();
- void InitBwe();
+ void SetBitrateEstimator(RemoteBitrateEstimator* rbe);
+ RemoteBitrateEstimator* GetBitrateEstimator() const {
+ return bitrate_estimator_.get();
+ }
+
// 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<DelayBasedBwe> delay_based_bwe_ GUARDED_BY(&bwe_lock_);
+ std::unique_ptr<RemoteBitrateEstimator> bitrate_estimator_;
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_CONGESTION_CONTROLLER_TRANSPORT_FEEDBACK_ADAPTER_H_
+#endif // WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TRANSPORT_FEEDBACK_ADAPTER_H_
diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc
similarity index 77%
rename from webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
rename to webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc
index 60c8514..e2755b3 100644
--- a/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
+++ b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc
@@ -12,12 +12,13 @@
#include <memory>
#include <vector>
-#include "webrtc/test/gmock.h"
-#include "webrtc/test/gtest.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
#include "webrtc/base/checks.h"
#include "webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h"
-#include "webrtc/modules/congestion_controller/transport_feedback_adapter.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/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"
@@ -31,16 +32,23 @@
class TransportFeedbackAdapterTest : public ::testing::Test {
public:
TransportFeedbackAdapterTest()
- : clock_(0), bitrate_controller_(this), receiver_estimated_bitrate_(0) {}
+ : clock_(0),
+ bitrate_estimator_(nullptr),
+ bitrate_controller_(this),
+ receiver_estimated_bitrate_(0) {}
virtual ~TransportFeedbackAdapterTest() {}
virtual void SetUp() {
- adapter_.reset(new TransportFeedbackAdapter(&clock_, &bitrate_controller_));
- adapter_->InitBwe();
+ adapter_.reset(new TransportFeedbackAdapter(&clock_));
+
+ bitrate_estimator_ = new MockRemoteBitrateEstimator();
+ adapter_->SetBitrateEstimator(bitrate_estimator_);
}
- virtual void TearDown() { adapter_.reset(); }
+ virtual void TearDown() {
+ adapter_.reset();
+ }
protected:
// Proxy class used since TransportFeedbackAdapter will own the instance
@@ -52,8 +60,9 @@
~MockBitrateControllerAdapter() override {}
- void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override {
- owner_->receiver_estimated_bitrate_ = result.target_bitrate_bps;
+ void OnReceiveBitrateChanged(const std::vector<uint32_t>& ssrcs,
+ uint32_t bitrate_bps) override {
+ owner_->receiver_estimated_bitrate_ = bitrate_bps;
}
TransportFeedbackAdapterTest* const owner_;
@@ -97,6 +106,7 @@
}
SimulatedClock clock_;
+ MockRemoteBitrateEstimator* bitrate_estimator_;
MockBitrateControllerAdapter bitrate_controller_;
std::unique_ptr<TransportFeedbackAdapter> adapter_;
@@ -125,8 +135,13 @@
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) {
@@ -162,9 +177,13 @@
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) {
@@ -198,9 +217,13 @@
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());
}
}
@@ -228,9 +251,13 @@
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) {
@@ -267,6 +294,14 @@
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);
@@ -292,18 +327,14 @@
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());
@@ -315,13 +346,18 @@
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);
}
} // namespace test
diff --git a/webrtc/tools/DEPS b/webrtc/tools/DEPS
index 507106a..cc33de1 100644
--- a/webrtc/tools/DEPS
+++ b/webrtc/tools/DEPS
@@ -4,7 +4,6 @@
"+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 d1dec22..260975b 100644
--- a/webrtc/tools/event_log_visualizer/analyzer.cc
+++ b/webrtc/tools/event_log_visualizer/analyzer.cc
@@ -24,7 +24,6 @@
#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"
@@ -1054,34 +1053,6 @@
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;
@@ -1102,8 +1073,7 @@
}
SimulatedClock clock(0);
- NullBitrateController null_controller;
- TransportFeedbackAdapter feedback_adapter(&clock, &null_controller);
+ TransportFeedbackAdapter feedback_adapter(&clock);
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 cce2955..656aadc 100644
--- a/webrtc/video/video_send_stream_tests.cc
+++ b/webrtc/video/video_send_stream_tests.cc
@@ -1135,44 +1135,20 @@
RunBaseTest(&test);
}
-TEST_F(VideoSendStreamTest, ChangingNetworkRoute) {
- static const int kStartBitrateBps = 300000;
- static const int kNewMaxBitrateBps = 1234567;
- static const uint8_t kExtensionId = 13;
+TEST_F(VideoSendStreamTest, DISABLED_ChangingNetworkRoute) {
class ChangingNetworkRouteTest : public test::EndToEndTest {
public:
+ const int kStartBitrateBps = 300000;
+ const int kNewMaxBitrateBps = 1234567;
+
ChangingNetworkRouteTest()
- : EndToEndTest(test::CallTest::kDefaultTimeoutMs), call_(nullptr) {
- EXPECT_TRUE(parser_->RegisterRtpHeaderExtension(
- kRtpExtensionTransportSequenceNumber, kExtensionId));
- }
+ : EndToEndTest(test::CallTest::kDefaultTimeoutMs),
+ call_(nullptr) {}
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();
