Initialize more RtpSender parameters via the constructor Initialize stream_ids, init_send_encodings, and send_codecs in AudioRtpSender and VideoRtpSender at construction time. This avoids post-creation signaling-thread state updates via ConfigureSender and ConfigureSendCodecs, simplifying the transceiver creation flow and improving memory and thread safety. Bug: none Change-Id: I9bb52538f5d98936d8c32f12905c8c5cadfd99f2 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/474940 Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47807}
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 81f6f1b..fa5108e 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc
@@ -1164,12 +1164,16 @@ } scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> new_sender; + CodecVendor codec_vendor(context_->media_engine(), false, trials()); + if (kind == MediaStreamTrackInterface::kAudioKind) { - auto audio_sender = - AudioRtpSender::Create(env_, signaling_thread(), worker_thread(), - CreateRandomUuid(), legacy_stats_.get(), nullptr, - /*enable_sframe_at_owner=*/nullptr, - rtp_manager()->voice_media_send_channel()); + auto audio_sender = AudioRtpSender::Create( + env_, signaling_thread(), worker_thread(), CreateRandomUuid(), + legacy_stats_.get(), nullptr, + /*enable_sframe_at_owner=*/nullptr, + rtp_manager()->voice_media_send_channel(), stream_ids, + /*init_send_encodings=*/std::vector<RtpEncodingParameters>(1), + codec_vendor.audio_send_codecs().codecs()); new_sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create( signaling_thread(), audio_sender); rtp_manager()->GetAudioTransceiver()->internal()->AddSenderPlanB( @@ -1180,7 +1184,8 @@ /*enable_sframe_at_owner=*/nullptr, rtp_manager()->video_media_send_channel(), /*init_send_encodings=*/{}, /*simulcast_rejected=*/false, - /*initial_simulcast_layers=*/{}); + /*initial_simulcast_layers=*/{}, stream_ids, + codec_vendor.video_send_codecs().codecs()); new_sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create( signaling_thread(), video_sender); rtp_manager()->GetVideoTransceiver()->internal()->AddSenderPlanB( @@ -1189,11 +1194,6 @@ RTC_LOG(LS_ERROR) << "CreateSender called with invalid kind: " << kind; } - if (!new_sender) { - return nullptr; - } - new_sender->internal()->set_stream_ids(stream_ids); - return new_sender; }
diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc index 070feed..0f8480a 100644 --- a/pc/rtp_sender.cc +++ b/pc/rtp_sender.cc
@@ -202,6 +202,16 @@ std::move(callback)); } +std::vector<std::string> GetUniqueStreamIds( + const std::vector<std::string>& stream_ids) { + std::vector<std::string> unique_ids; + absl::c_copy_if(stream_ids, std::back_inserter(unique_ids), + [&](const std::string& id) { + return !absl::c_linear_search(unique_ids, id); + }); + return unique_ids; +} + } // namespace // Returns true if any RtpParameters member that isn't implemented contains a @@ -229,12 +239,17 @@ MediaType media_type, SetStreamsObserver* set_streams_observer, absl::AnyInvocable<RTCError()> enable_sframe_at_owner, - MediaSendChannelInterface* media_channel) + MediaSendChannelInterface* media_channel, + std::vector<std::string> stream_ids, + std::vector<RtpEncodingParameters> init_send_encodings, + std::vector<Codec> send_codecs) : env_(env), signaling_thread_(signaling_thread), worker_thread_(worker_thread), id_(id), media_type_(media_type), + stream_ids_(GetUniqueStreamIds(stream_ids)), + send_codecs_(std::move(send_codecs)), media_channel_(nullptr), // Will be set in SetMediaChannel(). set_streams_observer_(set_streams_observer), worker_safety_(PendingTaskSafetyFlag::CreateAttachedToTaskQueue( @@ -245,7 +260,7 @@ signaling_thread_)), enable_sframe_at_owner_(std::move(enable_sframe_at_owner)) { RTC_DCHECK(worker_thread_); - init_parameters_.encodings.emplace_back(); + init_parameters_.encodings = std::move(init_send_encodings); if (media_channel) { // When initialized with a valid media channel, we need to be running on the // worker thread in order to set things up properly. @@ -703,11 +718,7 @@ } void RtpSenderBase::set_stream_ids(const std::vector<std::string>& stream_ids) { - stream_ids_.clear(); - absl::c_copy_if(stream_ids, std::back_inserter(stream_ids_), - [this](const std::string& stream_id) { - return !absl::c_linear_search(stream_ids_, stream_id); - }); + stream_ids_ = GetUniqueStreamIds(stream_ids); } void RtpSenderBase::SetStreams(const std::vector<std::string>& stream_ids) { @@ -1133,10 +1144,14 @@ LegacyStatsCollectorInterface* stats, SetStreamsObserver* set_streams_observer, absl::AnyInvocable<RTCError()> enable_sframe_at_owner, - MediaSendChannelInterface* media_channel) { + MediaSendChannelInterface* media_channel, + std::vector<std::string> stream_ids, + std::vector<RtpEncodingParameters> init_send_encodings, + std::vector<Codec> send_codecs) { return make_ref_counted<AudioRtpSender>( env, signaling_thread, worker_thread, id, stats, set_streams_observer, - std::move(enable_sframe_at_owner), media_channel); + std::move(enable_sframe_at_owner), media_channel, std::move(stream_ids), + std::move(init_send_encodings), std::move(send_codecs)); } AudioRtpSender::AudioRtpSender( @@ -1147,7 +1162,10 @@ LegacyStatsCollectorInterface* stats, SetStreamsObserver* set_streams_observer, absl::AnyInvocable<RTCError()> enable_sframe_at_owner, - MediaSendChannelInterface* media_channel) + MediaSendChannelInterface* media_channel, + std::vector<std::string> stream_ids, + std::vector<RtpEncodingParameters> init_send_encodings, + std::vector<Codec> send_codecs) : RtpSenderBase(env, signaling_thread, worker_thread, @@ -1155,7 +1173,10 @@ MediaType::AUDIO, set_streams_observer, std::move(enable_sframe_at_owner), - media_channel), + media_channel, + std::move(stream_ids), + std::move(init_send_encodings), + std::move(send_codecs)), legacy_stats_(stats), dtmf_sender_(DtmfSender::Create(signaling_thread, this)), dtmf_sender_proxy_( @@ -1312,11 +1333,14 @@ MediaSendChannelInterface* media_channel, const std::vector<RtpEncodingParameters>& init_send_encodings, bool simulcast_rejected, - const std::vector<SimulcastLayer>& initial_simulcast_layers) { + const std::vector<SimulcastLayer>& initial_simulcast_layers, + std::vector<std::string> stream_ids, + std::vector<Codec> send_codecs) { return make_ref_counted<VideoRtpSender>( env, signaling_thread, worker_thread, id, set_streams_observer, std::move(enable_sframe_at_owner), media_channel, init_send_encodings, - simulcast_rejected, initial_simulcast_layers); + simulcast_rejected, initial_simulcast_layers, std::move(stream_ids), + std::move(send_codecs)); } VideoRtpSender::VideoRtpSender( @@ -1329,19 +1353,24 @@ MediaSendChannelInterface* media_channel, const std::vector<RtpEncodingParameters>& init_send_encodings, bool simulcast_rejected, - const std::vector<SimulcastLayer>& initial_simulcast_layers) - : RtpSenderBase(env, - signaling_thread, - worker_thread, - id, - MediaType::VIDEO, - set_streams_observer, - std::move(enable_sframe_at_owner), - media_channel) { - set_init_send_encodings( - CalculateInitialEncodings(init_parameters_.encodings, init_send_encodings, - initial_simulcast_layers, simulcast_rejected)); -} + const std::vector<SimulcastLayer>& initial_simulcast_layers, + std::vector<std::string> stream_ids, + std::vector<Codec> send_codecs) + : RtpSenderBase( + env, + signaling_thread, + worker_thread, + id, + MediaType::VIDEO, + set_streams_observer, + std::move(enable_sframe_at_owner), + media_channel, + std::move(stream_ids), + CalculateInitialEncodings(std::vector<RtpEncodingParameters>(1), + init_send_encodings, + initial_simulcast_layers, + simulcast_rejected), + std::move(send_codecs)) {} VideoRtpSender::~VideoRtpSender() { Stop();
diff --git a/pc/rtp_sender.h b/pc/rtp_sender.h index f39ac69..1eeda57 100644 --- a/pc/rtp_sender.h +++ b/pc/rtp_sender.h
@@ -274,7 +274,10 @@ MediaType media_type, SetStreamsObserver* set_streams_observer, absl::AnyInvocable<RTCError()> enable_sframe_at_owner, - MediaSendChannelInterface* media_channel); + MediaSendChannelInterface* media_channel, + std::vector<std::string> stream_ids, + std::vector<RtpEncodingParameters> init_send_encodings, + std::vector<Codec> send_codecs); // TODO(bugs.webrtc.org/8694): Since SSRC == 0 is technically valid, figure // out some other way to test if we have a valid SSRC. @@ -415,7 +418,11 @@ LegacyStatsCollectorInterface* stats, SetStreamsObserver* set_streams_observer, absl::AnyInvocable<RTCError()> enable_sframe_at_owner, - MediaSendChannelInterface* media_channel); + MediaSendChannelInterface* media_channel, + std::vector<std::string> stream_ids = {}, + std::vector<RtpEncodingParameters> init_send_encodings = + std::vector<RtpEncodingParameters>(1), + std::vector<Codec> send_codecs = {}); ~AudioRtpSender() override; // DtmfSenderProvider implementation. @@ -440,7 +447,10 @@ LegacyStatsCollectorInterface* legacy_stats, SetStreamsObserver* set_streams_observer, absl::AnyInvocable<RTCError()> enable_sframe_at_owner, - MediaSendChannelInterface* media_channel); + MediaSendChannelInterface* media_channel, + std::vector<std::string> stream_ids, + std::vector<RtpEncodingParameters> init_send_encodings, + std::vector<Codec> send_codecs); void SetSend() override; void ClearSend() override; @@ -493,7 +503,9 @@ MediaSendChannelInterface* media_channel, const std::vector<RtpEncodingParameters>& init_send_encodings, bool simulcast_rejected, - const std::vector<SimulcastLayer>& initial_simulcast_layers); + const std::vector<SimulcastLayer>& initial_simulcast_layers, + std::vector<std::string> stream_ids = {}, + std::vector<Codec> send_codecs = {}); ~VideoRtpSender() override; // ObserverInterface implementation @@ -516,7 +528,9 @@ MediaSendChannelInterface* media_channel, const std::vector<RtpEncodingParameters>& init_send_encodings, bool simulcast_rejected, - const std::vector<SimulcastLayer>& initial_simulcast_layers); + const std::vector<SimulcastLayer>& initial_simulcast_layers, + std::vector<std::string> stream_ids, + std::vector<Codec> send_codecs); void SetSend() override; void ClearSend() override;
diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index 931f61a..db52919 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc
@@ -147,14 +147,6 @@ } } -void ConfigureSendCodecs(CodecVendor& codec_vendor, - MediaType media_type, - RtpSenderInternal* sender) { - sender->SetSendCodecs(media_type == MediaType::VIDEO - ? codec_vendor.video_send_codecs().codecs() - : codec_vendor.audio_send_codecs().codecs()); -} - scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> CreateSender( MediaType media_type, const Environment& env, @@ -166,7 +158,9 @@ MediaSendChannelInterface* media_send_channel, const std::vector<RtpEncodingParameters>& init_send_encodings, bool simulcast_rejected, - const std::vector<SimulcastLayer>& initial_simulcast_layers) { + const std::vector<SimulcastLayer>& initial_simulcast_layers, + std::vector<std::string> stream_ids, + std::vector<Codec> send_codecs) { if (media_type == MediaType::AUDIO) { return RtpSenderProxyWithInternal<RtpSenderInternal>::Create( context->signaling_thread(), @@ -174,7 +168,9 @@ env, context->signaling_thread(), context->worker_thread(), sender_id, legacy_stats, set_streams_observer, std::move(enable_sframe_at_owner), - static_cast<VoiceMediaSendChannelInterface*>(media_send_channel))); + static_cast<VoiceMediaSendChannelInterface*>(media_send_channel), + std::move(stream_ids), init_send_encodings, + std::move(send_codecs))); } RTC_DCHECK_EQ(media_type, MediaType::VIDEO); return RtpSenderProxyWithInternal<RtpSenderInternal>::Create( @@ -183,21 +179,8 @@ env, context->signaling_thread(), context->worker_thread(), sender_id, set_streams_observer, std::move(enable_sframe_at_owner), static_cast<VideoMediaSendChannelInterface*>(media_send_channel), - init_send_encodings, simulcast_rejected, initial_simulcast_layers)); -} - -void ConfigureSender( - scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>>& sender, - MediaStreamTrackInterface* track, - const std::vector<std::string>& stream_ids, - const std::vector<RtpEncodingParameters>& send_encodings, - CodecVendor& codec_vendor) { - bool set_track_succeeded = sender->SetTrack(track); - RTC_DCHECK(set_track_succeeded); - auto* internal = sender->internal(); - internal->set_stream_ids(stream_ids); - internal->set_init_send_encodings(send_encodings); - ConfigureSendCodecs(codec_vendor, sender->media_type(), internal); + init_send_encodings, simulcast_rejected, initial_simulcast_layers, + std::move(stream_ids), std::move(send_codecs))); } template <typename RtpReceiverT, typename ReceiveInterface> @@ -277,6 +260,13 @@ } // namespace +std::vector<Codec> RtpTransceiver::GetSendCodecs() { + RTC_DCHECK_RUN_ON(thread_); + return media_type_ == MediaType::VIDEO + ? codec_vendor().video_send_codecs().codecs() + : codec_vendor().audio_send_codecs().codecs(); +} + RtpTransceiver::RtpTransceiver(const Environment& env, MediaType media_type, ConnectionContext* context, @@ -344,7 +334,7 @@ .encodings, header_extensions_to_negotiate_); } - ConfigureSendCodecs(codec_vendor(), media_type_, sender_internal); + sender_internal->SetSendCodecs(GetSendCodecs()); } RtpTransceiver::RtpTransceiver( @@ -399,6 +389,7 @@ } auto encoder_switch_callback = GetEncoderSwitchRequestCallback(); + std::vector<Codec> send_codecs = GetSendCodecs(); worker_tasks.AddWithFinalizer( [this, call, media_config, audio_options, video_options, crypto_options, @@ -406,6 +397,7 @@ encoder_switch_callback = std::move(encoder_switch_callback), sender_id = std::string(sender_id), init_send_encodings, simulcast_rejected, initial_simulcast_layers, track, stream_ids, + send_codecs = std::move(send_codecs), receiver_id = std::string( receiver_id)]() mutable -> ScopedOperationsBatcher::FinalizerTask { RTC_DCHECK_RUN_ON(this->context()->worker_thread()); @@ -419,17 +411,17 @@ sender_id, absl::bind_front(&RtpTransceiver::TryToEnableSframe, this), channels.first.get(), init_send_encodings, simulcast_rejected, - initial_simulcast_layers); + initial_simulcast_layers, stream_ids, std::move(send_codecs)); return ScopedOperationsBatcher::FinalizerTask( [this, channels = std::move(channels), sender = std::move(sender), - track, stream_ids, init_send_encodings, receiver_id]() mutable { + track, receiver_id]() mutable { RTC_DCHECK_RUN_ON(thread_); owned_send_channel_ = std::move(channels.first); owned_receive_channel_ = std::move(channels.second); senders_.push_back(std::move(sender)); - ConfigureSender(senders_.back(), track.get(), stream_ids, - init_send_encodings, codec_vendor()); + bool set_track_succeeded = senders_.back()->SetTrack(track.get()); + RTC_DCHECK(set_track_succeeded); receivers_.push_back(CreateReceiver( media_type_, context_->signaling_thread(), @@ -750,7 +742,7 @@ RTC_DCHECK(sender); RTC_DCHECK_EQ(media_type(), sender->media_type()); RTC_DCHECK(!absl::c_linear_search(senders_, sender)); - ConfigureSendCodecs(codec_vendor(), media_type(), sender->internal()); + sender->internal()->SetSendCodecs(GetSendCodecs()); senders_.push_back(sender); } @@ -765,16 +757,17 @@ RTC_DCHECK(!unified_plan_); RTC_DCHECK(media_type_ == MediaType::AUDIO || media_type_ == MediaType::VIDEO); + std::vector<Codec> send_codecs = GetSendCodecs(); context_->worker_thread()->BlockingCall([&]() mutable { RTC_DCHECK_RUN_ON(context()->worker_thread()); senders_.push_back(CreateSender( media_type_, env_, context_, legacy_stats_, set_streams_observer_, sender_id, /*enable_sframe_at_owner=*/nullptr, channel_ ? channel_->media_send_channel() : nullptr, send_encodings, - false, {})); + false, {}, stream_ids, std::move(send_codecs))); }); - ConfigureSender(senders_.back(), track.get(), stream_ids, send_encodings, - codec_vendor()); + bool set_track_succeeded = senders_.back()->SetTrack(track.get()); + RTC_DCHECK(set_track_succeeded); return senders_.back(); }
diff --git a/pc/rtp_transceiver.h b/pc/rtp_transceiver.h index 6708022..c8904aa 100644 --- a/pc/rtp_transceiver.h +++ b/pc/rtp_transceiver.h
@@ -41,6 +41,7 @@ #include "api/task_queue/pending_task_safety_flag.h" #include "api/task_queue/task_queue_base.h" #include "api/video/video_bitrate_allocator_factory.h" +#include "media/base/codec.h" #include "media/base/media_channel.h" #include "media/base/media_config.h" #include "media/base/media_engine.h" @@ -441,6 +442,7 @@ CodecVendor& codec_vendor() { return *codec_lookup_helper_->GetCodecVendor(); } + std::vector<Codec> GetSendCodecs(); void OnFirstPacketReceived(uint32_t ssrc); void OnPacketReceived(uint32_t ssrc, scoped_refptr<PendingTaskSafetyFlag> safety)