Adds BitrateAllocation struct to OnBitrateUpdated.
This prepares for adding parameters to OnBitrateUpdated. By using a
struct, additional fields doesn't require a change in the signature and
only the obeservers that use the new fields will be affected by the
change.
Bug: webrtc:9718
Change-Id: I7dd6c9577afd77af06da5f56aea312356f80f9c0
Reviewed-on: https://webrtc-review.googlesource.com/c/107727
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25366}diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 052cd73..c8af43e 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -424,24 +424,22 @@
return channel_proxy_->ReceivedRTCPPacket(packet, length);
}
-uint32_t AudioSendStream::OnBitrateUpdated(uint32_t bitrate_bps,
- uint8_t fraction_loss,
- int64_t rtt,
- int64_t bwe_period_ms) {
+uint32_t AudioSendStream::OnBitrateUpdated(BitrateAllocationUpdate update) {
// A send stream may be allocated a bitrate of zero if the allocator decides
// to disable it. For now we ignore this decision and keep sending on min
// bitrate.
- if (bitrate_bps == 0) {
- bitrate_bps = config_.min_bitrate_bps;
+ if (update.bitrate_bps == 0) {
+ update.bitrate_bps = config_.min_bitrate_bps;
}
- RTC_DCHECK_GE(bitrate_bps, static_cast<uint32_t>(config_.min_bitrate_bps));
+ RTC_DCHECK_GE(update.bitrate_bps,
+ static_cast<uint32_t>(config_.min_bitrate_bps));
// The bitrate allocator might allocate an higher than max configured bitrate
// if there is room, to allow for, as example, extra FEC. Ignore that for now.
const uint32_t max_bitrate_bps = config_.max_bitrate_bps;
- if (bitrate_bps > max_bitrate_bps)
- bitrate_bps = max_bitrate_bps;
+ if (update.bitrate_bps > max_bitrate_bps)
+ update.bitrate_bps = max_bitrate_bps;
- channel_proxy_->SetBitrate(bitrate_bps, bwe_period_ms);
+ channel_proxy_->SetBitrate(update.bitrate_bps, update.bwe_period_ms);
// The amount of audio protection is not exposed by the encoder, hence
// always returning 0.
diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h
index 46c68b3..3683c91 100644
--- a/audio/audio_send_stream.h
+++ b/audio/audio_send_stream.h
@@ -83,10 +83,7 @@
bool DeliverRtcp(const uint8_t* packet, size_t length);
// Implements BitrateAllocatorObserver.
- uint32_t OnBitrateUpdated(uint32_t bitrate_bps,
- uint8_t fraction_loss,
- int64_t rtt,
- int64_t bwe_period_ms) override;
+ uint32_t OnBitrateUpdated(BitrateAllocationUpdate update) override;
// From PacketFeedbackObserver.
void OnPacketAdded(uint32_t ssrc, uint16_t seq_num) override;
diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc
index 0a954f8..30d2c54 100644
--- a/audio/audio_send_stream_unittest.cc
+++ b/audio/audio_send_stream_unittest.cc
@@ -470,15 +470,24 @@
auto send_stream = helper.CreateAudioSendStream();
EXPECT_CALL(*helper.channel_proxy(),
SetBitrate(helper.config().max_bitrate_bps, _));
- send_stream->OnBitrateUpdated(helper.config().max_bitrate_bps + 5000, 0.0, 50,
- 6000);
+ BitrateAllocationUpdate update;
+ update.bitrate_bps = helper.config().max_bitrate_bps + 5000;
+ update.fraction_loss = 0;
+ update.rtt = 50;
+ update.bwe_period_ms = 6000;
+ send_stream->OnBitrateUpdated(update);
}
TEST(AudioSendStreamTest, ProbingIntervalOnBitrateUpdated) {
ConfigHelper helper(false, true);
auto send_stream = helper.CreateAudioSendStream();
EXPECT_CALL(*helper.channel_proxy(), SetBitrate(_, 5000));
- send_stream->OnBitrateUpdated(50000, 0.0, 50, 5000);
+ BitrateAllocationUpdate update;
+ update.bitrate_bps = helper.config().max_bitrate_bps + 5000;
+ update.fraction_loss = 0;
+ update.rtt = 50;
+ update.bwe_period_ms = 5000;
+ send_stream->OnBitrateUpdated(update);
}
// Test that AudioSendStream doesn't recreate the encoder unnecessarily.
diff --git a/call/bitrate_allocator.cc b/call/bitrate_allocator.cc
index fc2618b..64d1a486 100644
--- a/call/bitrate_allocator.cc
+++ b/call/bitrate_allocator.cc
@@ -111,7 +111,8 @@
for (auto& config : bitrate_observer_configs_) {
uint32_t allocated_bitrate = allocation[config.observer];
uint32_t protection_bitrate = config.observer->OnBitrateUpdated(
- allocated_bitrate, last_fraction_loss_, last_rtt_, last_bwe_period_ms_);
+ BitrateAllocationUpdate{allocated_bitrate, last_fraction_loss_,
+ last_rtt_, last_bwe_period_ms_});
if (allocated_bitrate == 0 && config.allocated_bitrate_bps > 0) {
if (target_bitrate_bps > 0)
@@ -170,8 +171,8 @@
for (auto& config : bitrate_observer_configs_) {
uint32_t allocated_bitrate = allocation[config.observer];
uint32_t protection_bitrate = config.observer->OnBitrateUpdated(
- allocated_bitrate, last_fraction_loss_, last_rtt_,
- last_bwe_period_ms_);
+ BitrateAllocationUpdate{allocated_bitrate, last_fraction_loss_,
+ last_rtt_, last_bwe_period_ms_});
config.allocated_bitrate_bps = allocated_bitrate;
if (allocated_bitrate > 0)
config.media_ratio = MediaRatio(allocated_bitrate, protection_bitrate);
@@ -181,8 +182,8 @@
// But we still have to return the initial config bitrate + let the
// observer know that it can not produce frames.
allocation = AllocateBitrates(last_non_zero_bitrate_bps_);
- observer->OnBitrateUpdated(0, last_fraction_loss_, last_rtt_,
- last_bwe_period_ms_);
+ observer->OnBitrateUpdated(BitrateAllocationUpdate{
+ 0, last_fraction_loss_, last_rtt_, last_bwe_period_ms_});
}
UpdateAllocationLimits();
}
diff --git a/call/bitrate_allocator.h b/call/bitrate_allocator.h
index 8d00086..22fad7c 100644
--- a/call/bitrate_allocator.h
+++ b/call/bitrate_allocator.h
@@ -26,6 +26,12 @@
class Clock;
+struct BitrateAllocationUpdate {
+ uint32_t bitrate_bps;
+ uint8_t fraction_loss;
+ int64_t rtt;
+ int64_t bwe_period_ms;
+};
// Used by all send streams with adaptive bitrate, to get the currently
// allocated bitrate for the send stream. The current network properties are
// given at the same time, to let the send stream decide about possible loss
@@ -34,10 +40,7 @@
public:
// Returns the amount of protection used by the BitrateAllocatorObserver
// implementation, as bitrate in bps.
- virtual uint32_t OnBitrateUpdated(uint32_t bitrate_bps,
- uint8_t fraction_loss,
- int64_t rtt,
- int64_t bwe_period_ms) = 0;
+ virtual uint32_t OnBitrateUpdated(BitrateAllocationUpdate update) = 0;
protected:
virtual ~BitrateAllocatorObserver() {}
diff --git a/call/bitrate_allocator_unittest.cc b/call/bitrate_allocator_unittest.cc
index 2961fd4..1fc6cd6 100644
--- a/call/bitrate_allocator_unittest.cc
+++ b/call/bitrate_allocator_unittest.cc
@@ -59,15 +59,12 @@
protection_ratio_ = protection_ratio;
}
- uint32_t OnBitrateUpdated(uint32_t bitrate_bps,
- uint8_t fraction_loss,
- int64_t rtt,
- int64_t probing_interval_ms) override {
- last_bitrate_bps_ = bitrate_bps;
- last_fraction_loss_ = fraction_loss;
- last_rtt_ms_ = rtt;
- last_probing_interval_ms_ = probing_interval_ms;
- return bitrate_bps * protection_ratio_;
+ uint32_t OnBitrateUpdated(BitrateAllocationUpdate update) override {
+ last_bitrate_bps_ = update.bitrate_bps;
+ last_fraction_loss_ = update.fraction_loss;
+ last_rtt_ms_ = update.rtt;
+ last_probing_interval_ms_ = update.bwe_period_ms;
+ return update.bitrate_bps * protection_ratio_;
}
uint32_t last_bitrate_bps_;
uint8_t last_fraction_loss_;
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index 8454597..e6d97e9 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -611,21 +611,19 @@
return rtp_video_sender_->GetRtpPayloadStates();
}
-uint32_t VideoSendStreamImpl::OnBitrateUpdated(uint32_t bitrate_bps,
- uint8_t fraction_loss,
- int64_t rtt,
- int64_t probing_interval_ms) {
+uint32_t VideoSendStreamImpl::OnBitrateUpdated(BitrateAllocationUpdate update) {
RTC_DCHECK_RUN_ON(worker_queue_);
RTC_DCHECK(rtp_video_sender_->IsActive())
<< "VideoSendStream::Start has not been called.";
- rtp_video_sender_->OnBitrateUpdated(bitrate_bps, fraction_loss, rtt,
+ rtp_video_sender_->OnBitrateUpdated(update.bitrate_bps, update.fraction_loss,
+ update.rtt,
stats_proxy_->GetSendFrameRate());
encoder_target_rate_bps_ = rtp_video_sender_->GetPayloadBitrateBps();
encoder_target_rate_bps_ =
std::min(encoder_max_bitrate_bps_, encoder_target_rate_bps_);
video_stream_encoder_->OnBitrateUpdated(encoder_target_rate_bps_,
- fraction_loss, rtt);
+ update.fraction_loss, update.rtt);
stats_proxy_->OnSetEncoderTargetRate(encoder_target_rate_bps_);
return rtp_video_sender_->GetProtectionBitrateBps();
}
diff --git a/video/video_send_stream_impl.h b/video/video_send_stream_impl.h
index eed9f56..6197a04 100644
--- a/video/video_send_stream_impl.h
+++ b/video/video_send_stream_impl.h
@@ -83,10 +83,7 @@
class CheckEncoderActivityTask;
// Implements BitrateAllocatorObserver.
- uint32_t OnBitrateUpdated(uint32_t bitrate_bps,
- uint8_t fraction_loss,
- int64_t rtt,
- int64_t probing_interval_ms) override;
+ uint32_t OnBitrateUpdated(BitrateAllocationUpdate update) override;
void OnEncoderConfigurationChanged(std::vector<VideoStream> streams,
int min_transmit_bitrate_bps) override;
diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc
index 7d94c40..c93f8ff 100644
--- a/video/video_send_stream_impl_unittest.cc
+++ b/video/video_send_stream_impl_unittest.cc
@@ -69,6 +69,14 @@
MOCK_CONST_METHOD0(GetProtectionBitrateBps, uint32_t());
MOCK_METHOD3(SetEncodingData, void(size_t, size_t, size_t));
};
+
+BitrateAllocationUpdate CreateAllocation(int bitrate_bps) {
+ BitrateAllocationUpdate update;
+ update.bitrate_bps = bitrate_bps;
+ update.fraction_loss = 0;
+ update.rtt = 0;
+ return update;
+}
} // namespace
class VideoSendStreamImplTest : public ::testing::Test {
@@ -367,7 +375,7 @@
.Times(1)
.WillOnce(Return(kBitrateBps));
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
- ->OnBitrateUpdated(kBitrateBps, 0, 0, 0);
+ ->OnBitrateUpdated(CreateAllocation(kBitrateBps));
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc)).Times(1);
observer->OnBitrateAllocationUpdated(alloc);
@@ -376,7 +384,7 @@
.Times(1)
.WillOnce(Return(0));
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
- ->OnBitrateUpdated(0, 0, 0, 0);
+ ->OnBitrateUpdated(CreateAllocation(0));
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc)).Times(0);
observer->OnBitrateAllocationUpdated(alloc);
@@ -396,7 +404,7 @@
.Times(1)
.WillOnce(Return(kBitrateBps));
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
- ->OnBitrateUpdated(kBitrateBps, 0, 0, 0);
+ ->OnBitrateUpdated(CreateAllocation(kBitrateBps));
VideoBitrateAllocationObserver* const observer =
static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
@@ -450,7 +458,7 @@
.Times(1)
.WillOnce(Return(kBitrateBps));
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
- ->OnBitrateUpdated(kBitrateBps, 0, 0, 0);
+ ->OnBitrateUpdated(CreateAllocation(kBitrateBps));
VideoBitrateAllocationObserver* const observer =
static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
@@ -494,7 +502,7 @@
.Times(1)
.WillRepeatedly(Return(kBitrateBps));
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
- ->OnBitrateUpdated(kBitrateBps, 0, 0, 0);
+ ->OnBitrateUpdated(CreateAllocation(kBitrateBps));
VideoBitrateAllocationObserver* const observer =
static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
