diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 5e3b9ffc9c..ba13fcbe8b 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -342,6 +342,8 @@ void AudioSendStream::Start() { config_.max_bitrate_bps != -1 && (allocate_audio_without_feedback_ || TransportSeqNumId(config_) != 0)) { rtp_transport_->AccountForAudioPacketsInPacedSender(true); + if (send_side_bwe_with_overhead_) + rtp_transport_->IncludeOverheadInPacedSender(); rtp_rtcp_module_->SetAsPartOfAllocation(true); rtc::Event thread_sync_event; worker_queue_->PostTask([&] { @@ -765,6 +767,8 @@ void AudioSendStream::ReconfigureBitrateObserver( if (!new_config.has_dscp && new_config.min_bitrate_bps != -1 && new_config.max_bitrate_bps != -1 && TransportSeqNumId(new_config) != 0) { rtp_transport_->AccountForAudioPacketsInPacedSender(true); + if (send_side_bwe_with_overhead_) + rtp_transport_->IncludeOverheadInPacedSender(); rtc::Event thread_sync_event; worker_queue_->PostTask([&] { RTC_DCHECK_RUN_ON(worker_queue_); diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 04723664ee..3b9fbb7f39 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -490,6 +490,8 @@ TEST(AudioSendStreamTest, SendCodecAppliesAudioNetworkAdaptor) { const std::string kAnaConfigString = "abcde"; const std::string kAnaReconfigString = "12345"; + helper.config().rtp.extensions.push_back(RtpExtension( + RtpExtension::kTransportSequenceNumberUri, kTransportSequenceNumberId)); helper.config().audio_network_adaptor_config = kAnaConfigString; EXPECT_CALL(helper.mock_encoder_factory(), MakeAudioEncoderMock(_, _, _, _)) diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 62b7008396..c2946adbaf 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -434,6 +434,10 @@ void RtpTransportControllerSend::AccountForAudioPacketsInPacedSender( pacer()->SetAccountForAudioPackets(account_for_audio); } +void RtpTransportControllerSend::IncludeOverheadInPacedSender() { + pacer()->SetIncludeOverhead(); +} + void RtpTransportControllerSend::OnReceivedEstimatedBitrate(uint32_t bitrate) { RemoteBitrateReport msg; msg.receive_time = Timestamp::ms(clock_->TimeInMilliseconds()); diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index f74c4e598f..b07bea73d8 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -107,6 +107,7 @@ class RtpTransportControllerSend final size_t transport_overhead_per_packet) override; void AccountForAudioPacketsInPacedSender(bool account_for_audio) override; + void IncludeOverheadInPacedSender() override; // Implements RtcpBandwidthObserver interface void OnReceivedEstimatedBitrate(uint32_t bitrate) override; diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index 1e881dc42c..b40aabdc2c 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -150,6 +150,7 @@ class RtpTransportControllerSendInterface { size_t transport_overhead_per_packet) = 0; virtual void AccountForAudioPacketsInPacedSender(bool account_for_audio) = 0; + virtual void IncludeOverheadInPacedSender() = 0; }; } // namespace webrtc diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index a926eb514c..413171fa67 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -279,6 +279,11 @@ absl::optional GetVideoCodecType(const RtpConfig& config) { } return PayloadStringToCodecType(config.payload_name); } +bool TransportSeqNumExtensionConfigured(const RtpConfig& config_config) { + return absl::c_any_of(config_config.extensions, [](const RtpExtension& ext) { + return ext.uri == RtpExtension::kTransportSequenceNumberUri; + }); +} } // namespace RtpVideoSender::RtpVideoSender( @@ -301,6 +306,7 @@ RtpVideoSender::RtpVideoSender( "WebRTC-SubtractPacketizationOverhead")), use_early_loss_detection_( !webrtc::field_trial::IsDisabled("WebRTC-UseEarlyLossDetection")), + has_packet_feedback_(TransportSeqNumExtensionConfigured(rtp_config)), active_(false), module_process_thread_(nullptr), suspended_ssrcs_(std::move(suspended_ssrcs)), @@ -330,6 +336,8 @@ RtpVideoSender::RtpVideoSender( frame_counts_(rtp_config.ssrcs.size()), frame_count_observer_(observers.frame_count_observer) { RTC_DCHECK_EQ(rtp_config_.ssrcs.size(), rtp_streams_.size()); + if (send_side_bwe_with_overhead_ && has_packet_feedback_) + transport_->IncludeOverheadInPacedSender(); module_process_thread_checker_.Detach(); // SSRCs are assumed to be sorted in the same order as |rtp_modules|. for (uint32_t ssrc : rtp_config_.ssrcs) { @@ -700,7 +708,7 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, DataSize max_total_packet_size = DataSize::bytes( rtp_config_.max_packet_size + transport_overhead_bytes_per_packet_); uint32_t payload_bitrate_bps = update.target_bitrate.bps(); - if (send_side_bwe_with_overhead_) { + if (send_side_bwe_with_overhead_ && has_packet_feedback_) { DataRate overhead_rate = CalculateOverheadRate( update.target_bitrate, max_total_packet_size, packet_overhead); // TODO(srte): We probably should not accept 0 payload bitrate here. @@ -736,7 +744,7 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, loss_mask_vector_.clear(); uint32_t encoder_overhead_rate_bps = 0; - if (send_side_bwe_with_overhead_) { + if (send_side_bwe_with_overhead_ && has_packet_feedback_) { // TODO(srte): The packet size should probably be the same as in the // CalculateOverheadRate call above (just max_total_packet_size), it doesn't // make sense to use different packet rates for different overhead diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index fb01f1b263..eb7e4315be 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -163,6 +163,7 @@ class RtpVideoSender : public RtpVideoSenderInterface, const bool send_side_bwe_with_overhead_; const bool account_for_packetization_overhead_; const bool use_early_loss_detection_; + const bool has_packet_feedback_; // TODO(holmer): Remove crit_ once RtpVideoSender runs on the // transport task queue. diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index 04dac29f33..fad27b018f 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -67,6 +67,7 @@ class MockRtpTransportControllerSend MOCK_METHOD1(SetClientBitratePreferences, void(const BitrateSettings&)); MOCK_METHOD1(OnTransportOverheadChanged, void(size_t)); MOCK_METHOD1(AccountForAudioPacketsInPacedSender, void(bool)); + MOCK_METHOD0(IncludeOverheadInPacedSender, void()); MOCK_METHOD1(OnReceivedPacket, void(const ReceivedPacket&)); }; } // namespace webrtc diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc index 44cfe9e5a2..168bcec241 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc @@ -593,6 +593,11 @@ void AudioEncoderOpusImpl::OnReceivedUplinkPacketLossFraction( ApplyAudioNetworkAdaptor(); } +void AudioEncoderOpusImpl::OnReceivedTargetAudioBitrate( + int target_audio_bitrate_bps) { + SetTargetBitrate(target_audio_bitrate_bps); +} + void AudioEncoderOpusImpl::OnReceivedUplinkBandwidth( int target_audio_bitrate_bps, absl::optional bwe_period_ms, diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.h b/modules/audio_coding/codecs/opus/audio_encoder_opus.h index 66c489f79b..40fd167c10 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus.h +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.h @@ -104,6 +104,7 @@ class AudioEncoderOpusImpl final : public AudioEncoder { void DisableAudioNetworkAdaptor() override; void OnReceivedUplinkPacketLossFraction( float uplink_packet_loss_fraction) override; + void OnReceivedTargetAudioBitrate(int target_audio_bitrate_bps) override; void OnReceivedUplinkBandwidth( int target_audio_bitrate_bps, absl::optional bwe_period_ms) override; diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index f6c85d4ed3..6dc47b6892 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -126,6 +126,11 @@ void PacedSender::SetAccountForAudioPackets(bool account_for_audio) { pacing_controller_.SetAccountForAudioPackets(account_for_audio); } +void PacedSender::SetIncludeOverhead() { + rtc::CritScope cs(&critsect_); + pacing_controller_.SetIncludeOverhead(); +} + TimeDelta PacedSender::ExpectedQueueTime() const { rtc::CritScope cs(&critsect_); return pacing_controller_.ExpectedQueueTime(); diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h index 06a6c26e16..36913080e0 100644 --- a/modules/pacing/paced_sender.h +++ b/modules/pacing/paced_sender.h @@ -97,6 +97,8 @@ class PacedSender : public Module, // at high priority. void SetAccountForAudioPackets(bool account_for_audio) override; + void SetIncludeOverhead() override; + // Returns the time since the oldest queued packet was enqueued. TimeDelta OldestPacketWaitTime() const override; diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index e6dd7ac93a..09b76301fb 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -99,8 +99,6 @@ PacingController::PacingController(Clock* clock, pace_audio_(IsEnabled(*field_trials_, "WebRTC-Pacer-BlockAudio")), small_first_probe_packet_( IsEnabled(*field_trials_, "WebRTC-Pacer-SmallFirstProbePacket")), - send_side_bwe_with_overhead_( - IsEnabled(*field_trials_, "WebRTC-SendSideBwe-WithOverhead")), min_packet_limit_(kDefaultMinPacketLimit), last_timestamp_(clock_->CurrentTime()), paused_(false), @@ -120,7 +118,8 @@ PacingController::PacingController(Clock* clock, congestion_window_size_(DataSize::PlusInfinity()), outstanding_data_(DataSize::Zero()), queue_time_limit(kMaxExpectedQueueLength), - account_for_audio_(false) { + account_for_audio_(false), + include_overhead_(false) { if (!drain_large_queues_) { RTC_LOG(LS_WARNING) << "Pacer queues will not be drained," "pushback experiment must be enabled."; @@ -226,6 +225,11 @@ void PacingController::SetAccountForAudioPackets(bool account_for_audio) { account_for_audio_ = account_for_audio; } +void PacingController::SetIncludeOverhead() { + include_overhead_ = true; + packet_queue_.SetIncludeOverhead(); +} + TimeDelta PacingController::ExpectedQueueTime() const { RTC_DCHECK_GT(pacing_bitrate_, DataRate::Zero()); return TimeDelta::ms( @@ -517,10 +521,10 @@ void PacingController::ProcessPackets() { RTC_DCHECK(rtp_packet); RTC_DCHECK(rtp_packet->packet_type().has_value()); const RtpPacketToSend::Type packet_type = *rtp_packet->packet_type(); - const DataSize packet_size = DataSize::bytes( - send_side_bwe_with_overhead_ - ? rtp_packet->size() - : rtp_packet->payload_size() + rtp_packet->padding_size()); + const DataSize packet_size = + DataSize::bytes(include_overhead_ ? rtp_packet->size() + : rtp_packet->payload_size() + + rtp_packet->padding_size()); packet_sender_->SendRtpPacket(std::move(rtp_packet), pacing_info); data_sent += packet_size; diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index 75c0aa3b64..fb4d9d30c7 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -107,6 +107,7 @@ class PacingController { // the pacer budget calculation. The audio traffic still will be injected // at high priority. void SetAccountForAudioPackets(bool account_for_audio); + void SetIncludeOverhead(); // Returns the time since the oldest queued packet was enqueued. TimeDelta OldestPacketWaitTime() const; @@ -176,7 +177,6 @@ class PacingController { const bool send_padding_if_silent_; const bool pace_audio_; const bool small_first_probe_packet_; - const bool send_side_bwe_with_overhead_; TimeDelta min_packet_limit_; @@ -219,6 +219,7 @@ class PacingController { TimeDelta queue_time_limit; bool account_for_audio_; + bool include_overhead_; }; } // namespace webrtc diff --git a/modules/pacing/round_robin_packet_queue.cc b/modules/pacing/round_robin_packet_queue.cc index 16542b3a81..754ff5888a 100644 --- a/modules/pacing/round_robin_packet_queue.cc +++ b/modules/pacing/round_robin_packet_queue.cc @@ -93,6 +93,16 @@ void RoundRobinPacketQueue::QueuedPacket::SubtractPauseTime( enqueue_time_ -= pause_time_sum; } +RoundRobinPacketQueue::PriorityPacketQueue::const_iterator +RoundRobinPacketQueue::PriorityPacketQueue::begin() const { + return c.begin(); +} + +RoundRobinPacketQueue::PriorityPacketQueue::const_iterator +RoundRobinPacketQueue::PriorityPacketQueue::end() const { + return c.end(); +} + RoundRobinPacketQueue::Stream::Stream() : size(DataSize::Zero()), ssrc(0) {} RoundRobinPacketQueue::Stream::Stream(const Stream& stream) = default; RoundRobinPacketQueue::Stream::~Stream() = default; @@ -114,8 +124,7 @@ RoundRobinPacketQueue::RoundRobinPacketQueue( max_size_(kMaxLeadingSize), queue_time_sum_(TimeDelta::Zero()), pause_time_sum_(TimeDelta::Zero()), - send_side_bwe_with_overhead_( - IsEnabled(field_trials, "WebRTC-SendSideBwe-WithOverhead")) {} + include_overhead_(false) {} RoundRobinPacketQueue::~RoundRobinPacketQueue() { // Make sure to release any packets owned by raw pointer in QueuedPacket. @@ -158,7 +167,7 @@ std::unique_ptr RoundRobinPacketQueue::Pop() { // case a "budget" will be built up for the stream sending at the lower // rate. To avoid building a too large budget we limit |bytes| to be within // kMaxLeading bytes of the stream that has sent the most amount of bytes. - DataSize packet_size = queued_packet.Size(send_side_bwe_with_overhead_); + DataSize packet_size = queued_packet.Size(include_overhead_); stream->size = std::max(stream->size + packet_size, max_size_ - kMaxLeadingSize); max_size_ = std::max(max_size_, stream->size); @@ -238,6 +247,17 @@ void RoundRobinPacketQueue::SetPauseState(bool paused, Timestamp now) { paused_ = paused; } +void RoundRobinPacketQueue::SetIncludeOverhead() { + include_overhead_ = true; + // We need to update the size to reflect overhead for existing packets. + size_ = DataSize::Zero(); + for (const auto& stream : streams_) { + for (const QueuedPacket& packet : stream.second.packet_queue) { + size_ += packet.Size(include_overhead_); + } + } +} + TimeDelta RoundRobinPacketQueue::AverageQueueTime() const { if (Empty()) return TimeDelta::Zero(); @@ -279,7 +299,7 @@ void RoundRobinPacketQueue::Push(QueuedPacket packet) { packet.SubtractPauseTime(pause_time_sum_); size_packets_ += 1; - size_ += packet.Size(send_side_bwe_with_overhead_); + size_ += packet.Size(include_overhead_); stream->packet_queue.push(packet); } diff --git a/modules/pacing/round_robin_packet_queue.h b/modules/pacing/round_robin_packet_queue.h index 96b458f4c0..d0a2f7cb72 100644 --- a/modules/pacing/round_robin_packet_queue.h +++ b/modules/pacing/round_robin_packet_queue.h @@ -52,6 +52,7 @@ class RoundRobinPacketQueue { TimeDelta AverageQueueTime() const; void UpdateQueueTime(Timestamp now); void SetPauseState(bool paused, Timestamp now); + void SetIncludeOverhead(); private: struct QueuedPacket { @@ -89,6 +90,13 @@ class RoundRobinPacketQueue { RtpPacketToSend* owned_packet_; }; + class PriorityPacketQueue : public std::priority_queue { + public: + using const_iterator = container_type::const_iterator; + const_iterator begin() const; + const_iterator end() const; + }; + struct StreamPrioKey { StreamPrioKey(int priority, DataSize size) : priority(priority), size(size) {} @@ -111,7 +119,8 @@ class RoundRobinPacketQueue { DataSize size; uint32_t ssrc; - std::priority_queue packet_queue; + + PriorityPacketQueue packet_queue; // Whenever a packet is inserted for this stream we check if |priority_it| // points to an element in |stream_priorities_|, and if it does it means @@ -150,7 +159,7 @@ class RoundRobinPacketQueue { // the age of the oldest packet in the queue. std::multiset enqueue_times_; - const bool send_side_bwe_with_overhead_; + bool include_overhead_; }; } // namespace webrtc diff --git a/modules/pacing/rtp_packet_pacer.h b/modules/pacing/rtp_packet_pacer.h index 305be54234..2f11c1f5d6 100644 --- a/modules/pacing/rtp_packet_pacer.h +++ b/modules/pacing/rtp_packet_pacer.h @@ -64,6 +64,7 @@ class RtpPacketPacer { // the pacer budget calculation. The audio traffic still will be injected // at high priority. virtual void SetAccountForAudioPackets(bool account_for_audio) = 0; + virtual void SetIncludeOverhead() = 0; }; } // namespace webrtc diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc index e1745db9d5..54d2d844ca 100644 --- a/modules/pacing/task_queue_paced_sender.cc +++ b/modules/pacing/task_queue_paced_sender.cc @@ -136,6 +136,13 @@ void TaskQueuePacedSender::SetAccountForAudioPackets(bool account_for_audio) { }); } +void TaskQueuePacedSender::SetIncludeOverhead() { + task_queue_.PostTask([this]() { + RTC_DCHECK_RUN_ON(&task_queue_); + pacing_controller_.SetIncludeOverhead(); + }); +} + void TaskQueuePacedSender::SetQueueTimeLimit(TimeDelta limit) { task_queue_.PostTask([this, limit]() { RTC_DCHECK_RUN_ON(&task_queue_); diff --git a/modules/pacing/task_queue_paced_sender.h b/modules/pacing/task_queue_paced_sender.h index 719886a931..a50ffa2784 100644 --- a/modules/pacing/task_queue_paced_sender.h +++ b/modules/pacing/task_queue_paced_sender.h @@ -79,6 +79,7 @@ class TaskQueuePacedSender : public RtpPacketPacer, // at high priority. void SetAccountForAudioPackets(bool account_for_audio) override; + void SetIncludeOverhead() override; // Returns the time since the oldest queued packet was enqueued. TimeDelta OldestPacketWaitTime() const override;