diff --git a/webrtc/call/call_perf_tests.cc b/webrtc/call/call_perf_tests.cc index 13cec9c6e8..5a60acee5e 100644 --- a/webrtc/call/call_perf_tests.cc +++ b/webrtc/call/call_perf_tests.cc @@ -648,16 +648,14 @@ TEST_F(CallPerfTest, KeepsHighBitrateWhenReconfiguringSender) { size_t max_payload_size) override { ++encoder_inits_; if (encoder_inits_ == 1) { - // First time initialization. Frame size is known. - // |expected_bitrate| is affected by bandwidth estimation before the - // first frame arrives to the encoder. - uint32_t expected_bitrate = - last_set_bitrate_ > 0 ? last_set_bitrate_ : kInitialBitrateKbps; - EXPECT_EQ(expected_bitrate, config->startBitrate) + // First time initialization. Frame size is not known. + EXPECT_EQ(kInitialBitrateKbps, config->startBitrate) << "Encoder not initialized at expected bitrate."; + } else if (encoder_inits_ == 2) { + // First time initialization. Frame size is known. EXPECT_EQ(kDefaultWidth, config->width); EXPECT_EQ(kDefaultHeight, config->height); - } else if (encoder_inits_ == 2) { + } else if (encoder_inits_ == 3) { EXPECT_EQ(2 * kDefaultWidth, config->width); EXPECT_EQ(2 * kDefaultHeight, config->height); EXPECT_GE(last_set_bitrate_, kReconfigureThresholdKbps); @@ -673,7 +671,7 @@ TEST_F(CallPerfTest, KeepsHighBitrateWhenReconfiguringSender) { int32_t SetRates(uint32_t new_target_bitrate_kbps, uint32_t framerate) override { last_set_bitrate_ = new_target_bitrate_kbps; - if (encoder_inits_ == 1 && + if (encoder_inits_ == 2 && new_target_bitrate_kbps > kReconfigureThresholdKbps) { time_to_reconfigure_.Set(); } @@ -692,7 +690,6 @@ TEST_F(CallPerfTest, KeepsHighBitrateWhenReconfiguringSender) { std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { send_config->encoder_settings.encoder = this; - encoder_config->max_bitrate_bps = 2 * kReconfigureThresholdKbps * 1000; encoder_config->video_stream_factory = new rtc::RefCountedObject(); diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 5d7b2b3155..81bb1d3d9b 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -344,20 +344,24 @@ TEST_F(WebRtcVideoEngine2Test, UseExternalFactoryForVp8WhenSupported) { EXPECT_TRUE( channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); - EXPECT_EQ(0, encoder_factory.GetNumCreatedEncoders()); + ASSERT_TRUE(encoder_factory.WaitForCreatedVideoEncoders(1)); + ASSERT_EQ(1u, encoder_factory.encoders().size()); 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 allocate the encoder. - ASSERT_TRUE(encoder_factory.WaitForCreatedVideoEncoders(1)); + // 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)); EXPECT_TRUE_WAIT(encoder_factory.encoders()[0]->GetNumEncodedFrames() > 0, kTimeout); int num_created_encoders = encoder_factory.GetNumCreatedEncoders(); - EXPECT_EQ(num_created_encoders, 1); + EXPECT_EQ(num_created_encoders, 2); // Setting codecs of the same type should not reallocate any encoders // (expecting a no-op). @@ -665,14 +669,6 @@ 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 2663516b6b..9d00e007de 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -1785,9 +1785,12 @@ TEST_F(VideoSendStreamTest, EncoderIsProperlyInitializedAndDestroyed) { void PerformTest() override { EXPECT_TRUE(Wait()) << "Timed out while waiting for Encode."; - EXPECT_EQ(0u, num_releases()); + // 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()); stream_->ReconfigureVideoEncoder(std::move(encoder_config_)); - EXPECT_EQ(0u, num_releases()); + EXPECT_EQ(1u, num_releases()); stream_->Stop(); // Encoder should not be released before destroying the VideoSendStream. EXPECT_FALSE(IsReleased()); @@ -1809,7 +1812,7 @@ TEST_F(VideoSendStreamTest, EncoderIsProperlyInitializedAndDestroyed) { RunBaseTest(&test_encoder); EXPECT_TRUE(test_encoder.IsReleased()); - EXPECT_EQ(1u, test_encoder.num_releases()); + EXPECT_EQ(2u, test_encoder.num_releases()); } TEST_F(VideoSendStreamTest, EncoderSetupPropagatesCommonEncoderConfigValues) { @@ -1841,7 +1844,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_ == 0) { + if (num_initializations_ < 2) { // Verify default values. EXPECT_EQ(kRealtimeVideo, config->mode); } else { @@ -1849,18 +1852,23 @@ TEST_F(VideoSendStreamTest, EncoderSetupPropagatesCommonEncoderConfigValues) { EXPECT_EQ(kScreensharing, config->mode); } ++num_initializations_; - init_encode_event_.Set(); + 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(); + } return FakeEncoder::InitEncode(config, number_of_cores, max_payload_size); } void PerformTest() override { EXPECT_TRUE(init_encode_event_.Wait(kDefaultTimeoutMs)); - EXPECT_EQ(1u, num_initializations_) << "VideoEncoder not initialized."; + EXPECT_EQ(2u, 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(2u, num_initializations_) + EXPECT_EQ(3u, num_initializations_) << "ReconfigureVideoEncoder did not reinitialize the encoder with " "new encoder settings."; } @@ -1937,7 +1945,12 @@ class VideoCodecConfigObserver : public test::SendTest, EXPECT_EQ(video_codec_type_, config->codecType); VerifyCodecSpecifics(*config); ++num_initializations_; - init_encode_event_.Set(); + 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(); + } return FakeEncoder::InitEncode(config, number_of_cores, max_payload_size); } @@ -1948,14 +1961,14 @@ class VideoCodecConfigObserver : public test::SendTest, void PerformTest() override { EXPECT_TRUE( init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)); - ASSERT_EQ(1u, num_initializations_) << "VideoEncoder not initialized."; + ASSERT_EQ(2u, 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(2u, num_initializations_) + EXPECT_EQ(3u, num_initializations_) << "ReconfigureVideoEncoder did not reinitialize the encoder with " "new encoder settings."; } @@ -2202,7 +2215,8 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { size_t maxPayloadSize) override { EXPECT_GE(codecSettings->startBitrate, codecSettings->minBitrate); EXPECT_LE(codecSettings->startBitrate, codecSettings->maxBitrate); - if (num_initializations_ == 0) { + // First reinitialization happens due to that the frame size is updated. + if (num_initializations_ == 0 || num_initializations_ == 1) { EXPECT_EQ(static_cast(kMinBitrateKbps), codecSettings->minBitrate); EXPECT_EQ(static_cast(kStartBitrateKbps), @@ -2210,14 +2224,14 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { EXPECT_EQ(static_cast(kMaxBitrateKbps), codecSettings->maxBitrate); observation_complete_.Set(); - } else if (num_initializations_ == 1) { + } else if (num_initializations_ == 2) { 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_ == 2) { + } else if (num_initializations_ == 3) { EXPECT_EQ(static_cast(kIncreasedMaxBitrateKbps), codecSettings->maxBitrate); // The start bitrate will be whatever the rate BitRateController @@ -2225,8 +2239,9 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { // bitrate. } ++num_initializations_; - init_encode_event_.Set(); - + if (num_initializations_ > 1) { + init_encode_event_.Set(); + } return FakeEncoder::InitEncode(codecSettings, numberOfCores, maxPayloadSize); } @@ -2319,7 +2334,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { send_stream_->ReconfigureVideoEncoder(encoder_config_.Copy()); ASSERT_TRUE( init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)); - EXPECT_EQ(2, num_initializations_) + EXPECT_EQ(3, num_initializations_) << "Encoder should have been reconfigured with the new value."; WaitForSetRates(kLowerMaxBitrateKbps); @@ -2327,7 +2342,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(4, 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 f5bde2e359..a3fd98dd6f 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -410,14 +410,15 @@ void ViEEncoder::ConfigureEncoderOnTaskQueue(VideoEncoderConfig config, pending_encoder_reconfiguration_ = true; // Reconfigure the encoder now if the encoder has an internal source or - // 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)); + // 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)); + } ReconfigureEncoder(); } } @@ -541,7 +542,7 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame, if (pre_encode_callback_) pre_encode_callback_->OnFrame(video_frame); - if (!last_frame_info_ || video_frame.width() != last_frame_info_->width || + if (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 33caec42f4..68f043e1c5 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; - VideoRotation rotation; + webrtc::VideoRotation rotation; bool is_texture; }; diff --git a/webrtc/video/vie_encoder_unittest.cc b/webrtc/video/vie_encoder_unittest.cc index 78eb62f99f..5382fbf8eb 100644 --- a/webrtc/video/vie_encoder_unittest.cc +++ b/webrtc/video/vie_encoder_unittest.cc @@ -286,14 +286,13 @@ 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 once when the first frame is - // received. - EXPECT_EQ(1, sink_.number_of_reconfigurations()); + // 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()); VideoEncoderConfig video_encoder_config; test::FillEncoderConfiguration(1, &video_encoder_config); @@ -303,7 +302,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(2, sink_.number_of_reconfigurations()); + EXPECT_EQ(3, sink_.number_of_reconfigurations()); EXPECT_EQ(9999, sink_.last_min_transmit_bitrate()); vie_encoder_->Stop(); @@ -316,8 +315,9 @@ 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 once. - EXPECT_EQ(1, sink_.number_of_reconfigurations()); + // 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()); 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(2, sink_.number_of_reconfigurations()); + EXPECT_EQ(3, sink_.number_of_reconfigurations()); vie_encoder_->Stop(); }