Remove AsyncInvoker from WebRtcVideoChannel.
RequestEncoderFallback, RequestEncoderSwitch and SetVideoCodecSwitchingEnabledRequest are now all called on the worker thread. Before, the work already happened on that thread but WebRtcVideoChannel adapted internally when needed. With this CL, there are thread checks to make sure that these calls are always made the same way, we don't need the async invoker and there are fewer calls out from the encoder thread in VideoStreamEncoder (reducing the chance of unintentional blocking). Bug: webrtc:11908 Change-Id: If8738bc2a708a0fefc6fe850b32655f049f30bdc Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/184603 Commit-Queue: Tommi <tommi@webrtc.org> Reviewed-by: Henrik Boström <hbos@webrtc.org> Cr-Commit-Position: refs/heads/master@{#32151}
This commit is contained in:
committed by
Commit Bot
parent
612445ea60
commit
d41c2a6b8a
@ -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<void>(
|
||||
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<void>(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<void>(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<void>(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,
|
||||
|
||||
@ -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<EncoderSwitchRequestCallback::Config>
|
||||
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
|
||||
|
||||
@ -4082,16 +4082,24 @@ RTCError PeerConnection::SetConfiguration(
|
||||
}
|
||||
|
||||
if (modified_config.allow_codec_switching.has_value()) {
|
||||
std::vector<cricket::VideoMediaChannel*> 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<cricket::VideoChannel*>(
|
||||
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<void>(
|
||||
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;
|
||||
|
||||
@ -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> resource,
|
||||
VideoAdaptationReason reason) {
|
||||
|
||||
@ -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_;
|
||||
|
||||
@ -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
|
||||
|
||||
Reference in New Issue
Block a user