Fix bug that can cause invalid reset of corruption detection state.

`VideoStreamEncoder` should not recreate the
`FrameInstrumentationGenerator` instace unless the encoder is actually
released. Otherwise it will restart and expect a keyframe the encoder
will likely not produce for a while.

Bug: webrtc:358039777
Change-Id: I111149d5e9b632df9eeb88bbbe8a07969c3e3f1d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/369740
Auto-Submit: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43468}
This commit is contained in:
Erik Språng 2024-11-28 15:05:19 +01:00 committed by WebRTC LUCI CQ
parent c75fbe24e6
commit b4d09df6c2
2 changed files with 50 additions and 6 deletions

View File

@ -943,12 +943,6 @@ void VideoStreamEncoder::ConfigureEncoder(VideoEncoderConfig config,
max_data_payload_length_ = max_data_payload_length;
pending_encoder_reconfiguration_ = true;
if (settings_.enable_frame_instrumentation_generator) {
frame_instrumentation_generator_ =
std::make_unique<FrameInstrumentationGenerator>(
encoder_config_.codec_type);
}
// Reconfigure the encoder now if the frame resolution is known.
// Otherwise, the reconfiguration is deferred until the next frame to
// minimize the number of reconfigurations. The codec configuration
@ -1314,6 +1308,11 @@ void VideoStreamEncoder::ReconfigureEncoder() {
next_frame_types_.resize(
std::max(static_cast<int>(codec.numberOfSimulcastStreams), 1),
VideoFrameType::kVideoFrameKey);
if (settings_.enable_frame_instrumentation_generator) {
frame_instrumentation_generator_ =
std::make_unique<FrameInstrumentationGenerator>(
encoder_config_.codec_type);
}
}
frame_encode_metadata_writer_.Reset();
@ -2495,6 +2494,7 @@ void VideoStreamEncoder::ReleaseEncoder() {
}
encoder_->Release();
encoder_initialized_ = false;
frame_instrumentation_generator_ = nullptr;
TRACE_EVENT0("webrtc", "VCMGenericEncoder::Release");
}

View File

@ -1525,6 +1525,11 @@ class VideoStreamEncoderTest : public ::testing::Test {
return last_frame_instrumentation_data_;
}
void ResetLastFrameInstrumentationData() {
MutexLock lock(&mutex_);
last_frame_instrumentation_data_.reset();
}
private:
Result OnEncodedImage(
const EncodedImage& encoded_image,
@ -1776,6 +1781,45 @@ TEST_F(VideoStreamEncoderTest,
video_stream_encoder_->Stop();
}
TEST_F(VideoStreamEncoderTest,
FrameInstrumentationGeneratorNotResetOnConfigurationUnlessEncoderIsToo) {
// Enable frame instrumentation generator and produce the first keyframe.
video_send_config_.encoder_settings.enable_frame_instrumentation_generator =
true;
ConfigureEncoder(video_encoder_config_.Copy());
video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources(
kTargetBitrate, kTargetBitrate, kTargetBitrate, 0, 0, 0);
// We need a QP for the encoded frame.
fake_encoder_.SetEncodedImageData(EncodedImageBuffer::Create(
kCodedFrameVp8Qp25, sizeof(kCodedFrameVp8Qp25)));
video_source_.IncomingCapturedFrame(
CreateFrame(1, codec_width_, codec_height_));
WaitForEncodedFrame(1);
EXPECT_TRUE(sink_.GetLastFrameInstrumentationData().has_value());
sink_.ResetLastFrameInstrumentationData();
// Apply the same configuration again. Encoder should not be reinitilized.
video_stream_encoder_->ConfigureEncoder(video_encoder_config_.Copy(),
kMaxPayloadLength, nullptr);
// Insert delta frames until a frame instrumentation should definitely have
// been sent.
for (int i = 1; i < 40; ++i) {
int timestamp = 1 + (33 * i);
fake_encoder_.SetEncodedImageData(EncodedImageBuffer::Create(
kCodedFrameVp8Qp25, sizeof(kCodedFrameVp8Qp25)));
video_source_.IncomingCapturedFrame(
CreateFrame(timestamp, codec_width_, codec_height_));
WaitForEncodedFrame(timestamp);
}
EXPECT_TRUE(sink_.GetLastFrameInstrumentationData().has_value());
video_stream_encoder_->Stop();
}
TEST_F(VideoStreamEncoderTest, DropsFramesBeforeFirstOnBitrateUpdated) {
// Dropped since no target bitrate has been set.
rtc::Event frame_destroyed_event;