Cleanup: Propagating BitrateAllocationUpdate to RtpVideoSender
Bug: webrtc:9883
Change-Id: I12d342ecd5eb0cc859123fe31fc759f6f60f7c8b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156940
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29492}
diff --git a/call/BUILD.gn b/call/BUILD.gn
index 09ef540..48a6504 100644
--- a/call/BUILD.gn
+++ b/call/BUILD.gn
@@ -134,6 +134,7 @@
":bitrate_configurator",
":rtp_interfaces",
"../api:array_view",
+ "../api:bitrate_allocation",
"../api:fec_controller_api",
"../api:network_state_predictor_api",
"../api:rtp_parameters",
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index d667072..77a1338 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -712,9 +712,7 @@
overhead_bytes_per_packet_ = overhead_bytes_per_packet;
}
-void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps,
- uint8_t fraction_loss,
- int64_t rtt,
+void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update,
int framerate) {
// Substract overhead from bitrate.
rtc::CritScope lock(&crit_);
@@ -722,19 +720,22 @@
overhead_bytes_per_packet_ + transport_overhead_bytes_per_packet_);
DataSize max_total_packet_size = DataSize::bytes(
rtp_config_.max_packet_size + transport_overhead_bytes_per_packet_);
- uint32_t payload_bitrate_bps = bitrate_bps;
+ uint32_t payload_bitrate_bps = update.target_bitrate.bps();
if (send_side_bwe_with_overhead_) {
DataRate overhead_rate = CalculateOverheadRate(
- DataRate::bps(bitrate_bps), max_total_packet_size, packet_overhead);
+ update.target_bitrate, max_total_packet_size, packet_overhead);
// TODO(srte): We probably should not accept 0 payload bitrate here.
- payload_bitrate_bps =
- rtc::saturated_cast<uint32_t>(bitrate_bps - overhead_rate.bps());
+ payload_bitrate_bps = rtc::saturated_cast<uint32_t>(payload_bitrate_bps -
+ overhead_rate.bps());
}
// Get the encoder target rate. It is the estimated network rate -
// protection overhead.
+ // TODO(srte): We should multiply with 255 here.
encoder_target_rate_bps_ = fec_controller_->UpdateFecRates(
- payload_bitrate_bps, framerate, fraction_loss, loss_mask_vector_, rtt);
+ payload_bitrate_bps, framerate,
+ rtc::saturated_cast<uint8_t>(update.packet_loss_ratio * 256),
+ loss_mask_vector_, update.round_trip_time.ms());
if (!fec_allowed_) {
encoder_target_rate_bps_ = payload_bitrate_bps;
// fec_controller_->UpdateFecRates() was still called so as to allow
@@ -765,17 +766,17 @@
DataRate::bps(encoder_target_rate_bps_),
max_total_packet_size - DataSize::bytes(overhead_bytes_per_packet_),
packet_overhead);
- encoder_overhead_rate_bps =
- std::min(encoder_overhead_rate.bps<uint32_t>(),
- bitrate_bps - encoder_target_rate_bps_);
+ encoder_overhead_rate_bps = std::min(
+ encoder_overhead_rate.bps<uint32_t>(),
+ update.target_bitrate.bps<uint32_t>() - encoder_target_rate_bps_);
}
// When the field trial "WebRTC-SendSideBwe-WithOverhead" is enabled
// protection_bitrate includes overhead.
const uint32_t media_rate = encoder_target_rate_bps_ +
encoder_overhead_rate_bps +
packetization_rate_bps;
- RTC_DCHECK_GE(bitrate_bps, media_rate);
- protection_bitrate_bps_ = bitrate_bps - media_rate;
+ RTC_DCHECK_GE(update.target_bitrate, DataRate::bps(media_rate));
+ protection_bitrate_bps_ = update.target_bitrate.bps() - media_rate;
}
uint32_t RtpVideoSender::GetPayloadBitrateBps() const {
diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h
index b5f8a8f..9458f13 100644
--- a/call/rtp_video_sender.h
+++ b/call/rtp_video_sender.h
@@ -136,10 +136,7 @@
size_t transport_overhead_bytes_per_packet) override;
// Implements OverheadObserver.
void OnOverheadChanged(size_t overhead_bytes_per_packet) override;
- void OnBitrateUpdated(uint32_t bitrate_bps,
- uint8_t fraction_loss,
- int64_t rtt,
- int framerate) override;
+ void OnBitrateUpdated(BitrateAllocationUpdate update, int framerate) override;
uint32_t GetPayloadBitrateBps() const override;
uint32_t GetProtectionBitrateBps() const override;
void SetEncodingData(size_t width,
diff --git a/call/rtp_video_sender_interface.h b/call/rtp_video_sender_interface.h
index ae9cdaf..bb72eb5 100644
--- a/call/rtp_video_sender_interface.h
+++ b/call/rtp_video_sender_interface.h
@@ -16,6 +16,7 @@
#include "absl/types/optional.h"
#include "api/array_view.h"
+#include "api/call/bitrate_allocation.h"
#include "api/fec_controller_override.h"
#include "call/rtp_config.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
@@ -49,9 +50,7 @@
virtual void OnBitrateAllocationUpdated(
const VideoBitrateAllocation& bitrate) = 0;
- virtual void OnBitrateUpdated(uint32_t bitrate_bps,
- uint8_t fraction_loss,
- int64_t rtt,
+ virtual void OnBitrateUpdated(BitrateAllocationUpdate update,
int framerate) = 0;
virtual void OnTransportOverheadChanged(
size_t transport_overhead_bytes_per_packet) = 0;
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index 39d25e4..bac60f8 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -629,19 +629,22 @@
TEST(RtpVideoSenderTest, CanSetZeroBitrateWithOverhead) {
test::ScopedFieldTrials trials("WebRTC-SendSideBwe-WithOverhead/Enabled/");
RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {});
+ BitrateAllocationUpdate update;
+ update.target_bitrate = DataRate::Zero();
+ update.packet_loss_ratio = 0;
+ update.round_trip_time = TimeDelta::Zero();
- test.router()->OnBitrateUpdated(/*bitrate_bps*/ 0,
- /*fraction_loss*/ 0,
- /*rtt*/ 0,
- /*framerate*/ 0);
+ test.router()->OnBitrateUpdated(update, /*framerate*/ 0);
}
TEST(RtpVideoSenderTest, CanSetZeroBitrateWithoutOverhead) {
RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {});
- test.router()->OnBitrateUpdated(/*bitrate_bps*/ 0,
- /*fraction_loss*/ 0,
- /*rtt*/ 0,
- /*framerate*/ 0);
+ BitrateAllocationUpdate update;
+ update.target_bitrate = DataRate::Zero();
+ update.packet_loss_ratio = 0;
+ update.round_trip_time = TimeDelta::Zero();
+
+ test.router()->OnBitrateUpdated(update, /*framerate*/ 0);
}
} // namespace webrtc
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index f1c2d3f..26f55ff 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -647,10 +647,7 @@
update.stable_target_bitrate = update.target_bitrate;
}
- rtp_video_sender_->OnBitrateUpdated(
- update.target_bitrate.bps(),
- rtc::dchecked_cast<uint8_t>(update.packet_loss_ratio * 256),
- update.round_trip_time.ms(), stats_proxy_->GetSendFrameRate());
+ rtp_video_sender_->OnBitrateUpdated(update, stats_proxy_->GetSendFrameRate());
encoder_target_rate_bps_ = rtp_video_sender_->GetPayloadBitrateBps();
const uint32_t protection_bitrate_bps =
rtp_video_sender_->GetProtectionBitrateBps();
diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc
index fdf0024..2bbdefb 100644
--- a/video/video_send_stream_impl_unittest.cc
+++ b/video/video_send_stream_impl_unittest.cc
@@ -31,6 +31,14 @@
#include "video/test/mock_video_stream_encoder.h"
namespace webrtc {
+
+bool operator==(const BitrateAllocationUpdate& a,
+ const BitrateAllocationUpdate& b) {
+ return a.target_bitrate == b.target_bitrate &&
+ a.round_trip_time == b.round_trip_time &&
+ a.packet_loss_ratio == b.packet_loss_ratio;
+}
+
namespace internal {
namespace {
using ::testing::_;
@@ -66,7 +74,7 @@
const RTPFragmentationHeader*));
MOCK_METHOD1(OnTransportOverheadChanged, void(size_t));
MOCK_METHOD1(OnOverheadChanged, void(size_t));
- MOCK_METHOD4(OnBitrateUpdated, void(uint32_t, uint8_t, int64_t, int));
+ MOCK_METHOD2(OnBitrateUpdated, void(BitrateAllocationUpdate, int));
MOCK_CONST_METHOD0(GetPayloadBitrateBps, uint32_t());
MOCK_CONST_METHOD0(GetProtectionBitrateBps, uint32_t());
MOCK_METHOD3(SetEncodingData, void(size_t, size_t, size_t));
@@ -693,9 +701,7 @@
update.target_bitrate = network_constrained_rate;
update.stable_target_bitrate = network_constrained_rate;
update.round_trip_time = TimeDelta::ms(1);
- EXPECT_CALL(rtp_video_sender_,
- OnBitrateUpdated(network_constrained_rate.bps(), _,
- update.round_trip_time.ms(), _));
+ EXPECT_CALL(rtp_video_sender_, OnBitrateUpdated(update, _));
EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps())
.WillOnce(Return(network_constrained_rate.bps()));
EXPECT_CALL(
@@ -711,16 +717,14 @@
DataRate::bps(qvga_stream.max_bitrate_bps);
const DataRate headroom = DataRate::bps(50000);
const DataRate rate_with_headroom = qvga_max_bitrate + headroom;
- EXPECT_CALL(rtp_video_sender_,
- OnBitrateUpdated(rate_with_headroom.bps(), _,
- update.round_trip_time.ms(), _));
+ update.target_bitrate = rate_with_headroom;
+ update.stable_target_bitrate = rate_with_headroom;
+ EXPECT_CALL(rtp_video_sender_, OnBitrateUpdated(update, _));
EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps())
.WillOnce(Return(rate_with_headroom.bps()));
EXPECT_CALL(video_stream_encoder_,
OnBitrateUpdated(qvga_max_bitrate, qvga_max_bitrate,
rate_with_headroom, 0, _));
- update.target_bitrate = rate_with_headroom;
- update.stable_target_bitrate = rate_with_headroom;
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
->OnBitrateUpdated(update);
@@ -730,9 +734,7 @@
EXPECT_CALL(rtp_video_sender_, GetProtectionBitrateBps())
.WillOnce(Return(protection_bitrate_bps));
- EXPECT_CALL(rtp_video_sender_,
- OnBitrateUpdated(rate_with_headroom.bps(), _,
- update.round_trip_time.ms(), _));
+ EXPECT_CALL(rtp_video_sender_, OnBitrateUpdated(update, _));
EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps())
.WillOnce(Return(rate_with_headroom.bps()));
const DataRate headroom_minus_protection =
@@ -747,9 +749,7 @@
// capped to target bitrate.
EXPECT_CALL(rtp_video_sender_, GetProtectionBitrateBps())
.WillOnce(Return(headroom.bps() + 1000));
- EXPECT_CALL(rtp_video_sender_,
- OnBitrateUpdated(rate_with_headroom.bps(), _,
- update.round_trip_time.ms(), _));
+ EXPECT_CALL(rtp_video_sender_, OnBitrateUpdated(update, _));
EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps())
.WillOnce(Return(rate_with_headroom.bps()));
EXPECT_CALL(video_stream_encoder_,
