Proper assignment of FEC setting to encoders in SoftwareFallbackWrapper.

When using VideoEncoderSoftwareFallbackWrapper, releasing and
initialization of encoder_ (H/W) and fallback_encoder_(S/W) happen
repeatedly as reconfiguration procedure is called from higher layer.

Below problems would occur when our encoder_(H/W) fails to initialize
or encode.

Firstly, some encoders' SetFecControllerOverride() functions will fail
during repeated calls since they have checks like
RTC_DCHECK(!fec_controller_override_) to avoid repeated assignment of
fec_controller_override_.
(see : LibvpxVp8Encoder::SetFecControllerOverride())

Secondly, if main_ encoder fails to initialize at first attempt, FEC
setting (fec_controller_override) will not set until reconfiguration
procedure is called again.

This CL comes with two changes to fix above problems.
1. Sets fec_controller_override to both encoders when
SoftwareFallbackWrapper::SetFecController() is called.
2. Removes the current_encoder()->SetFecControllerOverride() in
PrimeEncoder() to avoid redundant calls which may involve fatal error.

Bug: webrtc:13184
Change-Id: I082c93de552bc9ec3141c6490d35acfcee2f8935
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/234301
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35231}
This commit is contained in:
Jiwon Jung
2021-10-08 18:22:57 +09:00
committed by WebRTC LUCI CQ
parent bf2a70a14d
commit 3791d332c5
2 changed files with 8 additions and 12 deletions

View File

@ -982,7 +982,7 @@ TEST_F(PreferTemporalLayersFallbackTest, PrimesEncoderOnSwitch) {
wrapper_->RegisterEncodeCompleteCallback(&callback1);
EXPECT_CALL(*hw_, SetFecControllerOverride(&fec_controller_override1));
EXPECT_CALL(*sw_, SetFecControllerOverride).Times(0);
EXPECT_CALL(*sw_, SetFecControllerOverride).Times(1);
wrapper_->SetFecControllerOverride(&fec_controller_override1);
EXPECT_CALL(*hw_, SetRates(rate_params1));
@ -1007,7 +1007,7 @@ TEST_F(PreferTemporalLayersFallbackTest, PrimesEncoderOnSwitch) {
EXPECT_CALL(*sw_, RegisterEncodeCompleteCallback(&callback1));
EXPECT_CALL(*hw_, RegisterEncodeCompleteCallback).Times(0);
EXPECT_CALL(*sw_, SetFecControllerOverride(&fec_controller_override1));
EXPECT_CALL(*sw_, SetFecControllerOverride).Times(0);
EXPECT_CALL(*hw_, SetFecControllerOverride).Times(0);
// Rate control parameters are cleared on InitEncode.
@ -1041,7 +1041,7 @@ TEST_F(PreferTemporalLayersFallbackTest, PrimesEncoderOnSwitch) {
wrapper_->RegisterEncodeCompleteCallback(&callback2);
EXPECT_CALL(*sw_, SetFecControllerOverride(&fec_controller_override2));
EXPECT_CALL(*hw_, SetFecControllerOverride).Times(0);
EXPECT_CALL(*hw_, SetFecControllerOverride).Times(1);
wrapper_->SetFecControllerOverride(&fec_controller_override2);
EXPECT_CALL(*sw_, SetRates(rate_params2));
@ -1066,7 +1066,7 @@ TEST_F(PreferTemporalLayersFallbackTest, PrimesEncoderOnSwitch) {
EXPECT_CALL(*hw_, RegisterEncodeCompleteCallback(&callback2));
EXPECT_CALL(*sw_, RegisterEncodeCompleteCallback).Times(0);
EXPECT_CALL(*hw_, SetFecControllerOverride(&fec_controller_override2));
EXPECT_CALL(*hw_, SetFecControllerOverride).Times(0);
EXPECT_CALL(*sw_, SetFecControllerOverride).Times(0);
// Rate control parameters are cleared on InitEncode.

View File

@ -180,7 +180,6 @@ class VideoEncoderSoftwareFallbackWrapper final : public VideoEncoder {
// The last channel parameters set.
absl::optional<float> packet_loss_;
absl::optional<int64_t> rtt_;
FecControllerOverride* fec_controller_override_;
absl::optional<LossNotification> loss_notification_;
enum class EncoderState {
@ -205,8 +204,7 @@ VideoEncoderSoftwareFallbackWrapper::VideoEncoderSoftwareFallbackWrapper(
std::unique_ptr<webrtc::VideoEncoder> sw_encoder,
std::unique_ptr<webrtc::VideoEncoder> hw_encoder,
bool prefer_temporal_support)
: fec_controller_override_(nullptr),
encoder_state_(EncoderState::kUninitialized),
: encoder_state_(EncoderState::kUninitialized),
encoder_(std::move(hw_encoder)),
fallback_encoder_(std::move(sw_encoder)),
callback_(nullptr),
@ -234,9 +232,7 @@ void VideoEncoderSoftwareFallbackWrapper::PrimeEncoder(
if (packet_loss_.has_value()) {
encoder->OnPacketLossRateUpdate(packet_loss_.value());
}
if (fec_controller_override_) {
encoder->SetFecControllerOverride(fec_controller_override_);
}
if (loss_notification_.has_value()) {
encoder->OnLossNotification(loss_notification_.value());
}
@ -277,8 +273,8 @@ void VideoEncoderSoftwareFallbackWrapper::SetFecControllerOverride(
// `fec_controller_override` at a given time. This is the responsibility
// of `this` to maintain.
fec_controller_override_ = fec_controller_override;
current_encoder()->SetFecControllerOverride(fec_controller_override);
encoder_->SetFecControllerOverride(fec_controller_override);
fallback_encoder_->SetFecControllerOverride(fec_controller_override);
}
int32_t VideoEncoderSoftwareFallbackWrapper::InitEncode(