diff --git a/rtc_base/experiments/keyframe_interval_settings.cc b/rtc_base/experiments/keyframe_interval_settings.cc index 2f19a1c53f..76c85cbbad 100644 --- a/rtc_base/experiments/keyframe_interval_settings.cc +++ b/rtc_base/experiments/keyframe_interval_settings.cc @@ -22,11 +22,8 @@ constexpr char kFieldTrialName[] = "WebRTC-KeyframeInterval"; KeyframeIntervalSettings::KeyframeIntervalSettings( const WebRtcKeyValueConfig* const key_value_config) - : min_keyframe_send_interval_ms_("min_keyframe_send_interval_ms"), - max_wait_for_keyframe_ms_("max_wait_for_keyframe_ms"), - max_wait_for_frame_ms_("max_wait_for_frame_ms") { - ParseFieldTrial({&min_keyframe_send_interval_ms_, &max_wait_for_keyframe_ms_, - &max_wait_for_frame_ms_}, + : min_keyframe_send_interval_ms_("min_keyframe_send_interval_ms") { + ParseFieldTrial({&min_keyframe_send_interval_ms_}, key_value_config->Lookup(kFieldTrialName)); } @@ -39,13 +36,4 @@ absl::optional KeyframeIntervalSettings::MinKeyframeSendIntervalMs() const { return min_keyframe_send_interval_ms_.GetOptional(); } - -absl::optional KeyframeIntervalSettings::MaxWaitForKeyframeMs() const { - return max_wait_for_keyframe_ms_.GetOptional(); -} - -absl::optional KeyframeIntervalSettings::MaxWaitForFrameMs() const { - return max_wait_for_frame_ms_.GetOptional(); -} - } // namespace webrtc diff --git a/rtc_base/experiments/keyframe_interval_settings.h b/rtc_base/experiments/keyframe_interval_settings.h index 7c8d6d364a..3f253f0022 100644 --- a/rtc_base/experiments/keyframe_interval_settings.h +++ b/rtc_base/experiments/keyframe_interval_settings.h @@ -17,6 +17,9 @@ namespace webrtc { +// TODO(bugs.webrtc.org/10427): Remove and replace with proper configuration +// parameter, or move to using FIR if intent is to avoid triggering multiple +// times to PLIs corresponding to the same request when RTT is large. class KeyframeIntervalSettings final { public: static KeyframeIntervalSettings ParseFromFieldTrials(); @@ -25,22 +28,11 @@ class KeyframeIntervalSettings final { // The encoded keyframe send rate is <= 1/MinKeyframeSendIntervalMs(). absl::optional MinKeyframeSendIntervalMs() const; - // Receiver side. - // The keyframe request send rate is - // - when we have not received a key frame at all: - // <= 1/MaxWaitForKeyframeMs() - // - when we have not received a frame recently: - // <= 1/MaxWaitForFrameMs() - absl::optional MaxWaitForKeyframeMs() const; - absl::optional MaxWaitForFrameMs() const; - private: explicit KeyframeIntervalSettings( const WebRtcKeyValueConfig* key_value_config); FieldTrialOptional min_keyframe_send_interval_ms_; - FieldTrialOptional max_wait_for_keyframe_ms_; - FieldTrialOptional max_wait_for_frame_ms_; }; } // namespace webrtc diff --git a/rtc_base/experiments/keyframe_interval_settings_unittest.cc b/rtc_base/experiments/keyframe_interval_settings_unittest.cc index 7d89a4c000..25cebbcd70 100644 --- a/rtc_base/experiments/keyframe_interval_settings_unittest.cc +++ b/rtc_base/experiments/keyframe_interval_settings_unittest.cc @@ -27,60 +27,16 @@ TEST(KeyframeIntervalSettingsTest, ParsesMinKeyframeSendIntervalMs) { 100); } -TEST(KeyframeIntervalSettingsTest, ParsesMaxWaitForKeyframeMs) { - EXPECT_FALSE( - KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForKeyframeMs()); - - test::ScopedFieldTrials field_trials( - "WebRTC-KeyframeInterval/max_wait_for_keyframe_ms:100/"); - EXPECT_EQ( - KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForKeyframeMs(), - 100); -} - -TEST(KeyframeIntervalSettingsTest, ParsesMaxWaitForFrameMs) { - EXPECT_FALSE( - KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForFrameMs()); - - test::ScopedFieldTrials field_trials( - "WebRTC-KeyframeInterval/max_wait_for_frame_ms:100/"); - EXPECT_EQ( - KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForFrameMs(), - 100); -} - -TEST(KeyframeIntervalSettingsTest, ParsesAllValues) { - test::ScopedFieldTrials field_trials( - "WebRTC-KeyframeInterval/" - "min_keyframe_send_interval_ms:100," - "max_wait_for_keyframe_ms:101," - "max_wait_for_frame_ms:102/"); - EXPECT_EQ(KeyframeIntervalSettings::ParseFromFieldTrials() - .MinKeyframeSendIntervalMs(), - 100); - EXPECT_EQ( - KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForKeyframeMs(), - 101); - EXPECT_EQ( - KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForFrameMs(), - 102); -} - -TEST(KeyframeIntervalSettingsTest, DoesNotParseAllValuesWhenIncorrectlySet) { - EXPECT_FALSE( - KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForFrameMs()); - - test::ScopedFieldTrials field_trials( - "WebRTC-KeyframeInterval/" - "min_keyframe_send_interval_ms:a," - "max_wait_for_keyframe_ms:b," - "max_wait_for_frame_ms:c/"); +TEST(KeyframeIntervalSettingsTest, DoesNotParseIncorrectValues) { + EXPECT_FALSE(KeyframeIntervalSettings::ParseFromFieldTrials() + .MinKeyframeSendIntervalMs()); + + test::ScopedFieldTrials field_trials( + "WebRTC-KeyframeInterval/min_keyframe_send_interval_ms:a/"); + EXPECT_FALSE(KeyframeIntervalSettings::ParseFromFieldTrials() + .MinKeyframeSendIntervalMs()); EXPECT_FALSE(KeyframeIntervalSettings::ParseFromFieldTrials() .MinKeyframeSendIntervalMs()); - EXPECT_FALSE( - KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForKeyframeMs()); - EXPECT_FALSE( - KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForFrameMs()); } } // namespace diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index d69eba57a2..12cb01d68b 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -39,7 +39,6 @@ #include "modules/video_coding/timing.h" #include "modules/video_coding/utility/vp8_header_parser.h" #include "rtc_base/checks.h" -#include "rtc_base/experiments/keyframe_interval_settings.h" #include "rtc_base/location.h" #include "rtc_base/logging.h" #include "rtc_base/strings/string_builder.h" @@ -220,12 +219,8 @@ VideoReceiveStream::VideoReceiveStream( config_.frame_decryptor, config_.frame_transformer), rtp_stream_sync_(this), - max_wait_for_keyframe_ms_(KeyframeIntervalSettings::ParseFromFieldTrials() - .MaxWaitForKeyframeMs() - .value_or(kMaxWaitForKeyFrameMs)), - max_wait_for_frame_ms_(KeyframeIntervalSettings::ParseFromFieldTrials() - .MaxWaitForFrameMs() - .value_or(kMaxWaitForFrameMs)), + max_wait_for_keyframe_ms_(kMaxWaitForKeyFrameMs), + max_wait_for_frame_ms_(kMaxWaitForFrameMs), decode_queue_(task_queue_factory_->CreateTaskQueue( "DecodingQueue", TaskQueueFactory::Priority::HIGH)) { diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 0b4b66f422..073fd93f7f 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -38,7 +38,6 @@ #include "modules/video_coding/timing.h" #include "modules/video_coding/utility/vp8_header_parser.h" #include "rtc_base/checks.h" -#include "rtc_base/experiments/keyframe_interval_settings.h" #include "rtc_base/location.h" #include "rtc_base/logging.h" #include "rtc_base/strings/string_builder.h" @@ -226,12 +225,8 @@ VideoReceiveStream2::VideoReceiveStream2( config_.frame_decryptor, config_.frame_transformer), rtp_stream_sync_(current_queue, this), - max_wait_for_keyframe_ms_(KeyframeIntervalSettings::ParseFromFieldTrials() - .MaxWaitForKeyframeMs() - .value_or(kMaxWaitForKeyFrameMs)), - max_wait_for_frame_ms_(KeyframeIntervalSettings::ParseFromFieldTrials() - .MaxWaitForFrameMs() - .value_or(kMaxWaitForFrameMs)), + max_wait_for_keyframe_ms_(kMaxWaitForKeyFrameMs), + max_wait_for_frame_ms_(kMaxWaitForFrameMs), low_latency_renderer_enabled_("enabled", true), low_latency_renderer_include_predecode_buffer_("include_predecode_buffer", true),