From 65fb4049c182cfd969f16cf6ca81085a1b06a8b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Wed, 25 Apr 2018 14:46:06 +0200 Subject: [PATCH] Don't expose resilience mode in VP8 and VP9 configuration. This deletes the resilienceOn flag in VideoCodecVP8 and VideoCodecVP9. Instead, the implementations of VP8 and VP9 set resilience mode internally, based on the configuration of temporal and spatial layers. The nack_enabled argument to VideoCodecInitializer::SetupCodec becomes unused with this cl. In a followup, it will be deleted, together with the corresponding argument to VideoStreamEncoder methods. An applications which really wants to configure resilience differently can do that by injecting an EncoderFactory with encoders behaving as desired. Bug: webrtc:8830 Change-Id: I9990faf07d3e95c0fb4a56fcc9a56c2005b4a6fa Reviewed-on: https://webrtc-review.googlesource.com/71380 Reviewed-by: Karl Wiberg Reviewed-by: Sergey Silkin Reviewed-by: Rasmus Brandt Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#23025} --- api/video_codecs/video_encoder.cc | 2 - common_types.cc | 2 - common_types.h | 2 - .../simulcast_encoder_adapter_unittest.cc | 1 - .../video_coding/codecs/test/test_config.cc | 5 - .../video_coding/codecs/test/test_config.h | 1 - .../videoprocessor_integrationtest_libvpx.cc | 37 ++-- ...deoprocessor_integrationtest_mediacodec.cc | 8 +- ...videoprocessor_integrationtest_openh264.cc | 5 +- ...processor_integrationtest_parameterized.cc | 3 +- ...oprocessor_integrationtest_videotoolbox.cc | 4 +- .../codecs/test/videoprocessor_unittest.cc | 2 +- .../codecs/vp8/libvpx_vp8_encoder.cc | 6 +- .../codecs/vp8/simulcast_test_utility.h | 1 - modules/video_coding/codecs/vp9/vp9_impl.cc | 4 +- .../video_coding/video_codec_initializer.cc | 11 -- video/video_stream_encoder_unittest.cc | 172 ------------------ 17 files changed, 34 insertions(+), 232 deletions(-) diff --git a/api/video_codecs/video_encoder.cc b/api/video_codecs/video_encoder.cc index 4de6829fa8..fd8f1425f4 100644 --- a/api/video_codecs/video_encoder.cc +++ b/api/video_codecs/video_encoder.cc @@ -17,7 +17,6 @@ VideoCodecVP8 VideoEncoder::GetDefaultVp8Settings() { VideoCodecVP8 vp8_settings; memset(&vp8_settings, 0, sizeof(vp8_settings)); - vp8_settings.resilienceOn = true; vp8_settings.numberOfTemporalLayers = 1; vp8_settings.denoisingOn = true; vp8_settings.automaticResizeOn = false; @@ -31,7 +30,6 @@ VideoCodecVP9 VideoEncoder::GetDefaultVp9Settings() { VideoCodecVP9 vp9_settings; memset(&vp9_settings, 0, sizeof(vp9_settings)); - vp9_settings.resilienceOn = true; vp9_settings.numberOfTemporalLayers = 1; vp9_settings.denoisingOn = true; vp9_settings.frameDroppingOn = true; diff --git a/common_types.cc b/common_types.cc index 20766cd0f9..a8a660622d 100644 --- a/common_types.cc +++ b/common_types.cc @@ -23,7 +23,6 @@ namespace webrtc { bool VideoCodecVP8::operator==(const VideoCodecVP8& other) const { return (complexity == other.complexity && - resilienceOn == other.resilienceOn && numberOfTemporalLayers == other.numberOfTemporalLayers && denoisingOn == other.denoisingOn && automaticResizeOn == other.automaticResizeOn && @@ -33,7 +32,6 @@ bool VideoCodecVP8::operator==(const VideoCodecVP8& other) const { bool VideoCodecVP9::operator==(const VideoCodecVP9& other) const { return (complexity == other.complexity && - resilienceOn == other.resilienceOn && numberOfTemporalLayers == other.numberOfTemporalLayers && denoisingOn == other.denoisingOn && frameDroppingOn == other.frameDroppingOn && diff --git a/common_types.h b/common_types.h index a6ec5f24b1..30e91633c2 100644 --- a/common_types.h +++ b/common_types.h @@ -354,7 +354,6 @@ struct VideoCodecVP8 { return !(*this == other); } VideoCodecComplexity complexity; - bool resilienceOn; unsigned char numberOfTemporalLayers; bool denoisingOn; bool automaticResizeOn; @@ -369,7 +368,6 @@ struct VideoCodecVP9 { return !(*this == other); } VideoCodecComplexity complexity; - bool resilienceOn; unsigned char numberOfTemporalLayers; bool denoisingOn; bool frameDroppingOn; diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index 2930e25d66..f046bf74ed 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -330,7 +330,6 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, EXPECT_EQ(ref.minBitrate, target.minBitrate); EXPECT_EQ(ref.maxFramerate, target.maxFramerate); EXPECT_EQ(ref.VP8().complexity, target.VP8().complexity); - EXPECT_EQ(ref.VP8().resilienceOn, target.VP8().resilienceOn); EXPECT_EQ(ref.VP8().numberOfTemporalLayers, target.VP8().numberOfTemporalLayers); EXPECT_EQ(ref.VP8().denoisingOn, target.VP8().denoisingOn); diff --git a/modules/video_coding/codecs/test/test_config.cc b/modules/video_coding/codecs/test/test_config.cc index 727a009a18..de6beb9263 100644 --- a/modules/video_coding/codecs/test/test_config.cc +++ b/modules/video_coding/codecs/test/test_config.cc @@ -68,7 +68,6 @@ std::string CodecSpecificToString(const VideoCodec& codec) { switch (codec.codecType) { case kVideoCodecVP8: ss << "complexity: " << codec.VP8().complexity; - ss << "\nresilience: " << codec.VP8().resilienceOn; ss << "\nnum_temporal_layers: " << static_cast(codec.VP8().numberOfTemporalLayers); ss << "\ndenoising: " << codec.VP8().denoisingOn; @@ -78,7 +77,6 @@ std::string CodecSpecificToString(const VideoCodec& codec) { break; case kVideoCodecVP9: ss << "complexity: " << codec.VP9().complexity; - ss << "\nresilience: " << codec.VP9().resilienceOn; ss << "\nnum_temporal_layers: " << static_cast(codec.VP9().numberOfTemporalLayers); ss << "\nnum_spatial_layers: " @@ -111,7 +109,6 @@ void TestConfig::SetCodecSettings(std::string codec_name, bool denoising_on, bool frame_dropper_on, bool spatial_resize_on, - bool resilience_on, size_t width, size_t height) { this->codec_name = codec_name; @@ -143,7 +140,6 @@ void TestConfig::SetCodecSettings(std::string codec_name, switch (codec_settings.codecType) { case kVideoCodecVP8: - codec_settings.VP8()->resilienceOn = resilience_on; codec_settings.VP8()->numberOfTemporalLayers = static_cast(num_temporal_layers); codec_settings.VP8()->denoisingOn = denoising_on; @@ -152,7 +148,6 @@ void TestConfig::SetCodecSettings(std::string codec_name, codec_settings.VP8()->keyFrameInterval = kBaseKeyFrameInterval; break; case kVideoCodecVP9: - codec_settings.VP9()->resilienceOn = resilience_on; codec_settings.VP9()->numberOfTemporalLayers = static_cast(num_temporal_layers); codec_settings.VP9()->denoisingOn = denoising_on; diff --git a/modules/video_coding/codecs/test/test_config.h b/modules/video_coding/codecs/test/test_config.h index 4c66e45ca5..8c2b725c3e 100644 --- a/modules/video_coding/codecs/test/test_config.h +++ b/modules/video_coding/codecs/test/test_config.h @@ -38,7 +38,6 @@ struct TestConfig { bool denoising_on, bool frame_dropper_on, bool spatial_resize_on, - bool resilience_on, size_t width, size_t height); diff --git a/modules/video_coding/codecs/test/videoprocessor_integrationtest_libvpx.cc b/modules/video_coding/codecs/test/videoprocessor_integrationtest_libvpx.cc index 5eb7df7c7d..05abfdfc2d 100644 --- a/modules/video_coding/codecs/test/videoprocessor_integrationtest_libvpx.cc +++ b/modules/video_coding/codecs/test/videoprocessor_integrationtest_libvpx.cc @@ -24,7 +24,6 @@ namespace test { namespace { // Codec settings. -const bool kResilienceOn = true; const int kCifWidth = 352; const int kCifHeight = 288; #if !defined(WEBRTC_IOS) @@ -101,13 +100,13 @@ class VideoProcessorIntegrationTestLibvpx #if !defined(RTC_DISABLE_VP9) TEST_F(VideoProcessorIntegrationTestLibvpx, HighBitrateVP9) { config_.SetCodecSettings(cricket::kVp9CodecName, 1, 1, 1, false, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); config_.num_frames = kNumFramesShort; std::vector rate_profiles = {{500, 30, kNumFramesShort}}; std::vector rc_thresholds = { - {5, 1, 0, 0.1, 0.3, 0.1, 0, 1}}; + {5, 1, 0, 0.11, 0.3, 0.1, 0, 1}}; std::vector quality_thresholds = {{37, 36, 0.94, 0.92}}; @@ -117,7 +116,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, HighBitrateVP9) { TEST_F(VideoProcessorIntegrationTestLibvpx, ChangeBitrateVP9) { config_.SetCodecSettings(cricket::kVp9CodecName, 1, 1, 1, false, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); std::vector rate_profiles = { {200, 30, 100}, // target_kbps, input_fps, frame_index_rate_update @@ -138,7 +137,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, ChangeBitrateVP9) { TEST_F(VideoProcessorIntegrationTestLibvpx, ChangeFramerateVP9) { config_.SetCodecSettings(cricket::kVp9CodecName, 1, 1, 1, false, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); std::vector rate_profiles = { {100, 24, 100}, // target_kbps, input_fps, frame_index_rate_update @@ -149,7 +148,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, ChangeFramerateVP9) { std::vector rc_thresholds = { {10, 2, 40, 0.4, 0.5, 0.2, 0, 1}, {8, 2, 5, 0.2, 0.5, 0.2, 0, 0}, - {5, 2, 0, 0.2, 0.5, 0.3, 0, 0}}; + {5, 2, 0, 0.21, 0.5, 0.3, 0, 0}}; // Quality should be higher for lower framerates for the same content. std::vector quality_thresholds = { @@ -161,13 +160,13 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, ChangeFramerateVP9) { TEST_F(VideoProcessorIntegrationTestLibvpx, DenoiserOnVP9) { config_.SetCodecSettings(cricket::kVp9CodecName, 1, 1, 1, true, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); config_.num_frames = kNumFramesShort; std::vector rate_profiles = {{500, 30, kNumFramesShort}}; std::vector rc_thresholds = { - {5, 1, 0, 0.1, 0.3, 0.1, 0, 1}}; + {5, 1, 0, 0.11, 0.3, 0.1, 0, 1}}; std::vector quality_thresholds = {{37.5, 36, 0.94, 0.93}}; @@ -177,7 +176,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, DenoiserOnVP9) { TEST_F(VideoProcessorIntegrationTestLibvpx, VeryLowBitrateVP9) { config_.SetCodecSettings(cricket::kVp9CodecName, 1, 1, 1, false, true, true, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); std::vector rate_profiles = {{50, 30, kNumFramesLong}}; @@ -197,7 +196,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, VeryLowBitrateVP9) { TEST_F(VideoProcessorIntegrationTestLibvpx, HighBitrateVP8) { config_.SetCodecSettings(cricket::kVp8CodecName, 1, 1, 1, true, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); config_.num_frames = kNumFramesShort; std::vector rate_profiles = {{500, 30, kNumFramesShort}}; @@ -233,7 +232,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, HighBitrateVP8) { #endif TEST_F(VideoProcessorIntegrationTestLibvpx, MAYBE_ChangeBitrateVP8) { config_.SetCodecSettings(cricket::kVp8CodecName, 1, 1, 1, true, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); std::vector rate_profiles = { {200, 30, 100}, // target_kbps, input_fps, frame_index_rate_update @@ -242,7 +241,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, MAYBE_ChangeBitrateVP8) { std::vector rc_thresholds = { {5, 1, 0, 0.1, 0.2, 0.1, 0, 1}, - {15, 1, 0, 0.1, 0.2, 0.1, 0, 0}, + {15.5, 1, 0, 0.1, 0.2, 0.1, 0, 0}, {15, 1, 0, 0.3, 0.2, 0.1, 0, 0}}; // std::vector quality_thresholds = { @@ -264,7 +263,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, MAYBE_ChangeBitrateVP8) { #endif TEST_F(VideoProcessorIntegrationTestLibvpx, MAYBE_ChangeFramerateVP8) { config_.SetCodecSettings(cricket::kVp8CodecName, 1, 1, 1, true, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); std::vector rate_profiles = { {80, 24, 100}, // target_kbps, input_fps, frame_index_rate_update @@ -301,7 +300,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, MAYBE_ChangeFramerateVP8) { #endif TEST_F(VideoProcessorIntegrationTestLibvpx, MAYBE_TemporalLayersVP8) { config_.SetCodecSettings(cricket::kVp8CodecName, 1, 1, 3, true, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); std::vector rate_profiles = {{200, 30, 150}, {400, 30, kNumFramesLong}}; @@ -336,7 +335,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, MAYBE_MultiresVP8) { config_.filepath = ResourcePath(config_.filename, "yuv"); config_.num_frames = 100; config_.SetCodecSettings(cricket::kVp8CodecName, 3, 1, 3, true, true, false, - kResilienceOn, 1280, 720); + 1280, 720); std::vector rate_profiles = {{1500, 30, config_.num_frames}}; @@ -360,7 +359,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, MAYBE_SimulcastVP8) { config_.num_frames = 100; config_.simulcast_adapted_encoder = true; config_.SetCodecSettings(cricket::kVp8CodecName, 3, 1, 3, true, true, false, - kResilienceOn, 1280, 720); + 1280, 720); std::vector rate_profiles = {{1500, 30, config_.num_frames}}; @@ -383,7 +382,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, MAYBE_SvcVP9) { config_.filepath = ResourcePath(config_.filename, "yuv"); config_.num_frames = 100; config_.SetCodecSettings(cricket::kVp9CodecName, 1, 3, 3, true, true, false, - kResilienceOn, 1280, 720); + 1280, 720); std::vector rate_profiles = {{1500, 30, config_.num_frames}}; @@ -401,7 +400,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, DISABLED_MultiresVP8RdPerf) { config_.num_frames = 300; config_.print_frame_level_stats = true; config_.SetCodecSettings(cricket::kVp8CodecName, 3, 1, 3, true, true, false, - kResilienceOn, 1280, 720); + 1280, 720); std::map> rd_stats; for (size_t bitrate_kbps : kBitrateRdPerfKbps) { @@ -424,7 +423,7 @@ TEST_F(VideoProcessorIntegrationTestLibvpx, DISABLED_SvcVP9RdPerf) { config_.num_frames = 300; config_.print_frame_level_stats = true; config_.SetCodecSettings(cricket::kVp9CodecName, 1, 3, 3, true, true, false, - kResilienceOn, 1280, 720); + 1280, 720); std::map> rd_stats; for (size_t bitrate_kbps : kBitrateRdPerfKbps) { diff --git a/modules/video_coding/codecs/test/videoprocessor_integrationtest_mediacodec.cc b/modules/video_coding/codecs/test/videoprocessor_integrationtest_mediacodec.cc index 8b758ae357..8760be9e42 100644 --- a/modules/video_coding/codecs/test/videoprocessor_integrationtest_mediacodec.cc +++ b/modules/video_coding/codecs/test/videoprocessor_integrationtest_mediacodec.cc @@ -41,7 +41,7 @@ class VideoProcessorIntegrationTestMediaCodec TEST_F(VideoProcessorIntegrationTestMediaCodec, ForemanCif500kbpsVp8) { config_.SetCodecSettings(cricket::kVp8CodecName, 1, 1, 1, false, false, false, - false, 352, 288); + 352, 288); std::vector rate_profiles = { {500, kForemanFramerateFps, kForemanNumFrames}}; @@ -61,7 +61,7 @@ TEST_F(VideoProcessorIntegrationTestMediaCodec, ForemanCif500kbpsVp8) { TEST_F(VideoProcessorIntegrationTestMediaCodec, ForemanCif500kbpsH264CBP) { config_.encoded_frame_checker = &h264_keyframe_checker_; config_.SetCodecSettings(cricket::kH264CodecName, 1, 1, 1, false, false, - false, false, 352, 288); + false, 352, 288); std::vector rate_profiles = { {500, kForemanFramerateFps, kForemanNumFrames}}; @@ -87,7 +87,7 @@ TEST_F(VideoProcessorIntegrationTestMediaCodec, config_.h264_codec_settings.profile = H264::kProfileConstrainedHigh; config_.encoded_frame_checker = &h264_keyframe_checker_; config_.SetCodecSettings(cricket::kH264CodecName, 1, 1, 1, false, false, - false, false, 352, 288); + false, 352, 288); std::vector rate_profiles = { {500, kForemanFramerateFps, kForemanNumFrames}}; @@ -123,7 +123,7 @@ TEST_F(VideoProcessorIntegrationTestMediaCodec, ForemanMixedRes100kbpsVp8H264) { std::to_string(height); config_.filepath = ResourcePath(config_.filename, "yuv"); config_.num_frames = kNumFrames; - config_.SetCodecSettings(codec, 1, 1, 1, false, false, false, false, + config_.SetCodecSettings(codec, 1, 1, 1, false, false, false, width, height); ProcessFramesAndMaybeVerify( diff --git a/modules/video_coding/codecs/test/videoprocessor_integrationtest_openh264.cc b/modules/video_coding/codecs/test/videoprocessor_integrationtest_openh264.cc index 5e20832412..d218b0c660 100644 --- a/modules/video_coding/codecs/test/videoprocessor_integrationtest_openh264.cc +++ b/modules/video_coding/codecs/test/videoprocessor_integrationtest_openh264.cc @@ -22,7 +22,6 @@ namespace test { namespace { // Codec settings. -const bool kResilienceOn = true; const int kCifWidth = 352; const int kCifHeight = 288; const int kNumFrames = 100; @@ -45,7 +44,7 @@ class VideoProcessorIntegrationTestOpenH264 TEST_F(VideoProcessorIntegrationTestOpenH264, ConstantHighBitrate) { config_.SetCodecSettings(cricket::kH264CodecName, 1, 1, 1, false, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); std::vector rate_profiles = {{500, 30, kNumFrames}}; @@ -65,7 +64,7 @@ TEST_F(VideoProcessorIntegrationTestOpenH264, SingleNalUnit) { H264PacketizationMode::SingleNalUnit; config_.max_payload_size_bytes = 500; config_.SetCodecSettings(cricket::kH264CodecName, 1, 1, 1, false, true, false, - kResilienceOn, kCifWidth, kCifHeight); + kCifWidth, kCifHeight); std::vector rate_profiles = {{500, 30, kNumFrames}}; diff --git a/modules/video_coding/codecs/test/videoprocessor_integrationtest_parameterized.cc b/modules/video_coding/codecs/test/videoprocessor_integrationtest_parameterized.cc index 62f448e2bc..2499ede07e 100644 --- a/modules/video_coding/codecs/test/videoprocessor_integrationtest_parameterized.cc +++ b/modules/video_coding/codecs/test/videoprocessor_integrationtest_parameterized.cc @@ -25,7 +25,6 @@ const bool kHwCodec[] = {false}; // Codec settings. const int kNumSpatialLayers = 1; const int kNumTemporalLayers = 1; -const bool kResilienceOn = kNumSpatialLayers > 1 || kNumTemporalLayers > 1; const bool kDenoisingOn = false; const bool kSpatialResizeOn = false; const bool kFrameDropperOn = false; @@ -75,7 +74,7 @@ class VideoProcessorIntegrationTestParameterized config_.SetCodecSettings(codec_name, num_simulcast_streams, num_spatial_layers, kNumTemporalLayers, kDenoisingOn, kFrameDropperOn, kSpatialResizeOn, - kResilienceOn, width, height); + width, height); std::vector rate_profiles = { {bitrate_, framerate, kNumFrames}}; diff --git a/modules/video_coding/codecs/test/videoprocessor_integrationtest_videotoolbox.cc b/modules/video_coding/codecs/test/videoprocessor_integrationtest_videotoolbox.cc index 24a4b90e3c..5840bb0e70 100644 --- a/modules/video_coding/codecs/test/videoprocessor_integrationtest_videotoolbox.cc +++ b/modules/video_coding/codecs/test/videoprocessor_integrationtest_videotoolbox.cc @@ -70,7 +70,7 @@ class VideoProcessorIntegrationTestVideoToolbox MAYBE_TEST_F(VideoProcessorIntegrationTestVideoToolbox, ForemanCif500kbpsH264CBP) { config_.SetCodecSettings(cricket::kH264CodecName, 1, 1, 1, false, false, - false, false, 352, 288); + false, 352, 288); std::vector rate_profiles = {{500, 30, kForemanNumFrames}}; @@ -86,7 +86,7 @@ MAYBE_TEST_F(VideoProcessorIntegrationTestVideoToolbox, config_.h264_codec_settings.profile = H264::kProfileConstrainedHigh; config_.SetCodecSettings(cricket::kH264CodecName, 1, 1, 1, false, false, - false, false, 352, 288); + false, 352, 288); std::vector rate_profiles = {{500, 30, kForemanNumFrames}}; diff --git a/modules/video_coding/codecs/test/videoprocessor_unittest.cc b/modules/video_coding/codecs/test/videoprocessor_unittest.cc index baf97bf393..4f496ae856 100644 --- a/modules/video_coding/codecs/test/videoprocessor_unittest.cc +++ b/modules/video_coding/codecs/test/videoprocessor_unittest.cc @@ -53,7 +53,7 @@ class VideoProcessorTest : public testing::Test { protected: VideoProcessorTest() : q_("VP queue") { config_.SetCodecSettings(cricket::kVp8CodecName, 1, 1, 1, false, false, - false, false, kWidth, kHeight); + false, kWidth, kHeight); decoder_mock_ = new MockVideoDecoder(); decoders_.push_back(std::unique_ptr(decoder_mock_)); diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index d84d0011da..8410e37de7 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -440,9 +440,9 @@ int LibvpxVp8Encoder::InitEncode(const VideoCodec* inst, configurations_[0].g_timebase.den = 90000; configurations_[0].g_lag_in_frames = 0; // 0- no frame lagging - // Set the error resilience mode according to user settings. + // Set the error resilience mode for temporal layers (but not simulcast). configurations_[0].g_error_resilient = - inst->VP8().resilienceOn ? VPX_ERROR_RESILIENT_DEFAULT : 0; + (num_temporal_layers > 1) ? VPX_ERROR_RESILIENT_DEFAULT : 0; // rate control settings configurations_[0].rc_dropframe_thresh = inst->VP8().frameDroppingOn ? 30 : 0; @@ -667,7 +667,7 @@ int LibvpxVp8Encoder::InitAndSetControlSettings() { vpx_codec_control(&(encoders_[i]), VP8E_SET_SCREEN_CONTENT_MODE, codec_.mode == kScreensharing ? 2 : 0); // Apply boost on golden frames (has only effect when resilience is off). - if (use_gf_boost_ && !codec_.VP8()->resilienceOn) { + if (use_gf_boost_ && configurations_[0].g_error_resilient == 0) { int gf_boost_percent; if (GetGfBoostPercentageFromFieldTrialGroup(&gf_boost_percent)) { vpx_codec_control(&(encoders_[i]), VP8E_SET_GF_CBR_BOOST_PCT, diff --git a/modules/video_coding/codecs/vp8/simulcast_test_utility.h b/modules/video_coding/codecs/vp8/simulcast_test_utility.h index 59d8226c79..f0b1cccdc8 100644 --- a/modules/video_coding/codecs/vp8/simulcast_test_utility.h +++ b/modules/video_coding/codecs/vp8/simulcast_test_utility.h @@ -214,7 +214,6 @@ class TestVp8Simulcast : public ::testing::Test { ConfigureStream(kDefaultWidth, kDefaultHeight, kMaxBitrates[2], kMinBitrates[2], kTargetBitrates[2], &settings->simulcastStream[2], temporal_layer_profile[2]); - settings->VP8()->resilienceOn = true; settings->VP8()->denoisingOn = true; settings->VP8()->automaticResizeOn = false; settings->VP8()->frameDroppingOn = true; diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc index d133e66962..48b7176c6d 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -266,6 +266,7 @@ int VP9EncoderImpl::InitEncode(const VideoCodec* inst, } num_spatial_layers_ = inst->VP9().numberOfSpatialLayers; + RTC_DCHECK_GT(num_spatial_layers_, 0); num_temporal_layers_ = inst->VP9().numberOfTemporalLayers; if (num_temporal_layers_ == 0) num_temporal_layers_ = 1; @@ -290,7 +291,8 @@ 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 = inst->VP9().resilienceOn ? 1 : 0; + config_->g_error_resilient = + (num_spatial_layers_ > 1 || num_temporal_layers_ > 1) ? 1 : 0; // 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 57bce50843..34fcb8dc5c 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -183,11 +183,6 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( RTC_DCHECK_LE(video_codec.VP8()->numberOfTemporalLayers, kMaxTemporalStreams); - if (nack_enabled && video_codec.VP8()->numberOfTemporalLayers == 1) { - RTC_LOG(LS_INFO) - << "No temporal layers and nack enabled -> resilience off"; - video_codec.VP8()->resilienceOn = false; - } break; } case kVideoCodecVP9: { @@ -250,12 +245,6 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( kMaxTemporalStreams); } - if (nack_enabled && video_codec.VP9()->numberOfTemporalLayers == 1 && - video_codec.VP9()->numberOfSpatialLayers == 1) { - RTC_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/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 5289f9a9a5..ba21cf9959 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -35,7 +35,6 @@ const int kMinPixelsPerFrame = 320 * 180; const int kMinFramerateFps = 2; const int kMinBalancedFramerateFps = 7; const int64_t kFrameTimeoutMs = 100; -const unsigned char kNumSlDummy = 0; } // namespace namespace webrtc { @@ -837,177 +836,6 @@ TEST_F(VideoStreamEncoderTest, FrameResolutionChangeReconfigureEncoder) { video_stream_encoder_->Stop(); } -TEST_F(VideoStreamEncoderTest, Vp8ResilienceIsOffFor1S1TLWithNackEnabled) { - const bool kNackEnabled = true; - const size_t kNumStreams = 1; - const size_t kNumTl = 1; - ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled, false); - video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); - - // Capture a frame and wait for it to synchronize with the encoder thread. - video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); - WaitForEncodedFrame(1); - // The encoder have been configured once when the first frame is received. - EXPECT_EQ(1, sink_.number_of_reconfigurations()); - EXPECT_EQ(kVideoCodecVP8, fake_encoder_.codec_config().codecType); - EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams); - EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP8()->numberOfTemporalLayers); - // Resilience is off for no temporal layers with nack on. - EXPECT_FALSE(fake_encoder_.codec_config().VP8()->resilienceOn); - video_stream_encoder_->Stop(); -} - -TEST_F(VideoStreamEncoderTest, Vp8ResilienceIsOffFor2S1TlWithNackEnabled) { - const bool kNackEnabled = true; - const size_t kNumStreams = 2; - const size_t kNumTl = 1; - ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled, false); - video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); - - // Capture a frame and wait for it to synchronize with the encoder thread. - video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); - WaitForEncodedFrame(1); - // The encoder have been configured once when the first frame is received. - EXPECT_EQ(1, sink_.number_of_reconfigurations()); - EXPECT_EQ(kVideoCodecVP8, fake_encoder_.codec_config().codecType); - EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams); - EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP8()->numberOfTemporalLayers); - // Resilience is off for no temporal layers and >1 streams with nack on. - EXPECT_FALSE(fake_encoder_.codec_config().VP8()->resilienceOn); - video_stream_encoder_->Stop(); -} - -TEST_F(VideoStreamEncoderTest, Vp8ResilienceIsOnFor1S1TLWithNackDisabled) { - const bool kNackEnabled = false; - const size_t kNumStreams = 1; - const size_t kNumTl = 1; - ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled, false); - video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); - - // Capture a frame and wait for it to synchronize with the encoder thread. - video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); - WaitForEncodedFrame(1); - // The encoder have been configured once when the first frame is received. - EXPECT_EQ(1, sink_.number_of_reconfigurations()); - EXPECT_EQ(kVideoCodecVP8, fake_encoder_.codec_config().codecType); - EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams); - EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP8()->numberOfTemporalLayers); - // Resilience is on for no temporal layers with nack off. - EXPECT_TRUE(fake_encoder_.codec_config().VP8()->resilienceOn); - video_stream_encoder_->Stop(); -} - -TEST_F(VideoStreamEncoderTest, Vp8ResilienceIsOnFor1S2TlWithNackEnabled) { - const bool kNackEnabled = true; - const size_t kNumStreams = 1; - const size_t kNumTl = 2; - ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled, false); - video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); - - // Capture a frame and wait for it to synchronize with the encoder thread. - video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); - WaitForEncodedFrame(1); - // The encoder have been configured once when the first frame is received. - EXPECT_EQ(1, sink_.number_of_reconfigurations()); - EXPECT_EQ(kVideoCodecVP8, fake_encoder_.codec_config().codecType); - EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams); - EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP8()->numberOfTemporalLayers); - // Resilience is on for temporal layers. - EXPECT_TRUE(fake_encoder_.codec_config().VP8()->resilienceOn); - video_stream_encoder_->Stop(); -} - -TEST_F(VideoStreamEncoderTest, 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, false); - video_stream_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); - video_stream_encoder_->Stop(); -} - -TEST_F(VideoStreamEncoderTest, 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, false); - video_stream_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); - video_stream_encoder_->Stop(); -} - -TEST_F(VideoStreamEncoderTest, Vp9ResilienceIsOnFor2SL1TLWithNackEnabled) { - const bool kNackEnabled = true; - const size_t kNumStreams = 1; - const size_t kNumTl = 1; - const unsigned char kNumSl = 2; - const int kFrameWidth = kMinVp9SpatialLayerWidth << (kNumSl - 1); - const int kFrameHeight = kMinVp9SpatialLayerHeight << (kNumSl - 1); - ResetEncoder("VP9", kNumStreams, kNumTl, kNumSl, kNackEnabled, false); - video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); - - // Capture a frame and wait for it to synchronize with the encoder thread. - video_source_.IncomingCapturedFrame( - CreateFrame(1, kFrameWidth, kFrameHeight)); - 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); - video_stream_encoder_->Stop(); -} - -TEST_F(VideoStreamEncoderTest, 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, false); - video_stream_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); - video_stream_encoder_->Stop(); -} - TEST_F(VideoStreamEncoderTest, SwitchSourceDeregisterEncoderAsSink) { EXPECT_TRUE(video_source_.has_sinks()); test::FrameForwarder new_video_source;