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

Revert of Reland Change ViEEncoder to not reconfigure the encoder until the video resolution is known. (patchset #2 id:20001 of https://codereview.webrtc.org/2455963004/ )

Browse Source 
Reason for revert:
It breaks webrtc.fyi bots, see
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Mac%20Tester/builds/35251.

Original issue's description:
> Reland Change ViEEncoder to not reconfigure the encoder until the video resolution is known.
>
> Patchset 1 contain the originally reviewed cl in https://codereview.webrtc.org/2455063002/
> TBR=stefan@webrtc.org, pbos@webrtc.org, skvlad@webrtc.org
>
> BUG=webrtc:6371 b/32285861
>
> Committed: https://crrev.com/5f1b05129e4770c98429164761779d99a410e7c8
> Cr-Commit-Position: refs/heads/master@{#14823}

TBR=pbos@webrtc.org,skvlad@webrtc.org,stefan@webrtc.org,perkj@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6371 b/32285861

Review-Url: https://codereview.webrtc.org/2457203002
Cr-Commit-Position: refs/heads/master@{#14829}
This commit is contained in:
emircan
2016-10-28 14:06:29 -07:00
committed by Commit bot
parent e7fc7d5c02
commit 05a55b500d
6 changed files with 65 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
webrtc/call/call_perf_tests.cc
View File

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

49
webrtc/video/video_send_stream_tests.cc
View File

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

19
webrtc/video/vie_encoder.cc
View File

@ -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>(
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>(
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) {

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;
VideoRotation rotation;
webrtc::VideoRotation rotation;
bool is_texture;
};

16
webrtc/video/vie_encoder_unittest.cc
View File

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