Simplify WebRTC Voice Engine, remove `friend`. Simplify webrtc_voice_engine.* by removing unused member variables, eliminating redundant state, decoupling shared engine state from individual receive channels, and improving encapsulation by removing friend declarations. Bug: none Change-Id: I6d79795e2780b1e3937da5c365a45ec342e7c573 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/472140 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47699}
diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index f7b5256..8eb262a 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h
@@ -816,11 +816,11 @@ // TODO: https://issues.webrtc.org/360058654 - stop faking codecs here. const std::vector<Codec>& LegacySendCodecs() const override; const std::vector<Codec>& LegacyRecvCodecs() const override; - AudioEncoderFactory* encoder_factory() const override { - return encoder_factory_.get(); + const scoped_refptr<AudioEncoderFactory>& encoder_factory() const override { + return encoder_factory_; } - AudioDecoderFactory* decoder_factory() const override { - return decoder_factory_.get(); + const scoped_refptr<AudioDecoderFactory>& decoder_factory() const override { + return decoder_factory_; } void SetCodecs(const std::vector<Codec>& codecs); void SetRecvCodecs(const std::vector<Codec>& codecs); @@ -909,8 +909,8 @@ std::vector<Codec> recv_codecs_; std::vector<Codec> send_codecs_; - scoped_refptr<FakeVoiceEncoderFactory> encoder_factory_; - scoped_refptr<FakeVoiceDecoderFactory> decoder_factory_; + scoped_refptr<AudioEncoderFactory> encoder_factory_; + scoped_refptr<AudioDecoderFactory> decoder_factory_; std::vector<RtpHeaderExtensionCapability> header_extensions_; friend class FakeMediaEngine;
diff --git a/media/base/media_engine.h b/media/base/media_engine.h index 1facc38..c2be2c9 100644 --- a/media/base/media_engine.h +++ b/media/base/media_engine.h
@@ -122,8 +122,8 @@ virtual const std::vector<Codec>& LegacySendCodecs() const = 0; virtual const std::vector<Codec>& LegacyRecvCodecs() const = 0; - virtual AudioEncoderFactory* encoder_factory() const = 0; - virtual AudioDecoderFactory* decoder_factory() const = 0; + virtual const scoped_refptr<AudioEncoderFactory>& encoder_factory() const = 0; + virtual const scoped_refptr<AudioDecoderFactory>& decoder_factory() const = 0; // Starts AEC dump using existing file, a maximum file size in bytes can be // specified. Logging is stopped just before the size limit is exceeded.
diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 683b582..3629b4d 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc
@@ -292,7 +292,7 @@ RtcpMode rtcp_mode, const std::vector<std::string>& stream_ids, Transport* rtcp_send_transport, - const scoped_refptr<AudioDecoderFactory>& decoder_factory, + scoped_refptr<AudioDecoderFactory> decoder_factory, const std::map<int, SdpAudioFormat>& decoder_map, size_t jitter_buffer_max_packets, bool jitter_buffer_fast_accelerate, @@ -309,7 +309,7 @@ } config.rtcp_send_transport = rtcp_send_transport; config.enable_non_sender_rtt = enable_non_sender_rtt; - config.decoder_factory = decoder_factory; + config.decoder_factory = std::move(decoder_factory); config.decoder_map = decoder_map; config.jitter_buffer_max_packets = jitter_buffer_max_packets; config.jitter_buffer_fast_accelerate = jitter_buffer_fast_accelerate; @@ -700,19 +700,6 @@ audio_state()->SetStereoChannelSwapping(*options.stereo_swapping); } - if (options.audio_jitter_buffer_max_packets) { - audio_jitter_buffer_max_packets_ = - std::max(20, *options.audio_jitter_buffer_max_packets); - } - if (options.audio_jitter_buffer_fast_accelerate) { - audio_jitter_buffer_fast_accelerate_ = - *options.audio_jitter_buffer_fast_accelerate; - } - if (options.audio_jitter_buffer_min_delay_ms) { - audio_jitter_buffer_min_delay_ms_ = - *options.audio_jitter_buffer_min_delay_ms; - } - AudioProcessing* ap = apm(); if (!ap) { return; @@ -1476,7 +1463,7 @@ SdpAudioFormat format(voice_codec.name, voice_codec.clockrate, voice_codec.channels, voice_codec.params); - voice_codec_info = engine()->encoder_factory_->QueryAudioEncoder(format); + voice_codec_info = engine()->encoder_factory()->QueryAudioEncoder(format); if (!voice_codec_info) { RTC_LOG(LS_WARNING) << "Unknown codec " << ToString(voice_codec); send_codec_position++; @@ -1633,7 +1620,7 @@ env_, ssrc, mid_, sp.cname, sp.id, send_codec_spec_, ExtmapAllowMixed(), send_rtp_extensions_, rtcp_cc_ack_type_, max_send_bitrate_bps_, audio_config_.rtcp_report_interval_ms, audio_network_adaptor_config, - call_, transport(), engine()->encoder_factory_, crypto_options_); + call_, transport(), engine()->encoder_factory(), crypto_options_); send_streams_.insert(std::make_pair(ssrc, stream)); if (ssrc_list_changed_callback_) { std::set<uint32_t> ssrcs_in_use; @@ -2236,7 +2223,7 @@ recv_params_.extensions, RtpExtension::IsSupportedForAudio, false, env_.field_trials()); - for (const Codec& codec : recv_codecs_) { + for (const Codec& codec : recv_params_.codecs) { rtp_params.codecs.push_back(codec.ToCodecParameters()); } rtp_params.rtcp.reduced_size = recv_rtcp_mode_ == RtcpMode::kReducedSize; @@ -2255,7 +2242,7 @@ } rtp_params.encodings.emplace_back(); - for (const Codec& codec : recv_codecs_) { + for (const Codec& codec : recv_params_.codecs) { rtp_params.codecs.push_back(codec.ToCodecParameters()); } return rtp_params; @@ -2273,10 +2260,10 @@ // Check if any options changed that should apply to receive streams. if (options.audio_jitter_buffer_max_packets && - *options.audio_jitter_buffer_max_packets != + std::max(20, *options.audio_jitter_buffer_max_packets) != audio_config_.audio_jitter_buffer_max_packets) { audio_config_.audio_jitter_buffer_max_packets = - *options.audio_jitter_buffer_max_packets; + std::max(20, *options.audio_jitter_buffer_max_packets); for (auto& [unused, stream] : recv_streams_) { stream->SetJitterBufferMaxPackets( audio_config_.audio_jitter_buffer_max_packets); @@ -2315,7 +2302,7 @@ for (const Codec& codec : codecs) { // Log a warning if a codec's payload type is changing. This used to be // treated as an error. It's abnormal, but not really illegal. - std::optional<Codec> old_codec = FindCodec(recv_codecs_, codec); + std::optional<Codec> old_codec = FindCodec(recv_params_.codecs, codec); if (old_codec && old_codec->id != codec.id) { RTC_LOG(LS_WARNING) << codec.name << " mapped to a second payload type (" << codec.id << ", was already mapped to " @@ -2324,7 +2311,7 @@ auto format = AudioCodecToSdpAudioFormat(codec); if (!IsCodec(codec, kCnCodecName) && !IsCodec(codec, kDtmfCodecName) && !IsCodec(codec, kRedCodecName) && - !engine()->decoder_factory_->IsSupportedDecoder(format)) { + !engine()->decoder_factory()->IsSupportedDecoder(format)) { RTC_LOG(LS_ERROR) << "Unsupported codec: " << absl::StrCat(format); return false; } @@ -2366,8 +2353,6 @@ kv.second->SetDecoderMap(decoder_map_); } - recv_codecs_ = codecs; - SetReceive(playout_enabled); RTC_DCHECK_EQ(playout_, playout_enabled); @@ -2429,7 +2414,6 @@ bool WebRtcVoiceReceiveChannel::AddRecvStream(const StreamParams& sp) { TRACE_EVENT0("webrtc", "WebRtcVoiceMediaChannel::AddRecvStream"); RTC_DCHECK_RUN_ON(worker_thread_); - RTC_DCHECK_RUN_ON(&engine()->worker_thread_checker_); RTC_LOG(LS_INFO) << "AddRecvStream: " << sp.ToString(); if (!sp.has_ssrcs()) { @@ -2461,13 +2445,14 @@ } // Create a new channel for receiving audio data. - auto config = BuildReceiveStreamConfig( + AudioReceiveStreamInterface::Config config = BuildReceiveStreamConfig( ssrc, recv_nack_enabled_, enable_non_sender_rtt_, recv_rtcp_mode_, - sp.stream_ids(), transport(), engine()->decoder_factory_, decoder_map_, - engine()->audio_jitter_buffer_max_packets_, - engine()->audio_jitter_buffer_fast_accelerate_, - engine()->audio_jitter_buffer_min_delay_ms_, unsignaled_frame_decryptor_, - crypto_options_, unsignaled_frame_transformer_); + sp.stream_ids(), transport(), engine()->decoder_factory(), decoder_map_, + audio_config_.audio_jitter_buffer_max_packets, + audio_config_.audio_jitter_buffer_fast_accelerate, + options_.audio_jitter_buffer_min_delay_ms.value_or(0), + unsignaled_frame_decryptor_, crypto_options_, + unsignaled_frame_transformer_); recv_streams_.insert(std::make_pair( ssrc, new WebRtcAudioReceiveStream(std::move(config), call_))); @@ -2801,11 +2786,12 @@ VoiceMediaReceiveInfo* voice_media_info) { RTC_DCHECK_RUN_ON(worker_thread_); for (const auto& receiver : voice_media_info->receivers) { - auto codec = absl::c_find_if(recv_codecs_, [&receiver](const Codec& c) { - return receiver.codec_payload_type && - *receiver.codec_payload_type == c.id; - }); - if (codec != recv_codecs_.end()) { + std::vector<Codec>::const_iterator codec = + absl::c_find_if(recv_params_.codecs, [&receiver](const Codec& c) { + return receiver.codec_payload_type && + *receiver.codec_payload_type == c.id; + }); + if (codec != recv_params_.codecs.end()) { voice_media_info->receive_codecs.insert( std::make_pair(codec->id.value(), codec->ToCodecParameters())); }
diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index 6e60fc2..47e09cf 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h
@@ -73,9 +73,6 @@ // WebRtcVoiceEngine is a class to be used with CompositeMediaEngine. // It uses the WebRtc VoiceEngine library for audio handling. class WebRtcVoiceEngine final : public VoiceEngineInterface { - friend class WebRtcVoiceSendChannel; - friend class WebRtcVoiceReceiveChannel; - public: WebRtcVoiceEngine(const Environment& env, scoped_refptr<AudioDeviceModule> adm, @@ -113,12 +110,20 @@ const std::vector<Codec>& LegacySendCodecs() const override; const std::vector<Codec>& LegacyRecvCodecs() const override; - AudioEncoderFactory* encoder_factory() const override { - return encoder_factory_.get(); + const scoped_refptr<AudioEncoderFactory>& encoder_factory() const override { + return encoder_factory_; } - AudioDecoderFactory* decoder_factory() const override { - return decoder_factory_.get(); + const scoped_refptr<AudioDecoderFactory>& decoder_factory() const override { + return decoder_factory_; } + + // Every option that is "set" will be applied. Every option not "set" will be + // ignored. This allows us to selectively turn on and off different options + // easily at any time. + void ApplyOptions(const AudioOptions& options); + + AudioDeviceModule* adm(); + AudioProcessing* apm() const; std::vector<RtpHeaderExtensionCapability> GetRtpHeaderExtensions( const FieldTrialsView* field_trials) const override; @@ -134,16 +139,9 @@ std::optional<AudioDeviceModule::Stats> GetAudioDeviceStats() override; private: - // Every option that is "set" will be applied. Every option not "set" will be - // ignored. This allows us to selectively turn on and off different options - // easily at any time. - void ApplyOptions(const AudioOptions& options); - const Environment env_; std::unique_ptr<TaskQueueBase, TaskQueueDeleter> low_priority_worker_queue_; - AudioDeviceModule* adm(); - AudioProcessing* apm() const; AudioState* audio_state(); SequenceChecker signal_thread_checker_{SequenceChecker::kDetached}; @@ -164,14 +162,6 @@ const std::vector<Codec> legacy_send_codecs_; const std::vector<Codec> legacy_recv_codecs_; bool initialized_ RTC_GUARDED_BY(worker_thread_checker_) = false; - - // Jitter buffer settings for new streams. - size_t audio_jitter_buffer_max_packets_ - RTC_GUARDED_BY(worker_thread_checker_) = 200; - bool audio_jitter_buffer_fast_accelerate_ - RTC_GUARDED_BY(worker_thread_checker_) = false; - int audio_jitter_buffer_min_delay_ms_ RTC_GUARDED_BY(worker_thread_checker_) = - 0; }; class WebRtcVoiceSendChannel final : public MediaChannelUtil, @@ -287,7 +277,6 @@ AudioOptions options_ RTC_GUARDED_BY(worker_thread_); PayloadType dtmf_payload_type_ RTC_GUARDED_BY(worker_thread_); int dtmf_payload_freq_ RTC_GUARDED_BY(worker_thread_) = -1; - bool enable_non_sender_rtt_ RTC_GUARDED_BY(worker_thread_) = false; bool send_ RTC_GUARDED_BY(worker_thread_) = false; Call* const call_ = nullptr; @@ -430,10 +419,7 @@ WebRtcVoiceEngine* const engine_ = nullptr; - // TODO(kwiberg): decoder_map_ and recv_codecs_ store the exact same - // information, in slightly different formats. Eliminate recv_codecs_. std::map<int, SdpAudioFormat> decoder_map_ RTC_GUARDED_BY(worker_thread_); - std::vector<Codec> recv_codecs_ RTC_GUARDED_BY(worker_thread_); AudioOptions options_ RTC_GUARDED_BY(worker_thread_); bool recv_nack_enabled_ RTC_GUARDED_BY(worker_thread_) = false;