Remove SendPacer from ViEEncoder and make sure SendPacer starts at a valid bitrate
This reverts commit e30c27205148b34ba421184efe65f6a0780b436d (https://codereview.webrtc.org/1958053002/)
Original reverted cl is in patch set #1.
Changes in following patch sets.
The cl now also make sure SendPacer starts with the configured bitrate provided in a call to CongestionController::SetBweBitrates)()
It turns out that the failing tests in 609816 is due to a bug in the current code that runs the proper at 300kbit regardless of configured start bitrate.
Original cl description:
Remove SendPacer from ViEEncoder
This CL moves the logic where the ViEEncoder pause if the pacer is full to the BitrateController. If the queue is full, the controller reports a bitrate of zero to Call (and BitrateAllocator)
BUG=chromium:609816, webrtc:5687
TBR=mflodman@webrtc.org
NOTRY=True // Due to bug in android_x86 cq builder....
Review-Url: https://codereview.webrtc.org/1958113003
Cr-Commit-Position: refs/heads/master@{#12688}
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
index 3c0d37c..09652d8 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
@@ -83,6 +83,10 @@
return new BitrateControllerImpl(clock, observer);
}
+BitrateController* BitrateController::CreateBitrateController(Clock* clock) {
+ return new BitrateControllerImpl(clock, nullptr);
+}
+
BitrateControllerImpl::BitrateControllerImpl(Clock* clock,
BitrateObserver* observer)
: clock_(clock),
@@ -94,8 +98,8 @@
last_fraction_loss_(0),
last_rtt_ms_(0),
last_reserved_bitrate_bps_(0) {
- // This calls the observer_, which means that the observer provided by the
- // user must be ready to accept a bitrate update when it constructs the
+ // This calls the observer_ if set, which means that the observer provided by
+ // the user must be ready to accept a bitrate update when it constructs the
// controller. We do this to avoid having to keep synchronized initial values
// in both the controller and the allocator.
MaybeTriggerOnNetworkChanged();
@@ -199,11 +203,15 @@
}
void BitrateControllerImpl::MaybeTriggerOnNetworkChanged() {
- uint32_t bitrate;
+ if (!observer_)
+ return;
+
+ uint32_t bitrate_bps;
uint8_t fraction_loss;
int64_t rtt;
- if (GetNetworkParameters(&bitrate, &fraction_loss, &rtt))
- observer_->OnNetworkChanged(bitrate, fraction_loss, rtt);
+
+ if (GetNetworkParameters(&bitrate_bps, &fraction_loss, &rtt))
+ observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt);
}
bool BitrateControllerImpl::GetNetworkParameters(uint32_t* bitrate,
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
index a966121..5a61379 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
@@ -28,6 +28,8 @@
class BitrateControllerImpl : public BitrateController {
public:
+ // TODO(perkj): BitrateObserver has been deprecated and is not used in WebRTC.
+ // |observer| is left for project that is not yet updated.
BitrateControllerImpl(Clock* clock, BitrateObserver* observer);
virtual ~BitrateControllerImpl() {}
@@ -50,6 +52,11 @@
void SetEventLog(RtcEventLog* event_log) override;
+ // Returns true if the parameters have changed since the last call.
+ bool GetNetworkParameters(uint32_t* bitrate,
+ uint8_t* fraction_loss,
+ int64_t* rtt) override;
+
int64_t TimeUntilNextProcess() override;
void Process() override;
@@ -64,20 +71,16 @@
int number_of_packets,
int64_t now_ms);
+ // Deprecated
void MaybeTriggerOnNetworkChanged();
- // Returns true if the parameters have changed since the last call.
- bool GetNetworkParameters(uint32_t* bitrate,
- uint8_t* fraction_loss,
- int64_t* rtt);
-
void OnNetworkChanged(uint32_t bitrate,
uint8_t fraction_loss, // 0 - 255.
int64_t rtt) EXCLUSIVE_LOCKS_REQUIRED(critsect_);
// Used by process thread.
- Clock* clock_;
- BitrateObserver* observer_;
+ Clock* const clock_;
+ BitrateObserver* const observer_;
int64_t last_bitrate_update_ms_;
rtc::CriticalSection critsect_;
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
index 3f467ef..4f92a38 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
@@ -14,11 +14,16 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
+#include "webrtc/modules/pacing/mock/mock_paced_sender.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
-using webrtc::RtcpBandwidthObserver;
-using webrtc::BitrateObserver;
+using ::testing::Exactly;
+using ::testing::Return;
+
using webrtc::BitrateController;
+using webrtc::BitrateObserver;
+using webrtc::PacedSender;
+using webrtc::RtcpBandwidthObserver;
uint8_t WeightedLoss(int num_packets1, uint8_t fraction_loss1,
int num_packets2, uint8_t fraction_loss2) {
diff --git a/webrtc/modules/bitrate_controller/include/bitrate_controller.h b/webrtc/modules/bitrate_controller/include/bitrate_controller.h
index d6cbc02..a61cf6a 100644
--- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h
+++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h
@@ -18,6 +18,7 @@
#include <map>
#include "webrtc/modules/include/module.h"
+#include "webrtc/modules/pacing/paced_sender.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
namespace webrtc {
@@ -26,6 +27,8 @@
class RtcEventLog;
struct PacketInfo;
+// Deprecated
+// TODO(perkj): Remove BitrateObserver when no implementations use it.
class BitrateObserver {
// Observer class for bitrate changes announced due to change in bandwidth
// estimate or due to bitrate allocation changes. Fraction loss and rtt is
@@ -46,10 +49,15 @@
// estimation and divide the available bitrate between all its registered
// BitrateObservers.
public:
- static const int kDefaultStartBitrateKbps = 300;
+ static const int kDefaultStartBitratebps = 300000;
+ // Deprecated:
+ // TODO(perkj): BitrateObserver has been deprecated and is not used in WebRTC.
+ // Remove this method once other other projects does not use it.
static BitrateController* CreateBitrateController(Clock* clock,
BitrateObserver* observer);
+ static BitrateController* CreateBitrateController(Clock* clock);
+
virtual ~BitrateController() {}
virtual RtcpBandwidthObserver* CreateRtcpBandwidthObserver() = 0;
@@ -71,6 +79,10 @@
virtual bool AvailableBandwidth(uint32_t* bandwidth) const = 0;
virtual void SetReservedBitrate(uint32_t reserved_bitrate_bps) = 0;
+
+ virtual bool GetNetworkParameters(uint32_t* bitrate,
+ uint8_t* fraction_loss,
+ int64_t* rtt) = 0;
};
} // namespace webrtc
#endif // WEBRTC_MODULES_BITRATE_CONTROLLER_INCLUDE_BITRATE_CONTROLLER_H_
diff --git a/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h b/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h
index 5290b01..da6169e 100644
--- a/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h
+++ b/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h
@@ -39,6 +39,8 @@
MOCK_METHOD1(SetEventLog, void(RtcEventLog* event_log));
MOCK_CONST_METHOD1(AvailableBandwidth, bool(uint32_t* bandwidth));
MOCK_METHOD1(SetReservedBitrate, void(uint32_t reserved_bitrate_bps));
+ MOCK_METHOD3(GetNetworkParameters,
+ bool(uint32_t* bitrate, uint8_t* fraction_loss, int64_t* rtt));
MOCK_METHOD0(Process, void());
MOCK_METHOD0(TimeUntilNextProcess, int64_t());