From 79f0d4d0c7f92af925f9c8d6fc6f8678d51f2c92 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Wed, 23 Jan 2019 09:41:43 +0100 Subject: [PATCH] Enables feature to account for unacknowledged data. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By enabling this trial, we can also remove reporting of packet feedback status from send streams that was used before. Bug: webrtc:9718 Change-Id: I3e7c4656b0ac6592a834617e044f23a072454181 Reviewed-on: https://webrtc-review.googlesource.com/c/118281 Reviewed-by: Oskar Sundbom Reviewed-by: Erik Språng Reviewed-by: Christoffer Rodbro Cr-Commit-Position: refs/heads/master@{#26363} --- api/transport/network_types.h | 2 -- audio/audio_send_stream.cc | 20 ++++++------ audio/audio_send_stream.h | 3 +- audio/audio_send_stream_unittest.cc | 6 ++-- call/bitrate_allocator.cc | 31 +++++-------------- call/bitrate_allocator.h | 25 ++++----------- call/bitrate_allocator_unittest.cc | 12 ++----- call/call.cc | 11 ++----- call/rtp_transport_controller_send.cc | 23 -------------- call/rtp_transport_controller_send.h | 4 --- .../rtp_transport_controller_send_interface.h | 3 -- .../test/mock_rtp_transport_controller_send.h | 2 -- .../bbr/bbr_network_controller.cc | 2 -- .../goog_cc/acknowledged_bitrate_estimator.cc | 24 ++------------ .../goog_cc/acknowledged_bitrate_estimator.h | 3 -- .../goog_cc/goog_cc_network_control.cc | 2 -- .../pcc/pcc_network_controller.cc | 2 -- .../send_side_congestion_controller.cc | 2 -- video/video_send_stream_impl.cc | 6 ++-- video/video_send_stream_impl_unittest.cc | 23 -------------- 20 files changed, 36 insertions(+), 170 deletions(-) diff --git a/api/transport/network_types.h b/api/transport/network_types.h index ad484ea800..6caa7a1356 100644 --- a/api/transport/network_types.h +++ b/api/transport/network_types.h @@ -36,8 +36,6 @@ struct StreamsConfig { absl::optional min_pacing_rate; absl::optional max_padding_rate; absl::optional max_total_allocated_bitrate; - // The send rate of traffic for which feedback is not received. - DataRate unacknowledged_rate_allocation = DataRate::Zero(); }; struct TargetRateConstraints { diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 0c9a9ffe10..99a237f722 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -320,8 +320,7 @@ void AudioSendStream::Start() { rtp_transport_->packet_sender()->SetAccountForAudioPackets(true); rtp_rtcp_module_->SetAsPartOfAllocation(true); ConfigureBitrateObserver(config_.min_bitrate_bps, config_.max_bitrate_bps, - config_.bitrate_priority, - has_transport_sequence_number); + config_.bitrate_priority); } else { rtp_rtcp_module_->SetAsPartOfAllocation(false); } @@ -729,10 +728,10 @@ void AudioSendStream::ReconfigureBitrateObserver( (has_transport_sequence_number || !webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe"))) { stream->rtp_transport_->packet_sender()->SetAccountForAudioPackets(true); - stream->ConfigureBitrateObserver( - new_config.min_bitrate_bps, new_config.max_bitrate_bps, - new_config.bitrate_priority, has_transport_sequence_number); stream->rtp_rtcp_module_->SetAsPartOfAllocation(true); + stream->ConfigureBitrateObserver(new_config.min_bitrate_bps, + new_config.max_bitrate_bps, + new_config.bitrate_priority); } else { stream->rtp_transport_->packet_sender()->SetAccountForAudioPackets(false); stream->RemoveBitrateObserver(); @@ -742,8 +741,7 @@ void AudioSendStream::ReconfigureBitrateObserver( void AudioSendStream::ConfigureBitrateObserver(int min_bitrate_bps, int max_bitrate_bps, - double bitrate_priority, - bool has_packet_feedback) { + double bitrate_priority) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); RTC_DCHECK_GE(max_bitrate_bps, min_bitrate_bps); rtc::Event thread_sync_event; @@ -755,10 +753,10 @@ 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, has_packet_feedback}); + this, + MediaStreamAllocationConfig{static_cast(min_bitrate_bps), + static_cast(max_bitrate_bps), 0, + true, config_.track_id, bitrate_priority}); thread_sync_event.Set(); }); thread_sync_event.Wait(rtc::Event::kForever); diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index 2ba8d061c7..38cccd4f31 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h @@ -112,8 +112,7 @@ class AudioSendStream final : public webrtc::AudioSendStream, void ConfigureBitrateObserver(int min_bitrate_bps, int max_bitrate_bps, - double bitrate_priority, - bool has_packet_feedback); + double bitrate_priority); void RemoveBitrateObserver(); void RegisterCngPayloadType(int payload_type, int clockrate_hz); diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 1bf8bcae4d..5aaa07ece5 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -72,12 +72,10 @@ const AudioCodecSpec kCodecSpecs[] = { class MockLimitObserver : public BitrateAllocator::LimitObserver { public: - MOCK_METHOD5(OnAllocationLimitsChanged, + MOCK_METHOD3(OnAllocationLimitsChanged, void(uint32_t min_send_bitrate_bps, uint32_t max_padding_bitrate_bps, - uint32_t total_bitrate_bps, - uint32_t allocated_without_feedback_bps, - bool has_packet_feedback)); + uint32_t total_bitrate_bps)); }; std::unique_ptr SetupAudioEncoderMock( diff --git a/call/bitrate_allocator.cc b/call/bitrate_allocator.cc index e23fa89db3..172f9401f1 100644 --- a/call/bitrate_allocator.cc +++ b/call/bitrate_allocator.cc @@ -64,8 +64,6 @@ BitrateAllocator::BitrateAllocator(LimitObserver* limit_observer) total_requested_padding_bitrate_(0), total_requested_min_bitrate_(0), total_requested_max_bitrate_(0), - allocated_without_feedback_(0), - has_packet_feedback_(false), bitrate_allocation_strategy_(nullptr), transmission_max_bitrate_multiplier_( GetTransmissionMaxBitrateMultiplier()) { @@ -175,10 +173,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, config.has_packet_feedback)); + 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)); } if (last_target_bps_ > 0) { @@ -221,8 +219,6 @@ void BitrateAllocator::UpdateAllocationLimits() { uint32_t total_requested_padding_bitrate = 0; uint32_t total_requested_min_bitrate = 0; uint32_t total_requested_max_bitrate = 0; - bool has_packet_feedback = false; - uint32_t allocated_without_feedback = 0; for (const auto& config : bitrate_observer_configs_) { uint32_t stream_padding = config.pad_up_bitrate_bps; if (config.enforce_min_bitrate) { @@ -240,27 +236,17 @@ void BitrateAllocator::UpdateAllocationLimits() { max_bitrate_bps *= 2; } total_requested_max_bitrate += max_bitrate_bps; - if (config.allocated_bitrate_bps > 0 && config.has_packet_feedback) - has_packet_feedback = true; - // TODO(srte): Remove field trial check. - if (!config.has_packet_feedback && - field_trial::IsEnabled("WebRTC-Audio-ABWENoTWCC")) - allocated_without_feedback += config.allocated_bitrate_bps; } if (total_requested_padding_bitrate == total_requested_padding_bitrate_ && total_requested_min_bitrate == total_requested_min_bitrate_ && - total_requested_max_bitrate == total_requested_max_bitrate_ && - allocated_without_feedback == allocated_without_feedback_ && - has_packet_feedback == has_packet_feedback_) { + total_requested_max_bitrate == total_requested_max_bitrate_) { return; } total_requested_min_bitrate_ = total_requested_min_bitrate; total_requested_padding_bitrate_ = total_requested_padding_bitrate; total_requested_max_bitrate_ = total_requested_max_bitrate; - allocated_without_feedback_ = allocated_without_feedback; - has_packet_feedback_ = has_packet_feedback; RTC_LOG(LS_INFO) << "UpdateAllocationLimits : total_requested_min_bitrate: " << total_requested_min_bitrate @@ -268,10 +254,9 @@ void BitrateAllocator::UpdateAllocationLimits() { << total_requested_padding_bitrate << "bps, total_requested_max_bitrate: " << total_requested_max_bitrate << "bps"; - limit_observer_->OnAllocationLimitsChanged( - total_requested_min_bitrate, total_requested_padding_bitrate, - total_requested_max_bitrate, allocated_without_feedback, - has_packet_feedback); + limit_observer_->OnAllocationLimitsChanged(total_requested_min_bitrate, + total_requested_padding_bitrate, + total_requested_max_bitrate); } void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) { diff --git a/call/bitrate_allocator.h b/call/bitrate_allocator.h index 0259da7e72..1323aa3288 100644 --- a/call/bitrate_allocator.h +++ b/call/bitrate_allocator.h @@ -47,11 +47,7 @@ class BitrateAllocatorObserver { // |enforce_min_bitrate| = 'true' will allocate at least |min_bitrate_bps| for // this observer, even if the BWE is too low, 'false' will allocate 0 to // the observer if BWE doesn't allow |min_bitrate_bps|. -// |has_packet_feedback| indicates whether the data produced by the -// corresponding media stream will receive per packet feedback. This is -// tracked here to communicate to limit observers whether packet feedback can -// be expected, which is true if any of the active observers has packet -// feedback enabled. Note that |observer|->OnBitrateUpdated() will be called +// Note that |observer|->OnBitrateUpdated() will be called // within the scope of this method with the current rtt, fraction_loss and // available bitrate and that the bitrate in OnBitrateUpdated will be zero if // the |observer| is currently not allowed to send data. @@ -62,7 +58,6 @@ struct MediaStreamAllocationConfig { bool enforce_min_bitrate; std::string track_id; double bitrate_priority; - bool has_packet_feedback; }; // Interface used for mocking @@ -86,12 +81,9 @@ class BitrateAllocator : public BitrateAllocatorInterface { // bitrate and max padding bitrate is changed. class LimitObserver { public: - virtual void OnAllocationLimitsChanged( - uint32_t min_send_bitrate_bps, - uint32_t max_padding_bitrate_bps, - uint32_t total_bitrate_bps, - uint32_t allocated_without_feedback_bps, - bool has_packet_feedback) = 0; + virtual void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, + uint32_t max_padding_bitrate_bps, + uint32_t total_bitrate_bps) = 0; protected: virtual ~LimitObserver() = default; @@ -138,8 +130,7 @@ class BitrateAllocator : public BitrateAllocatorInterface { uint32_t pad_up_bitrate_bps, bool enforce_min_bitrate, std::string track_id, - double bitrate_priority, - bool has_packet_feedback) + double bitrate_priority) : TrackConfig(min_bitrate_bps, max_bitrate_bps, enforce_min_bitrate, @@ -148,8 +139,7 @@ class BitrateAllocator : public BitrateAllocatorInterface { pad_up_bitrate_bps(pad_up_bitrate_bps), allocated_bitrate_bps(-1), media_ratio(1.0), - bitrate_priority(bitrate_priority), - has_packet_feedback(has_packet_feedback) {} + bitrate_priority(bitrate_priority) {} BitrateAllocatorObserver* observer; uint32_t pad_up_bitrate_bps; @@ -159,7 +149,6 @@ class BitrateAllocator : public BitrateAllocatorInterface { // observers. If an observer has twice the bitrate_priority of other // observers, it should be allocated twice the bitrate above its min. double bitrate_priority; - bool has_packet_feedback; uint32_t LastAllocatedBitrate() const; // The minimum bitrate required by this observer, including @@ -250,8 +239,6 @@ class BitrateAllocator : public BitrateAllocatorInterface { uint32_t total_requested_padding_bitrate_ RTC_GUARDED_BY(&sequenced_checker_); uint32_t total_requested_min_bitrate_ RTC_GUARDED_BY(&sequenced_checker_); uint32_t total_requested_max_bitrate_ RTC_GUARDED_BY(&sequenced_checker_); - uint32_t allocated_without_feedback_ RTC_GUARDED_BY(&sequenced_checker_); - bool has_packet_feedback_ RTC_GUARDED_BY(&sequenced_checker_); std::unique_ptr bitrate_allocation_strategy_ RTC_GUARDED_BY(&sequenced_checker_); const uint8_t transmission_max_bitrate_multiplier_; diff --git a/call/bitrate_allocator_unittest.cc b/call/bitrate_allocator_unittest.cc index db085f35ff..b1df347caf 100644 --- a/call/bitrate_allocator_unittest.cc +++ b/call/bitrate_allocator_unittest.cc @@ -25,14 +25,6 @@ namespace webrtc { // TODO(srte): Update tests to reflect new interface. class LimitObserverWrapper : public BitrateAllocator::LimitObserver { public: - void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, - uint32_t max_padding_bitrate_bps, - uint32_t total_bitrate_bps, - uint32_t allocated_without_feedback_bps, - bool has_packet_feedback) override { - OnAllocationLimitsChanged(min_send_bitrate_bps, max_padding_bitrate_bps, - total_bitrate_bps); - } virtual void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, uint32_t max_padding_bitrate_bps, uint32_t total_bitrate_bps) = 0; @@ -107,7 +99,7 @@ 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, false}); + enforce_min_bitrate, track_id, bitrate_priority}); } NiceMock limit_observer_; @@ -240,7 +232,7 @@ class BitrateAllocatorTestNoEnforceMin : 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, false}); + enforce_min_bitrate, track_id, bitrate_priority}); } NiceMock limit_observer_; std::unique_ptr allocator_; diff --git a/call/call.cc b/call/call.cc index a57a41c1a7..2361c72c08 100644 --- a/call/call.cc +++ b/call/call.cc @@ -220,9 +220,7 @@ class Call final : public webrtc::Call, // Implements BitrateAllocator::LimitObserver. void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, uint32_t max_padding_bitrate_bps, - uint32_t total_bitrate_bps, - uint32_t allocated_without_feedback_bps, - bool has_packet_feedback) override; + uint32_t total_bitrate_bps) override; // This method is invoked when the media transport is created and when the // media transport is being destructed. @@ -1178,14 +1176,9 @@ void Call::OnTargetTransferRate(TargetTransferRate msg) { void Call::OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, uint32_t max_padding_bitrate_bps, - uint32_t total_bitrate_bps, - uint32_t allocated_without_feedback_bps, - bool has_packet_feedback) { + uint32_t total_bitrate_bps) { transport_send_ptr_->SetAllocatedSendBitrateLimits( min_send_bitrate_bps, max_padding_bitrate_bps, total_bitrate_bps); - transport_send_ptr_->SetPerPacketFeedbackAvailable(has_packet_feedback); - transport_send_ptr_->SetAllocatedBitrateWithoutFeedback( - allocated_without_feedback_bps); rtc::CritScope lock(&bitrate_crit_); min_allocated_send_bitrate_bps_ = min_send_bitrate_bps; diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index e519a76544..777d7f8061 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -78,7 +78,6 @@ RtpTransportControllerSend::RtpTransportControllerSend( field_trial::IsEnabled("WebRTC-AddPacingToCongestionWindowPushback")), transport_overhead_bytes_per_packet_(0), network_available_(false), - packet_feedback_available_(false), retransmission_rate_limiter_(clock, kRetransmitWindowSizeMs), task_queue_("rtp_send_controller") { initial_config_.constraints = ConvertConstraints(bitrate_config, clock_); @@ -303,12 +302,6 @@ int64_t RtpTransportControllerSend::GetPacerQueuingDelayMs() const { int64_t RtpTransportControllerSend::GetFirstPacketTimeMs() const { return pacer_.FirstSentPacketTimeMs(); } -void RtpTransportControllerSend::SetPerPacketFeedbackAvailable(bool available) { - RTC_DCHECK_RUN_ON(&task_queue_); - packet_feedback_available_ = available; - if (!controller_) - MaybeCreateControllers(); -} void RtpTransportControllerSend::EnablePeriodicAlrProbing(bool enable) { task_queue_.PostTask([this, enable]() { RTC_DCHECK_RUN_ON(&task_queue_); @@ -373,22 +366,6 @@ void RtpTransportControllerSend::SetClientBitratePreferences( } } -void RtpTransportControllerSend::SetAllocatedBitrateWithoutFeedback( - uint32_t bitrate_bps) { - // Audio transport feedback will not be reported in this mode, instead update - // acknowledged bitrate estimator with the bitrate allocated for audio. - if (field_trial::IsEnabled("WebRTC-Audio-ABWENoTWCC")) { - // TODO(srte): Make sure it's safe to always report this and remove the - // field trial check. - task_queue_.PostTask([this, bitrate_bps]() { - RTC_DCHECK_RUN_ON(&task_queue_); - streams_config_.unacknowledged_rate_allocation = - DataRate::bps(bitrate_bps); - UpdateStreamsConfig(); - }); - } -} - void RtpTransportControllerSend::OnTransportOverheadChanged( size_t transport_overhead_bytes_per_packet) { if (transport_overhead_bytes_per_packet >= kMaxOverheadBytes) { diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index 6a15648738..deef04028e 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -94,15 +94,12 @@ class RtpTransportControllerSend final RtcpBandwidthObserver* GetBandwidthObserver() override; int64_t GetPacerQueuingDelayMs() const override; int64_t GetFirstPacketTimeMs() const override; - void SetPerPacketFeedbackAvailable(bool available) override; void EnablePeriodicAlrProbing(bool enable) override; void OnSentPacket(const rtc::SentPacket& sent_packet) override; void SetSdpBitrateParameters(const BitrateConstraints& constraints) override; void SetClientBitratePreferences(const BitrateSettings& preferences) override; - void SetAllocatedBitrateWithoutFeedback(uint32_t bitrate_bps) override; - void OnTransportOverheadChanged( size_t transport_overhead_per_packet) override; @@ -178,7 +175,6 @@ class RtpTransportControllerSend final // TODO(srte): Remove atomic when feedback adapter runs on task queue. std::atomic transport_overhead_bytes_per_packet_; bool network_available_ RTC_GUARDED_BY(task_queue_); - bool packet_feedback_available_ RTC_GUARDED_BY(task_queue_); RepeatingTaskHandle pacer_queue_update_task_ RTC_GUARDED_BY(task_queue_); RepeatingTaskHandle controller_task_ RTC_GUARDED_BY(task_queue_); // Protects access to last_packet_feedback_vector_ in feedback adapter. diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index ef6fad296f..e8c1fccdd0 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -151,15 +151,12 @@ class RtpTransportControllerSendInterface { virtual int64_t GetFirstPacketTimeMs() const = 0; virtual void EnablePeriodicAlrProbing(bool enable) = 0; virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0; - virtual void SetPerPacketFeedbackAvailable(bool available) = 0; virtual void SetSdpBitrateParameters( const BitrateConstraints& constraints) = 0; virtual void SetClientBitratePreferences( const BitrateSettings& preferences) = 0; - virtual void SetAllocatedBitrateWithoutFeedback(uint32_t bitrate_bps) = 0; - virtual void OnTransportOverheadChanged( size_t transport_overhead_per_packet) = 0; }; diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index 18bb117fac..dd9fc85bf8 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -63,12 +63,10 @@ class MockRtpTransportControllerSend MOCK_METHOD0(GetBandwidthObserver, RtcpBandwidthObserver*()); MOCK_CONST_METHOD0(GetPacerQueuingDelayMs, int64_t()); MOCK_CONST_METHOD0(GetFirstPacketTimeMs, int64_t()); - MOCK_METHOD1(SetPerPacketFeedbackAvailable, void(bool)); MOCK_METHOD1(EnablePeriodicAlrProbing, void(bool)); MOCK_METHOD1(OnSentPacket, void(const rtc::SentPacket&)); MOCK_METHOD1(SetSdpBitrateParameters, void(const BitrateConstraints&)); MOCK_METHOD1(SetClientBitratePreferences, void(const BitrateSettings&)); - MOCK_METHOD1(SetAllocatedBitrateWithoutFeedback, void(uint32_t)); MOCK_METHOD1(OnTransportOverheadChanged, void(size_t)); }; } // namespace webrtc diff --git a/modules/congestion_controller/bbr/bbr_network_controller.cc b/modules/congestion_controller/bbr/bbr_network_controller.cc index 863ce4bf98..c61a794f97 100644 --- a/modules/congestion_controller/bbr/bbr_network_controller.cc +++ b/modules/congestion_controller/bbr/bbr_network_controller.cc @@ -323,8 +323,6 @@ NetworkControlUpdate BbrNetworkController::OnProcessInterval( } NetworkControlUpdate BbrNetworkController::OnStreamsConfig(StreamsConfig msg) { - // TODO(srte): Handle unacknowledged rate allocation. - RTC_DCHECK(msg.unacknowledged_rate_allocation.IsZero()); return NetworkControlUpdate(); } diff --git a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.cc b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.cc index 37be68dcf4..c37c978dd0 100644 --- a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.cc +++ b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.cc @@ -38,10 +38,7 @@ AcknowledgedBitrateEstimator::~AcknowledgedBitrateEstimator() {} AcknowledgedBitrateEstimator::AcknowledgedBitrateEstimator( const WebRtcKeyValueConfig* key_value_config, std::unique_ptr bitrate_estimator) - : account_for_unacknowledged_traffic_( - key_value_config->Lookup("WebRTC-Bwe-AccountForUnacked") - .find("Enabled") == 0), - bitrate_estimator_(std::move(bitrate_estimator)) {} + : bitrate_estimator_(std::move(bitrate_estimator)) {} void AcknowledgedBitrateEstimator::IncomingPacketFeedbackVector( const std::vector& packet_feedback_vector) { @@ -52,24 +49,14 @@ void AcknowledgedBitrateEstimator::IncomingPacketFeedbackVector( if (IsInSendTimeHistory(packet)) { MaybeExpectFastRateChange(packet.send_time_ms); int acknowledged_estimate = rtc::dchecked_cast(packet.payload_size); - if (account_for_unacknowledged_traffic_) - acknowledged_estimate += packet.unacknowledged_data; + acknowledged_estimate += packet.unacknowledged_data; bitrate_estimator_->Update(packet.arrival_time_ms, acknowledged_estimate); } } } absl::optional AcknowledgedBitrateEstimator::bitrate_bps() const { - auto estimated_bitrate = bitrate_estimator_->bitrate_bps(); - // If we account for unacknowledged traffic, we should not add the allocated - // bitrate for unallocated stream as we expect it to be included already. - if (account_for_unacknowledged_traffic_) { - return estimated_bitrate; - } else { - return estimated_bitrate - ? *estimated_bitrate + allocated_bitrate_without_feedback_bps_ - : estimated_bitrate; - } + return bitrate_estimator_->bitrate_bps(); } absl::optional AcknowledgedBitrateEstimator::PeekBps() const { @@ -93,11 +80,6 @@ void AcknowledgedBitrateEstimator::SetAlrEndedTimeMs( alr_ended_time_ms_.emplace(alr_ended_time_ms); } -void AcknowledgedBitrateEstimator::SetAllocatedBitrateWithoutFeedback( - uint32_t bitrate_bps) { - allocated_bitrate_without_feedback_bps_ = bitrate_bps; -} - void AcknowledgedBitrateEstimator::MaybeExpectFastRateChange( int64_t packet_send_time_ms) { if (alr_ended_time_ms_ && packet_send_time_ms > *alr_ended_time_ms_) { diff --git a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h index 48f8057a10..343cef9335 100644 --- a/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h +++ b/modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h @@ -40,14 +40,11 @@ class AcknowledgedBitrateEstimator { absl::optional bitrate() const; absl::optional PeekRate() const; void SetAlrEndedTimeMs(int64_t alr_ended_time_ms); - void SetAllocatedBitrateWithoutFeedback(uint32_t bitrate_bps); private: void MaybeExpectFastRateChange(int64_t packet_arrival_time_ms); - const bool account_for_unacknowledged_traffic_; absl::optional alr_ended_time_ms_; std::unique_ptr bitrate_estimator_; - uint32_t allocated_bitrate_without_feedback_bps_ = 0; }; } // namespace webrtc diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc index d7b056523a..9c82b70e78 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -333,8 +333,6 @@ NetworkControlUpdate GoogCcNetworkController::OnStreamsConfig( max_padding_rate_ = *msg.max_padding_rate; pacing_changed = true; } - acknowledged_bitrate_estimator_->SetAllocatedBitrateWithoutFeedback( - msg.unacknowledged_rate_allocation.bps()); if (pacing_changed) update.pacer_config = GetPacingRates(msg.at_time); diff --git a/modules/congestion_controller/pcc/pcc_network_controller.cc b/modules/congestion_controller/pcc/pcc_network_controller.cc index 586f198386..e5385f6f3e 100644 --- a/modules/congestion_controller/pcc/pcc_network_controller.cc +++ b/modules/congestion_controller/pcc/pcc_network_controller.cc @@ -373,8 +373,6 @@ NetworkControlUpdate PccNetworkController::OnTransportLossReport( } NetworkControlUpdate PccNetworkController::OnStreamsConfig(StreamsConfig msg) { - // TODO(srte): Handle unacknowledged rate allocation. - RTC_DCHECK(msg.unacknowledged_rate_allocation.IsZero()); return NetworkControlUpdate(); } diff --git a/modules/congestion_controller/send_side_congestion_controller.cc b/modules/congestion_controller/send_side_congestion_controller.cc index ef1e2a1e55..af0b14fc1b 100644 --- a/modules/congestion_controller/send_side_congestion_controller.cc +++ b/modules/congestion_controller/send_side_congestion_controller.cc @@ -475,8 +475,6 @@ void DEPRECATED_SendSideCongestionController::SetPacingFactor( void DEPRECATED_SendSideCongestionController:: SetAllocatedBitrateWithoutFeedback(uint32_t bitrate_bps) { - acknowledged_bitrate_estimator_->SetAllocatedBitrateWithoutFeedback( - bitrate_bps); } void DEPRECATED_SendSideCongestionController::MaybeTriggerOnNetworkChanged() { diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index 2b6dbb5e11..22c485ddbb 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -375,7 +375,7 @@ void VideoSendStreamImpl::StartupVideoSendStream() { 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_, has_packet_feedback_}); + encoder_bitrate_priority_}); // Start monitoring encoder activity. { RTC_DCHECK(!check_encoder_activity_task_.Running()); @@ -488,7 +488,7 @@ void VideoSendStreamImpl::SignalEncoderActive() { 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_, has_packet_feedback_}); + encoder_bitrate_priority_}); } void VideoSendStreamImpl::OnEncoderConfigurationChanged( @@ -557,7 +557,7 @@ void VideoSendStreamImpl::OnEncoderConfigurationChanged( encoder_max_bitrate_bps_, static_cast(max_padding_bitrate_), !config_->suspend_below_min_bitrate, config_->track_id, - encoder_bitrate_priority_, has_packet_feedback_}); + encoder_bitrate_priority_}); } } diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index af8c307488..e57e36ddc8 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -166,7 +166,6 @@ TEST_F(VideoSendStreamImplTest, RegistersAsBitrateObserverOnStart) { EXPECT_EQ(config.enforce_min_bitrate, !kSuspend); EXPECT_EQ(config.track_id, "test"); EXPECT_EQ(config.bitrate_priority, kDefaultBitratePriority); - EXPECT_EQ(config.has_packet_feedback, false); })); vss_impl->Start(); EXPECT_CALL(bitrate_allocator_, RemoveObserver(vss_impl.get())).Times(1); @@ -224,7 +223,6 @@ TEST_F(VideoSendStreamImplTest, UpdatesObserverOnConfigurationChange) { static_cast(qvga_stream.target_bitrate_bps + vga_stream.min_bitrate_bps)); EXPECT_EQ(config.enforce_min_bitrate, !kSuspend); - EXPECT_EQ(config.has_packet_feedback, true); })); static_cast(vss_impl.get()) @@ -289,7 +287,6 @@ TEST_F(VideoSendStreamImplTest, UpdatesObserverOnConfigurationChangeWithAlr) { EXPECT_EQ(config.pad_up_bitrate_bps, static_cast(min_transmit_bitrate_bps)); EXPECT_EQ(config.enforce_min_bitrate, !kSuspend); - EXPECT_EQ(config.has_packet_feedback, true); })); static_cast(vss_impl.get()) @@ -300,26 +297,6 @@ TEST_F(VideoSendStreamImplTest, UpdatesObserverOnConfigurationChangeWithAlr) { }); } -TEST_F(VideoSendStreamImplTest, ReportFeedbackAvailability) { - test_queue_.SendTask([this] { - config_.rtp.extensions.emplace_back( - RtpExtension::kTransportSequenceNumberUri, - RtpExtension::kTransportSequenceNumberDefaultId); - - auto vss_impl = CreateVideoSendStreamImpl( - kDefaultInitialBitrateBps, kDefaultBitratePriority, - VideoEncoderConfig::ContentType::kRealtimeVideo); - EXPECT_CALL(bitrate_allocator_, AddObserver(vss_impl.get(), _)) - .WillOnce(Invoke( - [&](BitrateAllocatorObserver*, MediaStreamAllocationConfig config) { - EXPECT_EQ(config.has_packet_feedback, true); - })); - vss_impl->Start(); - EXPECT_CALL(bitrate_allocator_, RemoveObserver(vss_impl.get())).Times(1); - vss_impl->Stop(); - }); -} - TEST_F(VideoSendStreamImplTest, SetsScreensharePacingFactorWithFeedback) { test::ScopedFieldTrials alr_experiment(GetAlrProbingExperimentString());