diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 868ff42b7c..4d86e6c6f4 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -344,24 +344,20 @@ TEST_F(WebRtcVideoEngine2Test, UseExternalFactoryForVp8WhenSupported) { EXPECT_TRUE( channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); - ASSERT_TRUE(encoder_factory.WaitForCreatedVideoEncoders(1)); - ASSERT_EQ(1u, encoder_factory.encoders().size()); + EXPECT_EQ(0, encoder_factory.GetNumCreatedEncoders()); EXPECT_TRUE(channel->SetSend(true)); - cricket::FakeVideoCapturer capturer; EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer)); EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capturer.GetSupportedFormats()->front())); EXPECT_TRUE(capturer.CaptureFrame()); - // Sending one frame will have reallocated the encoder since input size - // changes from a small default to the actual frame width/height. Wait for - // that to happen then for the frame to be sent. - ASSERT_TRUE(encoder_factory.WaitForCreatedVideoEncoders(2)); + // Sending one frame will have allocate the encoder. + ASSERT_TRUE(encoder_factory.WaitForCreatedVideoEncoders(1)); EXPECT_TRUE_WAIT(encoder_factory.encoders()[0]->GetNumEncodedFrames() > 0, kTimeout); int num_created_encoders = encoder_factory.GetNumCreatedEncoders(); - EXPECT_EQ(num_created_encoders, 2); + EXPECT_EQ(num_created_encoders, 1); // Setting codecs of the same type should not reallocate any encoders // (expecting a no-op). @@ -669,6 +665,14 @@ TEST_F(WebRtcVideoEngine2Test, EXPECT_TRUE( channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); ASSERT_EQ(1u, encoder_factory.encoders().size()); + + // Send a frame of 720p. This should trigger a "real" encoder initialization. + cricket::VideoFormat format( + 1280, 720, cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420); + cricket::FakeVideoCapturer capturer; + EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer)); + EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(format)); + EXPECT_TRUE(capturer.CaptureFrame()); ASSERT_TRUE(encoder_factory.encoders()[0]->WaitForInitEncode()); EXPECT_EQ(webrtc::kVideoCodecH264, encoder_factory.encoders()[0]->GetCodecSettings().codecType); diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 9d00e007de..2663516b6b 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -1785,12 +1785,9 @@ TEST_F(VideoSendStreamTest, EncoderIsProperlyInitializedAndDestroyed) { void PerformTest() override { EXPECT_TRUE(Wait()) << "Timed out while waiting for Encode."; - // Expect |num_releases| == 1 since the encoder has been reconfigured - // once when the first frame is encoded. Not until at that point is the - // frame size known and the encoder can be properly initialized. - EXPECT_EQ(1u, num_releases()); + EXPECT_EQ(0u, num_releases()); stream_->ReconfigureVideoEncoder(std::move(encoder_config_)); - EXPECT_EQ(1u, num_releases()); + EXPECT_EQ(0u, num_releases()); stream_->Stop(); // Encoder should not be released before destroying the VideoSendStream. EXPECT_FALSE(IsReleased()); @@ -1812,7 +1809,7 @@ TEST_F(VideoSendStreamTest, EncoderIsProperlyInitializedAndDestroyed) { RunBaseTest(&test_encoder); EXPECT_TRUE(test_encoder.IsReleased()); - EXPECT_EQ(2u, test_encoder.num_releases()); + EXPECT_EQ(1u, test_encoder.num_releases()); } TEST_F(VideoSendStreamTest, EncoderSetupPropagatesCommonEncoderConfigValues) { @@ -1844,7 +1841,7 @@ TEST_F(VideoSendStreamTest, EncoderSetupPropagatesCommonEncoderConfigValues) { int32_t InitEncode(const VideoCodec* config, int32_t number_of_cores, size_t max_payload_size) override { - if (num_initializations_ < 2) { + if (num_initializations_ == 0) { // Verify default values. EXPECT_EQ(kRealtimeVideo, config->mode); } else { @@ -1852,23 +1849,18 @@ TEST_F(VideoSendStreamTest, EncoderSetupPropagatesCommonEncoderConfigValues) { EXPECT_EQ(kScreensharing, config->mode); } ++num_initializations_; - if (num_initializations_ > 1) { - // Skip the first two InitEncode events: one with QCIF resolution when - // the SendStream is created, the other with QVGA when the first frame - // is encoded. - init_encode_event_.Set(); - } + init_encode_event_.Set(); return FakeEncoder::InitEncode(config, number_of_cores, max_payload_size); } void PerformTest() override { EXPECT_TRUE(init_encode_event_.Wait(kDefaultTimeoutMs)); - EXPECT_EQ(2u, num_initializations_) << "VideoEncoder not initialized."; + EXPECT_EQ(1u, num_initializations_) << "VideoEncoder not initialized."; encoder_config_.content_type = VideoEncoderConfig::ContentType::kScreen; stream_->ReconfigureVideoEncoder(std::move(encoder_config_)); EXPECT_TRUE(init_encode_event_.Wait(kDefaultTimeoutMs)); - EXPECT_EQ(3u, num_initializations_) + EXPECT_EQ(2u, num_initializations_) << "ReconfigureVideoEncoder did not reinitialize the encoder with " "new encoder settings."; } @@ -1945,12 +1937,7 @@ class VideoCodecConfigObserver : public test::SendTest, EXPECT_EQ(video_codec_type_, config->codecType); VerifyCodecSpecifics(*config); ++num_initializations_; - if (num_initializations_ > 1) { - // Skip the first two InitEncode events: one with QCIF resolution when - // the SendStream is created, the other with QVGA when the first frame is - // encoded. - init_encode_event_.Set(); - } + init_encode_event_.Set(); return FakeEncoder::InitEncode(config, number_of_cores, max_payload_size); } @@ -1961,14 +1948,14 @@ class VideoCodecConfigObserver : public test::SendTest, void PerformTest() override { EXPECT_TRUE( init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)); - ASSERT_EQ(2u, num_initializations_) << "VideoEncoder not initialized."; + ASSERT_EQ(1u, num_initializations_) << "VideoEncoder not initialized."; encoder_settings_.frameDroppingOn = true; encoder_config_.encoder_specific_settings = GetEncoderSpecificSettings(); stream_->ReconfigureVideoEncoder(std::move(encoder_config_)); ASSERT_TRUE( init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)); - EXPECT_EQ(3u, num_initializations_) + EXPECT_EQ(2u, num_initializations_) << "ReconfigureVideoEncoder did not reinitialize the encoder with " "new encoder settings."; } @@ -2215,8 +2202,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { size_t maxPayloadSize) override { EXPECT_GE(codecSettings->startBitrate, codecSettings->minBitrate); EXPECT_LE(codecSettings->startBitrate, codecSettings->maxBitrate); - // First reinitialization happens due to that the frame size is updated. - if (num_initializations_ == 0 || num_initializations_ == 1) { + if (num_initializations_ == 0) { EXPECT_EQ(static_cast(kMinBitrateKbps), codecSettings->minBitrate); EXPECT_EQ(static_cast(kStartBitrateKbps), @@ -2224,14 +2210,14 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { EXPECT_EQ(static_cast(kMaxBitrateKbps), codecSettings->maxBitrate); observation_complete_.Set(); - } else if (num_initializations_ == 2) { + } else if (num_initializations_ == 1) { EXPECT_EQ(static_cast(kLowerMaxBitrateKbps), codecSettings->maxBitrate); // The start bitrate should be kept (-1) and capped to the max bitrate. // Since this is not an end-to-end call no receiver should have been // returning a REMB that could lower this estimate. EXPECT_EQ(codecSettings->startBitrate, codecSettings->maxBitrate); - } else if (num_initializations_ == 3) { + } else if (num_initializations_ == 2) { EXPECT_EQ(static_cast(kIncreasedMaxBitrateKbps), codecSettings->maxBitrate); // The start bitrate will be whatever the rate BitRateController @@ -2239,9 +2225,8 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { // bitrate. } ++num_initializations_; - if (num_initializations_ > 1) { - init_encode_event_.Set(); - } + init_encode_event_.Set(); + return FakeEncoder::InitEncode(codecSettings, numberOfCores, maxPayloadSize); } @@ -2334,7 +2319,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { send_stream_->ReconfigureVideoEncoder(encoder_config_.Copy()); ASSERT_TRUE( init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)); - EXPECT_EQ(3, num_initializations_) + EXPECT_EQ(2, num_initializations_) << "Encoder should have been reconfigured with the new value."; WaitForSetRates(kLowerMaxBitrateKbps); @@ -2342,7 +2327,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { send_stream_->ReconfigureVideoEncoder(encoder_config_.Copy()); ASSERT_TRUE( init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)); - EXPECT_EQ(4, num_initializations_) + EXPECT_EQ(3, num_initializations_) << "Encoder should have been reconfigured with the new value."; // Expected target bitrate is the start bitrate set in the call to // call_->SetBitrateConfig. diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index a3fd98dd6f..f5bde2e359 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -410,15 +410,14 @@ void ViEEncoder::ConfigureEncoderOnTaskQueue(VideoEncoderConfig config, pending_encoder_reconfiguration_ = true; // Reconfigure the encoder now if the encoder has an internal source or - // if this is the first time the encoder is configured. - // Otherwise, the reconfiguration is deferred until the next frame to minimize - // the number of reconfigurations. The codec configuration depends on incoming - // video frame size. - if (!last_frame_info_ || settings_.internal_source) { - if (!last_frame_info_) { - last_frame_info_ = rtc::Optional( - VideoFrameInfo(176, 144, kVideoRotation_0, false)); - } + // if the frame resolution is known. Otherwise, the reconfiguration is + // deferred until the next frame to minimize the number of reconfigurations. + // The codec configuration depends on incoming video frame size. + if (last_frame_info_) { + ReconfigureEncoder(); + } else if (settings_.internal_source) { + last_frame_info_ = rtc::Optional( + VideoFrameInfo(1, 1, kVideoRotation_0, true)); ReconfigureEncoder(); } } @@ -542,7 +541,7 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame, if (pre_encode_callback_) pre_encode_callback_->OnFrame(video_frame); - if (video_frame.width() != last_frame_info_->width || + if (!last_frame_info_ || video_frame.width() != last_frame_info_->width || video_frame.height() != last_frame_info_->height || video_frame.rotation() != last_frame_info_->rotation || video_frame.is_texture() != last_frame_info_->is_texture) { diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index 68f043e1c5..33caec42f4 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -114,7 +114,7 @@ class ViEEncoder : public rtc::VideoSinkInterface, is_texture(is_texture) {} int width; int height; - webrtc::VideoRotation rotation; + VideoRotation rotation; bool is_texture; }; diff --git a/webrtc/video/vie_encoder_unittest.cc b/webrtc/video/vie_encoder_unittest.cc index 5382fbf8eb..78eb62f99f 100644 --- a/webrtc/video/vie_encoder_unittest.cc +++ b/webrtc/video/vie_encoder_unittest.cc @@ -286,13 +286,14 @@ TEST_F(ViEEncoderTest, DropsPendingFramesOnSlowEncode) { TEST_F(ViEEncoderTest, ConfigureEncoderTriggersOnEncoderConfigurationChanged) { const int kTargetBitrateBps = 100000; vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + EXPECT_EQ(0, sink_.number_of_reconfigurations()); // Capture a frame and wait for it to synchronize with the encoder thread. video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); sink_.WaitForEncodedFrame(1); - // The encoder will have been configured twice. First time before the first - // frame has been received. Then a second time when the resolution is known. - EXPECT_EQ(2, sink_.number_of_reconfigurations()); + // The encoder will have been configured once when the first frame is + // received. + EXPECT_EQ(1, sink_.number_of_reconfigurations()); VideoEncoderConfig video_encoder_config; test::FillEncoderConfiguration(1, &video_encoder_config); @@ -302,7 +303,7 @@ TEST_F(ViEEncoderTest, ConfigureEncoderTriggersOnEncoderConfigurationChanged) { // Capture a frame and wait for it to synchronize with the encoder thread. video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr)); sink_.WaitForEncodedFrame(2); - EXPECT_EQ(3, sink_.number_of_reconfigurations()); + EXPECT_EQ(2, sink_.number_of_reconfigurations()); EXPECT_EQ(9999, sink_.last_min_transmit_bitrate()); vie_encoder_->Stop(); @@ -315,9 +316,8 @@ TEST_F(ViEEncoderTest, FrameResolutionChangeReconfigureEncoder) { // Capture a frame and wait for it to synchronize with the encoder thread. video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); sink_.WaitForEncodedFrame(1); - // The encoder will have been configured twice. First time before the first - // frame has been received. Then a second time when the resolution is known. - EXPECT_EQ(2, sink_.number_of_reconfigurations()); + // The encoder will have been configured once. + EXPECT_EQ(1, sink_.number_of_reconfigurations()); EXPECT_EQ(codec_width_, fake_encoder_.codec_config().width); EXPECT_EQ(codec_height_, fake_encoder_.codec_config().height); @@ -329,7 +329,7 @@ TEST_F(ViEEncoderTest, FrameResolutionChangeReconfigureEncoder) { sink_.WaitForEncodedFrame(2); EXPECT_EQ(codec_width_, fake_encoder_.codec_config().width); EXPECT_EQ(codec_height_, fake_encoder_.codec_config().height); - EXPECT_EQ(3, sink_.number_of_reconfigurations()); + EXPECT_EQ(2, sink_.number_of_reconfigurations()); vie_encoder_->Stop(); }