Make keyframe generation/request intervals configurable through field trials.
Bug: webrtc:10427
Change-Id: I5e7182fc8932943adc3e5f147be51b0b5df93172
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/127882
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27233}diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn
index afe174b..b8709dc 100644
--- a/rtc_base/experiments/BUILD.gn
+++ b/rtc_base/experiments/BUILD.gn
@@ -132,6 +132,19 @@
]
}
+rtc_static_library("keyframe_interval_settings_experiment") {
+ sources = [
+ "keyframe_interval_settings.cc",
+ "keyframe_interval_settings.h",
+ ]
+ deps = [
+ ":field_trial_parser",
+ "../../api/transport:field_trial_based_config",
+ "../../api/transport:webrtc_key_value_config",
+ "//third_party/abseil-cpp/absl/types:optional",
+ ]
+}
+
if (rtc_include_tests) {
rtc_source_set("experiments_unittests") {
testonly = true
@@ -140,6 +153,7 @@
"cpu_speed_experiment_unittest.cc",
"field_trial_parser_unittest.cc",
"field_trial_units_unittest.cc",
+ "keyframe_interval_settings_unittest.cc",
"normalize_simulcast_size_experiment_unittest.cc",
"quality_scaling_experiment_unittest.cc",
"rate_control_settings_unittest.cc",
@@ -148,6 +162,7 @@
deps = [
":cpu_speed_experiment",
":field_trial_parser",
+ ":keyframe_interval_settings_experiment",
":normalize_simulcast_size_experiment",
":quality_scaling_experiment",
":rate_control_settings",
diff --git a/rtc_base/experiments/OWNERS b/rtc_base/experiments/OWNERS
index c31e020..5b34315 100644
--- a/rtc_base/experiments/OWNERS
+++ b/rtc_base/experiments/OWNERS
@@ -4,6 +4,7 @@
per-file cpu_speed_experiment*=asapersson@webrtc.org
per-file field_trial*=srte@webrtc.org
per-file jitter_upper_bound_experiment*=sprang@webrtc.org
+per-file keyframe_interval_settings*=brandtr@webrtc.org
per-file normalize_simulcast_size_experiment*=asapersson@webrtc.org
per-file quality_scaling_experiment*=asapersson@webrtc.org
per-file rtt_mult_experiment*=mhoro@webrtc.org
diff --git a/rtc_base/experiments/keyframe_interval_settings.cc b/rtc_base/experiments/keyframe_interval_settings.cc
new file mode 100644
index 0000000..2f19a1c
--- /dev/null
+++ b/rtc_base/experiments/keyframe_interval_settings.cc
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "rtc_base/experiments/keyframe_interval_settings.h"
+
+#include "api/transport/field_trial_based_config.h"
+
+namespace webrtc {
+
+namespace {
+
+constexpr char kFieldTrialName[] = "WebRTC-KeyframeInterval";
+
+} // namespace
+
+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_},
+ key_value_config->Lookup(kFieldTrialName));
+}
+
+KeyframeIntervalSettings KeyframeIntervalSettings::ParseFromFieldTrials() {
+ FieldTrialBasedConfig field_trial_config;
+ return KeyframeIntervalSettings(&field_trial_config);
+}
+
+absl::optional<int> KeyframeIntervalSettings::MinKeyframeSendIntervalMs()
+ const {
+ return min_keyframe_send_interval_ms_.GetOptional();
+}
+
+absl::optional<int> KeyframeIntervalSettings::MaxWaitForKeyframeMs() const {
+ return max_wait_for_keyframe_ms_.GetOptional();
+}
+
+absl::optional<int> 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
new file mode 100644
index 0000000..7c8d6d3
--- /dev/null
+++ b/rtc_base/experiments/keyframe_interval_settings.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef RTC_BASE_EXPERIMENTS_KEYFRAME_INTERVAL_SETTINGS_H_
+#define RTC_BASE_EXPERIMENTS_KEYFRAME_INTERVAL_SETTINGS_H_
+
+#include "absl/types/optional.h"
+#include "api/transport/webrtc_key_value_config.h"
+#include "rtc_base/experiments/field_trial_parser.h"
+
+namespace webrtc {
+
+class KeyframeIntervalSettings final {
+ public:
+ static KeyframeIntervalSettings ParseFromFieldTrials();
+
+ // Sender side.
+ // The encoded keyframe send rate is <= 1/MinKeyframeSendIntervalMs().
+ absl::optional<int> 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<int> MaxWaitForKeyframeMs() const;
+ absl::optional<int> MaxWaitForFrameMs() const;
+
+ private:
+ explicit KeyframeIntervalSettings(
+ const WebRtcKeyValueConfig* key_value_config);
+
+ FieldTrialOptional<int> min_keyframe_send_interval_ms_;
+ FieldTrialOptional<int> max_wait_for_keyframe_ms_;
+ FieldTrialOptional<int> max_wait_for_frame_ms_;
+};
+
+} // namespace webrtc
+
+#endif // RTC_BASE_EXPERIMENTS_KEYFRAME_INTERVAL_SETTINGS_H_
diff --git a/rtc_base/experiments/keyframe_interval_settings_unittest.cc b/rtc_base/experiments/keyframe_interval_settings_unittest.cc
new file mode 100644
index 0000000..5457dc4
--- /dev/null
+++ b/rtc_base/experiments/keyframe_interval_settings_unittest.cc
@@ -0,0 +1,86 @@
+/*
+ * Copyright 2019 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "rtc_base/experiments/keyframe_interval_settings.h"
+#include "test/field_trial.h"
+#include "test/gtest.h"
+
+namespace webrtc {
+namespace {
+
+TEST(KeyframeIntervalSettingsTest, ParsesMinKeyframeSendIntervalMs) {
+ EXPECT_FALSE(KeyframeIntervalSettings::ParseFromFieldTrials()
+ .MinKeyframeSendIntervalMs());
+
+ test::ScopedFieldTrials field_trials(
+ "WebRTC-KeyframeInterval/min_keyframe_send_interval_ms:100/");
+ EXPECT_EQ(KeyframeIntervalSettings::ParseFromFieldTrials()
+ .MinKeyframeSendIntervalMs(),
+ 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/");
+ EXPECT_FALSE(KeyframeIntervalSettings::ParseFromFieldTrials()
+ .MinKeyframeSendIntervalMs());
+ EXPECT_FALSE(
+ KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForKeyframeMs());
+ EXPECT_FALSE(
+ KeyframeIntervalSettings::ParseFromFieldTrials().MaxWaitForFrameMs());
+}
+
+} // namespace
+} // namespace webrtc
diff --git a/video/BUILD.gn b/video/BUILD.gn
index 0e02401..8f89e22 100644
--- a/video/BUILD.gn
+++ b/video/BUILD.gn
@@ -99,6 +99,7 @@
"../rtc_base:stringutils",
"../rtc_base:weak_ptr",
"../rtc_base/experiments:alr_experiment",
+ "../rtc_base/experiments:keyframe_interval_settings_experiment",
"../rtc_base/experiments:quality_scaling_experiment",
"../rtc_base/experiments:rate_control_settings",
"../rtc_base/system:fallthrough",
diff --git a/video/encoder_key_frame_callback.cc b/video/encoder_key_frame_callback.cc
index 447a3af..3116221 100644
--- a/video/encoder_key_frame_callback.cc
+++ b/video/encoder_key_frame_callback.cc
@@ -10,12 +10,16 @@
#include "video/encoder_key_frame_callback.h"
+#include "absl/types/optional.h"
#include "rtc_base/checks.h"
-
-static const int kMinKeyFrameRequestIntervalMs = 300;
+#include "rtc_base/experiments/keyframe_interval_settings.h"
namespace webrtc {
+namespace {
+constexpr int kMinKeyframeSendIntervalMs = 300;
+} // namespace
+
EncoderKeyFrameCallback::EncoderKeyFrameCallback(
Clock* clock,
const std::vector<uint32_t>& ssrcs,
@@ -23,7 +27,11 @@
: clock_(clock),
ssrcs_(ssrcs),
video_stream_encoder_(encoder),
- time_last_intra_request_ms_(-1) {
+ time_last_intra_request_ms_(-1),
+ min_keyframe_send_interval_ms_(
+ KeyframeIntervalSettings::ParseFromFieldTrials()
+ .MinKeyframeSendIntervalMs()
+ .value_or(kMinKeyframeSendIntervalMs)) {
RTC_DCHECK(!ssrcs.empty());
}
@@ -41,7 +49,7 @@
{
int64_t now_ms = clock_->TimeInMilliseconds();
rtc::CritScope lock(&crit_);
- if (time_last_intra_request_ms_ + kMinKeyFrameRequestIntervalMs > now_ms) {
+ if (time_last_intra_request_ms_ + min_keyframe_send_interval_ms_ > now_ms) {
return;
}
time_last_intra_request_ms_ = now_ms;
diff --git a/video/encoder_key_frame_callback.h b/video/encoder_key_frame_callback.h
index 4e0ac56..9ed3c578 100644
--- a/video/encoder_key_frame_callback.h
+++ b/video/encoder_key_frame_callback.h
@@ -46,6 +46,8 @@
rtc::CriticalSection crit_;
int64_t time_last_intra_request_ms_ RTC_GUARDED_BY(crit_);
+
+ const int min_keyframe_send_interval_ms_;
};
} // namespace webrtc
diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc
index 790b7b3..bea06c0 100644
--- a/video/video_receive_stream.cc
+++ b/video/video_receive_stream.cc
@@ -39,6 +39,7 @@
#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/platform_file.h"
@@ -58,6 +59,9 @@
constexpr int kMinBaseMinimumDelayMs = 0;
constexpr int kMaxBaseMinimumDelayMs = 10000;
+constexpr int kMaxWaitForKeyFrameMs = 200;
+constexpr int kMaxWaitForFrameMs = 3000;
+
VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) {
VideoCodec codec;
memset(&codec, 0, sizeof(codec));
@@ -205,7 +209,13 @@
this, // KeyFrameRequestSender
this, // OnCompleteFrameCallback
config_.frame_decryptor),
- rtp_stream_sync_(this) {
+ 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)) {
RTC_LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString();
RTC_DCHECK(config_.renderer);
@@ -565,10 +575,9 @@
bool VideoReceiveStream::Decode() {
TRACE_EVENT0("webrtc", "VideoReceiveStream::Decode");
- static const int kMaxWaitForFrameMs = 3000;
- static const int kMaxWaitForKeyFrameMs = 200;
- int wait_ms = keyframe_required_ ? kMaxWaitForKeyFrameMs : kMaxWaitForFrameMs;
+ const int wait_ms =
+ keyframe_required_ ? max_wait_for_keyframe_ms_ : max_wait_for_frame_ms_;
std::unique_ptr<video_coding::EncodedFrame> frame;
// TODO(philipel): Call NextFrame with |keyframe_required| argument when
// downstream project has been fixed.
@@ -602,7 +611,8 @@
if (decode_result == WEBRTC_VIDEO_CODEC_OK_REQUEST_KEYFRAME)
RequestKeyFrame();
} else if (!frame_decoded_ || !keyframe_required_ ||
- (last_keyframe_request_ms_ + kMaxWaitForKeyFrameMs < now_ms)) {
+ (last_keyframe_request_ms_ + max_wait_for_keyframe_ms_ <
+ now_ms)) {
keyframe_required_ = true;
// TODO(philipel): Remove this keyframe request when downstream project
// has been fixed.
@@ -627,7 +637,7 @@
// we assume a keyframe is currently being received.
bool receiving_keyframe =
last_keyframe_packet_ms &&
- now_ms - *last_keyframe_packet_ms < kMaxWaitForKeyFrameMs;
+ now_ms - *last_keyframe_packet_ms < max_wait_for_keyframe_ms_;
if (stream_is_active && !receiving_keyframe &&
(!config_.crypto_options.sframe.require_frame_encryption ||
diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h
index 3730505..162ef8c 100644
--- a/video/video_receive_stream.h
+++ b/video/video_receive_stream.h
@@ -184,6 +184,10 @@
int64_t last_keyframe_request_ms_ = 0;
int64_t last_complete_frame_time_ms_ = 0;
+ // Keyframe request intervals are configurable through field trials.
+ const int max_wait_for_keyframe_ms_;
+ const int max_wait_for_frame_ms_;
+
rtc::CriticalSection playout_delay_lock_;
// All of them tries to change current min_playout_delay on |timing_| but
