Resolve cyclic dependency in remote bitrate estimator
Access SendTransportFeedback function through new interface to break rbe -> pacing -> rbe cycle
Depend on rtp_rtcp_format source set to break rbe -> rtp_rtcp -> rbe cycle.
Bug: webrtc:6828
Change-Id: Iae1c463a71871c0055485e2eca9b2235d770afec
Reviewed-on: https://webrtc-review.googlesource.com/1620
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19947}diff --git a/modules/remote_bitrate_estimator/BUILD.gn b/modules/remote_bitrate_estimator/BUILD.gn
index d29969a..4784a83 100644
--- a/modules/remote_bitrate_estimator/BUILD.gn
+++ b/modules/remote_bitrate_estimator/BUILD.gn
@@ -9,11 +9,6 @@
import("../../webrtc.gni")
rtc_static_library("remote_bitrate_estimator") {
- # TODO(mbonadei): Remove (bugs.webrtc.org/6828)
- # Errors on cyclic dependency with:
- # rtp_rtcp:rtp_rtcp if enabled.
- check_includes = false
-
sources = [
"aimd_rate_control.cc",
"aimd_rate_control.h",
@@ -51,7 +46,9 @@
deps = [
"../..:webrtc_common",
- "../../rtc_base:rtc_base",
+ "../../api:optional",
+ "../../modules:module_api",
+ "../../modules/rtp_rtcp:rtp_rtcp_format",
"../../rtc_base:rtc_base_approved",
"../../system_wrappers",
]
diff --git a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
index 31227e0..99f4ab8 100644
--- a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
+++ b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
@@ -23,6 +23,9 @@
#include "typedefs.h" // NOLINT(build/include)
namespace webrtc {
+namespace rtcp {
+class TransportFeedback;
+} // namespace rtcp
class Clock;
@@ -38,6 +41,12 @@
virtual ~RemoteBitrateObserver() {}
};
+class TransportFeedbackSenderInterface {
+ public:
+ virtual ~TransportFeedbackSenderInterface() = default;
+ virtual bool SendTransportFeedback(rtcp::TransportFeedback* packet) = 0;
+};
+
// TODO(holmer): Remove when all implementations have been updated.
struct ReceiveBandwidthEstimatorStats {};
diff --git a/modules/remote_bitrate_estimator/overuse_detector.cc b/modules/remote_bitrate_estimator/overuse_detector.cc
index 0042145..55c7b58 100644
--- a/modules/remote_bitrate_estimator/overuse_detector.cc
+++ b/modules/remote_bitrate_estimator/overuse_detector.cc
@@ -19,7 +19,6 @@
#include "modules/remote_bitrate_estimator/include/bwe_defines.h"
#include "modules/remote_bitrate_estimator/test/bwe_test_logging.h"
-#include "modules/rtp_rtcp/source/rtp_utility.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/safe_minmax.h"
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
index c521e53..ce8924d 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
@@ -14,7 +14,6 @@
#include <algorithm>
-#include "modules/pacing/paced_sender.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "rtc_base/checks.h"
#include "rtc_base/constructormagic.h"
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
index 0c459b3..0e6caff 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
@@ -13,8 +13,6 @@
#include <limits>
#include <algorithm>
-#include "modules/pacing/packet_router.h"
-#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
@@ -34,10 +32,11 @@
static constexpr int64_t kMaxTimeMs =
std::numeric_limits<int64_t>::max() / 1000;
-RemoteEstimatorProxy::RemoteEstimatorProxy(const Clock* clock,
- PacketRouter* packet_router)
+RemoteEstimatorProxy::RemoteEstimatorProxy(
+ const Clock* clock,
+ TransportFeedbackSenderInterface* feedback_sender)
: clock_(clock),
- packet_router_(packet_router),
+ feedback_sender_(feedback_sender),
last_process_time_ms_(-1),
media_ssrc_(0),
feedback_sequence_(0),
@@ -83,8 +82,8 @@
while (more_to_build) {
rtcp::TransportFeedback feedback_packet;
if (BuildFeedbackPacket(&feedback_packet)) {
- RTC_DCHECK(packet_router_ != nullptr);
- packet_router_->SendTransportFeedback(&feedback_packet);
+ RTC_DCHECK(feedback_sender_ != nullptr);
+ feedback_sender_->SendTransportFeedback(&feedback_packet);
} else {
more_to_build = false;
}
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/remote_estimator_proxy.h
index 7a69177..985ddd3 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h
@@ -32,7 +32,8 @@
class RemoteEstimatorProxy : public RemoteBitrateEstimator {
public:
- RemoteEstimatorProxy(const Clock* clock, PacketRouter* packet_router);
+ RemoteEstimatorProxy(const Clock* clock,
+ TransportFeedbackSenderInterface* feedback_sender);
virtual ~RemoteEstimatorProxy();
void IncomingPacket(int64_t arrival_time_ms,
@@ -58,7 +59,7 @@
bool BuildFeedbackPacket(rtcp::TransportFeedback* feedback_packet);
const Clock* const clock_;
- PacketRouter* const packet_router_;
+ TransportFeedbackSenderInterface* const feedback_sender_;
int64_t last_process_time_ms_;
rtc::CriticalSection lock_;
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
index 4f1d9e7..4197fcc 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
@@ -50,7 +50,7 @@
return timestamps;
}
-class MockPacketRouter : public PacketRouter {
+class MockTransportFeedbackSender : public TransportFeedbackSenderInterface {
public:
MOCK_METHOD1(SendTransportFeedback,
bool(rtcp::TransportFeedback* feedback_packet));
@@ -76,7 +76,7 @@
}
SimulatedClock clock_;
- testing::StrictMock<MockPacketRouter> router_;
+ testing::StrictMock<MockTransportFeedbackSender> router_;
RemoteEstimatorProxy proxy_;
};