From 17043b89ae6422eb055c13c5da191b73927ac99e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 4 Jan 2023 13:58:31 +0100 Subject: [PATCH] Make sure VP9 encoders are reconfigured on layer activation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When disabling a spatial layer, reconfiguration of the encoder is not necessary (bitrate will never be assigned to the inactive layer anway). This CL however makes sure we reconfigure the encoder when a spatial layer is activated. Some encoder implementations may encoder the wrong number of spatial layers if the active layers have not beens set correctly. Bug: webrtc:14809, b/261097903 Change-Id: I8d34aaec95eb50a9717c06ea38f25088e5a96429 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290560 Commit-Queue: Philip Eliasson Auto-Submit: Erik Språng Reviewed-by: Philip Eliasson Cr-Commit-Position: refs/heads/main@{#38999} --- video/video_stream_encoder.cc | 7 +++- video/video_stream_encoder_unittest.cc | 58 ++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 994173228b..c680fe12c8 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -150,6 +150,10 @@ bool RequiresEncoderReset(const VideoCodec& prev_send_codec, if (new_send_codec.codecType == kVideoCodecVP9) { size_t num_spatial_layers = new_send_codec.VP9().numberOfSpatialLayers; for (unsigned char i = 0; i < num_spatial_layers; ++i) { + if (!new_send_codec.spatialLayers[i].active) { + // No need to reset when layer is inactive. + continue; + } if (new_send_codec.spatialLayers[i].width != prev_send_codec.spatialLayers[i].width || new_send_codec.spatialLayers[i].height != @@ -157,7 +161,8 @@ bool RequiresEncoderReset(const VideoCodec& prev_send_codec, new_send_codec.spatialLayers[i].numberOfTemporalLayers != prev_send_codec.spatialLayers[i].numberOfTemporalLayers || new_send_codec.spatialLayers[i].qpMax != - prev_send_codec.spatialLayers[i].qpMax) { + prev_send_codec.spatialLayers[i].qpMax || + !prev_send_codec.spatialLayers[i].active) { return true; } } diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 1f328ccf81..cdd4c75ab7 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -8649,6 +8649,64 @@ TEST_F(VideoStreamEncoderTest, video_stream_encoder_->Stop(); } +TEST_F(VideoStreamEncoderTest, RecreatesEncoderWhenEnableVp9SpatialLayer) { + // Set up encoder to use VP9 SVC using two spatial layers. + fake_encoder_.SetTemporalLayersSupported(/*spatial_idx=*/0, true); + fake_encoder_.SetTemporalLayersSupported(/*spatial_idx*/ 1, true); + VideoEncoderConfig video_encoder_config; + test::FillEncoderConfiguration(VideoCodecType::kVideoCodecVP9, + /* num_streams*/ 1, &video_encoder_config); + video_encoder_config.max_bitrate_bps = 2 * kTargetBitrate.bps(); + video_encoder_config.content_type = + VideoEncoderConfig::ContentType::kRealtimeVideo; + VideoCodecVP9 vp9_settings = VideoEncoder::GetDefaultVp9Settings(); + vp9_settings.numberOfSpatialLayers = 2; + vp9_settings.numberOfTemporalLayers = 2; + vp9_settings.interLayerPred = InterLayerPredMode::kOn; + vp9_settings.automaticResizeOn = false; + video_encoder_config.encoder_specific_settings = + rtc::make_ref_counted( + vp9_settings); + video_encoder_config.spatial_layers = GetSvcConfig(1280, 720, + /*fps=*/30.0, + /*first_active_layer=*/0, + /*num_spatial_layers=*/2, + /*num_temporal_layers=*/3, + /*is_screenshare=*/false); + ConfigureEncoder(video_encoder_config.Copy(), + VideoStreamEncoder::BitrateAllocationCallbackType:: + kVideoLayersAllocation); + + video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( + kTargetBitrate, kTargetBitrate, kTargetBitrate, 0, 0, 0); + + video_source_.IncomingCapturedFrame(CreateFrame(CurrentTimeMs(), 1280, 720)); + WaitForEncodedFrame(CurrentTimeMs()); + EXPECT_EQ(fake_encoder_.GetNumInitializations(), 1); + + // Turn off the top spatial layer. This does not require an encoder reset. + video_encoder_config.spatial_layers[1].active = false; + video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), + kMaxPayloadLength, nullptr); + + time_controller_.AdvanceTime(TimeDelta::Millis(33)); + video_source_.IncomingCapturedFrame(CreateFrame(CurrentTimeMs(), 1280, 720)); + WaitForEncodedFrame(CurrentTimeMs()); + EXPECT_EQ(fake_encoder_.GetNumInitializations(), 1); + + // Turn on the top spatial layer again, this does require an encoder reset. + video_encoder_config.spatial_layers[1].active = true; + video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), + kMaxPayloadLength, nullptr); + + time_controller_.AdvanceTime(TimeDelta::Millis(33)); + video_source_.IncomingCapturedFrame(CreateFrame(CurrentTimeMs(), 1280, 720)); + WaitForEncodedFrame(CurrentTimeMs()); + EXPECT_EQ(fake_encoder_.GetNumInitializations(), 2); + + video_stream_encoder_->Stop(); +} + #endif // !defined(WEBRTC_IOS) // Test parameters: (VideoCodecType codec, bool allow_i420_conversion)