Reland "Follow pref order for codec fallback."
This is a reland of commit 1ad3e14e9981772554a848c5034c7c555680aef7 The original CL removes all sending streams since all codec types has been attempted for encoding and we have no other codecs to fallback to. However some downstream applications will still attempt to set the codec preferences even all send streams have been removed. As a result, we follow previous logic to keep the send streams, to avoid the regression. 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, b/384725754 Change-Id: Ifd48b30b80ae51c3ede9391ed62e8ce408864aa0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374852 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: Sergey Silkin <ssilkin@webrtc.org> Cr-Commit-Position: refs/heads/main@{#43874}
This commit is contained in:
parent
902bc24b6f
commit
c592e766a0
@ -152,6 +152,9 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([
|
||||
FieldTrial('WebRTC-SrtpRemoveReceiveStream',
|
||||
42225949,
|
||||
date(2024, 10, 1)),
|
||||
FieldTrial('WebRTC-SwitchEncoderFollowCodecPreferenceOrder',
|
||||
378566918,
|
||||
date(2025, 5, 1)),
|
||||
FieldTrial('WebRTC-TaskQueue-ReplaceLibeventWithStdlib',
|
||||
42224654,
|
||||
date(2024, 4, 1)),
|
||||
|
||||
@ -2575,6 +2575,38 @@ TEST_F(WebRtcVideoChannelBaseTest, TwoStreamsSendAndReceive) {
|
||||
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);
|
||||
|
||||
SendImpl()->RequestEncoderFallback();
|
||||
time_controller_.AdvanceTime(kFrameDuration);
|
||||
|
||||
webrtc::test::FrameForwarder frame_forwarder;
|
||||
EXPECT_TRUE(send_channel_->SetVideoSend(kSsrc, nullptr, &frame_forwarder));
|
||||
EXPECT_TRUE(send_channel_->RemoveSendStream(kSsrc));
|
||||
}
|
||||
|
||||
#if defined(RTC_ENABLE_VP9)
|
||||
|
||||
TEST_F(WebRtcVideoChannelBaseTest, RequestEncoderFallback) {
|
||||
|
||||
@ -80,6 +80,10 @@ constexpr char kFrameDropperFieldTrial[] = "WebRTC-FrameDropper";
|
||||
constexpr char kSwitchEncoderOnInitializationFailuresFieldTrial[] =
|
||||
"WebRTC-SwitchEncoderOnInitializationFailures";
|
||||
|
||||
// TODO(crbugs.com/378566918): Remove this kill switch after rollout.
|
||||
constexpr char kSwitchEncoderFollowCodecPreferenceOrderFieldTrial[] =
|
||||
"WebRTC-SwitchEncoderFollowCodecPreferenceOrder";
|
||||
|
||||
const size_t kDefaultPayloadSize = 1440;
|
||||
|
||||
const int64_t kParameterUpdateIntervalMs = 1000;
|
||||
@ -1483,15 +1487,21 @@ void VideoStreamEncoder::RequestEncoderSwitch() {
|
||||
}
|
||||
|
||||
// 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;
|
||||
if (is_encoder_selector_available) {
|
||||
preferred_fallback_encoder = encoder_selector_->OnEncoderBroken();
|
||||
}
|
||||
|
||||
if (!preferred_fallback_encoder) {
|
||||
preferred_fallback_encoder =
|
||||
SdpVideoFormat(CodecTypeToPayloadString(kVideoCodecVP8));
|
||||
if (!env_.field_trials().IsDisabled(
|
||||
kSwitchEncoderFollowCodecPreferenceOrderFieldTrial)) {
|
||||
encoder_fallback_requested_ = true;
|
||||
settings_.encoder_switch_request_callback->RequestEncoderFallback();
|
||||
return;
|
||||
} else {
|
||||
preferred_fallback_encoder =
|
||||
SdpVideoFormat(CodecTypeToPayloadString(kVideoCodecVP8));
|
||||
}
|
||||
}
|
||||
|
||||
settings_.encoder_switch_request_callback->RequestEncoderSwitch(
|
||||
@ -1894,11 +1904,14 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
|
||||
RTC_LOG(LS_VERBOSE) << __func__ << " posted " << time_when_posted_us
|
||||
<< " ntp time " << video_frame.ntp_time_ms();
|
||||
|
||||
// If the encoder fail we can't continue to encode frames. When this happens
|
||||
// the WebrtcVideoSender is notified and the whole VideoSendStream is
|
||||
// recreated.
|
||||
if (encoder_failed_ || !encoder_initialized_)
|
||||
// If encoder fallback is requested, but we run out of codecs to be
|
||||
// negotiated, we don't continue to encode frames. The send streams will still
|
||||
// be kept. Otherwise if WebRtcVideoEngine responds to the fallback request,
|
||||
// the send streams will be recreated and current VideoStreamEncoder will no
|
||||
// longer be used.
|
||||
if (encoder_fallback_requested_ || !encoder_initialized_) {
|
||||
return;
|
||||
}
|
||||
|
||||
// It's possible that EncodeVideoFrame can be called after we've completed
|
||||
// a Stop() operation. Check if the encoder_ is set before continuing.
|
||||
|
||||
@ -325,8 +325,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
|
||||
bool was_encode_called_since_last_initialization_
|
||||
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.
|
||||
int64_t last_captured_timestamp_ RTC_GUARDED_BY(encoder_queue_) = 0;
|
||||
// Delta used for translating between NTP and internal timestamps.
|
||||
@ -382,6 +380,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
|
||||
// Provides video stream input states: current resolution and frame rate.
|
||||
VideoStreamInputStateProvider input_state_provider_;
|
||||
|
||||
bool encoder_fallback_requested_ RTC_GUARDED_BY(encoder_queue_) = false;
|
||||
|
||||
const std::unique_ptr<VideoStreamAdapter> video_stream_adapter_
|
||||
RTC_GUARDED_BY(encoder_queue_);
|
||||
// Responsible for adapting input resolution or frame rate to ensure resources
|
||||
|
||||
@ -94,6 +94,7 @@
|
||||
#include "rtc_base/event.h"
|
||||
#include "rtc_base/experiments/encoder_info_settings.h"
|
||||
#include "rtc_base/experiments/rate_control_settings.h"
|
||||
#include "rtc_base/gunit.h"
|
||||
#include "rtc_base/logging.h"
|
||||
#include "rtc_base/ref_counted_object.h"
|
||||
#include "rtc_base/strings/string_builder.h"
|
||||
@ -1273,6 +1274,11 @@ class VideoStreamEncoderTest : public ::testing::Test {
|
||||
return num_set_rates_;
|
||||
}
|
||||
|
||||
int GetNumEncodes() const {
|
||||
MutexLock lock(&local_mutex_);
|
||||
return num_encodes_;
|
||||
}
|
||||
|
||||
void SetPreferredPixelFormats(
|
||||
absl::InlinedVector<VideoFrameBuffer::Type, kMaxPreferredPixelFormats>
|
||||
pixel_formats) {
|
||||
@ -1295,6 +1301,7 @@ class VideoStreamEncoderTest : public ::testing::Test {
|
||||
const std::vector<VideoFrameType>* frame_types) override {
|
||||
{
|
||||
MutexLock lock(&local_mutex_);
|
||||
num_encodes_++;
|
||||
if (expect_null_frame_) {
|
||||
EXPECT_EQ(input_image.rtp_timestamp(), 0u);
|
||||
EXPECT_EQ(input_image.width(), 1);
|
||||
@ -1426,6 +1433,7 @@ class VideoStreamEncoderTest : public ::testing::Test {
|
||||
std::vector<ResolutionBitrateLimits> resolution_bitrate_limits_
|
||||
RTC_GUARDED_BY(local_mutex_);
|
||||
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_
|
||||
RTC_GUARDED_BY(local_mutex_);
|
||||
absl::InlinedVector<VideoFrameBuffer::Type, kMaxPreferredPixelFormats>
|
||||
@ -8253,9 +8261,7 @@ TEST_F(VideoStreamEncoderTest,
|
||||
.WillByDefault(Return(WEBRTC_VIDEO_CODEC_ENCODER_FAILURE));
|
||||
|
||||
rtc::Event encode_attempted;
|
||||
EXPECT_CALL(switch_callback,
|
||||
RequestEncoderSwitch(Field(&SdpVideoFormat::name, "VP8"),
|
||||
/*allow_default_fallback=*/true))
|
||||
EXPECT_CALL(switch_callback, RequestEncoderFallback())
|
||||
.WillOnce([&encode_attempted]() { encode_attempted.Set(); });
|
||||
|
||||
video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
|
||||
@ -8323,6 +8329,115 @@ TEST_F(VideoStreamEncoderTest, NullEncoderReturnSwitch) {
|
||||
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));
|
||||
}
|
||||
|
||||
ASSERT_THAT(WaitUntil([&] { return fake_encoder_.GetNumEncodes() == 0; },
|
||||
::testing::IsTrue()),
|
||||
IsRtcOk());
|
||||
|
||||
// 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,
|
||||
AllocationPropagatedToEncoderWhenTargetRateChanged) {
|
||||
const int kFrameWidth = 320;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user