Revert of Turn off error resilience for VP9 if no spatial or temporal layers are configured and NACK is enabl… (patchset #1 id:40001 of https://codereview.webrtc.org/2532053002/ )
Reason for revert:
Increase in encode time larger than expected.
Original issue's description:
> 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
>
> Committed: https://crrev.com/4eb03c76fa2320534d669fda2aabf800e7a6f579
> Cr-Commit-Position: refs/heads/master@{#15390}
TBR=stefan@webrtc.org,marpan@webrtc.org,mflodman@webrtc.org,marpan@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:6783
Review-Url: https://codereview.webrtc.org/2554403006
Cr-Original-Commit-Position: refs/heads/master@{#15501}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: a90799d5fb8dcb56595c5b6708eea41f3cc8bf5e
diff --git a/common_types.h b/common_types.h
index dc7442c..2988bf9 100644
--- a/common_types.h
+++ b/common_types.h
@@ -537,7 +537,7 @@
// VP9 specific.
struct VideoCodecVP9 {
VideoCodecComplexity complexity;
- bool resilienceOn;
+ int resilience;
unsigned char numberOfTemporalLayers;
bool denoisingOn;
bool frameDroppingOn;
diff --git a/modules/video_coding/codec_database.cc b/modules/video_coding/codec_database.cc
index 40e31c6..75e7043 100644
--- a/modules/video_coding/codec_database.cc
+++ b/modules/video_coding/codec_database.cc
@@ -46,7 +46,7 @@
VideoCodecVP9 vp9_settings;
memset(&vp9_settings, 0, sizeof(vp9_settings));
- vp9_settings.resilienceOn = true;
+ vp9_settings.resilience = 1;
vp9_settings.numberOfTemporalLayers = 1;
vp9_settings.denoisingOn = false;
vp9_settings.frameDroppingOn = true;
diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc
index f8b6e1b..bfb5606 100644
--- a/modules/video_coding/codecs/vp9/vp9_impl.cc
+++ b/modules/video_coding/codecs/vp9/vp9_impl.cc
@@ -298,7 +298,7 @@
config_->g_w = codec_.width;
config_->g_h = codec_.height;
config_->rc_target_bitrate = inst->startBitrate; // in kbit/s
- config_->g_error_resilient = inst->VP9().resilienceOn ? 1 : 0;
+ config_->g_error_resilient = 1;
// Setting the time base of the codec.
config_->g_timebase.num = 1;
config_->g_timebase.den = 90000;
diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc
index d7b6131..dbdede0 100644
--- a/modules/video_coding/video_codec_initializer.cc
+++ b/modules/video_coding/video_codec_initializer.cc
@@ -21,15 +21,6 @@
#include "webrtc/system_wrappers/include/clock.h"
namespace webrtc {
-namespace {
-bool TemporalLayersConfigured(const std::vector<VideoStream>& 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,
@@ -128,8 +119,12 @@
*video_codec.VP8() = VideoEncoder::GetDefaultVp8Settings();
video_codec.VP8()->numberOfTemporalLayers = static_cast<unsigned char>(
streams.back().temporal_layer_thresholds_bps.size() + 1);
-
- if (nack_enabled && !TemporalLayersConfigured(streams)) {
+ 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) {
LOG(LS_INFO) << "No temporal layers and nack enabled -> resilience off";
video_codec.VP8()->resilience = kResilienceOff;
}
@@ -147,13 +142,6 @@
}
video_codec.VP9()->numberOfTemporalLayers = static_cast<unsigned char>(
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/video/vie_encoder_unittest.cc b/video/vie_encoder_unittest.cc
index da925e5..1bb904b 100644
--- a/video/vie_encoder_unittest.cc
+++ b/video/vie_encoder_unittest.cc
@@ -28,7 +28,6 @@
namespace {
const size_t kMaxPayloadLength = 1440;
const int kTargetBitrateBps = 100000;
-const unsigned char kNumSlDummy = 0;
class TestBuffer : public webrtc::I420Buffer {
public:
@@ -142,7 +141,6 @@
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;
@@ -151,13 +149,6 @@
video_encoder_config.max_bitrate_bps = 1000000;
video_encoder_config.video_stream_factory =
new rtc::RefCountedObject<VideoStreamFactory>(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);
}
@@ -442,7 +433,7 @@
const bool kNackEnabled = true;
const size_t kNumStreams = 1;
const size_t kNumTl = 1;
- ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled);
+ ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled);
vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
// Capture a frame and wait for it to synchronize with the encoder thread.
@@ -462,7 +453,7 @@
const bool kNackEnabled = true;
const size_t kNumStreams = 2;
const size_t kNumTl = 1;
- ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled);
+ ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled);
vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
// Capture a frame and wait for it to synchronize with the encoder thread.
@@ -482,7 +473,7 @@
const bool kNackEnabled = false;
const size_t kNumStreams = 1;
const size_t kNumTl = 1;
- ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled);
+ ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled);
vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
// Capture a frame and wait for it to synchronize with the encoder thread.
@@ -502,7 +493,7 @@
const bool kNackEnabled = true;
const size_t kNumStreams = 1;
const size_t kNumTl = 2;
- ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled);
+ ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled);
vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
// Capture a frame and wait for it to synchronize with the encoder thread.
@@ -518,94 +509,6 @@
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;
