zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove SendPacer from ViEEncoder and make sure SendPacer starts at a valid bitrate

Browse Source 
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}
This commit is contained in:
perkj
2016-05-11 06:01:13 -07:00
committed by Commit bot
parent 2f5ae66471
commit ec81bcd519
29 changed files with 575 additions and 372 deletions
Download Patch File Download Diff File Expand all files Collapse all files

104
webrtc/modules/congestion_controller/congestion_controller.cc
View File

@ -20,7 +20,6 @@
#include "webrtc/base/socket.h"
#include "webrtc/base/thread_annotations.h"
#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
#include "webrtc/modules/pacing/paced_sender.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"
#include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h"
@ -141,30 +140,71 @@ CongestionController::CongestionController(
BitrateObserver* bitrate_observer,
RemoteBitrateObserver* remote_bitrate_observer)
: clock_(clock),
observer_(nullptr),
packet_router_(new PacketRouter()),
pacer_(new PacedSender(clock_,
&packet_router_,
BitrateController::kDefaultStartBitrateKbps,
PacedSender::kDefaultPaceMultiplier *
BitrateController::kDefaultStartBitrateKbps,
0)),
packet_router_.get())),
remote_bitrate_estimator_(
new WrappingBitrateEstimator(remote_bitrate_observer, clock_)),
bitrate_controller_(
BitrateController::CreateBitrateController(clock_, bitrate_observer)),
remote_estimator_proxy_(clock_, packet_router_.get()),
transport_feedback_adapter_(bitrate_controller_.get(), clock_),
min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
send_queue_is_full_(false) {
Init();
}
CongestionController::CongestionController(
Clock* clock,
Observer* observer,
RemoteBitrateObserver* remote_bitrate_observer)
: clock_(clock),
observer_(observer),
packet_router_(new PacketRouter()),
pacer_(new PacedSender(clock_,
packet_router_.get())),
remote_bitrate_estimator_(
new WrappingBitrateEstimator(remote_bitrate_observer, clock_)),
bitrate_controller_(BitrateController::CreateBitrateController(clock_)),
remote_estimator_proxy_(clock_, packet_router_.get()),
transport_feedback_adapter_(bitrate_controller_.get(), clock_),
min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
send_queue_is_full_(false) {
Init();
}
CongestionController::CongestionController(
Clock* clock,
Observer* observer,
RemoteBitrateObserver* remote_bitrate_observer,
std::unique_ptr<PacketRouter> packet_router,
std::unique_ptr<PacedSender> pacer)
: clock_(clock),
observer_(observer),
packet_router_(std::move(packet_router)),
pacer_(std::move(pacer)),
remote_bitrate_estimator_(
new WrappingBitrateEstimator(remote_bitrate_observer, clock_)),
// Constructed last as this object calls the provided callback on
// construction.
bitrate_controller_(
BitrateController::CreateBitrateController(clock_, bitrate_observer)),
remote_estimator_proxy_(clock_, &packet_router_),
bitrate_controller_(BitrateController::CreateBitrateController(clock_)),
remote_estimator_proxy_(clock_, packet_router_.get()),
transport_feedback_adapter_(bitrate_controller_.get(), clock_),
min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps) {
min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
send_queue_is_full_(false) {
Init();
}
CongestionController::~CongestionController() {}
void CongestionController::Init() {
transport_feedback_adapter_.SetBitrateEstimator(
new RemoteBitrateEstimatorAbsSendTime(&transport_feedback_adapter_));
transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate(
min_bitrate_bps_);
}
CongestionController::~CongestionController() {
}
void CongestionController::SetBweBitrates(int min_bitrate_bps,
int start_bitrate_bps,
@ -189,6 +229,7 @@ void CongestionController::SetBweBitrates(int min_bitrate_bps,
min_bitrate_bps_ = min_bitrate_bps;
transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate(
min_bitrate_bps_);
MaybeTriggerOnNetworkChanged();
}
BitrateController* CongestionController::GetBitrateController() const {
@ -209,10 +250,9 @@ CongestionController::GetTransportFeedbackObserver() {
return &transport_feedback_adapter_;
}
void CongestionController::UpdatePacerBitrate(int bitrate_kbps,
int max_bitrate_kbps,
int min_bitrate_kbps) {
pacer_->UpdateBitrate(bitrate_kbps, max_bitrate_kbps, min_bitrate_kbps);
void CongestionController::SetAllocatedSendBitrate(int allocated_bitrate_bps,
int padding_bitrate_bps) {
pacer_->SetAllocatedSendBitrate(allocated_bitrate_bps, padding_bitrate_bps);
}
int64_t CongestionController::GetPacerQueuingDelayMs() const {
@ -245,6 +285,36 @@ int64_t CongestionController::TimeUntilNextProcess() {
void CongestionController::Process() {
bitrate_controller_->Process();
remote_bitrate_estimator_->Process();
MaybeTriggerOnNetworkChanged();
}
void CongestionController::MaybeTriggerOnNetworkChanged() {
// TODO(perkj): |observer_| can be nullptr if the ctor that accepts a
// BitrateObserver is used. Remove this check once the ctor is removed.
if (!observer_)
return;
uint32_t bitrate_bps;
uint8_t fraction_loss;
int64_t rtt;
bool network_changed = bitrate_controller_->GetNetworkParameters(
&bitrate_bps, &fraction_loss, &rtt);
if (network_changed)
pacer_->SetEstimatedBitrate(bitrate_bps);
bool send_queue_is_full =
pacer_->ExpectedQueueTimeMs() > PacedSender::kMaxQueueLengthMs;
bitrate_bps = send_queue_is_full ? 0 : bitrate_bps;
if ((network_changed && !send_queue_is_full) ||
UpdateSendQueueStatus(send_queue_is_full)) {
observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt);
}
}
bool CongestionController::UpdateSendQueueStatus(bool send_queue_is_full) {
rtc::CritScope cs(&critsect_);
bool result = send_queue_is_full_ != send_queue_is_full;
send_queue_is_full_ = send_queue_is_full;
return result;
}
} // namespace webrtc

117
webrtc/modules/congestion_controller/congestion_controller_unittest.cc Normal file
View File

@ -0,0 +1,117 @@
/*
* Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
*
* Use of this source code is governed by a BSD-style license
* that can be found in the LICENSE file in the root of the source
* tree. An additional intellectual property rights grant can be found
* in the file PATENTS. All contributing project authors may
* be found in the AUTHORS file in the root of the source tree.
*/
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "webrtc/modules/pacing/mock/mock_paced_sender.h"
#include "webrtc/modules/congestion_controller/include/congestion_controller.h"
#include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h"
#include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h"
#include "webrtc/system_wrappers/include/clock.h"
using testing::_;
using testing::NiceMock;
using testing::Return;
using testing::SaveArg;
using testing::StrictMock;
namespace webrtc {
namespace test {
class CongestionControllerTest : public ::testing::Test {
protected:
CongestionControllerTest() : clock_(123456) {}
~CongestionControllerTest() override {}
void SetUp() override {
pacer_ = new NiceMock<MockPacedSender>();
std::unique_ptr<PacedSender> pacer(pacer_); // Passes ownership.
std::unique_ptr<PacketRouter> packet_router(new PacketRouter());
controller_.reset(
new CongestionController(&clock_, &observer_, &remote_bitrate_observer_,
std::move(packet_router), std::move(pacer)));
bandwidth_observer_.reset(
controller_->GetBitrateController()->CreateRtcpBandwidthObserver());
// Set the initial bitrate estimate and expect the |observer| and |pacer_|
// to be updated.
EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _));
EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps));
controller_->SetBweBitrates(0, kInitialBitrateBps, 5 * kInitialBitrateBps);
}
SimulatedClock clock_;
StrictMock<MockCongestionObserver> observer_;
NiceMock<MockPacedSender>* pacer_;
NiceMock<MockRemoteBitrateObserver> remote_bitrate_observer_;
std::unique_ptr<RtcpBandwidthObserver> bandwidth_observer_;
std::unique_ptr<CongestionController> controller_;
const uint32_t kInitialBitrateBps = 60000;
};
TEST_F(CongestionControllerTest, OnNetworkChanged) {
// Test no change.
clock_.AdvanceTimeMilliseconds(25);
controller_->Process();
EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _));
EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps * 2));
bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps * 2);
clock_.AdvanceTimeMilliseconds(25);
controller_->Process();
EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _));
EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps));
bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps);
clock_.AdvanceTimeMilliseconds(25);
controller_->Process();
}
TEST_F(CongestionControllerTest, OnSendQueueFull) {
EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
.WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1));
EXPECT_CALL(observer_, OnNetworkChanged(0, _, _));
controller_->Process();
// Let the pacer not be full next time the controller checks.
EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
.WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1));
EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _));
controller_->Process();
}
TEST_F(CongestionControllerTest, OnSendQueueFullAndEstimateChange) {
EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
.WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1));
EXPECT_CALL(observer_, OnNetworkChanged(0, _, _));
controller_->Process();
// Receive new estimate but let the queue still be full.
bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps * 2);
EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
.WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1));
// The send pacer should get the new estimate though.
EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps * 2));
clock_.AdvanceTimeMilliseconds(25);
controller_->Process();
// Let the pacer not be full next time the controller checks.
// |OnNetworkChanged| should be called with the new estimate.
EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
.WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1));
EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _));
clock_.AdvanceTimeMilliseconds(25);
controller_->Process();
}
} // namespace test
} // namespace webrtc

