From 464a5576ea2734b3eb0b983fdea14c98c2f6b777 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Tue, 12 Feb 2019 13:32:32 +0100 Subject: [PATCH] Adds audio priority bitrate field trial parameter. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This replaces the functionality provided by AudioPriorityBitrateAllocationStrategy, removing the need provide that component via injection in all clients using audio bitrate priority. Bug: webrtc:10286 Change-Id: I3bafab56d24459d9d27dc07abffdc8538440a346 Reviewed-on: https://webrtc-review.googlesource.com/c/121402 Reviewed-by: Oskar Sundbom Reviewed-by: Erik Språng Reviewed-by: Christoffer Rodbro Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#26651} --- audio/audio_send_stream.cc | 9 +-- call/bitrate_allocator.cc | 23 ++++++-- call/bitrate_allocator.h | 4 ++ call/bitrate_allocator_unittest.cc | 58 +++++++++++++++++-- .../experiments/audio_allocation_settings.cc | 27 +++++++-- .../experiments/audio_allocation_settings.h | 9 +++ video/video_send_stream_impl.cc | 35 +++++------ video/video_send_stream_impl.h | 2 + 8 files changed, 127 insertions(+), 40 deletions(-) diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index baf51b2bb0..8ced40eacd 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -785,10 +785,11 @@ void AudioSendStream::ConfigureBitrateObserver(int min_bitrate_bps, config_.bitrate_priority = bitrate_priority; // This either updates the current observer or adds a new observer. bitrate_allocator_->AddObserver( - this, - MediaStreamAllocationConfig{static_cast(min_bitrate_bps), - static_cast(max_bitrate_bps), 0, - true, config_.track_id, bitrate_priority}); + this, MediaStreamAllocationConfig{ + static_cast(min_bitrate_bps), + static_cast(max_bitrate_bps), 0, + allocation_settings_.DefaultPriorityBitrate().bps(), true, + config_.track_id, bitrate_priority}); thread_sync_event.Set(); }); thread_sync_event.Wait(rtc::Event::kForever); diff --git a/call/bitrate_allocator.cc b/call/bitrate_allocator.cc index 06b74ba4e4..21939a7e47 100644 --- a/call/bitrate_allocator.cc +++ b/call/bitrate_allocator.cc @@ -174,10 +174,10 @@ void BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, it->enforce_min_bitrate = config.enforce_min_bitrate; it->bitrate_priority = config.bitrate_priority; } else { - bitrate_observer_configs_.push_back( - ObserverConfig(observer, config.min_bitrate_bps, config.max_bitrate_bps, - config.pad_up_bitrate_bps, config.enforce_min_bitrate, - config.track_id, config.bitrate_priority)); + bitrate_observer_configs_.push_back(ObserverConfig( + observer, config.min_bitrate_bps, config.max_bitrate_bps, + config.pad_up_bitrate_bps, config.priority_bitrate_bps, + config.enforce_min_bitrate, config.track_id, config.bitrate_priority)); } if (last_target_bps_ > 0) { @@ -439,6 +439,21 @@ BitrateAllocator::ObserverAllocation BitrateAllocator::NormalRateAllocation( } bitrate -= sum_min_bitrates; + + // TODO(srte): Implement fair sharing between prioritized streams, currently + // they are treated on a first come first serve basis. + for (const auto& observer_config : bitrate_observer_configs_) { + int64_t priority_margin = observer_config.priority_bitrate_bps - + allocation[observer_config.observer]; + if (priority_margin > 0 && bitrate > 0) { + int64_t extra_bitrate = std::min(priority_margin, bitrate); + allocation[observer_config.observer] += + rtc::dchecked_cast(extra_bitrate); + observers_capacities[observer_config.observer] -= extra_bitrate; + bitrate -= extra_bitrate; + } + } + // From the remaining bitrate, allocate a proportional amount to each observer // above the min bitrate already allocated. if (bitrate > 0) diff --git a/call/bitrate_allocator.h b/call/bitrate_allocator.h index 1323aa3288..43e89b2936 100644 --- a/call/bitrate_allocator.h +++ b/call/bitrate_allocator.h @@ -55,6 +55,7 @@ struct MediaStreamAllocationConfig { uint32_t min_bitrate_bps; uint32_t max_bitrate_bps; uint32_t pad_up_bitrate_bps; + int64_t priority_bitrate_bps; bool enforce_min_bitrate; std::string track_id; double bitrate_priority; @@ -128,6 +129,7 @@ class BitrateAllocator : public BitrateAllocatorInterface { uint32_t min_bitrate_bps, uint32_t max_bitrate_bps, uint32_t pad_up_bitrate_bps, + int64_t priority_bitrate_bps, bool enforce_min_bitrate, std::string track_id, double bitrate_priority) @@ -137,12 +139,14 @@ class BitrateAllocator : public BitrateAllocatorInterface { track_id), observer(observer), pad_up_bitrate_bps(pad_up_bitrate_bps), + priority_bitrate_bps(priority_bitrate_bps), allocated_bitrate_bps(-1), media_ratio(1.0), bitrate_priority(bitrate_priority) {} BitrateAllocatorObserver* observer; uint32_t pad_up_bitrate_bps; + int64_t priority_bitrate_bps; int64_t allocated_bitrate_bps; double media_ratio; // Part of the total bitrate used for media [0.0, 1.0]. // The amount of bitrate allocated to this observer relative to all other diff --git a/call/bitrate_allocator_unittest.cc b/call/bitrate_allocator_unittest.cc index 9956a88d71..33747efb34 100644 --- a/call/bitrate_allocator_unittest.cc +++ b/call/bitrate_allocator_unittest.cc @@ -100,13 +100,60 @@ class BitrateAllocatorTest : public ::testing::Test { double bitrate_priority) { allocator_->AddObserver( observer, {min_bitrate_bps, max_bitrate_bps, pad_up_bitrate_bps, - enforce_min_bitrate, track_id, bitrate_priority}); + /* priority_bitrate */ 0, enforce_min_bitrate, track_id, + bitrate_priority}); + } + MediaStreamAllocationConfig DefaultConfig() const { + MediaStreamAllocationConfig default_config; + default_config.min_bitrate_bps = 0; + default_config.max_bitrate_bps = 1500000; + default_config.pad_up_bitrate_bps = 0; + default_config.priority_bitrate_bps = 0; + default_config.enforce_min_bitrate = true; + default_config.track_id = ""; + default_config.bitrate_priority = kDefaultBitratePriority; + return default_config; } NiceMock limit_observer_; std::unique_ptr allocator_; }; +TEST_F(BitrateAllocatorTest, RespectsPriorityBitrate) { + TestBitrateObserver stream_a; + auto config_a = DefaultConfig(); + config_a.min_bitrate_bps = 100000; + config_a.priority_bitrate_bps = 0; + allocator_->AddObserver(&stream_a, config_a); + + TestBitrateObserver stream_b; + auto config_b = DefaultConfig(); + config_b.min_bitrate_bps = 100000; + config_b.max_bitrate_bps = 300000; + config_b.priority_bitrate_bps = 300000; + allocator_->AddObserver(&stream_b, config_b); + + allocator_->OnNetworkChanged(100000, 0, 0, 0); + EXPECT_EQ(stream_a.last_bitrate_bps_, 100000u); + EXPECT_EQ(stream_b.last_bitrate_bps_, 100000u); + + allocator_->OnNetworkChanged(200000, 0, 0, 0); + EXPECT_EQ(stream_a.last_bitrate_bps_, 100000u); + EXPECT_EQ(stream_b.last_bitrate_bps_, 100000u); + + allocator_->OnNetworkChanged(300000, 0, 0, 0); + EXPECT_EQ(stream_a.last_bitrate_bps_, 100000u); + EXPECT_EQ(stream_b.last_bitrate_bps_, 200000u); + + allocator_->OnNetworkChanged(400000, 0, 0, 0); + EXPECT_EQ(stream_a.last_bitrate_bps_, 100000u); + EXPECT_EQ(stream_b.last_bitrate_bps_, 300000u); + + allocator_->OnNetworkChanged(800000, 0, 0, 0); + EXPECT_EQ(stream_a.last_bitrate_bps_, 500000u); + EXPECT_EQ(stream_b.last_bitrate_bps_, 300000u); +} + TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { TestBitrateObserver bitrate_observer; const uint32_t kMinSendBitrateBps = 100000; @@ -232,7 +279,7 @@ class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { std::string track_id, double bitrate_priority) { allocator_->AddObserver( - observer, {min_bitrate_bps, max_bitrate_bps, pad_up_bitrate_bps, + observer, {min_bitrate_bps, max_bitrate_bps, pad_up_bitrate_bps, 0, enforce_min_bitrate, track_id, bitrate_priority}); } NiceMock limit_observer_; @@ -404,9 +451,10 @@ TEST_F(BitrateAllocatorTest, EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinBitrateBps, 0, kMaxBitrateBps)) .Times(1); - allocator_->AddObserver( - &bitrate_observer, - {kMinBitrateBps, kMaxBitrateBps, 0, true, "", kDefaultBitratePriority}); + MediaStreamAllocationConfig allocation_config = DefaultConfig(); + allocation_config.min_bitrate_bps = kMinBitrateBps; + allocation_config.max_bitrate_bps = kMaxBitrateBps; + allocator_->AddObserver(&bitrate_observer, allocation_config); // Observer uses 20% of it's allocated bitrate for protection. bitrate_observer.SetBitrateProtectionRatio(/*protection_ratio=*/0.2); diff --git a/rtc_base/experiments/audio_allocation_settings.cc b/rtc_base/experiments/audio_allocation_settings.cc index 51c859170a..79c5a0dfe7 100644 --- a/rtc_base/experiments/audio_allocation_settings.cc +++ b/rtc_base/experiments/audio_allocation_settings.cc @@ -15,13 +15,18 @@ namespace { // For SendSideBwe, Opus bitrate should be in the range between 6000 and 32000. const int kOpusMinBitrateBps = 6000; const int kOpusBitrateFbBps = 32000; +// OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12) +constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12; } // namespace AudioAllocationSettings::AudioAllocationSettings() : audio_send_side_bwe_("Enabled"), allocate_audio_without_feedback_("Enabled"), force_no_audio_feedback_("Enabled"), audio_feedback_to_improve_video_bwe_("Enabled"), - send_side_bwe_with_overhead_("Enabled") { + send_side_bwe_with_overhead_("Enabled"), + default_min_bitrate_("min", DataRate::bps(kOpusMinBitrateBps)), + default_max_bitrate_("max", DataRate::bps(kOpusBitrateFbBps)), + priority_bitrate_("prio", DataRate::Zero()) { ParseFieldTrial({&audio_send_side_bwe_}, field_trial::FindFullName("WebRTC-Audio-SendSideBwe")); ParseFieldTrial({&allocate_audio_without_feedback_}, @@ -31,16 +36,17 @@ AudioAllocationSettings::AudioAllocationSettings() ParseFieldTrial( {&audio_feedback_to_improve_video_bwe_}, field_trial::FindFullName("WebRTC-Audio-SendSideBwe-For-Video")); + ParseFieldTrial({&send_side_bwe_with_overhead_}, field_trial::FindFullName("WebRTC-SendSideBwe-WithOverhead")); + ParseFieldTrial( + {&default_min_bitrate_, &default_max_bitrate_, &priority_bitrate_}, + field_trial::FindFullName("WebRTC-Audio-Allocation")); // TODO(mflodman): Keep testing this and set proper values. // Note: This is an early experiment currently only supported by Opus. if (send_side_bwe_with_overhead_) { constexpr int kMaxPacketSizeMs = WEBRTC_OPUS_SUPPORT_120MS_PTIME ? 120 : 60; - - // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12) - constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12; min_overhead_bps_ = kOverheadPerPacket * 8 * 1000 / kMaxPacketSizeMs; } } @@ -116,7 +122,7 @@ bool AudioAllocationSettings::IncludeAudioInAllocationOnReconfigure( } int AudioAllocationSettings::MinBitrateBps() const { - return kOpusMinBitrateBps + min_overhead_bps_; + return default_min_bitrate_->bps() + min_overhead_bps_; } int AudioAllocationSettings::MaxBitrateBps( @@ -133,6 +139,15 @@ int AudioAllocationSettings::MaxBitrateBps( // meanwhile change the cap to the output of BWE. if (rtp_parameter_max_bitrate_bps) return *rtp_parameter_max_bitrate_bps + min_overhead_bps_; - return kOpusBitrateFbBps + min_overhead_bps_; + return default_max_bitrate_->bps() + min_overhead_bps_; } +DataRate AudioAllocationSettings::DefaultPriorityBitrate() const { + DataRate max_overhead = DataRate::Zero(); + if (send_side_bwe_with_overhead_) { + const TimeDelta kMinPacketDuration = TimeDelta::ms(20); + max_overhead = DataSize::bytes(kOverheadPerPacket) / kMinPacketDuration; + } + return priority_bitrate_.Get() + max_overhead; +} + } // namespace webrtc diff --git a/rtc_base/experiments/audio_allocation_settings.h b/rtc_base/experiments/audio_allocation_settings.h index a932c1334d..bcc0f3e90b 100644 --- a/rtc_base/experiments/audio_allocation_settings.h +++ b/rtc_base/experiments/audio_allocation_settings.h @@ -75,6 +75,10 @@ class AudioAllocationSettings { // overhead. |rtp_parameter_max_bitrate_bps| max bitrate as configured in rtp // parameters, excluding overhead. int MaxBitrateBps(absl::optional rtp_parameter_max_bitrate_bps) const; + // Indicates the default priority bitrate for audio streams. The bitrate + // allocator will prioritize audio until it reaches this bitrate and will + // divide bitrate evently between audio and video above this bitrate. + DataRate DefaultPriorityBitrate() const; private: FieldTrialFlag audio_send_side_bwe_; @@ -83,6 +87,11 @@ class AudioAllocationSettings { FieldTrialFlag audio_feedback_to_improve_video_bwe_; FieldTrialFlag send_side_bwe_with_overhead_; int min_overhead_bps_ = 0; + // Default bitrates to use as range if there's no user configured + // bitrate range but audio bitrate allocation is enabled. + FieldTrialParameter default_min_bitrate_; + FieldTrialParameter default_max_bitrate_; + FieldTrialParameter priority_bitrate_; }; } // namespace webrtc diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index 5d60a418ce..b8088ea8bd 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -385,13 +385,7 @@ void VideoSendStreamImpl::Start() { void VideoSendStreamImpl::StartupVideoSendStream() { RTC_DCHECK_RUN_ON(worker_queue_); - bitrate_allocator_->AddObserver( - this, - MediaStreamAllocationConfig{ - static_cast(encoder_min_bitrate_bps_), - encoder_max_bitrate_bps_, static_cast(max_padding_bitrate_), - !config_->suspend_below_min_bitrate, config_->track_id, - encoder_bitrate_priority_}); + bitrate_allocator_->AddObserver(this, GetAllocationConfig()); // Start monitoring encoder activity. { RTC_DCHECK(!check_encoder_activity_task_.Running()); @@ -498,13 +492,18 @@ void VideoSendStreamImpl::OnBitrateAllocationUpdated( void VideoSendStreamImpl::SignalEncoderActive() { RTC_DCHECK_RUN_ON(worker_queue_); RTC_LOG(LS_INFO) << "SignalEncoderActive, Encoder is active."; - bitrate_allocator_->AddObserver( - this, - MediaStreamAllocationConfig{ - static_cast(encoder_min_bitrate_bps_), - encoder_max_bitrate_bps_, static_cast(max_padding_bitrate_), - !config_->suspend_below_min_bitrate, config_->track_id, - encoder_bitrate_priority_}); + bitrate_allocator_->AddObserver(this, GetAllocationConfig()); +} + +MediaStreamAllocationConfig VideoSendStreamImpl::GetAllocationConfig() const { + return MediaStreamAllocationConfig{ + static_cast(encoder_min_bitrate_bps_), + encoder_max_bitrate_bps_, + static_cast(max_padding_bitrate_), + /* priority_bitrate */ 0, + !config_->suspend_below_min_bitrate, + config_->track_id, + encoder_bitrate_priority_}; } void VideoSendStreamImpl::OnEncoderConfigurationChanged( @@ -571,13 +570,7 @@ void VideoSendStreamImpl::OnEncoderConfigurationChanged( if (rtp_video_sender_->IsActive()) { // The send stream is started already. Update the allocator with new bitrate // limits. - bitrate_allocator_->AddObserver( - this, MediaStreamAllocationConfig{ - static_cast(encoder_min_bitrate_bps_), - encoder_max_bitrate_bps_, - static_cast(max_padding_bitrate_), - !config_->suspend_below_min_bitrate, config_->track_id, - encoder_bitrate_priority_}); + bitrate_allocator_->AddObserver(this, GetAllocationConfig()); } } diff --git a/video/video_send_stream_impl.h b/video/video_send_stream_impl.h index 4c5e3e4634..1cdad60d2c 100644 --- a/video/video_send_stream_impl.h +++ b/video/video_send_stream_impl.h @@ -139,6 +139,8 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, void ConfigureSsrcs(); void SignalEncoderTimedOut(); void SignalEncoderActive(); + MediaStreamAllocationConfig GetAllocationConfig() const + RTC_RUN_ON(worker_queue_); const bool has_alr_probing_; const PacingConfig pacing_config_;