diff --git a/modules/video_coding/include/video_codec_initializer.h b/modules/video_coding/include/video_codec_initializer.h index 0748db07ea..fa2582fa27 100644 --- a/modules/video_coding/include/video_codec_initializer.h +++ b/modules/video_coding/include/video_codec_initializer.h @@ -34,10 +34,20 @@ class VideoCodecInitializer { static bool SetupCodec( const VideoEncoderConfig& config, const std::vector& streams, - bool nack_enabled, VideoCodec* codec, std::unique_ptr* bitrate_allocator); + // TODO(nisse): Deprecated, delete as soon as downstream projects are + // updated. + static bool SetupCodec( + const VideoEncoderConfig& config, + const std::vector& streams, + bool /* nack_enabled */, + VideoCodec* codec, + std::unique_ptr* bitrate_allocator) { + return SetupCodec(config, streams, codec, bitrate_allocator); + } + // Create a bitrate allocator for the specified codec. static std::unique_ptr CreateBitrateAllocator( const VideoCodec& codec); @@ -45,8 +55,7 @@ class VideoCodecInitializer { private: static VideoCodec VideoEncoderConfigToVideoCodec( const VideoEncoderConfig& config, - const std::vector& streams, - bool nack_enabled); + const std::vector& streams); }; } // namespace webrtc diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index 34fcb8dc5c..192f80fc3e 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -29,13 +29,12 @@ namespace webrtc { bool VideoCodecInitializer::SetupCodec( const VideoEncoderConfig& config, const std::vector& streams, - bool nack_enabled, VideoCodec* codec, std::unique_ptr* 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 @@ bool VideoCodecInitializer::SetupCodec( } *codec = - VideoEncoderConfigToVideoCodec(config, streams, nack_enabled); + VideoEncoderConfigToVideoCodec(config, streams); *bitrate_allocator = CreateBitrateAllocator(*codec); return true; @@ -73,8 +72,7 @@ VideoCodecInitializer::CreateBitrateAllocator(const VideoCodec& codec) { // TODO(sprang): Split this up and separate the codec specific parts. VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( const VideoEncoderConfig& config, - const std::vector& streams, - bool nack_enabled) { + const std::vector& 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 2ef1159ff2..104b149c00 100644 --- a/modules/video_coding/video_codec_initializer_unittest.cc +++ b/modules/video_coding/video_codec_initializer_unittest.cc @@ -39,7 +39,7 @@ static const uint32_t kHighScreenshareTl1Bps = 1200000; // 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 @@ class VideoCodecInitializerTest : public ::testing::Test { 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 @@ class VideoCodecInitializerTest : public ::testing::Test { // Input settings. VideoEncoderConfig config_; std::vector 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 55f6063f23..684962df17 100644 --- a/video/test/mock_video_stream_encoder.h +++ b/video/test/mock_video_stream_encoder.h @@ -28,14 +28,13 @@ class MockVideoStreamEncoder : public VideoStreamEncoderInterface { 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 1dfb6933b4..88362f6288 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -169,8 +169,7 @@ void VideoSendStream::ReconfigureVideoEncoder(VideoEncoderConfig config) { 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 6135064a37..1b7f2f782c 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -344,7 +344,6 @@ VideoStreamEncoder::VideoStreamEncoder( 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::SetStartBitrate(int start_bitrate_bps) { } 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 @@ void VideoStreamEncoder::ReconfigureEncoder() { 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 e2c7d5da44..34cfc95ffc 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -63,8 +63,7 @@ class VideoStreamEncoderInterface : public rtc::VideoSinkInterface { 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 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, 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 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, }; 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 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, 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 ba21cf9959..ab2d8af99c 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -301,11 +301,10 @@ class VideoStreamEncoderTest : public ::testing::Test { 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 @@ class VideoStreamEncoderTest : public ::testing::Test { 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 @@ class VideoStreamEncoderTest : public ::testing::Test { 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 @@ class VideoStreamEncoderTest : public ::testing::Test { 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_F(VideoStreamEncoderTest, 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 @@ TEST_F(VideoStreamEncoderTest, // 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 @@ TEST_F(VideoStreamEncoderTest, OveruseDetectorUpdatedOnReconfigureAndAdaption) { video_encoder_config.video_stream_factory = new rtc::RefCountedObject(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 @@ TEST_F(VideoStreamEncoderTest, new rtc::RefCountedObject(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 @@ TEST_F(VideoStreamEncoderTest, new rtc::RefCountedObject(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 @@ TEST_F(VideoStreamEncoderTest, new rtc::RefCountedObject(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 @@ TEST_F(VideoStreamEncoderTest, InitialFrameDropOffWhenEncoderDisabledScaling) { // 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, 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 @@ TEST_F(VideoStreamEncoderTest, DoesntAdaptDownPastMinFramerate) { // 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 @@ TEST_F(VideoStreamEncoderTest, AcceptsFullHdAdaptedDownSimulcastFrames) { video_encoder_config.video_stream_factory = new rtc::RefCountedObject(1, kFramerate); video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config), - kMaxPayloadLength, false); + kMaxPayloadLength); video_stream_encoder_->WaitUntilTaskQueueIsIdle(); video_source_.set_adaptation_enabled(true);