Revert "Follow codec preference order for sending codec fallback."
This reverts commit 1ad3e14e9981772554a848c5034c7c555680aef7. Reason for revert: Breaks downstream project. We are investigating into a potential problem when running on mobile platforms. We will get back with info or reland. Original change's description: > Follow codec preference order for sending codec fallback. > > When encoder selector is not enabled, currently we always fallback to > VP8 no matter how the codec preference is setup. Update to follow codec > preference order for the fallback. > > Bug: chromium:378566918 > Change-Id: Ia3fbfc9d407683ef7b3d6246af7e9ec58535dc89 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370707 > Reviewed-by: Erik Språng <sprang@webrtc.org> > Reviewed-by: Henrik Boström <hbos@webrtc.org> > Commit-Queue: Jianlin Qiu <jianlin.qiu@intel.com> > Reviewed-by: Harald Alvestrand <hta@webrtc.org> > Reviewed-by: Sergey Silkin <ssilkin@webrtc.org> > Cr-Commit-Position: refs/heads/main@{#43566} Bug: chromium:378566918 Change-Id: I09086b5ad100a8f66e87df167e903d0b5fe5b589 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372080 Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org> Reviewed-by: Erik Språng <sprang@webrtc.org> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Reviewed-by: Sergey Silkin <ssilkin@webrtc.org> Cr-Commit-Position: refs/heads/main@{#43600}
This commit is contained in:
parent
29af9f0c87
commit
7976b77345
@ -152,9 +152,6 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([
|
|||||||
FieldTrial('WebRTC-SrtpRemoveReceiveStream',
|
FieldTrial('WebRTC-SrtpRemoveReceiveStream',
|
||||||
42225949,
|
42225949,
|
||||||
date(2024, 10, 1)),
|
date(2024, 10, 1)),
|
||||||
FieldTrial('WebRTC-SwitchEncoderFollowCodecPreferenceOrder',
|
|
||||||
378566918,
|
|
||||||
date(2025, 5, 1)),
|
|
||||||
FieldTrial('WebRTC-TaskQueue-ReplaceLibeventWithStdlib',
|
FieldTrial('WebRTC-TaskQueue-ReplaceLibeventWithStdlib',
|
||||||
42224654,
|
42224654,
|
||||||
date(2024, 4, 1)),
|
date(2024, 4, 1)),
|
||||||
|
|||||||
@ -1233,10 +1233,6 @@ void WebRtcVideoSendChannel::RequestEncoderFallback() {
|
|||||||
RTC_DCHECK_RUN_ON(&thread_checker_);
|
RTC_DCHECK_RUN_ON(&thread_checker_);
|
||||||
if (negotiated_codecs_.size() <= 1) {
|
if (negotiated_codecs_.size() <= 1) {
|
||||||
RTC_LOG(LS_WARNING) << "Encoder failed but no fallback codec is available";
|
RTC_LOG(LS_WARNING) << "Encoder failed but no fallback codec is available";
|
||||||
// Remove all the send streams since we cannot encode any more.
|
|
||||||
while (!send_streams_.empty()) {
|
|
||||||
RemoveSendStream(send_streams_.begin()->first);
|
|
||||||
}
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -2547,31 +2547,6 @@ TEST_F(WebRtcVideoChannelBaseTest, TwoStreamsSendAndReceive) {
|
|||||||
TwoStreamsSendAndReceive(codec);
|
TwoStreamsSendAndReceive(codec);
|
||||||
}
|
}
|
||||||
|
|
||||||
TEST_F(WebRtcVideoChannelBaseTest,
|
|
||||||
RequestEncoderFallbackNextCodecFollowNegotiatedOrder) {
|
|
||||||
cricket::VideoSenderParameters parameters;
|
|
||||||
parameters.codecs.push_back(GetEngineCodec("VP9"));
|
|
||||||
parameters.codecs.push_back(GetEngineCodec("AV1"));
|
|
||||||
parameters.codecs.push_back(GetEngineCodec("VP8"));
|
|
||||||
EXPECT_TRUE(send_channel_->SetSenderParameters(parameters));
|
|
||||||
|
|
||||||
std::optional<Codec> codec = send_channel_->GetSendCodec();
|
|
||||||
ASSERT_TRUE(codec);
|
|
||||||
EXPECT_EQ("VP9", codec->name);
|
|
||||||
|
|
||||||
SendImpl()->RequestEncoderFallback();
|
|
||||||
time_controller_.AdvanceTime(kFrameDuration);
|
|
||||||
codec = send_channel_->GetSendCodec();
|
|
||||||
ASSERT_TRUE(codec);
|
|
||||||
EXPECT_EQ("AV1", codec->name);
|
|
||||||
|
|
||||||
SendImpl()->RequestEncoderFallback();
|
|
||||||
time_controller_.AdvanceTime(kFrameDuration);
|
|
||||||
codec = send_channel_->GetSendCodec();
|
|
||||||
ASSERT_TRUE(codec);
|
|
||||||
EXPECT_EQ("VP8", codec->name);
|
|
||||||
}
|
|
||||||
|
|
||||||
#if defined(RTC_ENABLE_VP9)
|
#if defined(RTC_ENABLE_VP9)
|
||||||
|
|
||||||
TEST_F(WebRtcVideoChannelBaseTest, RequestEncoderFallback) {
|
TEST_F(WebRtcVideoChannelBaseTest, RequestEncoderFallback) {
|
||||||
|
|||||||
@ -80,10 +80,6 @@ constexpr char kFrameDropperFieldTrial[] = "WebRTC-FrameDropper";
|
|||||||
constexpr char kSwitchEncoderOnInitializationFailuresFieldTrial[] =
|
constexpr char kSwitchEncoderOnInitializationFailuresFieldTrial[] =
|
||||||
"WebRTC-SwitchEncoderOnInitializationFailures";
|
"WebRTC-SwitchEncoderOnInitializationFailures";
|
||||||
|
|
||||||
// TODO(crbugs.com/378566918): Remove this kill switch after rollout.
|
|
||||||
constexpr char kSwitchEncoderFollowCodecPreferenceOrderFieldTrial[] =
|
|
||||||
"WebRTC-SwitchEncoderFollowCodecPreferenceOrder";
|
|
||||||
|
|
||||||
const size_t kDefaultPayloadSize = 1440;
|
const size_t kDefaultPayloadSize = 1440;
|
||||||
|
|
||||||
const int64_t kParameterUpdateIntervalMs = 1000;
|
const int64_t kParameterUpdateIntervalMs = 1000;
|
||||||
@ -1485,22 +1481,16 @@ void VideoStreamEncoder::RequestEncoderSwitch() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// If encoder selector is available, switch to the encoder it prefers.
|
// If encoder selector is available, switch to the encoder it prefers.
|
||||||
|
// Otherwise try switching to VP8 (default WebRTC codec).
|
||||||
std::optional<SdpVideoFormat> preferred_fallback_encoder;
|
std::optional<SdpVideoFormat> preferred_fallback_encoder;
|
||||||
if (is_encoder_selector_available) {
|
if (is_encoder_selector_available) {
|
||||||
preferred_fallback_encoder = encoder_selector_->OnEncoderBroken();
|
preferred_fallback_encoder = encoder_selector_->OnEncoderBroken();
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!preferred_fallback_encoder) {
|
if (!preferred_fallback_encoder) {
|
||||||
if (!env_.field_trials().IsDisabled(
|
|
||||||
kSwitchEncoderFollowCodecPreferenceOrderFieldTrial)) {
|
|
||||||
encoder_fallback_requested_ = true;
|
|
||||||
settings_.encoder_switch_request_callback->RequestEncoderFallback();
|
|
||||||
return;
|
|
||||||
} else {
|
|
||||||
preferred_fallback_encoder =
|
preferred_fallback_encoder =
|
||||||
SdpVideoFormat(CodecTypeToPayloadString(kVideoCodecVP8));
|
SdpVideoFormat(CodecTypeToPayloadString(kVideoCodecVP8));
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
settings_.encoder_switch_request_callback->RequestEncoderSwitch(
|
settings_.encoder_switch_request_callback->RequestEncoderSwitch(
|
||||||
*preferred_fallback_encoder, /*allow_default_fallback=*/true);
|
*preferred_fallback_encoder, /*allow_default_fallback=*/true);
|
||||||
@ -1902,15 +1892,11 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
|
|||||||
RTC_LOG(LS_VERBOSE) << __func__ << " posted " << time_when_posted_us
|
RTC_LOG(LS_VERBOSE) << __func__ << " posted " << time_when_posted_us
|
||||||
<< " ntp time " << video_frame.ntp_time_ms();
|
<< " ntp time " << video_frame.ntp_time_ms();
|
||||||
|
|
||||||
// If encoder fallback is requested, but WebRtcVideoEngine cannot switch codec
|
// If the encoder fail we can't continue to encode frames. When this happens
|
||||||
// due to negotatied codec list is already exhausted, we don't continue to
|
// the WebrtcVideoSender is notified and the whole VideoSendStream is
|
||||||
// encode frames, and WebRtcVideoEngine will destroy the VideoSendStreams
|
// recreated.
|
||||||
// along with current VideoStreamEncoder. If the encoder fallback is requested
|
if (encoder_failed_ || !encoder_initialized_)
|
||||||
// and WebRtcVideoEngine responds to the fallback request, the VideoSendStream
|
|
||||||
// is recreated and current VideoStreamEncoder will no longer be used.
|
|
||||||
if (encoder_fallback_requested_ || !encoder_initialized_) {
|
|
||||||
return;
|
return;
|
||||||
}
|
|
||||||
|
|
||||||
// It's possible that EncodeVideoFrame can be called after we've completed
|
// It's possible that EncodeVideoFrame can be called after we've completed
|
||||||
// a Stop() operation. Check if the encoder_ is set before continuing.
|
// a Stop() operation. Check if the encoder_ is set before continuing.
|
||||||
|
|||||||
@ -325,6 +325,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
|
|||||||
bool was_encode_called_since_last_initialization_
|
bool was_encode_called_since_last_initialization_
|
||||||
RTC_GUARDED_BY(encoder_queue_) = false;
|
RTC_GUARDED_BY(encoder_queue_) = false;
|
||||||
|
|
||||||
|
bool encoder_failed_ RTC_GUARDED_BY(encoder_queue_) = false;
|
||||||
|
|
||||||
// Used to make sure incoming time stamp is increasing for every frame.
|
// Used to make sure incoming time stamp is increasing for every frame.
|
||||||
int64_t last_captured_timestamp_ RTC_GUARDED_BY(encoder_queue_) = 0;
|
int64_t last_captured_timestamp_ RTC_GUARDED_BY(encoder_queue_) = 0;
|
||||||
// Delta used for translating between NTP and internal timestamps.
|
// Delta used for translating between NTP and internal timestamps.
|
||||||
@ -380,8 +382,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
|
|||||||
// Provides video stream input states: current resolution and frame rate.
|
// Provides video stream input states: current resolution and frame rate.
|
||||||
VideoStreamInputStateProvider input_state_provider_;
|
VideoStreamInputStateProvider input_state_provider_;
|
||||||
|
|
||||||
bool encoder_fallback_requested_ RTC_GUARDED_BY(encoder_queue_) = false;
|
|
||||||
|
|
||||||
const std::unique_ptr<VideoStreamAdapter> video_stream_adapter_
|
const std::unique_ptr<VideoStreamAdapter> video_stream_adapter_
|
||||||
RTC_GUARDED_BY(encoder_queue_);
|
RTC_GUARDED_BY(encoder_queue_);
|
||||||
// Responsible for adapting input resolution or frame rate to ensure resources
|
// Responsible for adapting input resolution or frame rate to ensure resources
|
||||||
|
|||||||
@ -1237,11 +1237,6 @@ class VideoStreamEncoderTest : public ::testing::Test {
|
|||||||
return num_set_rates_;
|
return num_set_rates_;
|
||||||
}
|
}
|
||||||
|
|
||||||
int GetNumEncodes() const {
|
|
||||||
MutexLock lock(&local_mutex_);
|
|
||||||
return num_encodes_;
|
|
||||||
}
|
|
||||||
|
|
||||||
void SetPreferredPixelFormats(
|
void SetPreferredPixelFormats(
|
||||||
absl::InlinedVector<VideoFrameBuffer::Type, kMaxPreferredPixelFormats>
|
absl::InlinedVector<VideoFrameBuffer::Type, kMaxPreferredPixelFormats>
|
||||||
pixel_formats) {
|
pixel_formats) {
|
||||||
@ -1264,7 +1259,6 @@ class VideoStreamEncoderTest : public ::testing::Test {
|
|||||||
const std::vector<VideoFrameType>* frame_types) override {
|
const std::vector<VideoFrameType>* frame_types) override {
|
||||||
{
|
{
|
||||||
MutexLock lock(&local_mutex_);
|
MutexLock lock(&local_mutex_);
|
||||||
num_encodes_++;
|
|
||||||
if (expect_null_frame_) {
|
if (expect_null_frame_) {
|
||||||
EXPECT_EQ(input_image.rtp_timestamp(), 0u);
|
EXPECT_EQ(input_image.rtp_timestamp(), 0u);
|
||||||
EXPECT_EQ(input_image.width(), 1);
|
EXPECT_EQ(input_image.width(), 1);
|
||||||
@ -1396,7 +1390,6 @@ class VideoStreamEncoderTest : public ::testing::Test {
|
|||||||
std::vector<ResolutionBitrateLimits> resolution_bitrate_limits_
|
std::vector<ResolutionBitrateLimits> resolution_bitrate_limits_
|
||||||
RTC_GUARDED_BY(local_mutex_);
|
RTC_GUARDED_BY(local_mutex_);
|
||||||
int num_set_rates_ RTC_GUARDED_BY(local_mutex_) = 0;
|
int num_set_rates_ RTC_GUARDED_BY(local_mutex_) = 0;
|
||||||
int num_encodes_ RTC_GUARDED_BY(local_mutex_) = 0;
|
|
||||||
std::optional<VideoFrameBuffer::Type> last_input_pixel_format_
|
std::optional<VideoFrameBuffer::Type> last_input_pixel_format_
|
||||||
RTC_GUARDED_BY(local_mutex_);
|
RTC_GUARDED_BY(local_mutex_);
|
||||||
absl::InlinedVector<VideoFrameBuffer::Type, kMaxPreferredPixelFormats>
|
absl::InlinedVector<VideoFrameBuffer::Type, kMaxPreferredPixelFormats>
|
||||||
@ -8203,7 +8196,9 @@ TEST_F(VideoStreamEncoderTest,
|
|||||||
.WillByDefault(Return(WEBRTC_VIDEO_CODEC_ENCODER_FAILURE));
|
.WillByDefault(Return(WEBRTC_VIDEO_CODEC_ENCODER_FAILURE));
|
||||||
|
|
||||||
rtc::Event encode_attempted;
|
rtc::Event encode_attempted;
|
||||||
EXPECT_CALL(switch_callback, RequestEncoderFallback())
|
EXPECT_CALL(switch_callback,
|
||||||
|
RequestEncoderSwitch(Field(&SdpVideoFormat::name, "VP8"),
|
||||||
|
/*allow_default_fallback=*/true))
|
||||||
.WillOnce([&encode_attempted]() { encode_attempted.Set(); });
|
.WillOnce([&encode_attempted]() { encode_attempted.Set(); });
|
||||||
|
|
||||||
video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
|
video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
|
||||||
@ -8271,112 +8266,6 @@ TEST_F(VideoStreamEncoderTest, NullEncoderReturnSwitch) {
|
|||||||
video_stream_encoder_.reset();
|
video_stream_encoder_.reset();
|
||||||
}
|
}
|
||||||
|
|
||||||
TEST_F(VideoStreamEncoderTest, NoPreferenceDefaultFallbackToVP8Disabled) {
|
|
||||||
constexpr int kSufficientBitrateToNotDrop = 1000;
|
|
||||||
constexpr int kDontCare = 100;
|
|
||||||
constexpr int kNumFrames = 8;
|
|
||||||
|
|
||||||
NiceMock<MockVideoEncoder> video_encoder;
|
|
||||||
StrictMock<MockEncoderSwitchRequestCallback> switch_callback;
|
|
||||||
video_send_config_.encoder_settings.encoder_switch_request_callback =
|
|
||||||
&switch_callback;
|
|
||||||
auto encoder_factory = std::make_unique<test::VideoEncoderProxyFactory>(
|
|
||||||
&video_encoder, /*encoder_selector=*/nullptr);
|
|
||||||
video_send_config_.encoder_settings.encoder_factory = encoder_factory.get();
|
|
||||||
|
|
||||||
// Reset encoder for new configuration to take effect.
|
|
||||||
ConfigureEncoder(video_encoder_config_.Copy());
|
|
||||||
|
|
||||||
// The VideoStreamEncoder needs some bitrate before it can start encoding,
|
|
||||||
// setting some bitrate so that subsequent calls to WaitForEncodedFrame does
|
|
||||||
// not fail.
|
|
||||||
video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources(
|
|
||||||
/*target_bitrate=*/DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop),
|
|
||||||
/*stable_target_bitrate=*/
|
|
||||||
DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop),
|
|
||||||
/*link_allocation=*/DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop),
|
|
||||||
/*fraction_lost=*/0,
|
|
||||||
/*round_trip_time_ms=*/0,
|
|
||||||
/*cwnd_reduce_ratio=*/0);
|
|
||||||
|
|
||||||
EXPECT_CALL(video_encoder, Encode)
|
|
||||||
.WillOnce(Return(WEBRTC_VIDEO_CODEC_ENCODER_FAILURE));
|
|
||||||
|
|
||||||
EXPECT_CALL(switch_callback, RequestEncoderFallback());
|
|
||||||
|
|
||||||
VideoFrame frame = CreateFrame(1, kDontCare, kDontCare);
|
|
||||||
for (int i = 0; i < kNumFrames; ++i) {
|
|
||||||
int64_t timestamp_ms = CurrentTimeMs();
|
|
||||||
frame.set_ntp_time_ms(timestamp_ms);
|
|
||||||
frame.set_timestamp_us(timestamp_ms * 1000);
|
|
||||||
video_source_.IncomingCapturedFrame(frame);
|
|
||||||
time_controller_.AdvanceTime(TimeDelta::Millis(33));
|
|
||||||
}
|
|
||||||
|
|
||||||
EXPECT_TRUE_WAIT(fake_encoder_.GetNumEncodes() == 0, 5000);
|
|
||||||
// After requesting fallback failure, the encoder will be released.
|
|
||||||
EXPECT_CALL(video_encoder, Release()).Times(1);
|
|
||||||
|
|
||||||
AdvanceTime(TimeDelta::Zero());
|
|
||||||
video_stream_encoder_->Stop();
|
|
||||||
// The encoders produced by the VideoEncoderProxyFactory have a pointer back
|
|
||||||
// to it's factory, so in order for the encoder instance in the
|
|
||||||
// `video_stream_encoder_` to be destroyed before the `encoder_factory` we
|
|
||||||
// reset the `video_stream_encoder_` here.
|
|
||||||
video_stream_encoder_.reset();
|
|
||||||
}
|
|
||||||
|
|
||||||
TEST_F(VideoStreamEncoderTest, NoPreferenceDefaultFallbackToVP8Enabled) {
|
|
||||||
constexpr int kSufficientBitrateToNotDrop = 1000;
|
|
||||||
constexpr int kDontCare = 100;
|
|
||||||
|
|
||||||
webrtc::test::ScopedKeyValueConfig field_trials(
|
|
||||||
field_trials_,
|
|
||||||
"WebRTC-SwitchEncoderFollowCodecPreferenceOrder/Disabled/");
|
|
||||||
|
|
||||||
NiceMock<MockVideoEncoder> video_encoder;
|
|
||||||
StrictMock<MockEncoderSwitchRequestCallback> switch_callback;
|
|
||||||
video_send_config_.encoder_settings.encoder_switch_request_callback =
|
|
||||||
&switch_callback;
|
|
||||||
auto encoder_factory = std::make_unique<test::VideoEncoderProxyFactory>(
|
|
||||||
&video_encoder, /*encoder_selector=*/nullptr);
|
|
||||||
video_send_config_.encoder_settings.encoder_factory = encoder_factory.get();
|
|
||||||
video_encoder_config_.codec_type = kVideoCodecVP9;
|
|
||||||
|
|
||||||
// Reset encoder for new configuration to take effect.
|
|
||||||
ConfigureEncoder(video_encoder_config_.Copy());
|
|
||||||
|
|
||||||
// The VideoStreamEncoder needs some bitrate before it can start encoding,
|
|
||||||
// setting some bitrate so that subsequent calls to WaitForEncodedFrame does
|
|
||||||
// not fail.
|
|
||||||
video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources(
|
|
||||||
/*target_bitrate=*/DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop),
|
|
||||||
/*stable_target_bitrate=*/
|
|
||||||
DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop),
|
|
||||||
/*link_allocation=*/DataRate::KilobitsPerSec(kSufficientBitrateToNotDrop),
|
|
||||||
/*fraction_lost=*/0,
|
|
||||||
/*round_trip_time_ms=*/0,
|
|
||||||
/*cwnd_reduce_ratio=*/0);
|
|
||||||
|
|
||||||
EXPECT_CALL(video_encoder, Encode)
|
|
||||||
.WillOnce(Return(WEBRTC_VIDEO_CODEC_ENCODER_FAILURE));
|
|
||||||
|
|
||||||
// Fallback request will be asking for switching to VP8.
|
|
||||||
EXPECT_CALL(switch_callback,
|
|
||||||
RequestEncoderSwitch(Field(&SdpVideoFormat::name, "VP8"),
|
|
||||||
/*allow_default_fallback=*/true));
|
|
||||||
|
|
||||||
VideoFrame frame = CreateFrame(1, kDontCare, kDontCare);
|
|
||||||
video_source_.IncomingCapturedFrame(frame);
|
|
||||||
|
|
||||||
video_stream_encoder_->Stop();
|
|
||||||
// The encoders produced by the VideoEncoderProxyFactory have a pointer back
|
|
||||||
// to it's factory, so in order for the encoder instance in the
|
|
||||||
// `video_stream_encoder_` to be destroyed before the `encoder_factory` we
|
|
||||||
// reset the `video_stream_encoder_` here.
|
|
||||||
video_stream_encoder_.reset();
|
|
||||||
}
|
|
||||||
|
|
||||||
TEST_F(VideoStreamEncoderTest,
|
TEST_F(VideoStreamEncoderTest,
|
||||||
AllocationPropagatedToEncoderWhenTargetRateChanged) {
|
AllocationPropagatedToEncoderWhenTargetRateChanged) {
|
||||||
const int kFrameWidth = 320;
|
const int kFrameWidth = 320;
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user