From 4eb03c76fa2320534d669fda2aabf800e7a6f579 Mon Sep 17 00:00:00 2001 From: asapersson Date: Fri, 2 Dec 2016 08:57:55 -0800 Subject: [PATCH] Turn off error resilience for VP9 if no spatial or temporal layers are configured and NACK is enabled. Error resilience is currently always enabled for VP9 which reduces quality. BUG=webrtc:6783 Review-Url: https://codereview.webrtc.org/2532053002 Cr-Commit-Position: refs/heads/master@{#15390} --- webrtc/common_types.h | 2 +- webrtc/modules/video_coding/codec_database.cc | 2 +- .../video_coding/codecs/vp9/vp9_impl.cc | 2 +- .../video_coding/video_codec_initializer.cc | 24 +++- webrtc/video/vie_encoder_unittest.cc | 105 +++++++++++++++++- 5 files changed, 122 insertions(+), 13 deletions(-) diff --git a/webrtc/common_types.h b/webrtc/common_types.h index 2988bf9f41..dc7442c6bc 100644 --- a/webrtc/common_types.h +++ b/webrtc/common_types.h @@ -537,7 +537,7 @@ struct VideoCodecVP8 { // VP9 specific. struct VideoCodecVP9 { VideoCodecComplexity complexity; - int resilience; + bool resilienceOn; unsigned char numberOfTemporalLayers; bool denoisingOn; bool frameDroppingOn; diff --git a/webrtc/modules/video_coding/codec_database.cc b/webrtc/modules/video_coding/codec_database.cc index 75e7043509..40e31c6713 100644 --- a/webrtc/modules/video_coding/codec_database.cc +++ b/webrtc/modules/video_coding/codec_database.cc @@ -46,7 +46,7 @@ VideoCodecVP9 VideoEncoder::GetDefaultVp9Settings() { VideoCodecVP9 vp9_settings; memset(&vp9_settings, 0, sizeof(vp9_settings)); - vp9_settings.resilience = 1; + vp9_settings.resilienceOn = true; vp9_settings.numberOfTemporalLayers = 1; vp9_settings.denoisingOn = false; vp9_settings.frameDroppingOn = true; diff --git a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc index bfb5606e2c..f8b6e1bafa 100644 --- a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -298,7 +298,7 @@ int VP9EncoderImpl::InitEncode(const VideoCodec* inst, config_->g_w = codec_.width; config_->g_h = codec_.height; config_->rc_target_bitrate = inst->startBitrate; // in kbit/s - config_->g_error_resilient = 1; + config_->g_error_resilient = inst->VP9().resilienceOn ? 1 : 0; // Setting the time base of the codec. config_->g_timebase.num = 1; config_->g_timebase.den = 90000; diff --git a/webrtc/modules/video_coding/video_codec_initializer.cc b/webrtc/modules/video_coding/video_codec_initializer.cc index dbdede07ad..d7b6131fcc 100644 --- a/webrtc/modules/video_coding/video_codec_initializer.cc +++ b/webrtc/modules/video_coding/video_codec_initializer.cc @@ -21,6 +21,15 @@ #include "webrtc/system_wrappers/include/clock.h" namespace webrtc { +namespace { +bool TemporalLayersConfigured(const std::vector& streams) { + for (const VideoStream& stream : streams) { + if (stream.temporal_layer_thresholds_bps.size() > 0) + return true; + } + return false; +} +} // namespace bool VideoCodecInitializer::SetupCodec( const VideoEncoderConfig& config, @@ -119,12 +128,8 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( *video_codec.VP8() = VideoEncoder::GetDefaultVp8Settings(); video_codec.VP8()->numberOfTemporalLayers = static_cast( streams.back().temporal_layer_thresholds_bps.size() + 1); - bool temporal_layers_configured = false; - for (const VideoStream& stream : streams) { - if (stream.temporal_layer_thresholds_bps.size() > 0) - temporal_layers_configured = true; - } - if (nack_enabled && !temporal_layers_configured) { + + if (nack_enabled && !TemporalLayersConfigured(streams)) { LOG(LS_INFO) << "No temporal layers and nack enabled -> resilience off"; video_codec.VP8()->resilience = kResilienceOff; } @@ -142,6 +147,13 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( } video_codec.VP9()->numberOfTemporalLayers = static_cast( streams.back().temporal_layer_thresholds_bps.size() + 1); + + if (nack_enabled && !TemporalLayersConfigured(streams) && + video_codec.VP9()->numberOfSpatialLayers == 1) { + LOG(LS_INFO) << "No temporal or spatial layers and nack enabled -> " + << "resilience off"; + video_codec.VP9()->resilienceOn = false; + } break; } case kVideoCodecH264: { diff --git a/webrtc/video/vie_encoder_unittest.cc b/webrtc/video/vie_encoder_unittest.cc index 1bb904be7c..da925e5224 100644 --- a/webrtc/video/vie_encoder_unittest.cc +++ b/webrtc/video/vie_encoder_unittest.cc @@ -28,6 +28,7 @@ using ScaleReason = ScalingObserverInterface::ScaleReason; namespace { const size_t kMaxPayloadLength = 1440; const int kTargetBitrateBps = 100000; +const unsigned char kNumSlDummy = 0; class TestBuffer : public webrtc::I420Buffer { public: @@ -141,6 +142,7 @@ class ViEEncoderTest : public ::testing::Test { void ResetEncoder(const std::string& payload_name, size_t num_streams, size_t num_temporal_layers, + unsigned char num_spatial_layers, bool nack_enabled) { video_send_config_.encoder_settings.payload_name = payload_name; @@ -149,6 +151,13 @@ class ViEEncoderTest : public ::testing::Test { video_encoder_config.max_bitrate_bps = 1000000; video_encoder_config.video_stream_factory = new rtc::RefCountedObject(num_temporal_layers); + if (payload_name == "VP9") { + VideoCodecVP9 vp9_settings = VideoEncoder::GetDefaultVp9Settings(); + vp9_settings.numberOfSpatialLayers = num_spatial_layers; + video_encoder_config.encoder_specific_settings = + new rtc::RefCountedObject< + VideoEncoderConfig::Vp9EncoderSpecificSettings>(vp9_settings); + } ConfigureEncoder(std::move(video_encoder_config), nack_enabled); } @@ -433,7 +442,7 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOffFor1S1TLWithNackEnabled) { const bool kNackEnabled = true; const size_t kNumStreams = 1; const size_t kNumTl = 1; - ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled); + ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled); vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); // Capture a frame and wait for it to synchronize with the encoder thread. @@ -453,7 +462,7 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOffFor2S1TlWithNackEnabled) { const bool kNackEnabled = true; const size_t kNumStreams = 2; const size_t kNumTl = 1; - ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled); + ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled); vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); // Capture a frame and wait for it to synchronize with the encoder thread. @@ -473,7 +482,7 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOnFor1S1TLWithNackDisabled) { const bool kNackEnabled = false; const size_t kNumStreams = 1; const size_t kNumTl = 1; - ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled); + ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled); vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); // Capture a frame and wait for it to synchronize with the encoder thread. @@ -493,7 +502,7 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOnFor1S2TlWithNackEnabled) { const bool kNackEnabled = true; const size_t kNumStreams = 1; const size_t kNumTl = 2; - ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled); + ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled); vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); // Capture a frame and wait for it to synchronize with the encoder thread. @@ -509,6 +518,94 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOnFor1S2TlWithNackEnabled) { vie_encoder_->Stop(); } +TEST_F(ViEEncoderTest, Vp9ResilienceIsOffFor1SL1TLWithNackEnabled) { + const bool kNackEnabled = true; + const size_t kNumStreams = 1; + const size_t kNumTl = 1; + const unsigned char kNumSl = 1; + ResetEncoder("VP9", kNumStreams, kNumTl, kNumSl, kNackEnabled); + vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + + // Capture a frame and wait for it to synchronize with the encoder thread. + video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); + sink_.WaitForEncodedFrame(1); + // The encoder have been configured once when the first frame is received. + EXPECT_EQ(1, sink_.number_of_reconfigurations()); + EXPECT_EQ(kVideoCodecVP9, fake_encoder_.codec_config().codecType); + EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams); + EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP9()->numberOfTemporalLayers); + EXPECT_EQ(kNumSl, fake_encoder_.codec_config().VP9()->numberOfSpatialLayers); + // Resilience is off for no spatial and temporal layers with nack on. + EXPECT_FALSE(fake_encoder_.codec_config().VP9()->resilienceOn); + vie_encoder_->Stop(); +} + +TEST_F(ViEEncoderTest, Vp9ResilienceIsOnFor1SL1TLWithNackDisabled) { + const bool kNackEnabled = false; + const size_t kNumStreams = 1; + const size_t kNumTl = 1; + const unsigned char kNumSl = 1; + ResetEncoder("VP9", kNumStreams, kNumTl, kNumSl, kNackEnabled); + vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + + // Capture a frame and wait for it to synchronize with the encoder thread. + video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); + sink_.WaitForEncodedFrame(1); + // The encoder have been configured once when the first frame is received. + EXPECT_EQ(1, sink_.number_of_reconfigurations()); + EXPECT_EQ(kVideoCodecVP9, fake_encoder_.codec_config().codecType); + EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams); + EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP9()->numberOfTemporalLayers); + EXPECT_EQ(kNumSl, fake_encoder_.codec_config().VP9()->numberOfSpatialLayers); + // Resilience is on if nack is off. + EXPECT_TRUE(fake_encoder_.codec_config().VP9()->resilienceOn); + vie_encoder_->Stop(); +} + +TEST_F(ViEEncoderTest, Vp9ResilienceIsOnFor2SL1TLWithNackEnabled) { + const bool kNackEnabled = true; + const size_t kNumStreams = 1; + const size_t kNumTl = 1; + const unsigned char kNumSl = 2; + ResetEncoder("VP9", kNumStreams, kNumTl, kNumSl, kNackEnabled); + vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + + // Capture a frame and wait for it to synchronize with the encoder thread. + video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); + sink_.WaitForEncodedFrame(1); + // The encoder have been configured once when the first frame is received. + EXPECT_EQ(1, sink_.number_of_reconfigurations()); + EXPECT_EQ(kVideoCodecVP9, fake_encoder_.codec_config().codecType); + EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams); + EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP9()->numberOfTemporalLayers); + EXPECT_EQ(kNumSl, fake_encoder_.codec_config().VP9()->numberOfSpatialLayers); + // Resilience is on for spatial layers. + EXPECT_TRUE(fake_encoder_.codec_config().VP9()->resilienceOn); + vie_encoder_->Stop(); +} + +TEST_F(ViEEncoderTest, Vp9ResilienceIsOnFor1SL2TLWithNackEnabled) { + const bool kNackEnabled = true; + const size_t kNumStreams = 1; + const size_t kNumTl = 2; + const unsigned char kNumSl = 1; + ResetEncoder("VP9", kNumStreams, kNumTl, kNumSl, kNackEnabled); + vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + + // Capture a frame and wait for it to synchronize with the encoder thread. + video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); + sink_.WaitForEncodedFrame(1); + // The encoder have been configured once when the first frame is received. + EXPECT_EQ(1, sink_.number_of_reconfigurations()); + EXPECT_EQ(kVideoCodecVP9, fake_encoder_.codec_config().codecType); + EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams); + EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP9()->numberOfTemporalLayers); + EXPECT_EQ(kNumSl, fake_encoder_.codec_config().VP9()->numberOfSpatialLayers); + // Resilience is on for temporal layers. + EXPECT_TRUE(fake_encoder_.codec_config().VP9()->resilienceOn); + vie_encoder_->Stop(); +} + TEST_F(ViEEncoderTest, SwitchSourceDeregisterEncoderAsSink) { EXPECT_TRUE(video_source_.has_sinks()); test::FrameForwarder new_video_source;