44
webrtc/modules/congestion_controller/include/congestion_controller.h
View File

@ -19,6 +19,7 @@
#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"
@ -31,7 +32,6 @@ namespace webrtc {
class BitrateController;
class BitrateObserver;
class Clock;
class PacedSender;
class ProcessThread;
class RemoteBitrateEstimator;
class RemoteBitrateObserver;
@ -39,9 +39,33 @@ class TransportFeedbackObserver;
class CongestionController : public CallStatsObserver, public Module {
public:
// Observer class for bitrate changes announced due to change in bandwidth
// estimate or due to that the send pacer is full. Fraction loss and rtt is
// also part of this callback to allow the observer to optimize its settings
// for different types of network environments. The bitrate does not include
// packet headers and is measured in bits per second.
class Observer {
public:
virtual void OnNetworkChanged(uint32_t bitrate_bps,
uint8_t fraction_loss, // 0 - 255.
int64_t rtt_ms) = 0;
protected:
virtual ~Observer() {}
};
// Deprecated
// TODO(perkj): Remove once no other clients use this ctor.
CongestionController(Clock* clock,
BitrateObserver* bitrate_observer,
RemoteBitrateObserver* remote_bitrate_observer);
CongestionController(Clock* clock,
Observer* observer,
RemoteBitrateObserver* remote_bitrate_observer);
CongestionController(Clock* clock,
Observer* observer,
RemoteBitrateObserver* remote_bitrate_observer,
std::unique_ptr<PacketRouter> packet_router,
std::unique_ptr<PacedSender> pacer);
virtual ~CongestionController();
virtual void SetBweBitrates(int min_bitrate_bps,
@ -53,12 +77,11 @@ class CongestionController : public CallStatsObserver, public Module {
bool send_side_bwe);
virtual int64_t GetPacerQueuingDelayMs() const;
virtual PacedSender* pacer() { return pacer_.get(); }
virtual PacketRouter* packet_router() { return &packet_router_; }
virtual PacketRouter* packet_router() { return packet_router_.get(); }
virtual TransportFeedbackObserver* GetTransportFeedbackObserver();
virtual void UpdatePacerBitrate(int bitrate_kbps,
int max_bitrate_kbps,
int min_bitrate_kbps);
void SetAllocatedSendBitrate(int allocated_bitrate_bps,
int padding_bitrate_bps);
virtual void OnSentPacket(const rtc::SentPacket& sent_packet);
@ -70,14 +93,23 @@ class CongestionController : public CallStatsObserver, public Module {
void Process() override;
private:
void Init();
void MaybeTriggerOnNetworkChanged();
// Updates |send_queue_is_full_|. Returns true if |send_queue_is_full_|
// has changed.
bool UpdateSendQueueStatus(bool send_queue_is_full);
Clock* const clock_;
Observer* const observer_;
const std::unique_ptr<PacketRouter> packet_router_;
const std::unique_ptr<PacedSender> pacer_;
const std::unique_ptr<RemoteBitrateEstimator> remote_bitrate_estimator_;
const std::unique_ptr<BitrateController> bitrate_controller_;
PacketRouter packet_router_;
RemoteEstimatorProxy remote_estimator_proxy_;
TransportFeedbackAdapter transport_feedback_adapter_;
int min_bitrate_bps_;
rtc::CriticalSection critsect_;
bool send_queue_is_full_ GUARDED_BY(critsect_);
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(CongestionController);
};

14
webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h
View File

@ -19,14 +19,20 @@
namespace webrtc {
namespace test {
class MockCongestionObserver : public CongestionController::Observer {
public:
MOCK_METHOD3(OnNetworkChanged,
void(uint32_t bitrate_bps,
uint8_t fraction_loss,
int64_t rtt_ms));
};
class MockCongestionController : public CongestionController {
public:
MockCongestionController(Clock* clock,
BitrateObserver* bitrate_observer,
Observer* observer,
RemoteBitrateObserver* remote_bitrate_observer)
: CongestionController(clock,
bitrate_observer,
remote_bitrate_observer) {}
: CongestionController(clock, observer, remote_bitrate_observer) {}
MOCK_METHOD3(SetBweBitrates,
void(int min_bitrate_bps,
int start_bitrate_bps,