Skip to content

Commit

Permalink
Replacing bandwidth adaptation trial with stable target in Opus encoder.
Browse files Browse the repository at this point in the history
This also means that the NetworkEstimate::bandwidth can be deprecated
as it's currently just a copy of the target_rate.

Bug: webrtc:10981
Change-Id: I1bc57b98480bd77ce052736b19d630c775428546
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/153669
Commit-Queue: Sebastian Jansson <[email protected]>
Reviewed-by: Oskar Sundbom <[email protected]>
Reviewed-by: Per Kjellander <[email protected]>
Cr-Commit-Position: refs/heads/master@{#29288}
  • Loading branch information
jonex authored and Commit Bot committed Sep 24, 2019
1 parent c30bc16 commit f34116e
Show file tree
Hide file tree
Showing 16 changed files with 39 additions and 67 deletions.
2 changes: 0 additions & 2 deletions api/call/bitrate_allocation.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ struct BitrateAllocationUpdate {
double packet_loss_ratio = 0;
// Predicted round trip time.
TimeDelta round_trip_time = TimeDelta::PlusInfinity();
// |link_capacity| is deprecated, use |stable_target_bitrate| instead.
DataRate link_capacity = DataRate::Zero();
// |bwe_period| is deprecated, use |stable_target_bitrate| allocation instead.
TimeDelta bwe_period = TimeDelta::PlusInfinity();
};
Expand Down
1 change: 1 addition & 0 deletions api/transport/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ rtc_static_library("network_control") {

deps = [
":webrtc_key_value_config",
"../../rtc_base:deprecation",
"../units:data_rate",
"../units:data_size",
"../units:time_delta",
Expand Down
2 changes: 2 additions & 0 deletions api/transport/network_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "api/units/data_size.h"
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "rtc_base/deprecation.h"

namespace webrtc {

Expand Down Expand Up @@ -179,6 +180,7 @@ struct TransportPacketsFeedback {

struct NetworkEstimate {
Timestamp at_time = Timestamp::PlusInfinity();
// Deprecated, use TargetTransferRate::target_rate instead.
DataRate bandwidth = DataRate::Infinity();
TimeDelta round_trip_time = TimeDelta::PlusInfinity();
TimeDelta bwe_period = TimeDelta::PlusInfinity();
Expand Down
11 changes: 0 additions & 11 deletions call/bitrate_allocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ BitrateAllocator::BitrateAllocator(LimitObserver* limit_observer)
: limit_observer_(limit_observer),
last_target_bps_(0),
last_stable_target_bps_(0),
last_bandwidth_bps_(0),
last_non_zero_bitrate_bps_(kDefaultBitrateBps),
last_fraction_loss_(0),
last_rtt_(0),
Expand Down Expand Up @@ -95,7 +94,6 @@ uint8_t BitrateAllocator::GetTransmissionMaxBitrateMultiplier() {
void BitrateAllocator::OnNetworkEstimateChanged(TargetTransferRate msg) {
RTC_DCHECK_RUN_ON(&sequenced_checker_);
last_target_bps_ = msg.target_rate.bps();
last_bandwidth_bps_ = msg.network_estimate.bandwidth.bps();
last_stable_target_bps_ = msg.stable_target_rate.bps();
last_non_zero_bitrate_bps_ =
last_target_bps_ > 0 ? last_target_bps_ : last_non_zero_bitrate_bps_;
Expand All @@ -114,20 +112,16 @@ void BitrateAllocator::OnNetworkEstimateChanged(TargetTransferRate msg) {
}

ObserverAllocation allocation = AllocateBitrates(last_target_bps_);
ObserverAllocation bandwidth_allocation =
AllocateBitrates(last_bandwidth_bps_);
ObserverAllocation stable_bitrate_allocation =
AllocateBitrates(last_stable_target_bps_);

for (auto& config : allocatable_tracks_) {
uint32_t allocated_bitrate = allocation[config.observer];
uint32_t allocated_bandwidth = bandwidth_allocation[config.observer];
uint32_t allocated_stable_target_rate =
stable_bitrate_allocation[config.observer];
BitrateAllocationUpdate update;
update.target_bitrate = DataRate::bps(allocated_bitrate);
update.stable_target_bitrate = DataRate::bps(allocated_stable_target_rate);
update.link_capacity = DataRate::bps(allocated_bandwidth);
update.packet_loss_ratio = last_fraction_loss_ / 256.0;
update.round_trip_time = TimeDelta::ms(last_rtt_);
update.bwe_period = TimeDelta::ms(last_bwe_period_ms_);
Expand Down Expand Up @@ -183,19 +177,15 @@ void BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer,
// Calculate a new allocation and update all observers.

ObserverAllocation allocation = AllocateBitrates(last_target_bps_);
ObserverAllocation bandwidth_allocation =
AllocateBitrates(last_bandwidth_bps_);
ObserverAllocation stable_bitrate_allocation =
AllocateBitrates(last_stable_target_bps_);
for (auto& config : allocatable_tracks_) {
uint32_t allocated_bitrate = allocation[config.observer];
uint32_t allocated_stable_bitrate =
stable_bitrate_allocation[config.observer];
uint32_t bandwidth = bandwidth_allocation[config.observer];
BitrateAllocationUpdate update;
update.target_bitrate = DataRate::bps(allocated_bitrate);
update.stable_target_bitrate = DataRate::bps(allocated_stable_bitrate);
update.link_capacity = DataRate::bps(bandwidth);
update.packet_loss_ratio = last_fraction_loss_ / 256.0;
update.round_trip_time = TimeDelta::ms(last_rtt_);
update.bwe_period = TimeDelta::ms(last_bwe_period_ms_);
Expand All @@ -212,7 +202,6 @@ void BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer,
BitrateAllocationUpdate update;
update.target_bitrate = DataRate::Zero();
update.stable_target_bitrate = DataRate::Zero();
update.link_capacity = DataRate::Zero();
update.packet_loss_ratio = last_fraction_loss_ / 256.0;
update.round_trip_time = TimeDelta::ms(last_rtt_);
update.bwe_period = TimeDelta::ms(last_bwe_period_ms_);
Expand Down
1 change: 0 additions & 1 deletion call/bitrate_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ class BitrateAllocator : public BitrateAllocatorInterface {
RTC_GUARDED_BY(&sequenced_checker_);
uint32_t last_target_bps_ RTC_GUARDED_BY(&sequenced_checker_);
uint32_t last_stable_target_bps_ RTC_GUARDED_BY(&sequenced_checker_);
uint32_t last_bandwidth_bps_ RTC_GUARDED_BY(&sequenced_checker_);
uint32_t last_non_zero_bitrate_bps_ RTC_GUARDED_BY(&sequenced_checker_);
uint8_t last_fraction_loss_ RTC_GUARDED_BY(&sequenced_checker_);
int64_t last_rtt_ RTC_GUARDED_BY(&sequenced_checker_);
Expand Down
2 changes: 1 addition & 1 deletion call/call.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,7 @@ void Call::OnTargetTransferRate(TargetTransferRate msg) {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
{
rtc::CritScope cs(&last_bandwidth_bps_crit_);
last_bandwidth_bps_ = msg.network_estimate.bandwidth.bps();
last_bandwidth_bps_ = msg.target_rate.bps();
}

uint32_t target_bitrate_bps = msg.target_rate.bps();
Expand Down
3 changes: 1 addition & 2 deletions call/rtp_transport_controller_send.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ void RtpTransportControllerSend::UpdateControlState() {
absl::optional<TargetTransferRate> update = control_handler_->GetUpdate();
if (!update)
return;
retransmission_rate_limiter_.SetMaxRate(
update->network_estimate.bandwidth.bps());
retransmission_rate_limiter_.SetMaxRate(update->target_rate.bps());
// We won't create control_handler_ until we have an observers.
RTC_DCHECK(observer_ != nullptr);
observer_->OnTargetTransferRate(*update);
Expand Down
68 changes: 32 additions & 36 deletions modules/audio_coding/codecs/opus/audio_encoder_opus.cc
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,8 @@ AudioEncoderOpusImpl::AudioEncoderOpusImpl(
: payload_type_(payload_type),
send_side_bwe_with_overhead_(
webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")),
use_link_capacity_for_adaptation_(webrtc::field_trial::IsEnabled(
"WebRTC-Audio-LinkCapacityAdaptation")),
use_stable_target_for_adaptation_(webrtc::field_trial::IsEnabled(
"WebRTC-Audio-StableTargetAdaptation")),
adjust_bandwidth_(
webrtc::field_trial::IsEnabled("WebRTC-AdjustOpusBandwidth")),
bitrate_changed_(true),
Expand Down Expand Up @@ -563,26 +563,28 @@ void AudioEncoderOpusImpl::OnReceivedUplinkRecoverablePacketLossFraction(
void AudioEncoderOpusImpl::OnReceivedUplinkBandwidth(
int target_audio_bitrate_bps,
absl::optional<int64_t> bwe_period_ms,
absl::optional<int64_t> link_capacity_allocation_bps) {
absl::optional<int64_t> stable_target_bitrate_bps) {
if (audio_network_adaptor_) {
audio_network_adaptor_->SetTargetAudioBitrate(target_audio_bitrate_bps);
// We give smoothed bitrate allocation to audio network adaptor as
// the uplink bandwidth.
// The BWE spikes should not affect the bitrate smoother more than 25%.
// To simplify the calculations we use a step response as input signal.
// The step response of an exponential filter is
// u(t) = 1 - e^(-t / time_constant).
// In order to limit the affect of a BWE spike within 25% of its value
// before
// the next BWE update, we would choose a time constant that fulfills
// 1 - e^(-bwe_period_ms / time_constant) < 0.25
// Then 4 * bwe_period_ms is a good choice.
if (bwe_period_ms)
bitrate_smoother_->SetTimeConstantMs(*bwe_period_ms * 4);
bitrate_smoother_->AddSample(target_audio_bitrate_bps);

if (link_capacity_allocation_bps)
link_capacity_allocation_bps_ = link_capacity_allocation_bps;
if (use_stable_target_for_adaptation_) {
if (stable_target_bitrate_bps)
audio_network_adaptor_->SetUplinkBandwidth(*stable_target_bitrate_bps);
} else {
// We give smoothed bitrate allocation to audio network adaptor as
// the uplink bandwidth.
// The BWE spikes should not affect the bitrate smoother more than 25%.
// To simplify the calculations we use a step response as input signal.
// The step response of an exponential filter is
// u(t) = 1 - e^(-t / time_constant).
// In order to limit the affect of a BWE spike within 25% of its value
// before
// the next BWE update, we would choose a time constant that fulfills
// 1 - e^(-bwe_period_ms / time_constant) < 0.25
// Then 4 * bwe_period_ms is a good choice.
if (bwe_period_ms)
bitrate_smoother_->SetTimeConstantMs(*bwe_period_ms * 4);
bitrate_smoother_->AddSample(target_audio_bitrate_bps);
}

ApplyAudioNetworkAdaptor();
} else if (send_side_bwe_with_overhead_) {
Expand Down Expand Up @@ -612,7 +614,7 @@ void AudioEncoderOpusImpl::OnReceivedUplinkBandwidth(
void AudioEncoderOpusImpl::OnReceivedUplinkAllocation(
BitrateAllocationUpdate update) {
OnReceivedUplinkBandwidth(update.target_bitrate.bps(), update.bwe_period.ms(),
update.link_capacity.bps());
update.stable_target_bitrate.bps());
}

void AudioEncoderOpusImpl::OnReceivedRtt(int rtt_ms) {
Expand Down Expand Up @@ -857,21 +859,15 @@ AudioEncoderOpusImpl::DefaultAudioNetworkAdaptorCreator(
}

void AudioEncoderOpusImpl::MaybeUpdateUplinkBandwidth() {
if (audio_network_adaptor_) {
if (use_link_capacity_for_adaptation_ && link_capacity_allocation_bps_) {
audio_network_adaptor_->SetUplinkBandwidth(
*link_capacity_allocation_bps_);
} else {
int64_t now_ms = rtc::TimeMillis();
if (!bitrate_smoother_last_update_time_ ||
now_ms - *bitrate_smoother_last_update_time_ >=
config_.uplink_bandwidth_update_interval_ms) {
absl::optional<float> smoothed_bitrate =
bitrate_smoother_->GetAverage();
if (smoothed_bitrate)
audio_network_adaptor_->SetUplinkBandwidth(*smoothed_bitrate);
bitrate_smoother_last_update_time_ = now_ms;
}
if (audio_network_adaptor_ && !use_stable_target_for_adaptation_) {
int64_t now_ms = rtc::TimeMillis();
if (!bitrate_smoother_last_update_time_ ||
now_ms - *bitrate_smoother_last_update_time_ >=
config_.uplink_bandwidth_update_interval_ms) {
absl::optional<float> smoothed_bitrate = bitrate_smoother_->GetAverage();
if (smoothed_bitrate)
audio_network_adaptor_->SetUplinkBandwidth(*smoothed_bitrate);
bitrate_smoother_last_update_time_ = now_ms;
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions modules/audio_coding/codecs/opus/audio_encoder_opus.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class AudioEncoderOpusImpl final : public AudioEncoder {
AudioEncoderOpusConfig config_;
const int payload_type_;
const bool send_side_bwe_with_overhead_;
const bool use_link_capacity_for_adaptation_;
const bool use_stable_target_for_adaptation_;
const bool adjust_bandwidth_;
bool bitrate_changed_;
float packet_loss_rate_;
Expand All @@ -192,7 +192,6 @@ class AudioEncoderOpusImpl final : public AudioEncoder {
absl::optional<size_t> overhead_bytes_per_packet_;
const std::unique_ptr<SmoothingFilter> bitrate_smoother_;
absl::optional<int64_t> bitrate_smoother_last_update_time_;
absl::optional<int64_t> link_capacity_allocation_bps_;
int consecutive_dtx_frames_;

friend struct AudioEncoderOpus;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ NetworkControlUpdate BbrNetworkController::CreateRateUpdate(

TargetTransferRate target_rate_msg;
target_rate_msg.network_estimate.at_time = at_time;
target_rate_msg.network_estimate.bandwidth = bandwidth;
target_rate_msg.network_estimate.round_trip_time = rtt;

// TODO(srte): Fill in field below with proper value.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,6 @@ NetworkControlUpdate GoogCcNetworkController::GetNetworkState(
NetworkControlUpdate update;
update.target_rate = TargetTransferRate();
update.target_rate->network_estimate.at_time = at_time;
update.target_rate->network_estimate.bandwidth = last_raw_target_rate_;
update.target_rate->network_estimate.loss_rate_ratio =
last_estimated_fraction_loss_ / 255.0;
update.target_rate->network_estimate.round_trip_time = rtt;
Expand Down Expand Up @@ -635,7 +634,6 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged(
bandwidth_estimation_->GetEstimatedLinkCapacity(), target_rate);
target_rate_msg.network_estimate.at_time = at_time;
target_rate_msg.network_estimate.round_trip_time = TimeDelta::ms(rtt_ms);
target_rate_msg.network_estimate.bandwidth = last_raw_target_rate_;
target_rate_msg.network_estimate.loss_rate_ratio = fraction_loss / 255.0f;
target_rate_msg.network_estimate.bwe_period = bwe_period;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ TEST_F(GoogCcNetworkControllerTest, StableEstimateDoesNotVaryInSteadyState) {
// Measure variation in steady state.
for (int i = 0; i < 20; ++i) {
auto stable_target_rate = client->stable_target_rate();
auto target_rate = client->link_capacity();
auto target_rate = client->target_rate();
EXPECT_LE(stable_target_rate, target_rate);

min_stable_target = std::min(min_stable_target, stable_target_rate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ std::deque<FieldLogger*> GoogCcStatePrinter::CreateLoggers() {
};
std::deque<FieldLogger*> loggers({
Log("time", [=] { return target_.at_time; }),
Log("bandwidth", [=] { return target_.network_estimate.bandwidth; }),
Log("rtt", [=] { return target_.network_estimate.round_trip_time; }),
Log("target", [=] { return target_.target_rate; }),
Log("pacing", [=] { return pacing_.data_rate(); }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ NetworkControlUpdate PccNetworkController::CreateRateUpdate(
target_rate_msg.at_time = at_time;
target_rate_msg.network_estimate.at_time = at_time;
target_rate_msg.network_estimate.round_trip_time = rtt_tracker_.GetRtt();
target_rate_msg.network_estimate.bandwidth = bandwidth_estimate_;
// TODO(koloskova): Add correct estimate.
target_rate_msg.network_estimate.loss_rate_ratio = 0;
target_rate_msg.network_estimate.bwe_period =
Expand Down
5 changes: 0 additions & 5 deletions test/scenario/call_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,6 @@ DataRate CallClient::target_rate() const {
return network_controller_factory_.GetUpdate().target_rate->target_rate;
}

DataRate CallClient::link_capacity() const {
return network_controller_factory_.GetUpdate()
.target_rate->network_estimate.bandwidth;
}

DataRate CallClient::stable_target_rate() const {
return network_controller_factory_.GetUpdate()
.target_rate->stable_target_rate;
Expand Down
1 change: 0 additions & 1 deletion test/scenario/call_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ class CallClient : public EmulatedNetworkReceiverInterface {
}
DataRate target_rate() const;
DataRate stable_target_rate() const;
DataRate link_capacity() const;
DataRate padding_rate() const;

void OnPacketReceived(EmulatedIpPacket packet) override;
Expand Down

0 comments on commit f34116e

Please sign in to comment.