diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 2b4e3a4e39..4b7d9e5953 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -39,6 +39,7 @@ #include "rtc_base/logging.h" #include "rtc_base/numerics/safe_conversions.h" #include "rtc_base/strings/string_builder.h" +#include "rtc_base/thread.h" #include "rtc_base/time_utils.h" #include "rtc_base/trace_event.h" #include "system_wrappers/include/field_trial.h" @@ -836,97 +837,85 @@ bool WebRtcVideoChannel::SetSendParameters(const VideoSendParameters& params) { } void WebRtcVideoChannel::RequestEncoderFallback() { - invoker_.AsyncInvoke( - RTC_FROM_HERE, worker_thread_, [this] { - RTC_DCHECK_RUN_ON(&thread_checker_); - if (negotiated_codecs_.size() <= 1) { - RTC_LOG(LS_WARNING) - << "Encoder failed but no fallback codec is available"; - return; - } + RTC_DCHECK_RUN_ON(&thread_checker_); + if (negotiated_codecs_.size() <= 1) { + RTC_LOG(LS_WARNING) << "Encoder failed but no fallback codec is available"; + return; + } - ChangedSendParameters params; - params.negotiated_codecs = negotiated_codecs_; - params.negotiated_codecs->erase(params.negotiated_codecs->begin()); - params.send_codec = params.negotiated_codecs->front(); - ApplyChangedParams(params); - }); + ChangedSendParameters params; + params.negotiated_codecs = negotiated_codecs_; + params.negotiated_codecs->erase(params.negotiated_codecs->begin()); + params.send_codec = params.negotiated_codecs->front(); + ApplyChangedParams(params); } void WebRtcVideoChannel::RequestEncoderSwitch( const EncoderSwitchRequestCallback::Config& conf) { - invoker_.AsyncInvoke(RTC_FROM_HERE, worker_thread_, [this, conf] { - RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK_RUN_ON(&thread_checker_); - if (!allow_codec_switching_) { - RTC_LOG(LS_INFO) << "Encoder switch requested but codec switching has" - " not been enabled yet."; - requested_encoder_switch_ = conf; - return; - } + if (!allow_codec_switching_) { + RTC_LOG(LS_INFO) << "Encoder switch requested but codec switching has" + " not been enabled yet."; + requested_encoder_switch_ = conf; + return; + } - for (const VideoCodecSettings& codec_setting : negotiated_codecs_) { - if (codec_setting.codec.name == conf.codec_name) { - if (conf.param) { - auto it = codec_setting.codec.params.find(*conf.param); + for (const VideoCodecSettings& codec_setting : negotiated_codecs_) { + if (codec_setting.codec.name == conf.codec_name) { + if (conf.param) { + auto it = codec_setting.codec.params.find(*conf.param); + if (it == codec_setting.codec.params.end()) + continue; - if (it == codec_setting.codec.params.end()) { - continue; - } + if (conf.value && it->second != *conf.value) + continue; + } - if (conf.value && it->second != *conf.value) { - continue; - } - } - - if (send_codec_ == codec_setting) { - // Already using this codec, no switch required. - return; - } - - ChangedSendParameters params; - params.send_codec = codec_setting; - ApplyChangedParams(params); + if (send_codec_ == codec_setting) { + // Already using this codec, no switch required. return; } - } - RTC_LOG(LS_WARNING) << "Requested encoder with codec_name:" - << conf.codec_name - << ", param:" << conf.param.value_or("none") - << " and value:" << conf.value.value_or("none") - << "not found. No switch performed."; - }); + ChangedSendParameters params; + params.send_codec = codec_setting; + ApplyChangedParams(params); + return; + } + } + + RTC_LOG(LS_WARNING) << "Requested encoder with codec_name:" << conf.codec_name + << ", param:" << conf.param.value_or("none") + << " and value:" << conf.value.value_or("none") + << "not found. No switch performed."; } void WebRtcVideoChannel::RequestEncoderSwitch( const webrtc::SdpVideoFormat& format) { - invoker_.AsyncInvoke(RTC_FROM_HERE, worker_thread_, [this, format] { - RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK_RUN_ON(&thread_checker_); - for (const VideoCodecSettings& codec_setting : negotiated_codecs_) { - if (IsSameCodec(format.name, format.parameters, codec_setting.codec.name, - codec_setting.codec.params)) { - VideoCodecSettings new_codec_setting = codec_setting; - for (const auto& kv : format.parameters) { - new_codec_setting.codec.params[kv.first] = kv.second; - } + for (const VideoCodecSettings& codec_setting : negotiated_codecs_) { + if (IsSameCodec(format.name, format.parameters, codec_setting.codec.name, + codec_setting.codec.params)) { + VideoCodecSettings new_codec_setting = codec_setting; + for (const auto& kv : format.parameters) { + new_codec_setting.codec.params[kv.first] = kv.second; + } - if (send_codec_ == new_codec_setting) { - // Already using this codec, no switch required. - return; - } - - ChangedSendParameters params; - params.send_codec = new_codec_setting; - ApplyChangedParams(params); + if (send_codec_ == new_codec_setting) { + // Already using this codec, no switch required. return; } - } - RTC_LOG(LS_WARNING) << "Encoder switch failed: SdpVideoFormat " - << format.ToString() << " not negotiated."; - }); + ChangedSendParameters params; + params.send_codec = new_codec_setting; + ApplyChangedParams(params); + return; + } + } + + RTC_LOG(LS_WARNING) << "Encoder switch failed: SdpVideoFormat " + << format.ToString() << " not negotiated."; } bool WebRtcVideoChannel::ApplyChangedParams( @@ -1830,18 +1819,16 @@ void WebRtcVideoChannel::SetFrameEncryptor( } void WebRtcVideoChannel::SetVideoCodecSwitchingEnabled(bool enabled) { - invoker_.AsyncInvoke(RTC_FROM_HERE, worker_thread_, [this, enabled] { - RTC_DCHECK_RUN_ON(&thread_checker_); - allow_codec_switching_ = enabled; - if (allow_codec_switching_) { - RTC_LOG(LS_INFO) << "Encoder switching enabled."; - if (requested_encoder_switch_) { - RTC_LOG(LS_INFO) << "Executing cached video encoder switch request."; - RequestEncoderSwitch(*requested_encoder_switch_); - requested_encoder_switch_.reset(); - } + RTC_DCHECK_RUN_ON(&thread_checker_); + allow_codec_switching_ = enabled; + if (allow_codec_switching_) { + RTC_LOG(LS_INFO) << "Encoder switching enabled."; + if (requested_encoder_switch_) { + RTC_LOG(LS_INFO) << "Executing cached video encoder switch request."; + RequestEncoderSwitch(*requested_encoder_switch_); + requested_encoder_switch_.reset(); } - }); + } } bool WebRtcVideoChannel::SetBaseMinimumPlayoutDelayMs(uint32_t ssrc, diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index b9f27b4eec..f00d0c88e4 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -31,7 +31,6 @@ #include "media/base/media_engine.h" #include "media/engine/constants.h" #include "media/engine/unhandled_packets_buffer.h" -#include "rtc_base/async_invoker.h" #include "rtc_base/network_route.h" #include "rtc_base/synchronization/mutex.h" #include "rtc_base/thread_annotations.h" @@ -634,10 +633,6 @@ class WebRtcVideoChannel : public VideoMediaChannel, bool allow_codec_switching_ = false; absl::optional requested_encoder_switch_; - - // In order for the |invoker_| to protect other members from being destructed - // as they are used in asynchronous tasks it has to be destructed first. - rtc::AsyncInvoker invoker_; }; class EncoderStreamFactory diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 36dc5b27a4..303f5f97f0 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -4082,16 +4082,24 @@ RTCError PeerConnection::SetConfiguration( } if (modified_config.allow_codec_switching.has_value()) { + std::vector channels; for (const auto& transceiver : transceivers_) { - if (transceiver->media_type() != cricket::MEDIA_TYPE_VIDEO || - !transceiver->internal()->channel()) { + if (transceiver->media_type() != cricket::MEDIA_TYPE_VIDEO) continue; - } + auto* video_channel = static_cast( transceiver->internal()->channel()); - video_channel->media_channel()->SetVideoCodecSwitchingEnabled( - *modified_config.allow_codec_switching); + if (video_channel) + channels.push_back(video_channel->media_channel()); } + + worker_thread()->Invoke( + RTC_FROM_HERE, + [channels = std::move(channels), + allow_codec_switching = *modified_config.allow_codec_switching]() { + for (auto* ch : channels) + ch->SetVideoCodecSwitchingEnabled(allow_codec_switching); + }); } configuration_ = modified_config; diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index f8ca4dce3c..65bd27bf25 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -543,7 +543,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { conf.codec_name = encoder_switch_experiment_.to_codec; conf.param = encoder_switch_experiment_.to_param; conf.value = encoder_switch_experiment_.to_value; - settings_.encoder_switch_request_callback->RequestEncoderSwitch(conf); + QueueRequestEncoderSwitch(conf); encoder_switch_requested_ = true; } @@ -1425,12 +1425,14 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, if (settings_.encoder_switch_request_callback) { if (encoder_selector_) { if (auto encoder = encoder_selector_->OnEncoderBroken()) { - settings_.encoder_switch_request_callback->RequestEncoderSwitch( - *encoder); + QueueRequestEncoderSwitch(*encoder); } } else { encoder_failed_ = true; - settings_.encoder_switch_request_callback->RequestEncoderFallback(); + main_queue_->PostTask(ToQueuedTask(task_safety_, [this]() { + RTC_DCHECK_RUN_ON(main_queue_); + settings_.encoder_switch_request_callback->RequestEncoderFallback(); + })); } } else { RTC_LOG(LS_ERROR) @@ -1690,8 +1692,7 @@ void VideoStreamEncoder::OnBitrateUpdated(DataRate target_bitrate, if (encoder_selector_) { if (auto encoder = encoder_selector_->OnAvailableBitrate(link_allocation)) { - settings_.encoder_switch_request_callback->RequestEncoderSwitch( - *encoder); + QueueRequestEncoderSwitch(*encoder); } } else if (encoder_switch_experiment_.IsBitrateBelowThreshold( target_bitrate) && @@ -1700,7 +1701,7 @@ void VideoStreamEncoder::OnBitrateUpdated(DataRate target_bitrate, conf.codec_name = encoder_switch_experiment_.to_codec; conf.param = encoder_switch_experiment_.to_param; conf.value = encoder_switch_experiment_.to_value; - settings_.encoder_switch_request_callback->RequestEncoderSwitch(conf); + QueueRequestEncoderSwitch(conf); encoder_switch_requested_ = true; } @@ -2060,6 +2061,25 @@ void VideoStreamEncoder::CheckForAnimatedContent( })); } } + +// RTC_RUN_ON(&encoder_queue_) +void VideoStreamEncoder::QueueRequestEncoderSwitch( + const EncoderSwitchRequestCallback::Config& conf) { + main_queue_->PostTask(ToQueuedTask(task_safety_, [this, conf]() { + RTC_DCHECK_RUN_ON(main_queue_); + settings_.encoder_switch_request_callback->RequestEncoderSwitch(conf); + })); +} + +// RTC_RUN_ON(&encoder_queue_) +void VideoStreamEncoder::QueueRequestEncoderSwitch( + const webrtc::SdpVideoFormat& format) { + main_queue_->PostTask(ToQueuedTask(task_safety_, [this, format]() { + RTC_DCHECK_RUN_ON(main_queue_); + settings_.encoder_switch_request_callback->RequestEncoderSwitch(format); + })); +} + void VideoStreamEncoder::InjectAdaptationResource( rtc::scoped_refptr resource, VideoAdaptationReason reason) { diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 5ab0840059..e80d1203c7 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -214,6 +214,13 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, int64_t time_when_posted_in_ms) RTC_RUN_ON(&encoder_queue_); + // TODO(bugs.webrtc.org/11341) : Remove this version of RequestEncoderSwitch. + void QueueRequestEncoderSwitch( + const EncoderSwitchRequestCallback::Config& conf) + RTC_RUN_ON(&encoder_queue_); + void QueueRequestEncoderSwitch(const webrtc::SdpVideoFormat& format) + RTC_RUN_ON(&encoder_queue_); + TaskQueueBase* const main_queue_; const uint32_t number_of_cores_; diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 00c22ffdff..504b7b7b9c 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -5789,6 +5789,7 @@ TEST_F(VideoStreamEncoderTest, BitrateEncoderSwitch) { /*fraction_lost=*/0, /*rtt_ms=*/0, /*cwnd_reduce_ratio=*/0); + AdvanceTime(TimeDelta::Millis(0)); video_stream_encoder_->Stop(); } @@ -5924,6 +5925,7 @@ TEST_F(VideoStreamEncoderTest, EncoderSelectorBitrateSwitch) { /*fraction_lost=*/0, /*rtt_ms=*/0, /*cwnd_reduce_ratio=*/0); + AdvanceTime(TimeDelta::Millis(0)); video_stream_encoder_->Stop(); } @@ -5972,6 +5974,8 @@ TEST_F(VideoStreamEncoderTest, EncoderSelectorBrokenEncoderSwitch) { video_source_.IncomingCapturedFrame(CreateFrame(1, kDontCare, kDontCare)); encode_attempted.Wait(3000); + AdvanceTime(TimeDelta::Millis(0)); + video_stream_encoder_->Stop(); // The encoders produces by the VideoEncoderProxyFactory have a pointer back