From 0ee8008a0d746a8295d8d63b2518cf131e3f9ea2 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Wed, 14 Aug 2019 13:16:26 +0200 Subject: [PATCH] Use struct parser for rate control trial. Bug: webrtc:9883 Change-Id: I9ec7988da2e4d88bedd9b71cae00452f531980d6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/148581 Commit-Queue: Sebastian Jansson Reviewed-by: Jonas Olsson Cr-Commit-Position: refs/heads/master@{#28856} --- rtc_base/experiments/field_trial_parser.h | 2 + rtc_base/experiments/rate_control_settings.cc | 150 ++++++------ rtc_base/experiments/rate_control_settings.h | 53 +++-- .../experiments/struct_parameters_parser.cc | 109 +++++++-- .../experiments/struct_parameters_parser.h | 222 +++++------------- .../struct_parameters_parser_unittest.cc | 49 ++-- 6 files changed, 265 insertions(+), 320 deletions(-) diff --git a/rtc_base/experiments/field_trial_parser.h b/rtc_base/experiments/field_trial_parser.h index 3c3731c6bc..997a7fd1aa 100644 --- a/rtc_base/experiments/field_trial_parser.h +++ b/rtc_base/experiments/field_trial_parser.h @@ -243,6 +243,8 @@ absl::optional ParseTypedParameter(std::string str); template <> absl::optional ParseTypedParameter(std::string str); template <> +absl::optional ParseTypedParameter(std::string str); +template <> absl::optional ParseTypedParameter(std::string str); template <> diff --git a/rtc_base/experiments/rate_control_settings.cc b/rtc_base/experiments/rate_control_settings.cc index 69e685acfd..b82a981d0d 100644 --- a/rtc_base/experiments/rate_control_settings.cc +++ b/rtc_base/experiments/rate_control_settings.cc @@ -16,7 +16,6 @@ #include #include "api/transport/field_trial_based_config.h" -#include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/logging.h" #include "rtc_base/numerics/safe_conversions.h" @@ -35,68 +34,76 @@ const char kVp9TrustedRateControllerFieldTrialName[] = const char* kVideoHysteresisFieldTrialname = "WebRTC-SimulcastUpswitchHysteresisPercent"; -const double kDefaultVideoHysteresisFactor = 1.0; const char* kScreenshareHysteresisFieldTrialname = "WebRTC-SimulcastScreenshareUpswitchHysteresisPercent"; -// Default to 35% hysteresis for simulcast screenshare. -const double kDefaultScreenshareHysteresisFactor = 1.35; bool IsEnabled(const WebRtcKeyValueConfig* const key_value_config, absl::string_view key) { return key_value_config->Lookup(key).find("Enabled") == 0; } -double ParseHysteresisFactor(const WebRtcKeyValueConfig* const key_value_config, - absl::string_view key, - double default_value) { +void ParseHysteresisFactor(const WebRtcKeyValueConfig* const key_value_config, + absl::string_view key, + double* output_value) { std::string group_name = key_value_config->Lookup(key); int percent = 0; if (!group_name.empty() && sscanf(group_name.c_str(), "%d", &percent) == 1 && percent >= 0) { - return 1.0 + (percent / 100.0); + *output_value = 1.0 + (percent / 100.0); } - return default_value; } } // namespace +constexpr char CongestionWindowConfig::kKey[]; + +std::unique_ptr CongestionWindowConfig::Parser() { + return StructParametersParser::Create("QueueSize", &queue_size_ms, // + "MinBitrate", &min_bitrate_bps); +} + +// static +CongestionWindowConfig CongestionWindowConfig::Parse(absl::string_view config) { + CongestionWindowConfig res; + res.Parser()->Parse(config); + return res; +} + +constexpr char VideoRateControlConfig::kKey[]; + +std::unique_ptr VideoRateControlConfig::Parser() { + // The empty comments ensures that each pair is on a separate line. + return StructParametersParser::Create( + "pacing_factor", &pacing_factor, // + "alr_probing", &alr_probing, // + "vp8_qp_max", &vp8_qp_max, // + "vp8_min_pixels", &vp8_min_pixels, // + "trust_vp8", &trust_vp8, // + "trust_vp9", &trust_vp9, // + "video_hysteresis", &video_hysteresis, // + "screenshare_hysteresis", &screenshare_hysteresis, // + "probe_max_allocation", &probe_max_allocation, // + "bitrate_adjuster", &bitrate_adjuster, // + "adjuster_use_headroom", &adjuster_use_headroom, // + "vp8_s0_boost", &vp8_s0_boost, // + "vp8_dynamic_rate", &vp8_dynamic_rate, // + "vp9_dynamic_rate", &vp9_dynamic_rate); +} + RateControlSettings::RateControlSettings( const WebRtcKeyValueConfig* const key_value_config) - : congestion_window_("QueueSize"), - congestion_window_pushback_("MinBitrate"), - pacing_factor_("pacing_factor"), - alr_probing_("alr_probing", false), - vp8_qp_max_("vp8_qp_max"), - vp8_min_pixels_("vp8_min_pixels"), - trust_vp8_( - "trust_vp8", - IsEnabled(key_value_config, kVp8TrustedRateControllerFieldTrialName)), - trust_vp9_( - "trust_vp9", - IsEnabled(key_value_config, kVp9TrustedRateControllerFieldTrialName)), - video_hysteresis_("video_hysteresis", - ParseHysteresisFactor(key_value_config, - kVideoHysteresisFieldTrialname, - kDefaultVideoHysteresisFactor)), - screenshare_hysteresis_( - "screenshare_hysteresis", - ParseHysteresisFactor(key_value_config, - kScreenshareHysteresisFieldTrialname, - kDefaultScreenshareHysteresisFactor)), - probe_max_allocation_("probe_max_allocation", true), - bitrate_adjuster_("bitrate_adjuster", false), - adjuster_use_headroom_("adjuster_use_headroom", false), - vp8_s0_boost_("vp8_s0_boost", true), - vp8_dynamic_rate_("vp8_dynamic_rate", false), - vp9_dynamic_rate_("vp9_dynamic_rate", false) { - ParseFieldTrial({&congestion_window_, &congestion_window_pushback_}, - key_value_config->Lookup("WebRTC-CongestionWindow")); - ParseFieldTrial( - {&pacing_factor_, &alr_probing_, &vp8_qp_max_, &vp8_min_pixels_, - &trust_vp8_, &trust_vp9_, &video_hysteresis_, &screenshare_hysteresis_, - &probe_max_allocation_, &bitrate_adjuster_, &adjuster_use_headroom_, - &vp8_s0_boost_, &vp8_dynamic_rate_, &vp9_dynamic_rate_}, - key_value_config->Lookup("WebRTC-VideoRateControl")); + : congestion_window_config_(CongestionWindowConfig::Parse( + key_value_config->Lookup(CongestionWindowConfig::kKey))) { + video_config_.trust_vp8 = + IsEnabled(key_value_config, kVp8TrustedRateControllerFieldTrialName); + video_config_.trust_vp9 = + IsEnabled(key_value_config, kVp9TrustedRateControllerFieldTrialName); + ParseHysteresisFactor(key_value_config, kVideoHysteresisFieldTrialname, + &video_config_.video_hysteresis); + ParseHysteresisFactor(key_value_config, kScreenshareHysteresisFieldTrialname, + &video_config_.screenshare_hysteresis); + video_config_.Parser()->Parse( + key_value_config->Lookup(VideoRateControlConfig::kKey)); } RateControlSettings::~RateControlSettings() = default; @@ -115,100 +122,95 @@ RateControlSettings RateControlSettings::ParseFromKeyValueConfig( } bool RateControlSettings::UseCongestionWindow() const { - return static_cast(congestion_window_); + return static_cast(congestion_window_config_.queue_size_ms); } int64_t RateControlSettings::GetCongestionWindowAdditionalTimeMs() const { - return congestion_window_.GetOptional().value_or(kDefaultAcceptedQueueMs); + return congestion_window_config_.queue_size_ms.value_or( + kDefaultAcceptedQueueMs); } bool RateControlSettings::UseCongestionWindowPushback() const { - return congestion_window_ && congestion_window_pushback_; + return congestion_window_config_.queue_size_ms && + congestion_window_config_.min_bitrate_bps; } uint32_t RateControlSettings::CongestionWindowMinPushbackTargetBitrateBps() const { - return congestion_window_pushback_.GetOptional().value_or( + return congestion_window_config_.min_bitrate_bps.value_or( kDefaultMinPushbackTargetBitrateBps); } absl::optional RateControlSettings::GetPacingFactor() const { - return pacing_factor_.GetOptional(); + return video_config_.pacing_factor; } bool RateControlSettings::UseAlrProbing() const { - return alr_probing_.Get(); + return video_config_.alr_probing; } absl::optional RateControlSettings::LibvpxVp8QpMax() const { - if (vp8_qp_max_ && (vp8_qp_max_.Value() < 0 || vp8_qp_max_.Value() > 63)) { + if (video_config_.vp8_qp_max && + (*video_config_.vp8_qp_max < 0 || *video_config_.vp8_qp_max > 63)) { RTC_LOG(LS_WARNING) << "Unsupported vp8_qp_max_ value, ignored."; return absl::nullopt; } - return vp8_qp_max_.GetOptional(); + return video_config_.vp8_qp_max; } absl::optional RateControlSettings::LibvpxVp8MinPixels() const { - if (vp8_min_pixels_ && vp8_min_pixels_.Value() < 1) { + if (video_config_.vp8_min_pixels && *video_config_.vp8_min_pixels < 1) { return absl::nullopt; } - return vp8_min_pixels_.GetOptional(); + return video_config_.vp8_min_pixels; } bool RateControlSettings::LibvpxVp8TrustedRateController() const { - return trust_vp8_.Get(); + return video_config_.trust_vp8; } bool RateControlSettings::Vp8BoostBaseLayerQuality() const { - return vp8_s0_boost_.Get(); + return video_config_.vp8_s0_boost; } bool RateControlSettings::Vp8DynamicRateSettings() const { - return vp8_dynamic_rate_.Get(); + return video_config_.vp8_dynamic_rate; } bool RateControlSettings::LibvpxVp9TrustedRateController() const { - return trust_vp9_.Get(); + return video_config_.trust_vp9; } bool RateControlSettings::Vp9DynamicRateSettings() const { - return vp9_dynamic_rate_.Get(); + return video_config_.vp9_dynamic_rate; } double RateControlSettings::GetSimulcastHysteresisFactor( VideoCodecMode mode) const { if (mode == VideoCodecMode::kScreensharing) { - return GetSimulcastScreenshareHysteresisFactor(); + return video_config_.screenshare_hysteresis; } - return GetSimulcastVideoHysteresisFactor(); + return video_config_.video_hysteresis; } double RateControlSettings::GetSimulcastHysteresisFactor( VideoEncoderConfig::ContentType content_type) const { if (content_type == VideoEncoderConfig::ContentType::kScreen) { - return GetSimulcastScreenshareHysteresisFactor(); + return video_config_.screenshare_hysteresis; } - return GetSimulcastVideoHysteresisFactor(); -} - -double RateControlSettings::GetSimulcastVideoHysteresisFactor() const { - return video_hysteresis_.Get(); -} - -double RateControlSettings::GetSimulcastScreenshareHysteresisFactor() const { - return screenshare_hysteresis_.Get(); + return video_config_.video_hysteresis; } bool RateControlSettings::TriggerProbeOnMaxAllocatedBitrateChange() const { - return probe_max_allocation_.Get(); + return video_config_.probe_max_allocation; } bool RateControlSettings::UseEncoderBitrateAdjuster() const { - return bitrate_adjuster_.Get(); + return video_config_.bitrate_adjuster; } bool RateControlSettings::BitrateAdjusterCanUseNetworkHeadroom() const { - return adjuster_use_headroom_.Get(); + return video_config_.adjuster_use_headroom; } } // namespace webrtc diff --git a/rtc_base/experiments/rate_control_settings.h b/rtc_base/experiments/rate_control_settings.h index b003bee726..0781205dc4 100644 --- a/rtc_base/experiments/rate_control_settings.h +++ b/rtc_base/experiments/rate_control_settings.h @@ -15,11 +15,39 @@ #include "api/transport/webrtc_key_value_config.h" #include "api/video_codecs/video_codec.h" #include "api/video_codecs/video_encoder_config.h" -#include "rtc_base/experiments/field_trial_parser.h" -#include "rtc_base/experiments/field_trial_units.h" +#include "rtc_base/experiments/struct_parameters_parser.h" namespace webrtc { +struct CongestionWindowConfig { + static constexpr char kKey[] = "WebRTC-CongestionWindow"; + absl::optional queue_size_ms; + absl::optional min_bitrate_bps; + std::unique_ptr Parser(); + static CongestionWindowConfig Parse(absl::string_view config); +}; + +struct VideoRateControlConfig { + static constexpr char kKey[] = "WebRTC-VideoRateControl"; + absl::optional pacing_factor; + bool alr_probing = false; + absl::optional vp8_qp_max; + absl::optional vp8_min_pixels; + bool trust_vp8 = false; + bool trust_vp9 = false; + double video_hysteresis = 1.0; + // Default to 35% hysteresis for simulcast screenshare. + double screenshare_hysteresis = 1.35; + bool probe_max_allocation = true; + bool bitrate_adjuster = false; + bool adjuster_use_headroom = false; + bool vp8_s0_boost = true; + bool vp8_dynamic_rate = false; + bool vp9_dynamic_rate = false; + + std::unique_ptr Parser(); +}; + class RateControlSettings final { public: ~RateControlSettings(); @@ -62,25 +90,8 @@ class RateControlSettings final { explicit RateControlSettings( const WebRtcKeyValueConfig* const key_value_config); - double GetSimulcastVideoHysteresisFactor() const; - double GetSimulcastScreenshareHysteresisFactor() const; - - FieldTrialOptional congestion_window_; - FieldTrialOptional congestion_window_pushback_; - FieldTrialOptional pacing_factor_; - FieldTrialParameter alr_probing_; - FieldTrialOptional vp8_qp_max_; - FieldTrialOptional vp8_min_pixels_; - FieldTrialParameter trust_vp8_; - FieldTrialParameter trust_vp9_; - FieldTrialParameter video_hysteresis_; - FieldTrialParameter screenshare_hysteresis_; - FieldTrialParameter probe_max_allocation_; - FieldTrialParameter bitrate_adjuster_; - FieldTrialParameter adjuster_use_headroom_; - FieldTrialParameter vp8_s0_boost_; - FieldTrialParameter vp8_dynamic_rate_; - FieldTrialParameter vp9_dynamic_rate_; + const CongestionWindowConfig congestion_window_config_; + VideoRateControlConfig video_config_; }; } // namespace webrtc diff --git a/rtc_base/experiments/struct_parameters_parser.cc b/rtc_base/experiments/struct_parameters_parser.cc index cef9386544..24058b50bd 100644 --- a/rtc_base/experiments/struct_parameters_parser.cc +++ b/rtc_base/experiments/struct_parameters_parser.cc @@ -9,13 +9,9 @@ */ #include "rtc_base/experiments/struct_parameters_parser.h" -#include - #include "rtc_base/logging.h" -#include "rtc_base/strings/string_builder.h" namespace webrtc { -namespace struct_parser_impl { namespace { size_t FindOrEnd(absl::string_view str, size_t start, char delimiter) { size_t pos = str.find(delimiter, start); @@ -24,44 +20,105 @@ size_t FindOrEnd(absl::string_view str, size_t start, char delimiter) { } } // namespace -void ParseConfigParams( - absl::string_view config_str, - std::map> field_map) { +namespace struct_parser_impl { +namespace { +inline void StringEncode(std::string* target, bool val) { + *target += rtc::ToString(val); +} +inline void StringEncode(std::string* target, double val) { + *target += rtc::ToString(val); +} +inline void StringEncode(std::string* target, int val) { + *target += rtc::ToString(val); +} +inline void StringEncode(std::string* target, DataRate val) { + *target += webrtc::ToString(val); +} +inline void StringEncode(std::string* target, DataSize val) { + *target += webrtc::ToString(val); +} +inline void StringEncode(std::string* target, TimeDelta val) { + *target += webrtc::ToString(val); +} + +template +inline void StringEncode(std::string* sb, absl::optional val) { + if (val) + StringEncode(sb, *val); +} +} // namespace +template +bool TypedParser::Parse(absl::string_view src, void* target) { + auto parsed = ParseTypedParameter(std::string(src)); + if (parsed.has_value()) + *reinterpret_cast(target) = *parsed; + return parsed.has_value(); +} +template +void TypedParser::Encode(const void* src, std::string* target) { + StringEncode(target, *reinterpret_cast(src)); +} + +template class TypedParser; +template class TypedParser; +template class TypedParser; +template class TypedParser>; +template class TypedParser>; + +template class TypedParser; +template class TypedParser; +template class TypedParser; +template class TypedParser>; +template class TypedParser>; +template class TypedParser>; +} // namespace struct_parser_impl + +StructParametersParser::StructParametersParser( + std::vector members) + : members_(std::move(members)) {} + +void StructParametersParser::Parse(absl::string_view src) { size_t i = 0; - while (i < config_str.length()) { - size_t val_end = FindOrEnd(config_str, i, ','); - size_t colon_pos = FindOrEnd(config_str, i, ':'); + while (i < src.length()) { + size_t val_end = FindOrEnd(src, i, ','); + size_t colon_pos = FindOrEnd(src, i, ':'); size_t key_end = std::min(val_end, colon_pos); size_t val_begin = key_end + 1u; - std::string key(config_str.substr(i, key_end - i)); + absl::string_view key(src.substr(i, key_end - i)); absl::string_view opt_value; if (val_end >= val_begin) - opt_value = config_str.substr(val_begin, val_end - val_begin); + opt_value = src.substr(val_begin, val_end - val_begin); i = val_end + 1u; - auto field = field_map.find(key); - if (field != field_map.end()) { - if (!field->second(opt_value)) { - RTC_LOG(LS_WARNING) << "Failed to read field with key: '" << key - << "' in trial: \"" << config_str << "\""; + bool found = false; + for (auto& member : members_) { + if (key == member.key) { + found = true; + if (!member.parser.parse(opt_value, member.member_ptr)) { + RTC_LOG(LS_WARNING) << "Failed to read field with key: '" << key + << "' in trial: \"" << src << "\""; + } + break; } - } else { + } + if (!found) { RTC_LOG(LS_INFO) << "No field with key: '" << key - << "' (found in trial: \"" << config_str << "\")"; + << "' (found in trial: \"" << src << "\")"; } } } -std::string EncodeStringStringMap(std::map mapping) { - rtc::StringBuilder sb; +std::string StructParametersParser::Encode() const { + std::string res; bool first = true; - for (const auto& kv : mapping) { + for (const auto& member : members_) { if (!first) - sb << ","; - sb << kv.first << ":" << kv.second; + res += ","; + res += member.key; + res += ":"; + member.parser.encode(member.member_ptr, &res); first = false; } - return sb.Release(); + return res; } -} // namespace struct_parser_impl } // namespace webrtc diff --git a/rtc_base/experiments/struct_parameters_parser.h b/rtc_base/experiments/struct_parameters_parser.h index f6728f6ea5..b40f381594 100644 --- a/rtc_base/experiments/struct_parameters_parser.h +++ b/rtc_base/experiments/struct_parameters_parser.h @@ -26,189 +26,83 @@ namespace webrtc { namespace struct_parser_impl { -inline std::string StringEncode(bool val) { - return rtc::ToString(val); -} -inline std::string StringEncode(double val) { - return rtc::ToString(val); -} -inline std::string StringEncode(int val) { - return rtc::ToString(val); -} -inline std::string StringEncode(std::string val) { - return val; -} -inline std::string StringEncode(DataRate val) { - return ToString(val); -} -inline std::string StringEncode(DataSize val) { - return ToString(val); -} -inline std::string StringEncode(TimeDelta val) { - return ToString(val); -} +struct TypedMemberParser { + public: + bool (*parse)(const absl::string_view src, void* target); + void (*encode)(const void* src, std::string* target); +}; + +struct MemberParameter { + const char* key; + void* member_ptr; + TypedMemberParser parser; +}; template -inline std::string StringEncode(absl::optional val) { - if (val) - return StringEncode(*val); - return ""; -} +class TypedParser { + public: + static bool Parse(absl::string_view src, void* target); + static void Encode(const void* src, std::string* target); +}; + +// Instantiated in cc file to avoid duplication during compile. Add additional +// parsers as needed. Generally, try to use these suggested types even if the +// context where the value is used might require a different type. For instance, +// a size_t representing a packet size should use an int parameter as there's no +// need to support packet sizes larger than INT32_MAX. +extern template class TypedParser; +extern template class TypedParser; +extern template class TypedParser; +extern template class TypedParser>; +extern template class TypedParser>; + +extern template class TypedParser; +extern template class TypedParser; +extern template class TypedParser; +extern template class TypedParser>; +extern template class TypedParser>; +extern template class TypedParser>; template -struct LambdaTraits : public LambdaTraits {}; - -template -struct LambdaTraits { - using ret = RetType; - using src = SourceType; -}; - -void ParseConfigParams( - absl::string_view config_str, - std::map> field_map); - -std::string EncodeStringStringMap(std::map mapping); - -template -class StructParameterParser { - public: - virtual bool Parse(absl::string_view src, StructType* target) const = 0; - virtual bool Changed(const StructType& src, const StructType& base) const = 0; - virtual std::string Encode(const StructType& src) const = 0; - virtual ~StructParameterParser() = default; -}; - -template -class StructParameterImpl : public StructParameterParser { - public: - explicit StructParameterImpl(std::function field_getter) - : field_getter_(std::move(field_getter)) {} - bool Parse(absl::string_view src, StructType* target) const override { - auto parsed = ParseTypedParameter(std::string(src)); - if (parsed.has_value()) - *field_getter_(target) = *parsed; - return parsed.has_value(); - } - bool Changed(const StructType& src, const StructType& base) const override { - T base_value = *field_getter_(const_cast(&base)); - T value = *field_getter_(const_cast(&src)); - return value != base_value; - } - std::string Encode(const StructType& src) const override { - T value = *field_getter_(const_cast(&src)); - return struct_parser_impl::StringEncode(value); - } - - private: - const std::function field_getter_; -}; - -template -struct StructParameter { - std::string key; - StructParameterParser* parser; -}; - -template ::ret> -void AddParameters(std::vector>* out, - std::string key, - Closure getter) { - auto* parser = new StructParameterImpl(getter); - out->push_back(StructParameter{std::move(key), parser}); +void AddMembers(MemberParameter* out, const char* key, T* member) { + *out = MemberParameter{ + key, member, + TypedMemberParser{&TypedParser::Parse, &TypedParser::Encode}}; } -template ::ret, - typename... Args> -void AddParameters(std::vector>* out, - std::string key, - Closure getter, - Args... args) { - AddParameters(out, key, getter); - AddParameters(out, args...); +template +void AddMembers(MemberParameter* out, + const char* key, + T* member, + Args... args) { + AddMembers(out, key, member); + AddMembers(++out, args...); } - } // namespace struct_parser_impl -template class StructParametersParser { public: - ~StructParametersParser() { - for (auto& param : parameters_) { - delete param.parser; - } + template + static std::unique_ptr Create(const char* first_key, + T* first_member, + Args... args) { + std::vector members( + sizeof...(args) / 2 + 1); + struct_parser_impl::AddMembers(&members.front(), std::move(first_key), + first_member, args...); + return absl::WrapUnique(new StructParametersParser(std::move(members))); } - void Parse(StructType* target, absl::string_view src) { - std::map> field_parsers; - for (const auto& param : parameters_) { - field_parsers.emplace(param.key, [target, param](absl::string_view src) { - return param.parser->Parse(src, target); - }); - } - struct_parser_impl::ParseConfigParams(src, std::move(field_parsers)); - } - - StructType Parse(absl::string_view src) { - StructType res; - Parse(&res, src); - return res; - } - - std::string EncodeChanged(const StructType& src) { - static StructType base; - std::map pairs; - for (const auto& param : parameters_) { - if (param.parser->Changed(src, base)) - pairs[param.key] = param.parser->Encode(src); - } - return struct_parser_impl::EncodeStringStringMap(pairs); - } - - std::string EncodeAll(const StructType& src) { - std::map pairs; - for (const auto& param : parameters_) { - pairs[param.key] = param.parser->Encode(src); - } - return struct_parser_impl::EncodeStringStringMap(pairs); - } + void Parse(absl::string_view src); + std::string Encode() const; private: - template - friend std::unique_ptr> - CreateStructParametersParser(std::string, C, Args...); - explicit StructParametersParser( - std::vector> parameters) - : parameters_(parameters) {} + std::vector members); - std::vector> parameters_; + std::vector members_; }; -// Creates a struct parameters parser based on interleaved key and field -// accessor arguments, where the field accessor converts a struct pointer to a -// member pointer: FieldType*(StructType*). See the unit tests for example -// usage. Note that the struct type is inferred from the field getters. Beware -// of providing incorrect arguments to this, such as mixing the struct type or -// incorrect return values, as this will cause very confusing compile errors. -template ::src, - typename... Args> -std::unique_ptr> CreateStructParametersParser( - std::string first_key, - Closure first_getter, - Args... args) { - std::vector> parameters; - struct_parser_impl::AddParameters(¶meters, std::move(first_key), - first_getter, args...); - // absl::make_unique can't be used since the StructParametersParser - // constructor is only visible to this create function. - return absl::WrapUnique(new StructParametersParser(std::move(parameters))); -} } // namespace webrtc #endif // RTC_BASE_EXPERIMENTS_STRUCT_PARAMETERS_PARSER_H_ diff --git a/rtc_base/experiments/struct_parameters_parser_unittest.cc b/rtc_base/experiments/struct_parameters_parser_unittest.cc index 1e88f8cfc1..69103bd046 100644 --- a/rtc_base/experiments/struct_parameters_parser_unittest.cc +++ b/rtc_base/experiments/struct_parameters_parser_unittest.cc @@ -17,30 +17,25 @@ struct DummyConfig { double factor = 0.5; int retries = 5; bool ping = 0; - std::string hash = "a80"; absl::optional duration; absl::optional latency = TimeDelta::ms(100); - static StructParametersParser* Parser(); + std::unique_ptr Parser(); }; -StructParametersParser* DummyConfig::Parser() { - using C = DummyConfig; +std::unique_ptr DummyConfig::Parser() { // The empty comments ensures that each pair is on a separate line. - static auto parser = CreateStructParametersParser( - "e", [](C* c) { return &c->enabled; }, // - "f", [](C* c) { return &c->factor; }, // - "r", [](C* c) { return &c->retries; }, // - "p", [](C* c) { return &c->ping; }, // - "h", [](C* c) { return &c->hash; }, // - "d", [](C* c) { return &c->duration; }, // - "l", [](C* c) { return &c->latency; }); // - return parser.get(); + return StructParametersParser::Create("e", &enabled, // + "f", &factor, // + "r", &retries, // + "p", &ping, // + "d", &duration, // + "l", &latency); } } // namespace TEST(StructParametersParserTest, ParsesValidParameters) { - DummyConfig exp = - DummyConfig::Parser()->Parse("e:1,f:-1.7,r:2,p:1,h:x7c,d:8,l:,"); + DummyConfig exp; + exp.Parser()->Parse("e:1,f:-1.7,r:2,p:1,d:8,l:,"); EXPECT_TRUE(exp.enabled); EXPECT_EQ(exp.factor, -1.7); EXPECT_EQ(exp.retries, 2); @@ -50,35 +45,19 @@ TEST(StructParametersParserTest, ParsesValidParameters) { } TEST(StructParametersParserTest, UsesDefaults) { - DummyConfig exp = DummyConfig::Parser()->Parse(""); + DummyConfig exp; + exp.Parser()->Parse(""); EXPECT_FALSE(exp.enabled); EXPECT_EQ(exp.factor, 0.5); EXPECT_EQ(exp.retries, 5); EXPECT_EQ(exp.ping, false); - EXPECT_EQ(exp.hash, "a80"); -} - -TEST(StructParametersParserTest, EmptyDefaults) { - DummyConfig exp; - auto encoded = DummyConfig::Parser()->EncodeChanged(exp); - // Unchanged parameters are not encoded. - EXPECT_EQ(encoded, ""); } TEST(StructParametersParserTest, EncodeAll) { DummyConfig exp; - auto encoded = DummyConfig::Parser()->EncodeAll(exp); + auto encoded = exp.Parser()->Encode(); // All parameters are encoded. - EXPECT_EQ(encoded, "d:,e:false,f:0.5,h:a80,l:100 ms,p:false,r:5"); -} - -TEST(StructParametersParserTest, EncodeChanged) { - DummyConfig exp; - exp.ping = true; - exp.retries = 4; - auto encoded = DummyConfig::Parser()->EncodeChanged(exp); - // We expect the changed parameters to be encoded in alphabetical order. - EXPECT_EQ(encoded, "p:true,r:4"); + EXPECT_EQ(encoded, "e:false,f:0.5,r:5,p:false,d:,l:100 ms"); } } // namespace webrtc