Make audio_options_ const in SdpOfferAnswerHandler Initialize `audio_options_` directly in the constructor initializer list and declare it as a const member variable. This improves safety and thread-safety by avoiding post-construction modifications and removing the thread guard since const members are thread-safe to read. Bug: none Change-Id: Id14b0defc05a0fbc469ec23241dbe1114d5ee2cb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/476720 Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47850}
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index eb0fde1..6e6927a 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc
@@ -519,7 +519,7 @@ } sdp_handler_ = SdpOfferAnswerHandler::Create( - env_, this, configuration_, std::move(dependencies.cert_generator), + env_, this, std::move(dependencies.cert_generator), std::move(dependencies.video_bitrate_allocator_factory), context_.get(), codec_lookup_helper_.get()); sdp_handler_->UpdateCachedIceCredentials(std::move(pooled_credentials));
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index d4c0268..31132c1 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc
@@ -32,6 +32,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_split.h" #include "absl/strings/string_view.h" +#include "api/audio_options.h" #include "api/candidate.h" #include "api/crypto/crypto_options.h" #include "api/environment/environment.h" @@ -1687,6 +1688,17 @@ operations_chain_(OperationsChain::Create()), rtcp_cname_(GenerateRtcpCname()), local_ice_credentials_to_replace_(new LocalIceCredentialsToReplace()), + audio_options_([&]() { + AudioOptions options; + const auto& configuration = *pc->configuration(); + options.audio_jitter_buffer_max_packets = + configuration.audio_jitter_buffer_max_packets; + options.audio_jitter_buffer_fast_accelerate = + configuration.audio_jitter_buffer_fast_accelerate; + options.audio_jitter_buffer_min_delay_ms = + configuration.audio_jitter_buffer_min_delay_ms; + return options; + }()), pt_suggester_(pc_->configuration()->bundle_policy, env_), weak_ptr_factory_(this) { operations_chain_->SetOnChainEmptyCallback( @@ -1703,21 +1715,19 @@ std::unique_ptr<SdpOfferAnswerHandler> SdpOfferAnswerHandler::Create( const Environment& env, PeerConnectionSdpMethods* pc, - const PeerConnectionInterface::RTCConfiguration& configuration, std::unique_ptr<RTCCertificateGeneratorInterface> cert_generator, std::unique_ptr<VideoBitrateAllocatorFactory> video_bitrate_allocator_factory, ConnectionContext* context, CodecLookupHelper* codec_lookup_helper) { auto handler = absl::WrapUnique(new SdpOfferAnswerHandler(env, pc, context)); - handler->Initialize(configuration, std::move(cert_generator), + handler->Initialize(std::move(cert_generator), std::move(video_bitrate_allocator_factory), context, codec_lookup_helper); return handler; } void SdpOfferAnswerHandler::Initialize( - const PeerConnectionInterface::RTCConfiguration& configuration, std::unique_ptr<RTCCertificateGeneratorInterface> cert_generator, std::unique_ptr<VideoBitrateAllocatorFactory> video_bitrate_allocator_factory, @@ -1725,20 +1735,12 @@ CodecLookupHelper* codec_lookup_helper) { RTC_LOG_THREAD_BLOCK_COUNT(); RTC_DCHECK_RUN_ON(signaling_thread()); + const auto& configuration = *pc_->configuration(); // 100 kbps is used by default, but can be overriden by a non-standard // RTCConfiguration value (not available on Web). video_options_.screencast_min_bitrate_kbps = configuration.screencast_min_bitrate.value_or(100); - audio_options_.audio_jitter_buffer_max_packets = - configuration.audio_jitter_buffer_max_packets; - - audio_options_.audio_jitter_buffer_fast_accelerate = - configuration.audio_jitter_buffer_fast_accelerate; - - audio_options_.audio_jitter_buffer_min_delay_ms = - configuration.audio_jitter_buffer_min_delay_ms; - // Obtain a certificate from RTCConfiguration if any were provided (optional). scoped_refptr<RTCCertificate> certificate; if (!configuration.certificates.empty()) {
diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h index de62ef5..2c906c2 100644 --- a/pc/sdp_offer_answer.h +++ b/pc/sdp_offer_answer.h
@@ -90,7 +90,6 @@ static std::unique_ptr<SdpOfferAnswerHandler> Create( const Environment& env, PeerConnectionSdpMethods* pc, - const PeerConnectionInterface::RTCConfiguration& configuration, std::unique_ptr<RTCCertificateGeneratorInterface> cert_generator, std::unique_ptr<webrtc::VideoBitrateAllocatorFactory> video_bitrate_allocator_factory, @@ -111,7 +110,7 @@ return video_bitrate_allocator_factory_.get(); } - const AudioOptions& audio_options() { return audio_options_; } + const AudioOptions& audio_options() const { return audio_options_; } const VideoOptions& video_options() { return video_options_; } // Change signaling state to Closed, and perform appropriate actions. @@ -245,7 +244,6 @@ // Called from the `Create()` function. Can only be called // once. Modifies dependencies. void Initialize( - const PeerConnectionInterface::RTCConfiguration& configuration, std::unique_ptr<RTCCertificateGeneratorInterface> cert_generator, std::unique_ptr<webrtc::VideoBitrateAllocatorFactory> video_bitrate_allocator_factory, @@ -712,7 +710,7 @@ std::string session_error_desc_ RTC_GUARDED_BY(signaling_thread()); // Member variables for caching global options. - AudioOptions audio_options_ RTC_GUARDED_BY(signaling_thread()); + const AudioOptions audio_options_; VideoOptions video_options_ RTC_GUARDED_BY(signaling_thread()); std::vector<IceParameters> cached_pooled_ice_credentials_ RTC_GUARDED_BY(signaling_thread());