zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reland Change ViEEncoder to not reconfigure the encoder until the video resolution is known.

Browse Source 
This is the second reland.  Patchset 1 contains the reverted cl.
Patchset 2 revert the change to initialize the encoder with resolution 1*1pixels if an internal source is used.
This is to to fix the problem reported in https://codereview.webrtc.org/2457203002/ https://build.chromium.org/p/chromium.webrtc.fyi/builders/Mac%20Tester/builds/35251 remoting.
Fix has been verified to work in Chrome.
This reverts commit 05a55b500d83e4212d4e54f0fecf13097e782ffa.

BUG=webrtc:6371 b/32285861
TBR=pbos@webrtc.org, skvlad@webrtc.org, stefan@webrtc.org

Review URL: https://codereview.webrtc.org/2458363002 .

Cr-Commit-Position: refs/heads/master@{#14833}
This commit is contained in:
Per
2016-10-30 21:37:57 +01:00
parent b68d655f36
commit 21d45d2ab6
6 changed files with 56 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
webrtc/call/call_perf_tests.cc
View File

@ -648,14 +648,16 @@ TEST_F(CallPerfTest, KeepsHighBitrateWhenReconfiguringSender) {
size_t max_payload_size) override {
++encoder_inits_;
if (encoder_inits_ == 1) {
// 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.
// |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)
<< "Encoder not initialized at expected bitrate.";
EXPECT_EQ(kDefaultWidth, config->width);
EXPECT_EQ(kDefaultHeight, config->height);
} else if (encoder_inits_ == 3) {
} else if (encoder_inits_ == 2) {
EXPECT_EQ(2 * kDefaultWidth, config->width);
EXPECT_EQ(2 * kDefaultHeight, config->height);
EXPECT_GE(last_set_bitrate_, kReconfigureThresholdKbps);
@ -671,7 +673,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_ == 2 &&
if (encoder_inits_ == 1 &&
new_target_bitrate_kbps > kReconfigureThresholdKbps) {
time_to_reconfigure_.Set();
}
@ -690,6 +692,7 @@ TEST_F(CallPerfTest, KeepsHighBitrateWhenReconfiguringSender) {
std::vector<VideoReceiveStream::Config>* 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<VideoStreamFactory>();

20
webrtc/media/engine/webrtcvideoengine2_unittest.cc
View File

@ -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);

49
webrtc/video/video_send_stream_tests.cc
View File

@ -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<unsigned int>(kMinBitrateKbps),
codecSettings->minBitrate);
EXPECT_EQ(static_cast<unsigned int>(kStartBitrateKbps),
@ -2224,14 +2210,14 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) {
EXPECT_EQ(static_cast<unsigned int>(kMaxBitrateKbps),
codecSettings->maxBitrate);
observation_complete_.Set();
} else if (num_initializations_ == 2) {
} else if (num_initializations_ == 1) {
EXPECT_EQ(static_cast<unsigned int>(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<unsigned int>(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.

19
webrtc/video/vie_encoder.cc
View File

@ -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>(
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>(
VideoFrameInfo(176, 144, kVideoRotation_0, false));
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) {

2
webrtc/video/vie_encoder.h
View File

@ -114,7 +114,7 @@ class ViEEncoder : public rtc::VideoSinkInterface<VideoFrame>,
is_texture(is_texture) {}
int width;
int height;
webrtc::VideoRotation rotation;
VideoRotation rotation;
bool is_texture;
};

16
webrtc/video/vie_encoder_unittest.cc
View File

@ -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();
}