diff --git a/api/stats/attribute.h b/api/stats/attribute.h index dd21a985df..fdedc74082 100644 --- a/api/stats/attribute.h +++ b/api/stats/attribute.h @@ -74,11 +74,8 @@ class RTC_EXPORT Attribute { bool is_sequence() const; bool is_string() const; - // Converts the attribute to a string that is parseable as a JSON object. - std::string ToString() const; - // TODO(https://crbug.com/15164): Use ToString() instead and delete these. - std::string ValueToString() const { return ToString(); } - std::string ValueToJson() const { return ToString(); } + std::string ValueToString() const; + std::string ValueToJson() const; bool operator==(const Attribute& other) const; bool operator!=(const Attribute& other) const; diff --git a/api/stats/rtc_stats_member.h b/api/stats/rtc_stats_member.h index bf9cc0f400..d2aa539cf5 100644 --- a/api/stats/rtc_stats_member.h +++ b/api/stats/rtc_stats_member.h @@ -65,6 +65,13 @@ class RTCStatsMemberInterface { bool operator!=(const RTCStatsMemberInterface& other) const { return !(*this == other); } + virtual std::string ValueToString() const = 0; + // This is the same as ValueToString except for kInt64 and kUint64 types, + // where the value is represented as a double instead of as an integer. + // Since JSON stores numbers as floating point numbers, very large integers + // cannot be accurately represented, so we prefer to display them as doubles + // instead. + virtual std::string ValueToJson() const = 0; virtual const RTCStatsMemberInterface* member_ptr() const { return this; } template @@ -91,6 +98,8 @@ class RTCStatsMember : public RTCStatsMemberInterface { bool is_sequence() const override; bool is_string() const override; bool is_defined() const override { return value_.has_value(); } + std::string ValueToString() const override; + std::string ValueToJson() const override; template inline T value_or(U default_value) const { @@ -161,6 +170,10 @@ typedef std::map MapStringDouble; RTC_EXPORT bool RTCStatsMember::is_sequence() const; \ template <> \ RTC_EXPORT bool RTCStatsMember::is_string() const; \ + template <> \ + RTC_EXPORT std::string RTCStatsMember::ValueToString() const; \ + template <> \ + RTC_EXPORT std::string RTCStatsMember::ValueToJson() const; \ extern template class RTC_EXPORT_TEMPLATE_DECLARE(RTC_EXPORT) \ RTCStatsMember diff --git a/pc/peer_connection_rampup_tests.cc b/pc/peer_connection_rampup_tests.cc index e45821b31f..0fd3c27f7d 100644 --- a/pc/peer_connection_rampup_tests.cc +++ b/pc/peer_connection_rampup_tests.cc @@ -313,9 +313,7 @@ class PeerConnectionRampUpTest : public ::testing::Test { return 0; } std::string selected_ice_id = - transport_stats[0] - ->GetAttribute(transport_stats[0]->selected_candidate_pair_id) - .ToString(); + transport_stats[0]->selected_candidate_pair_id.ValueToString(); // Use the selected ICE candidate pair ID to get the appropriate ICE stats. const RTCIceCandidatePairStats ice_candidate_pair_stats = stats->Get(selected_ice_id)->cast_to(); diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc index 002f9d34b5..762dad6fcd 100644 --- a/pc/rtc_stats_integrationtest.cc +++ b/pc/rtc_stats_integrationtest.cc @@ -228,10 +228,9 @@ class RTCStatsVerifier { template void TestAttributeIsUndefined(const RTCStatsMember& field) { - Attribute attribute = stats_->GetAttribute(field); EXPECT_FALSE(field.has_value()) - << stats_->type() << "." << attribute.name() << "[" << stats_->id() - << "] was defined (" << attribute.ToString() << ")."; + << stats_->type() << "." << stats_->GetAttribute(field).name() << "[" + << stats_->id() << "] was defined (" << field.ValueToString() << ")."; MarkAttributeTested(field, !field.has_value()); } @@ -247,7 +246,7 @@ class RTCStatsVerifier { bool is_positive = field.value() > T(0); EXPECT_TRUE(is_positive) << stats_->type() << "." << attribute.name() << "[" << stats_->id() - << "] was not positive (" << attribute.ToString() << ")."; + << "] was not positive (" << attribute.ValueToString() << ")."; MarkAttributeTested(field, is_positive); } @@ -263,7 +262,7 @@ class RTCStatsVerifier { bool is_non_negative = field.value() >= T(0); EXPECT_TRUE(is_non_negative) << stats_->type() << "." << attribute.name() << "[" << stats_->id() - << "] was not non-negative (" << attribute.ToString() << ")."; + << "] was not non-negative (" << attribute.ValueToString() << ")."; MarkAttributeTested(field, is_non_negative); } @@ -324,7 +323,8 @@ class RTCStatsVerifier { << stats_->type() << "." << attribute.name() << " is not a reference to an " "existing dictionary of type " - << expected_type << " (value: " << attribute.ToString() << ")."; + << expected_type << " (value: " + << (field.has_value() ? attribute.ValueToString() : "null") << ")."; MarkAttributeTested(field, valid_reference); } diff --git a/stats/attribute.cc b/stats/attribute.cc index fab948b1bd..76956e25b8 100644 --- a/stats/attribute.cc +++ b/stats/attribute.cc @@ -10,13 +10,8 @@ #include "api/stats/attribute.h" -#include - #include "absl/types/variant.h" -#include "rtc_base/arraysize.h" #include "rtc_base/checks.h" -#include "rtc_base/string_encode.h" -#include "rtc_base/strings/string_builder.h" namespace webrtc { @@ -35,82 +30,6 @@ struct VisitIsSequence { } }; -// Converts the attribute to string in a JSON-compatible way. -struct VisitToString { - template || std::is_same_v || - std::is_same_v || std::is_same_v, - bool> = true> - std::string ValueToString(const T& value) { - return rtc::ToString(value); - } - // Convert 64-bit integers to doubles before converting to string because JSON - // represents all numbers as floating points with ~15 digits of precision. - template || - std::is_same_v || - std::is_same_v, - bool> = true> - std::string ValueToString(const T& value) { - char buf[32]; - const int len = std::snprintf(&buf[0], arraysize(buf), "%.16g", - static_cast(value)); - RTC_DCHECK_LE(len, arraysize(buf)); - return std::string(&buf[0], len); - } - - // Vector attributes. - template - std::string operator()(const RTCStatsMember>* attribute) { - rtc::StringBuilder sb; - sb << "["; - const char* separator = ""; - constexpr bool element_is_string = std::is_same::value; - for (const T& element : attribute->value()) { - sb << separator; - if (element_is_string) { - sb << "\""; - } - sb << ValueToString(element); - if (element_is_string) { - sb << "\""; - } - separator = ","; - } - sb << "]"; - return sb.Release(); - } - // Map attributes. - template - std::string operator()( - const RTCStatsMember>* attribute) { - rtc::StringBuilder sb; - sb << "{"; - const char* separator = ""; - constexpr bool element_is_string = std::is_same::value; - for (const auto& pair : attribute->value()) { - sb << separator; - sb << "\"" << pair.first << "\":"; - if (element_is_string) { - sb << "\""; - } - sb << ValueToString(pair.second); - if (element_is_string) { - sb << "\""; - } - separator = ","; - } - sb << "}"; - return sb.Release(); - } - // Simple attributes. - template - std::string operator()(const RTCStatsMember* attribute) { - return ValueToString(attribute->value()); - } -}; - struct VisitIsEqual { template bool operator()(const RTCStatsMember* attribute) { @@ -150,11 +69,14 @@ bool Attribute::is_string() const { attribute_); } -std::string Attribute::ToString() const { - if (!has_value()) { - return "null"; - } - return absl::visit(VisitToString(), attribute_); +std::string Attribute::ValueToString() const { + return absl::visit([](const auto* attr) { return attr->ValueToString(); }, + attribute_); +} + +std::string Attribute::ValueToJson() const { + return absl::visit([](const auto* attr) { return attr->ValueToJson(); }, + attribute_); } bool Attribute::operator==(const Attribute& other) const { diff --git a/stats/rtc_stats.cc b/stats/rtc_stats.cc index e7d72ee3a3..ea87379b95 100644 --- a/stats/rtc_stats.cc +++ b/stats/rtc_stats.cc @@ -54,7 +54,8 @@ std::string RTCStats::ToJson() const { if (attribute.holds_alternative()) { sb << "\""; } - sb << attribute.ToString(); + sb << absl::visit([](const auto* attr) { return attr->ValueToJson(); }, + attribute.as_variant()); if (attribute.holds_alternative()) { sb << "\""; } diff --git a/stats/rtc_stats_member.cc b/stats/rtc_stats_member.cc index 3f91988ed0..d7a3f6ccd2 100644 --- a/stats/rtc_stats_member.cc +++ b/stats/rtc_stats_member.cc @@ -10,53 +10,231 @@ #include "api/stats/rtc_stats_member.h" +#include "rtc_base/arraysize.h" +#include "rtc_base/strings/string_builder.h" + namespace webrtc { -#define WEBRTC_DEFINE_RTCSTATSMEMBER(T, type, is_seq, is_str) \ - template <> \ - RTCStatsMemberInterface::Type RTCStatsMember::StaticType() { \ - return type; \ - } \ - template <> \ - bool RTCStatsMember::is_sequence() const { \ - return is_seq; \ - } \ - template <> \ - bool RTCStatsMember::is_string() const { \ - return is_str; \ - } \ +namespace { + +// Produces "[a,b,c]". Works for non-vector `RTCStatsMemberInterface::Type` +// types. +template +std::string VectorToString(const std::vector& vector) { + rtc::StringBuilder sb; + sb << "["; + const char* separator = ""; + for (const T& element : vector) { + sb << separator << rtc::ToString(element); + separator = ","; + } + sb << "]"; + return sb.Release(); +} + +// This overload is required because std::vector range loops don't +// return references but objects, causing -Wrange-loop-analysis diagnostics. +std::string VectorToString(const std::vector& vector) { + rtc::StringBuilder sb; + sb << "["; + const char* separator = ""; + for (bool element : vector) { + sb << separator << rtc::ToString(element); + separator = ","; + } + sb << "]"; + return sb.Release(); +} + +// Produces "[\"a\",\"b\",\"c\"]". Works for vectors of both const char* and +// std::string element types. +template +std::string VectorOfStringsToString(const std::vector& strings) { + rtc::StringBuilder sb; + sb << "["; + const char* separator = ""; + for (const T& element : strings) { + sb << separator << "\"" << rtc::ToString(element) << "\""; + separator = ","; + } + sb << "]"; + return sb.Release(); +} + +template +std::string MapToString(const std::map& map) { + rtc::StringBuilder sb; + sb << "{"; + const char* separator = ""; + for (const auto& element : map) { + sb << separator << rtc::ToString(element.first) << ":" + << rtc::ToString(element.second); + separator = ","; + } + sb << "}"; + return sb.Release(); +} + +template +std::string ToStringAsDouble(const T value) { + // JSON represents numbers as floating point numbers with about 15 decimal + // digits of precision. + char buf[32]; + const int len = std::snprintf(&buf[0], arraysize(buf), "%.16g", + static_cast(value)); + RTC_DCHECK_LE(len, arraysize(buf)); + return std::string(&buf[0], len); +} + +template +std::string VectorToStringAsDouble(const std::vector& vector) { + rtc::StringBuilder sb; + sb << "["; + const char* separator = ""; + for (const T& element : vector) { + sb << separator << ToStringAsDouble(element); + separator = ","; + } + sb << "]"; + return sb.Release(); +} + +template +std::string MapToStringAsDouble(const std::map& map) { + rtc::StringBuilder sb; + sb << "{"; + const char* separator = ""; + for (const auto& element : map) { + sb << separator << "\"" << rtc::ToString(element.first) + << "\":" << ToStringAsDouble(element.second); + separator = ","; + } + sb << "}"; + return sb.Release(); +} + +} // namespace + +#define WEBRTC_DEFINE_RTCSTATSMEMBER(T, type, is_seq, is_str, to_str, to_json) \ + template <> \ + RTCStatsMemberInterface::Type RTCStatsMember::StaticType() { \ + return type; \ + } \ + template <> \ + bool RTCStatsMember::is_sequence() const { \ + return is_seq; \ + } \ + template <> \ + bool RTCStatsMember::is_string() const { \ + return is_str; \ + } \ + template <> \ + std::string RTCStatsMember::ValueToString() const { \ + RTC_DCHECK(value_.has_value()); \ + return to_str; \ + } \ + template <> \ + std::string RTCStatsMember::ValueToJson() const { \ + RTC_DCHECK(value_.has_value()); \ + return to_json; \ + } \ template class RTC_EXPORT_TEMPLATE_DEFINE(RTC_EXPORT) RTCStatsMember -WEBRTC_DEFINE_RTCSTATSMEMBER(bool, kBool, false, false); -WEBRTC_DEFINE_RTCSTATSMEMBER(int32_t, kInt32, false, false); -WEBRTC_DEFINE_RTCSTATSMEMBER(uint32_t, kUint32, false, false); -WEBRTC_DEFINE_RTCSTATSMEMBER(int64_t, kInt64, false, false); -WEBRTC_DEFINE_RTCSTATSMEMBER(uint64_t, kUint64, false, false); -WEBRTC_DEFINE_RTCSTATSMEMBER(double, kDouble, false, false); -WEBRTC_DEFINE_RTCSTATSMEMBER(std::string, kString, false, true); -WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, kSequenceBool, true, false); -WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, kSequenceInt32, true, false); +WEBRTC_DEFINE_RTCSTATSMEMBER(bool, + kBool, + false, + false, + rtc::ToString(*value_), + rtc::ToString(*value_)); +WEBRTC_DEFINE_RTCSTATSMEMBER(int32_t, + kInt32, + false, + false, + rtc::ToString(*value_), + rtc::ToString(*value_)); +WEBRTC_DEFINE_RTCSTATSMEMBER(uint32_t, + kUint32, + false, + false, + rtc::ToString(*value_), + rtc::ToString(*value_)); +WEBRTC_DEFINE_RTCSTATSMEMBER(int64_t, + kInt64, + false, + false, + rtc::ToString(*value_), + ToStringAsDouble(*value_)); +WEBRTC_DEFINE_RTCSTATSMEMBER(uint64_t, + kUint64, + false, + false, + rtc::ToString(*value_), + ToStringAsDouble(*value_)); +WEBRTC_DEFINE_RTCSTATSMEMBER(double, + kDouble, + false, + false, + rtc::ToString(*value_), + ToStringAsDouble(*value_)); +WEBRTC_DEFINE_RTCSTATSMEMBER(std::string, + kString, + false, + true, + *value_, + *value_); +WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, + kSequenceBool, + true, + false, + VectorToString(*value_), + VectorToString(*value_)); +WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, + kSequenceInt32, + true, + false, + VectorToString(*value_), + VectorToString(*value_)); WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, kSequenceUint32, true, - false); -WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, kSequenceInt64, true, false); + false, + VectorToString(*value_), + VectorToString(*value_)); +WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, + kSequenceInt64, + true, + false, + VectorToString(*value_), + VectorToStringAsDouble(*value_)); WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, kSequenceUint64, true, - false); -WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, kSequenceDouble, true, false); + false, + VectorToString(*value_), + VectorToStringAsDouble(*value_)); +WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, + kSequenceDouble, + true, + false, + VectorToString(*value_), + VectorToStringAsDouble(*value_)); WEBRTC_DEFINE_RTCSTATSMEMBER(std::vector, kSequenceString, true, - false); + false, + VectorOfStringsToString(*value_), + VectorOfStringsToString(*value_)); WEBRTC_DEFINE_RTCSTATSMEMBER(rtc_stats_internal::MapStringUint64, kMapStringUint64, false, - false); + false, + MapToString(*value_), + MapToStringAsDouble(*value_)); WEBRTC_DEFINE_RTCSTATSMEMBER(rtc_stats_internal::MapStringDouble, kMapStringDouble, false, - false); + false, + MapToString(*value_), + MapToStringAsDouble(*value_)); } // namespace webrtc diff --git a/stats/rtc_stats_unittest.cc b/stats/rtc_stats_unittest.cc index 40b05d79db..8837da7ce9 100644 --- a/stats/rtc_stats_unittest.cc +++ b/stats/rtc_stats_unittest.cc @@ -459,50 +459,45 @@ TEST(RTCStatsTest, IsString) { EXPECT_FALSE(stats.m_map_string_double.is_string()); } -TEST(RTCStatsTest, AttributeToString) { +TEST(RTCStatsTest, ValueToString) { RTCTestStats stats("statsId", Timestamp::Micros(42)); stats.m_bool = true; - EXPECT_EQ("true", stats.GetAttribute(stats.m_bool).ToString()); + EXPECT_EQ("true", stats.m_bool.ValueToString()); stats.m_string = "foo"; - EXPECT_EQ("foo", stats.GetAttribute(stats.m_string).ToString()); + EXPECT_EQ("foo", stats.m_string.ValueToString()); stats.m_int32 = -32; - EXPECT_EQ("-32", stats.GetAttribute(stats.m_int32).ToString()); + EXPECT_EQ("-32", stats.m_int32.ValueToString()); stats.m_uint32 = 32; - EXPECT_EQ("32", stats.GetAttribute(stats.m_uint32).ToString()); + EXPECT_EQ("32", stats.m_uint32.ValueToString()); stats.m_int64 = -64; - EXPECT_EQ("-64", stats.GetAttribute(stats.m_int64).ToString()); + EXPECT_EQ("-64", stats.m_int64.ValueToString()); stats.m_uint64 = 64; - EXPECT_EQ("64", stats.GetAttribute(stats.m_uint64).ToString()); + EXPECT_EQ("64", stats.m_uint64.ValueToString()); stats.m_double = 0.5; - EXPECT_EQ("0.5", stats.GetAttribute(stats.m_double).ToString()); + EXPECT_EQ("0.5", stats.m_double.ValueToString()); stats.m_sequence_bool = {true, false}; - EXPECT_EQ("[true,false]", - stats.GetAttribute(stats.m_sequence_bool).ToString()); + EXPECT_EQ("[true,false]", stats.m_sequence_bool.ValueToString()); stats.m_sequence_int32 = {-32, 32}; - EXPECT_EQ("[-32,32]", stats.GetAttribute(stats.m_sequence_int32).ToString()); + EXPECT_EQ("[-32,32]", stats.m_sequence_int32.ValueToString()); stats.m_sequence_uint32 = {64, 32}; - EXPECT_EQ("[64,32]", stats.GetAttribute(stats.m_sequence_uint32).ToString()); + EXPECT_EQ("[64,32]", stats.m_sequence_uint32.ValueToString()); stats.m_sequence_int64 = {-64, 32}; - EXPECT_EQ("[-64,32]", stats.GetAttribute(stats.m_sequence_int64).ToString()); + EXPECT_EQ("[-64,32]", stats.m_sequence_int64.ValueToString()); stats.m_sequence_uint64 = {16, 32}; - EXPECT_EQ("[16,32]", stats.GetAttribute(stats.m_sequence_uint64).ToString()); + EXPECT_EQ("[16,32]", stats.m_sequence_uint64.ValueToString()); stats.m_sequence_double = {0.5, 0.25}; - EXPECT_EQ("[0.5,0.25]", - stats.GetAttribute(stats.m_sequence_double).ToString()); + EXPECT_EQ("[0.5,0.25]", stats.m_sequence_double.ValueToString()); stats.m_sequence_string = {"foo", "bar"}; - EXPECT_EQ("[\"foo\",\"bar\"]", - stats.GetAttribute(stats.m_sequence_string).ToString()); + EXPECT_EQ("[\"foo\",\"bar\"]", stats.m_sequence_string.ValueToString()); stats.m_map_string_uint64 = std::map(); stats.m_map_string_uint64->emplace("foo", 32); stats.m_map_string_uint64->emplace("bar", 64); - EXPECT_EQ("{\"bar\":64,\"foo\":32}", - stats.GetAttribute(stats.m_map_string_uint64).ToString()); + EXPECT_EQ("{bar:64,foo:32}", stats.m_map_string_uint64.ValueToString()); stats.m_map_string_double = std::map(); stats.m_map_string_double->emplace("foo", 0.5); stats.m_map_string_double->emplace("bar", 0.25); - EXPECT_EQ("{\"bar\":0.25,\"foo\":0.5}", - stats.GetAttribute(stats.m_map_string_double).ToString()); + EXPECT_EQ("{bar:0.25,foo:0.5}", stats.m_map_string_double.ValueToString()); } // Death tests. diff --git a/test/pc/e2e/analyzer/video/video_quality_metrics_reporter.cc b/test/pc/e2e/analyzer/video/video_quality_metrics_reporter.cc index e66624127d..3ae7fda04b 100644 --- a/test/pc/e2e/analyzer/video/video_quality_metrics_reporter.cc +++ b/test/pc/e2e/analyzer/video/video_quality_metrics_reporter.cc @@ -63,9 +63,7 @@ void VideoQualityMetricsReporter::OnStatsReports( } RTC_DCHECK_EQ(transport_stats.size(), 1); std::string selected_ice_id = - transport_stats[0] - ->GetAttribute(transport_stats[0]->selected_candidate_pair_id) - .ToString(); + transport_stats[0]->selected_candidate_pair_id.ValueToString(); // Use the selected ICE candidate pair ID to get the appropriate ICE stats. const RTCIceCandidatePairStats ice_candidate_pair_stats = report->Get(selected_ice_id)->cast_to();