Delete nack_enabled flag in encoder configuration.
This is a followup to cl https://webrtc-review.googlesource.com/71380,
which reworked the way encoder resilience is done, and made the
nack_enabled flag unused.
Bug: webrtc:8830
Change-Id: I3de2508c97bc71e01c8f2232d16cd1f33e57fe4a
Reviewed-on: https://webrtc-review.googlesource.com/69986
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23080}diff --git a/modules/video_coding/include/video_codec_initializer.h b/modules/video_coding/include/video_codec_initializer.h
index 0748db0..fa2582f 100644
--- a/modules/video_coding/include/video_codec_initializer.h
+++ b/modules/video_coding/include/video_codec_initializer.h
@@ -34,10 +34,20 @@
static bool SetupCodec(
const VideoEncoderConfig& config,
const std::vector<VideoStream>& streams,
- bool nack_enabled,
VideoCodec* codec,
std::unique_ptr<VideoBitrateAllocator>* bitrate_allocator);
+ // TODO(nisse): Deprecated, delete as soon as downstream projects are
+ // updated.
+ static bool SetupCodec(
+ const VideoEncoderConfig& config,
+ const std::vector<VideoStream>& streams,
+ bool /* nack_enabled */,
+ VideoCodec* codec,
+ std::unique_ptr<VideoBitrateAllocator>* bitrate_allocator) {
+ return SetupCodec(config, streams, codec, bitrate_allocator);
+ }
+
// Create a bitrate allocator for the specified codec.
static std::unique_ptr<VideoBitrateAllocator> CreateBitrateAllocator(
const VideoCodec& codec);
@@ -45,8 +55,7 @@
private:
static VideoCodec VideoEncoderConfigToVideoCodec(
const VideoEncoderConfig& config,
- const std::vector<VideoStream>& streams,
- bool nack_enabled);
+ const std::vector<VideoStream>& streams);
};
} // namespace webrtc
diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc
index 34fcb8d..192f80f 100644
--- a/modules/video_coding/video_codec_initializer.cc
+++ b/modules/video_coding/video_codec_initializer.cc
@@ -29,13 +29,12 @@
bool VideoCodecInitializer::SetupCodec(
const VideoEncoderConfig& config,
const std::vector<VideoStream>& streams,
- bool nack_enabled,
VideoCodec* codec,
std::unique_ptr<VideoBitrateAllocator>* bitrate_allocator) {
if (config.codec_type == kVideoCodecMultiplex) {
VideoEncoderConfig associated_config = config.Copy();
associated_config.codec_type = kVideoCodecVP9;
- if (!SetupCodec(associated_config, streams, nack_enabled, codec,
+ if (!SetupCodec(associated_config, streams, codec,
bitrate_allocator)) {
RTC_LOG(LS_ERROR) << "Failed to create stereo encoder configuration.";
return false;
@@ -45,7 +44,7 @@
}
*codec =
- VideoEncoderConfigToVideoCodec(config, streams, nack_enabled);
+ VideoEncoderConfigToVideoCodec(config, streams);
*bitrate_allocator = CreateBitrateAllocator(*codec);
return true;
@@ -73,8 +72,7 @@
// TODO(sprang): Split this up and separate the codec specific parts.
VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec(
const VideoEncoderConfig& config,
- const std::vector<VideoStream>& streams,
- bool nack_enabled) {
+ const std::vector<VideoStream>& streams) {
static const int kEncoderMinBitrateKbps = 30;
RTC_DCHECK(!streams.empty());
RTC_DCHECK_GE(config.min_transmit_bitrate_bps, 0);
diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc
index 2ef1159..104b149 100644
--- a/modules/video_coding/video_codec_initializer_unittest.cc
+++ b/modules/video_coding/video_codec_initializer_unittest.cc
@@ -39,7 +39,7 @@
// TODO(sprang): Extend coverage to handle the rest of the codec initializer.
class VideoCodecInitializerTest : public ::testing::Test {
public:
- VideoCodecInitializerTest() : nack_enabled_(false) {}
+ VideoCodecInitializerTest() {}
virtual ~VideoCodecInitializerTest() {}
protected:
@@ -76,7 +76,7 @@
codec_out_ = VideoCodec();
bitrate_allocator_out_.reset();
temporal_layers_.clear();
- if (!VideoCodecInitializer::SetupCodec(config_, streams_, nack_enabled_,
+ if (!VideoCodecInitializer::SetupCodec(config_, streams_,
&codec_out_,
&bitrate_allocator_out_)) {
return false;
@@ -122,7 +122,6 @@
// Input settings.
VideoEncoderConfig config_;
std::vector<VideoStream> streams_;
- bool nack_enabled_;
// Output.
VideoCodec codec_out_;
diff --git a/video/test/mock_video_stream_encoder.h b/video/test/mock_video_stream_encoder.h
index 55f6063..684962d 100644
--- a/video/test/mock_video_stream_encoder.h
+++ b/video/test/mock_video_stream_encoder.h
@@ -28,14 +28,13 @@
MOCK_METHOD1(SetBitrateObserver, void(VideoBitrateAllocationObserver*));
MOCK_METHOD0(Stop, void());
- MOCK_METHOD3(MockedConfigureEncoder,
- void(const VideoEncoderConfig&, size_t, bool));
+ MOCK_METHOD2(MockedConfigureEncoder,
+ void(const VideoEncoderConfig&, size_t));
// gtest generates implicit copy which is not allowed on VideoEncoderConfig,
// so we can't mock ConfigureEncoder directly.
void ConfigureEncoder(VideoEncoderConfig config,
- size_t max_data_payload_length,
- bool nack_enabled) {
- MockedConfigureEncoder(config, max_data_payload_length, nack_enabled);
+ size_t max_data_payload_length) {
+ MockedConfigureEncoder(config, max_data_payload_length);
}
};
diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index 1dfb693..88362f6 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -169,8 +169,7 @@
RTC_DCHECK(content_type_ == config.content_type);
video_stream_encoder_->ConfigureEncoder(
std::move(config),
- config_.rtp.max_packet_size - CalculateMaxHeaderSize(config_.rtp),
- config_.rtp.nack.rtp_history_ms > 0);
+ config_.rtp.max_packet_size - CalculateMaxHeaderSize(config_.rtp));
}
VideoSendStream::Stats VideoSendStream::GetStats() {
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 6135064..1b7f2f7 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -344,7 +344,6 @@
pending_encoder_creation_(false),
encoder_start_bitrate_bps_(0),
max_data_payload_length_(0),
- nack_enabled_(false),
last_observed_bitrate_bps_(0),
encoder_paused_and_dropped_frame_(false),
clock_(Clock::GetRealTimeClock()),
@@ -448,34 +447,30 @@
}
void VideoStreamEncoder::ConfigureEncoder(VideoEncoderConfig config,
- size_t max_data_payload_length,
- bool nack_enabled) {
+ size_t max_data_payload_length) {
// TODO(srte): This struct should be replaced by a lambda with move capture
// when C++14 lambda is allowed.
struct ConfigureEncoderTask {
void operator()() {
encoder->ConfigureEncoderOnTaskQueue(
- std::move(config), max_data_payload_length, nack_enabled);
+ std::move(config), max_data_payload_length);
}
VideoStreamEncoder* encoder;
VideoEncoderConfig config;
size_t max_data_payload_length;
- bool nack_enabled;
};
encoder_queue_.PostTask(ConfigureEncoderTask{
- this, std::move(config), max_data_payload_length, nack_enabled});
+ this, std::move(config), max_data_payload_length});
}
void VideoStreamEncoder::ConfigureEncoderOnTaskQueue(
VideoEncoderConfig config,
- size_t max_data_payload_length,
- bool nack_enabled) {
+ size_t max_data_payload_length) {
RTC_DCHECK_RUN_ON(&encoder_queue_);
RTC_DCHECK(sink_);
RTC_LOG(LS_INFO) << "ConfigureEncoder requested.";
max_data_payload_length_ = max_data_payload_length;
- nack_enabled_ = nack_enabled;
pending_encoder_creation_ =
(!encoder_ || encoder_config_.video_format != config.video_format);
encoder_config_ = std::move(config);
@@ -520,7 +515,7 @@
VideoCodec codec;
if (!VideoCodecInitializer::SetupCodec(
- encoder_config_, streams, nack_enabled_, &codec, &rate_allocator_)) {
+ encoder_config_, streams, &codec, &rate_allocator_)) {
RTC_LOG(LS_ERROR) << "Failed to create encoder configuration.";
}
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h
index e2c7d5d..34cfc95 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -63,8 +63,7 @@
virtual void SetBitrateObserver(
VideoBitrateAllocationObserver* bitrate_observer) = 0;
virtual void ConfigureEncoder(VideoEncoderConfig config,
- size_t max_data_payload_length,
- bool nack_enabled) = 0;
+ size_t max_data_payload_length) = 0;
virtual void Stop() = 0;
protected:
@@ -115,8 +114,7 @@
VideoBitrateAllocationObserver* bitrate_observer) override;
void ConfigureEncoder(VideoEncoderConfig config,
- size_t max_data_payload_length,
- bool nack_enabled) override;
+ size_t max_data_payload_length) override;
// Permanently stop encoding. After this method has returned, it is
// guaranteed that no encoded frames will be delivered to the sink.
@@ -156,8 +154,7 @@
};
void ConfigureEncoderOnTaskQueue(VideoEncoderConfig config,
- size_t max_data_payload_length,
- bool nack_enabled);
+ size_t max_data_payload_length);
void ReconfigureEncoder() RTC_RUN_ON(&encoder_queue_);
void ConfigureQualityScaler();
@@ -276,7 +273,6 @@
int crop_height_ RTC_GUARDED_BY(&encoder_queue_);
uint32_t encoder_start_bitrate_bps_ RTC_GUARDED_BY(&encoder_queue_);
size_t max_data_payload_length_ RTC_GUARDED_BY(&encoder_queue_);
- bool nack_enabled_ RTC_GUARDED_BY(&encoder_queue_);
uint32_t last_observed_bitrate_bps_ RTC_GUARDED_BY(&encoder_queue_);
bool encoder_paused_and_dropped_frame_ RTC_GUARDED_BY(&encoder_queue_);
Clock* const clock_;
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index ba21cf9..ab2d8af 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -301,11 +301,10 @@
max_framerate_ = streams[0].max_framerate;
fake_clock_.SetTimeMicros(1234);
- ConfigureEncoder(std::move(video_encoder_config), true /* nack_enabled */);
+ ConfigureEncoder(std::move(video_encoder_config));
}
- void ConfigureEncoder(VideoEncoderConfig video_encoder_config,
- bool nack_enabled) {
+ void ConfigureEncoder(VideoEncoderConfig video_encoder_config) {
if (video_stream_encoder_)
video_stream_encoder_->Stop();
video_stream_encoder_.reset(new VideoStreamEncoderUnderTest(
@@ -316,7 +315,7 @@
VideoSendStream::DegradationPreference::kMaintainFramerate);
video_stream_encoder_->SetStartBitrate(kTargetBitrateBps);
video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
- kMaxPayloadLength, nack_enabled);
+ kMaxPayloadLength);
video_stream_encoder_->WaitUntilTaskQueueIsIdle();
}
@@ -324,7 +323,6 @@
size_t num_streams,
size_t num_temporal_layers,
unsigned char num_spatial_layers,
- bool nack_enabled,
bool screenshare) {
video_send_config_.rtp.payload_name = payload_name;
@@ -345,7 +343,7 @@
new rtc::RefCountedObject<
VideoEncoderConfig::Vp9EncoderSpecificSettings>(vp9_settings);
}
- ConfigureEncoder(std::move(video_encoder_config), nack_enabled);
+ ConfigureEncoder(std::move(video_encoder_config));
}
VideoFrame CreateFrame(int64_t ntp_time_ms,
@@ -800,8 +798,7 @@
test::FillEncoderConfiguration(kVideoCodecVP8, 1, &video_encoder_config);
video_encoder_config.min_transmit_bitrate_bps = 9999;
video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
- kMaxPayloadLength,
- true /* nack_enabled */);
+ kMaxPayloadLength);
// Capture a frame and wait for it to synchronize with the encoder thread.
video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr));
@@ -1278,8 +1275,7 @@
// Make format different, to force recreation of encoder.
video_encoder_config.video_format.parameters["foo"] = "foo";
video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
- kMaxPayloadLength,
- true /* nack_enabled */);
+ kMaxPayloadLength);
video_source_.IncomingCapturedFrame(CreateFrame(4, kWidth, kHeight));
WaitForEncodedFrame(4);
@@ -2136,7 +2132,7 @@
video_encoder_config.video_stream_factory =
new rtc::RefCountedObject<VideoStreamFactory>(1, kFramerate);
video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
- kMaxPayloadLength, false);
+ kMaxPayloadLength);
video_stream_encoder_->WaitUntilTaskQueueIsIdle();
// Detector should be updated with fps limit from codec config.
@@ -2189,7 +2185,7 @@
new rtc::RefCountedObject<VideoStreamFactory>(1, kLowFramerate);
source.IncomingCapturedFrame(CreateFrame(1, kFrameWidth, kFrameHeight));
video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
- kMaxPayloadLength, false);
+ kMaxPayloadLength);
video_stream_encoder_->WaitUntilTaskQueueIsIdle();
EXPECT_EQ(
@@ -2212,7 +2208,7 @@
new rtc::RefCountedObject<VideoStreamFactory>(1, kHighFramerate);
source.IncomingCapturedFrame(CreateFrame(1, kFrameWidth, kFrameHeight));
video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
- kMaxPayloadLength, false);
+ kMaxPayloadLength);
video_stream_encoder_->WaitUntilTaskQueueIsIdle();
EXPECT_EQ(
@@ -2252,7 +2248,7 @@
new rtc::RefCountedObject<VideoStreamFactory>(1, kFramerate);
source.IncomingCapturedFrame(CreateFrame(1, kFrameWidth, kFrameHeight));
video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
- kMaxPayloadLength, false);
+ kMaxPayloadLength);
video_stream_encoder_->WaitUntilTaskQueueIsIdle();
EXPECT_EQ(
@@ -2360,8 +2356,7 @@
// Make format different, to force recreation of encoder.
video_encoder_config.video_format.parameters["foo"] = "foo";
video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
- kMaxPayloadLength,
- true /* nack_enabled */);
+ kMaxPayloadLength);
video_stream_encoder_->OnBitrateUpdated(kLowTargetBitrateBps, 0, 0);
// Force quality scaler reconfiguration by resetting the source.
@@ -2441,7 +2436,7 @@
TEST_F(VideoStreamEncoderTest, FailingInitEncodeDoesntCauseCrash) {
fake_encoder_.ForceInitEncodeFailure(true);
video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
- ResetEncoder("VP8", 2, 1, 1, true, false);
+ ResetEncoder("VP8", 2, 1, 1, false);
const int kFrameWidth = 1280;
const int kFrameHeight = 720;
video_source_.IncomingCapturedFrame(
@@ -2587,7 +2582,7 @@
// Reconfigure encoder with two temporal layers and screensharing, which will
// disable frame dropping and make testing easier.
- ResetEncoder("VP8", 1, 2, 1, true, true);
+ ResetEncoder("VP8", 1, 2, 1, true);
video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
video_stream_encoder_->SetSource(
@@ -3053,7 +3048,7 @@
video_encoder_config.video_stream_factory =
new rtc::RefCountedObject<CroppingVideoStreamFactory>(1, kFramerate);
video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
- kMaxPayloadLength, false);
+ kMaxPayloadLength);
video_stream_encoder_->WaitUntilTaskQueueIsIdle();
video_source_.set_adaptation_enabled(true);