From 478f3b786e5d7dacbded7ed4e6346932d32ef0e3 Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Tue, 17 Jan 2023 15:40:36 +0100 Subject: [PATCH] Avoid waking up encoder thread when audio send stream is stopped. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the default enabled "WebRTC-Audio-FixTimestampStall" field trial which was rolled out 2 years ago without any issues. Also change the include audio level indication member to be atomic since it is accessed on multiple threads. Bug: webrtc:14804 Change-Id: I4c5145e1fb03351154162b4293a3bd870e4793cb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290886 Reviewed-by: Markus Handell Reviewed-by: Harald Alvestrand Commit-Queue: Jakob Ivarsson‎ Cr-Commit-Position: refs/heads/main@{#39125} --- audio/channel_send.cc | 57 +++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 37 deletions(-) diff --git a/audio/channel_send.cc b/audio/channel_send.cc index fdc11a83fb..361880d68f 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -197,16 +197,13 @@ class ChannelSend : public ChannelSendInterface, std::unique_ptr rtp_sender_audio_; std::unique_ptr audio_coding_; - uint32_t _timeStamp RTC_GUARDED_BY(encoder_queue_); - // uses + // This is just an offset, RTP module will add its own random offset. + uint32_t timestamp_ RTC_GUARDED_BY(audio_thread_race_checker_) = 0; + RmsLevel rms_level_ RTC_GUARDED_BY(encoder_queue_); - bool input_mute_ RTC_GUARDED_BY(volume_settings_mutex_); - bool previous_frame_muted_ RTC_GUARDED_BY(encoder_queue_); - // VoeRTP_RTCP - // TODO(henrika): can today be accessed on the main thread and on the - // task queue; hence potential race. - bool _includeAudioLevelIndication; + bool input_mute_ RTC_GUARDED_BY(volume_settings_mutex_) = false; + bool previous_frame_muted_ RTC_GUARDED_BY(encoder_queue_) = false; // RtcpBandwidthObserver const std::unique_ptr rtcp_observer_; @@ -219,7 +216,8 @@ class ChannelSend : public ChannelSendInterface, SequenceChecker construction_thread_; - bool encoder_queue_is_active_ RTC_GUARDED_BY(encoder_queue_) = false; + std::atomic include_audio_level_indication_ = false; + std::atomic encoder_queue_is_active_ = false; // E2EE Audio Frame Encryption rtc::scoped_refptr frame_encryptor_ @@ -233,8 +231,6 @@ class ChannelSend : public ChannelSendInterface, rtc::scoped_refptr frame_transformer_delegate_ RTC_GUARDED_BY(encoder_queue_); - const bool fixing_timestamp_stall_; - mutable Mutex rtcp_counter_mutex_; RtcpPacketTypeCounter rtcp_packet_type_counter_ RTC_GUARDED_BY(rtcp_counter_mutex_); @@ -373,7 +369,7 @@ int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType, uint32_t rtp_timestamp, rtc::ArrayView payload, int64_t absolute_capture_timestamp_ms) { - if (_includeAudioLevelIndication) { + if (include_audio_level_indication_.load()) { // Store current audio level in the RTP sender. // The level will be used in combination with voice-activity state // (frameType) to add an RTP header extension @@ -463,11 +459,6 @@ ChannelSend::ChannelSend( const FieldTrialsView& field_trials) : ssrc_(ssrc), event_log_(rtc_event_log), - _timeStamp(0), // This is just an offset, RTP module will add it's own - // random offset - input_mute_(false), - previous_frame_muted_(false), - _includeAudioLevelIndication(false), rtcp_observer_(new VoERtcpObserver(this)), feedback_observer_(feedback_observer), rtp_packet_pacer_proxy_(new RtpPacketSenderProxy()), @@ -475,8 +466,6 @@ ChannelSend::ChannelSend( new RateLimiter(clock, kMaxRetransmissionWindowMs)), frame_encryptor_(frame_encryptor), crypto_options_(crypto_options), - fixing_timestamp_stall_( - field_trials.IsDisabled("WebRTC-Audio-FixTimestampStall")), encoder_queue_(task_queue_factory->CreateTaskQueue( "AudioEncoder", TaskQueueFactory::Priority::NORMAL)) { @@ -540,10 +529,7 @@ void ChannelSend::StartSend() { RTC_DCHECK_EQ(0, ret); // It is now OK to start processing on the encoder task queue. - encoder_queue_.PostTask([this] { - RTC_DCHECK_RUN_ON(&encoder_queue_); - encoder_queue_is_active_ = true; - }); + encoder_queue_is_active_.store(true); } void ChannelSend::StopSend() { @@ -552,11 +538,12 @@ void ChannelSend::StopSend() { return; } sending_ = false; + encoder_queue_is_active_.store(false); + // Wait until all pending encode tasks are executed. rtc::Event flush; encoder_queue_.PostTask([this, &flush]() { RTC_DCHECK_RUN_ON(&encoder_queue_); - encoder_queue_is_active_ = false; flush.Set(); }); flush.Wait(rtc::Event::kForever); @@ -701,7 +688,7 @@ void ChannelSend::SetSendTelephoneEventPayloadType(int payload_type, void ChannelSend::SetSendAudioLevelIndicationStatus(bool enable, int id) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); - _includeAudioLevelIndication = enable; + include_audio_level_indication_.store(enable); if (enable) { rtp_rtcp_->RegisterRtpHeaderExtension(AudioLevel::Uri(), id); } else { @@ -815,17 +802,19 @@ void ChannelSend::ProcessAndEncodeAudio( RTC_DCHECK_GT(audio_frame->samples_per_channel_, 0); RTC_DCHECK_LE(audio_frame->num_channels_, 8); + audio_frame->timestamp_ = timestamp_; + timestamp_ += audio_frame->samples_per_channel_; + if (!encoder_queue_is_active_.load()) { + return; + } + // Profile time between when the audio frame is added to the task queue and // when the task is actually executed. audio_frame->UpdateProfileTimeStamp(); encoder_queue_.PostTask( [this, audio_frame = std::move(audio_frame)]() mutable { RTC_DCHECK_RUN_ON(&encoder_queue_); - if (!encoder_queue_is_active_) { - if (fixing_timestamp_stall_) { - _timeStamp += - static_cast(audio_frame->samples_per_channel_); - } + if (!encoder_queue_is_active_.load()) { return; } // Measure time between when the audio frame is added to the task queue @@ -838,7 +827,7 @@ void ChannelSend::ProcessAndEncodeAudio( AudioFrameOperations::Mute(audio_frame.get(), previous_frame_muted_, is_muted); - if (_includeAudioLevelIndication) { + if (include_audio_level_indication_.load()) { size_t length = audio_frame->samples_per_channel_ * audio_frame->num_channels_; RTC_CHECK_LE(length, AudioFrame::kMaxDataSizeBytes); @@ -851,10 +840,6 @@ void ChannelSend::ProcessAndEncodeAudio( } previous_frame_muted_ = is_muted; - // Add 10ms of raw (PCM) audio data to the encoder @ 32kHz. - - // The ACM resamples internally. - audio_frame->timestamp_ = _timeStamp; // This call will trigger AudioPacketizationCallback::SendData if // encoding is done and payload is ready for packetization and // transmission. Otherwise, it will return without invoking the @@ -863,8 +848,6 @@ void ChannelSend::ProcessAndEncodeAudio( RTC_DLOG(LS_ERROR) << "ACM::Add10MsData() failed."; return; } - - _timeStamp += static_cast(audio_frame->samples_per_channel_); }); }