From 1e4d4fdf88370637318d38cbf62c731d04fa3e56 Mon Sep 17 00:00:00 2001 From: Yun Zhang Date: Wed, 30 Sep 2020 00:22:46 -0700 Subject: [PATCH] Don't trigger key frame when encoder is not reset during reconfigure Currently, key frames are scheduled even when the encoder is not reset during reconfigeration. This means whenever new parameters like max bitrate or min bitrate are updated through SetRtpParameters(), the triggered encoder reconfigeration will always schedule key frames even they are not necessary. Since parameters' changes like bitrate doesn't require encoder instance reset. This causes flood of key frames in our app since we do regularly max bitrate update according to server control message. Bug: None Change-Id: I15d953b24c30e6026c0e97b30f44495d845f293f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/185380 Commit-Queue: Rasmus Brandt Reviewed-by: Rasmus Brandt Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#32245} --- video/video_stream_encoder.cc | 40 ++++++++---- video/video_stream_encoder_unittest.cc | 90 ++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 11 deletions(-) diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 65bd27bf25..360a419e3c 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -115,21 +115,39 @@ bool RequiresEncoderReset(const VideoCodec& prev_send_codec, } for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams; ++i) { - if (new_send_codec.simulcastStream[i].width != + if (!new_send_codec.simulcastStream[i].active) { + // No need to reset when stream is inactive. + continue; + } + + if (!prev_send_codec.simulcastStream[i].active || + new_send_codec.simulcastStream[i].width != prev_send_codec.simulcastStream[i].width || new_send_codec.simulcastStream[i].height != prev_send_codec.simulcastStream[i].height || - new_send_codec.simulcastStream[i].maxFramerate != - prev_send_codec.simulcastStream[i].maxFramerate || new_send_codec.simulcastStream[i].numberOfTemporalLayers != prev_send_codec.simulcastStream[i].numberOfTemporalLayers || new_send_codec.simulcastStream[i].qpMax != - prev_send_codec.simulcastStream[i].qpMax || - new_send_codec.simulcastStream[i].active != - prev_send_codec.simulcastStream[i].active) { + prev_send_codec.simulcastStream[i].qpMax) { return true; } } + + 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].width != + prev_send_codec.spatialLayers[i].width || + new_send_codec.spatialLayers[i].height != + prev_send_codec.spatialLayers[i].height || + new_send_codec.spatialLayers[i].numberOfTemporalLayers != + prev_send_codec.spatialLayers[i].numberOfTemporalLayers || + new_send_codec.spatialLayers[i].qpMax != + prev_send_codec.spatialLayers[i].qpMax) { + return true; + } + } + } return false; } @@ -756,7 +774,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { // start bitrate or max framerate has changed. if (!encoder_reset_required) { encoder_reset_required = RequiresEncoderReset( - codec, send_codec_, was_encode_called_since_last_initialization_); + send_codec_, codec, was_encode_called_since_last_initialization_); } send_codec_ = codec; @@ -787,6 +805,10 @@ void VideoStreamEncoder::ReconfigureEncoder() { encoder_->RegisterEncodeCompleteCallback(this); frame_encode_metadata_writer_.OnEncoderInit(send_codec_, HasInternalSource()); + next_frame_types_.clear(); + next_frame_types_.resize( + std::max(static_cast(codec.numberOfSimulcastStreams), 1), + VideoFrameType::kVideoFrameKey); } frame_encode_metadata_writer_.Reset(); @@ -798,10 +820,6 @@ void VideoStreamEncoder::ReconfigureEncoder() { OnEncoderSettingsChanged(); if (success) { - next_frame_types_.clear(); - next_frame_types_.resize( - std::max(static_cast(codec.numberOfSimulcastStreams), 1), - VideoFrameType::kVideoFrameKey); RTC_LOG(LS_VERBOSE) << " max bitrate " << codec.maxBitrate << " start bitrate " << codec.startBitrate << " max frame rate " << codec.maxFramerate diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 6909475efc..21a609794e 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -6149,4 +6149,94 @@ TEST_F(VideoStreamEncoderTest, ConfiguresVp9SvcAtOddResolutions) { video_stream_encoder_->Stop(); } +TEST_F(VideoStreamEncoderTest, EncoderResetAccordingToParameterChange) { + const float downscale_factors[] = {4.0, 2.0, 1.0}; + const int number_layers = + sizeof(downscale_factors) / sizeof(downscale_factors[0]); + VideoEncoderConfig config; + test::FillEncoderConfiguration(kVideoCodecVP8, number_layers, &config); + for (int i = 0; i < number_layers; ++i) { + config.simulcast_layers[i].scale_resolution_down_by = downscale_factors[i]; + config.simulcast_layers[i].active = true; + } + config.video_stream_factory = + new rtc::RefCountedObject( + "VP8", /*max qp*/ 56, /*screencast*/ false, + /*screenshare enabled*/ false); + video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( + DataRate::BitsPerSec(kSimulcastTargetBitrateBps), + DataRate::BitsPerSec(kSimulcastTargetBitrateBps), + DataRate::BitsPerSec(kSimulcastTargetBitrateBps), 0, 0, 0); + + // First initialization. + // Encoder should be initialized. Next frame should be key frame. + video_stream_encoder_->ConfigureEncoder(config.Copy(), kMaxPayloadLength); + sink_.SetNumExpectedLayers(number_layers); + int64_t timestamp_ms = kFrameIntervalMs; + video_source_.IncomingCapturedFrame(CreateFrame(timestamp_ms, 1280, 720)); + WaitForEncodedFrame(timestamp_ms); + EXPECT_EQ(1, fake_encoder_.GetNumEncoderInitializations()); + EXPECT_THAT(fake_encoder_.LastFrameTypes(), + ::testing::ElementsAreArray({VideoFrameType::kVideoFrameKey, + VideoFrameType::kVideoFrameKey, + VideoFrameType::kVideoFrameKey})); + + // Disable top layer. + // Encoder shouldn't be re-initialized. Next frame should be delta frame. + config.simulcast_layers[number_layers - 1].active = false; + video_stream_encoder_->ConfigureEncoder(config.Copy(), kMaxPayloadLength); + sink_.SetNumExpectedLayers(number_layers - 1); + timestamp_ms += kFrameIntervalMs; + video_source_.IncomingCapturedFrame(CreateFrame(timestamp_ms, 1280, 720)); + WaitForEncodedFrame(timestamp_ms); + EXPECT_EQ(1, fake_encoder_.GetNumEncoderInitializations()); + EXPECT_THAT(fake_encoder_.LastFrameTypes(), + ::testing::ElementsAreArray({VideoFrameType::kVideoFrameDelta, + VideoFrameType::kVideoFrameDelta, + VideoFrameType::kVideoFrameDelta})); + + // Re-enable top layer. + // Encoder should be re-initialized. Next frame should be key frame. + config.simulcast_layers[number_layers - 1].active = true; + video_stream_encoder_->ConfigureEncoder(config.Copy(), kMaxPayloadLength); + sink_.SetNumExpectedLayers(number_layers); + timestamp_ms += kFrameIntervalMs; + video_source_.IncomingCapturedFrame(CreateFrame(timestamp_ms, 1280, 720)); + WaitForEncodedFrame(timestamp_ms); + EXPECT_EQ(2, fake_encoder_.GetNumEncoderInitializations()); + EXPECT_THAT(fake_encoder_.LastFrameTypes(), + ::testing::ElementsAreArray({VideoFrameType::kVideoFrameKey, + VideoFrameType::kVideoFrameKey, + VideoFrameType::kVideoFrameKey})); + + // Top layer max rate change. + // Encoder shouldn't be re-initialized. Next frame should be delta frame. + config.simulcast_layers[number_layers - 1].max_bitrate_bps -= 100; + video_stream_encoder_->ConfigureEncoder(config.Copy(), kMaxPayloadLength); + sink_.SetNumExpectedLayers(number_layers); + timestamp_ms += kFrameIntervalMs; + video_source_.IncomingCapturedFrame(CreateFrame(timestamp_ms, 1280, 720)); + WaitForEncodedFrame(timestamp_ms); + EXPECT_EQ(2, fake_encoder_.GetNumEncoderInitializations()); + EXPECT_THAT(fake_encoder_.LastFrameTypes(), + ::testing::ElementsAreArray({VideoFrameType::kVideoFrameDelta, + VideoFrameType::kVideoFrameDelta, + VideoFrameType::kVideoFrameDelta})); + + // Top layer resolution change. + // Encoder should be re-initialized. Next frame should be key frame. + config.simulcast_layers[number_layers - 1].scale_resolution_down_by += 0.1; + video_stream_encoder_->ConfigureEncoder(config.Copy(), kMaxPayloadLength); + sink_.SetNumExpectedLayers(number_layers); + timestamp_ms += kFrameIntervalMs; + video_source_.IncomingCapturedFrame(CreateFrame(timestamp_ms, 1280, 720)); + WaitForEncodedFrame(timestamp_ms); + EXPECT_EQ(3, fake_encoder_.GetNumEncoderInitializations()); + EXPECT_THAT(fake_encoder_.LastFrameTypes(), + ::testing::ElementsAreArray({VideoFrameType::kVideoFrameKey, + VideoFrameType::kVideoFrameKey, + VideoFrameType::kVideoFrameKey})); + video_stream_encoder_->Stop(); +} + } // namespace webrtc