diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn index afe174b2ec..b8709dc25b 100644 --- a/rtc_base/experiments/BUILD.gn +++ b/rtc_base/experiments/BUILD.gn @@ -132,6 +132,19 @@ rtc_static_library("rate_control_settings") { ] } +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 @@ if (rtc_include_tests) { "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 @@ if (rtc_include_tests) { 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 c31e0209ea..5b34315e54 100644 --- a/rtc_base/experiments/OWNERS +++ b/rtc_base/experiments/OWNERS @@ -4,6 +4,7 @@ per-file congestion_controller_experiment*=srte@webrtc.org 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 0000000000..2f19a1c53f --- /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 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 new file mode 100644 index 0000000000..7c8d6d364a --- /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 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 + +#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 0000000000..5457dc4d79 --- /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 0e02401c61..8f89e221ac 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -99,6 +99,7 @@ rtc_static_library("video") { "../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 447a3af955..3116221da2 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& ssrcs, @@ -23,7 +27,11 @@ EncoderKeyFrameCallback::EncoderKeyFrameCallback( : 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 @@ void EncoderKeyFrameCallback::OnReceivedIntraFrameRequest(uint32_t ssrc) { { 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 4e0ac56405..9ed3c578a8 100644 --- a/video/encoder_key_frame_callback.h +++ b/video/encoder_key_frame_callback.h @@ -46,6 +46,8 @@ class EncoderKeyFrameCallback : public RtcpIntraFrameObserver, 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 790b7b3074..bea06c0853 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 @@ namespace { 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 @@ VideoReceiveStream::VideoReceiveStream( 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 @@ void VideoReceiveStream::DecodeThreadFunction(void* ptr) { 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 frame; // TODO(philipel): Call NextFrame with |keyframe_required| argument when // downstream project has been fixed. @@ -602,7 +611,8 @@ bool VideoReceiveStream::Decode() { 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 @@ bool VideoReceiveStream::Decode() { // 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 3730505e11..162ef8c8ee 100644 --- a/video/video_receive_stream.h +++ b/video/video_receive_stream.h @@ -184,6 +184,10 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, 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