diff --git a/api/uma_metrics.h b/api/uma_metrics.h index 4336c1edc7..51b0ff06c8 100644 --- a/api/uma_metrics.h +++ b/api/uma_metrics.h @@ -155,6 +155,14 @@ enum AddIceCandidateResult { kAddIceCandidateMax }; +// Metric for recording which api surface was used to enable simulcast. +enum SimulcastApiVersion { + kSimulcastApiVersionNone, + kSimulcastApiVersionLegacy, + kSimulcastApiVersionSpecCompliant, + kSimulcastApiVersionMax, +}; + } // namespace webrtc #endif // API_UMA_METRICS_H_ diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 6a5f925732..bcba7636af 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -104,6 +104,15 @@ const char kDtlsSrtpSetupFailureRtcp[] = namespace { +// UMA metric names. +const char kSimulcastVersionApplyLocalDescription[] = + "WebRTC.PeerConnection.Simulcast.ApplyLocalDescription"; +const char kSimulcastVersionApplyRemoteDescription[] = + "WebRTC.PeerConnection.Simulcast.ApplyRemoteDescription"; +const char kSimulcastNumberOfEncodings[] = + "WebRTC.PeerConnection.Simulcast.NumberOfSendEncodings"; +const char kSimulcastDisabled[] = "WebRTC.PeerConnection.Simulcast.Disabled"; + static const char kDefaultStreamId[] = "default"; static const char kDefaultAudioSenderId[] = "defaulta0"; static const char kDefaultVideoSenderId[] = "defaultv0"; @@ -658,6 +667,34 @@ DataMessageType ToWebrtcDataMessageType(cricket::DataMessageType type) { return DataMessageType::kControl; } +void ReportSimulcastApiVersion(const char* name, + const SessionDescription& session) { + bool has_legacy = false; + bool has_spec_compliant = false; + for (const ContentInfo& content : session.contents()) { + if (!content.description) { + continue; + } + has_spec_compliant |= content.description->HasSimulcast(); + for (const StreamParams& sp : content.description->streams()) { + has_legacy |= sp.has_ssrc_group(cricket::kSimSsrcGroupSemantics); + } + } + + if (has_legacy) { + RTC_HISTOGRAM_ENUMERATION(name, kSimulcastApiVersionLegacy, + kSimulcastApiVersionMax); + } + if (has_spec_compliant) { + RTC_HISTOGRAM_ENUMERATION(name, kSimulcastApiVersionSpecCompliant, + kSimulcastApiVersionMax); + } + if (!has_legacy && !has_spec_compliant) { + RTC_HISTOGRAM_ENUMERATION(name, kSimulcastApiVersionNone, + kSimulcastApiVersionMax); + } +} + } // namespace // Upon completion, posts a task to execute the callback of the @@ -1494,6 +1531,9 @@ PeerConnection::AddTransceiver( : cricket::MEDIA_TYPE_VIDEO)); } + RTC_HISTOGRAM_COUNTS_LINEAR(kSimulcastNumberOfEncodings, + init.send_encodings.size(), 0, 7, 8); + size_t num_rids = absl::c_count_if(init.send_encodings, [](const RtpEncodingParameters& encoding) { return !encoding.rid.empty(); @@ -2165,6 +2205,10 @@ RTCError PeerConnection::ApplyLocalDescription( // |local_description()|. RTC_DCHECK(local_description()); + // Report statistics about any use of simulcast. + ReportSimulcastApiVersion(kSimulcastVersionApplyLocalDescription, + *local_description()->description()); + if (!is_caller_) { if (remote_description()) { // Remote description was applied first, so this PC is the callee. @@ -2519,6 +2563,10 @@ RTCError PeerConnection::ApplyRemoteDescription( // |remote_description()|. RTC_DCHECK(remote_description()); + // Report statistics about any use of simulcast. + ReportSimulcastApiVersion(kSimulcastVersionApplyRemoteDescription, + *remote_description()->description()); + RTCError error = PushdownTransportDescription(cricket::CS_REMOTE, type); if (!error.ok()) { return error; @@ -3168,6 +3216,7 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, // Check if the offer indicated simulcast but the answer rejected it. // This can happen when simulcast is not supported on the remote party. if (SimulcastIsRejected(old_local_content, *media_desc)) { + RTC_HISTOGRAM_BOOLEAN(kSimulcastDisabled, true); RTCError error = DisableSimulcastInSender(transceiver->internal()->sender_internal()); if (!error.ok()) { diff --git a/pc/peer_connection_simulcast_unittest.cc b/pc/peer_connection_simulcast_unittest.cc index 6fa4bae403..3efc6cbf44 100644 --- a/pc/peer_connection_simulcast_unittest.cc +++ b/pc/peer_connection_simulcast_unittest.cc @@ -16,6 +16,7 @@ #include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/create_peerconnection_factory.h" #include "api/rtp_transceiver_interface.h" +#include "api/uma_metrics.h" #include "api/video_codecs/builtin_video_decoder_factory.h" #include "api/video_codecs/builtin_video_encoder_factory.h" #include "pc/peer_connection.h" @@ -23,6 +24,7 @@ #include "pc/test/fake_audio_capture_module.h" #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/gunit.h" +#include "system_wrappers/include/metrics.h" #include "test/gmock.h" using ::testing::Contains; @@ -33,6 +35,7 @@ using ::testing::Eq; using ::testing::Field; using ::testing::IsEmpty; using ::testing::Ne; +using ::testing::Pair; using ::testing::Property; using ::testing::SizeIs; @@ -72,6 +75,15 @@ std::vector CreateLayers(const std::vector& rids, return CreateLayers(rids, std::vector(rids.size(), active)); } +std::vector CreateLayers(int num_layers, bool active) { + rtc::UniqueStringGenerator rid_generator; + std::vector rids; + for (int i = 0; i < num_layers; ++i) { + rids.push_back(rid_generator()); + } + return CreateLayers(rids, active); +} + } // namespace namespace webrtc { @@ -163,10 +175,9 @@ class PeerConnectionSimulcastTests : public testing::Test { mcd->set_simulcast_description(simulcast); } - bool ValidateTransceiverParameters( + void ValidateTransceiverParameters( rtc::scoped_refptr transceiver, const std::vector& layers) { - bool errors_before = ::testing::Test::HasFailure(); auto parameters = transceiver->sender()->GetParameters(); std::vector result_layers; absl::c_transform(parameters.encodings, std::back_inserter(result_layers), @@ -174,15 +185,29 @@ class PeerConnectionSimulcastTests : public testing::Test { return SimulcastLayer(encoding.rid, !encoding.active); }); EXPECT_THAT(result_layers, ElementsAreArray(layers)); - // If there were errors before this code ran, we cannot tell if this - // validation succeeded or not. - return errors_before || !::testing::Test::HasFailure(); } private: rtc::scoped_refptr pc_factory_; }; +// This class is used to test the metrics emitted for simulcast. +class PeerConnectionSimulcastMetricsTests + : public PeerConnectionSimulcastTests, + public ::testing::WithParamInterface { + protected: + PeerConnectionSimulcastMetricsTests() { webrtc::metrics::Reset(); } + + std::map LocalDescriptionSamples() { + return metrics::Samples( + "WebRTC.PeerConnection.Simulcast.ApplyLocalDescription"); + } + std::map RemoteDescriptionSamples() { + return metrics::Samples( + "WebRTC.PeerConnection.Simulcast.ApplyRemoteDescription"); + } +}; + // Validates that RIDs are supported arguments when adding a transceiver. TEST_F(PeerConnectionSimulcastTests, CanCreateTransceiverWithRid) { auto pc = CreatePeerConnectionWrapper(); @@ -200,7 +225,7 @@ TEST_F(PeerConnectionSimulcastTests, CanCreateTransceiverWithSimulcast) { auto layers = CreateLayers({"f", "h", "q"}, true); auto transceiver = AddTransceiver(pc.get(), layers); ASSERT_TRUE(transceiver); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); + ValidateTransceiverParameters(transceiver, layers); } TEST_F(PeerConnectionSimulcastTests, RidsAreAutogeneratedIfNotProvided) { @@ -251,7 +276,7 @@ TEST_F(PeerConnectionSimulcastTests, SimulcastLayersRemovedFromTail) { std::copy_n(layers.begin(), kMaxSimulcastStreams, std::back_inserter(expected_layers)); auto transceiver = AddTransceiver(pc.get(), layers); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, expected_layers)); + ValidateTransceiverParameters(transceiver, expected_layers); } // Checks that an offfer to send simulcast contains a SimulcastDescription. @@ -291,8 +316,10 @@ TEST_F(PeerConnectionSimulcastTests, SimulcastLayersAreSetInSender) { auto layers = CreateLayers({"f", "h", "q"}, true); auto transceiver = AddTransceiver(local.get(), layers); auto offer = local->CreateOfferAndSetAsLocal(); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); - + { + SCOPED_TRACE("after create offer"); + ValidateTransceiverParameters(transceiver, layers); + } // Remove simulcast as the second peer connection won't support it. auto simulcast = RemoveSimulcast(offer.get()); std::string error; @@ -308,7 +335,10 @@ TEST_F(PeerConnectionSimulcastTests, SimulcastLayersAreSetInSender) { receive_layers.AddLayer(layer); } EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); + { + SCOPED_TRACE("after set remote"); + ValidateTransceiverParameters(transceiver, layers); + } } // Checks that paused Simulcast layers propagate to the sender parameters. @@ -320,7 +350,10 @@ TEST_F(PeerConnectionSimulcastTests, PausedSimulcastLayersAreDisabledInSender) { RTC_DCHECK_EQ(layers.size(), server_layers.size()); auto transceiver = AddTransceiver(local.get(), layers); auto offer = local->CreateOfferAndSetAsLocal(); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); + { + SCOPED_TRACE("after create offer"); + ValidateTransceiverParameters(transceiver, layers); + } // Remove simulcast as the second peer connection won't support it. RemoveSimulcast(offer.get()); @@ -336,7 +369,10 @@ TEST_F(PeerConnectionSimulcastTests, PausedSimulcastLayersAreDisabledInSender) { receive_layers.AddLayer(layer); } EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, server_layers)); + { + SCOPED_TRACE("after set remote"); + ValidateTransceiverParameters(transceiver, server_layers); + } } // Checks that when Simulcast is not supported by the remote party, then all @@ -362,7 +398,10 @@ TEST_F(PeerConnectionSimulcastTests, RejectedSimulcastLayersAreDeactivated) { auto expected_layers = CreateLayers({"2", "3", "4"}, true); auto transceiver = AddTransceiver(local.get(), layers); auto offer = local->CreateOfferAndSetAsLocal(); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); + { + SCOPED_TRACE("after create offer"); + ValidateTransceiverParameters(transceiver, layers); + } // Remove simulcast as the second peer connection won't support it. auto removed_simulcast = RemoveSimulcast(offer.get()); std::string error; @@ -379,7 +418,10 @@ TEST_F(PeerConnectionSimulcastTests, RejectedSimulcastLayersAreDeactivated) { } ASSERT_TRUE(mcd_answer->HasSimulcast()); EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, expected_layers)); + { + SCOPED_TRACE("after set remote"); + ValidateTransceiverParameters(transceiver, expected_layers); + } } // Checks that simulcast is set up correctly when the server sends an offer @@ -398,7 +440,7 @@ TEST_F(PeerConnectionSimulcastTests, ServerSendsOfferToReceiveSimulcast) { auto transceiver = remote->pc()->GetTransceivers()[0]; transceiver->SetDirection(RtpTransceiverDirection::kSendRecv); EXPECT_TRUE(remote->CreateAnswerAndSetAsLocal()); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); + ValidateTransceiverParameters(transceiver, layers); } // Checks that SetRemoteDescription doesn't attempt to associate a transceiver @@ -421,7 +463,7 @@ TEST_F(PeerConnectionSimulcastTests, TransceiverIsNotRecycledWithSimulcast) { auto transceiver = transceivers[1]; transceiver->SetDirection(RtpTransceiverDirection::kSendRecv); EXPECT_TRUE(remote->CreateAnswerAndSetAsLocal()); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, layers)); + ValidateTransceiverParameters(transceiver, layers); } // Checks that if the number of layers changes during negotiation, then any @@ -458,9 +500,11 @@ TEST_F(PeerConnectionSimulcastTests, auto result = transceiver->sender()->SetParameters(parameters); EXPECT_TRUE(result.ok()); - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, expected_layers)); + ValidateTransceiverParameters(transceiver, expected_layers); } +// Tests that simulcast is disabled if the RID extension is not negotiated +// regardless of if the RIDs and simulcast attribute were negotiated properly. TEST_F(PeerConnectionSimulcastTests, NegotiationDoesNotHaveRidExtension) { auto local = CreatePeerConnectionWrapper(); auto remote = CreatePeerConnectionWrapper(); @@ -492,7 +536,176 @@ TEST_F(PeerConnectionSimulcastTests, NegotiationDoesNotHaveRidExtension) { .GetAllLayers() .size()); EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &err)) << err; - EXPECT_TRUE(ValidateTransceiverParameters(transceiver, expected_layers)); + ValidateTransceiverParameters(transceiver, expected_layers); +} +// +// Checks the logged metrics when simulcast is not used. +TEST_F(PeerConnectionSimulcastMetricsTests, NoSimulcastUsageIsLogged) { + auto local = CreatePeerConnectionWrapper(); + auto remote = CreatePeerConnectionWrapper(); + auto layers = CreateLayers(0, true); + AddTransceiver(local.get(), layers); + ExchangeOfferAnswer(local.get(), remote.get(), layers); + + EXPECT_THAT(LocalDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 2))); + EXPECT_THAT(RemoteDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 2))); } +// Checks the logged metrics when spec-compliant simulcast is used. +TEST_F(PeerConnectionSimulcastMetricsTests, SpecComplianceIsLogged) { + auto local = CreatePeerConnectionWrapper(); + auto remote = CreatePeerConnectionWrapper(); + auto layers = CreateLayers(3, true); + AddTransceiver(local.get(), layers); + ExchangeOfferAnswer(local.get(), remote.get(), layers); + + // Expecting 2 invocations of each, because we have 2 peer connections. + // Only the local peer connection will be aware of simulcast. + // The remote peer connection will think that there is no simulcast. + EXPECT_THAT(LocalDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 1), + Pair(kSimulcastApiVersionSpecCompliant, 1))); + EXPECT_THAT(RemoteDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 1), + Pair(kSimulcastApiVersionSpecCompliant, 1))); +} + +// Checks the logged metrics when and incoming request to send spec-compliant +// simulcast is received from the remote party. +TEST_F(PeerConnectionSimulcastMetricsTests, IncomingSimulcastIsLogged) { + auto local = CreatePeerConnectionWrapper(); + auto remote = CreatePeerConnectionWrapper(); + auto layers = CreateLayers(3, true); + AddTransceiver(local.get(), layers); + auto offer = local->CreateOfferAndSetAsLocal(); + EXPECT_THAT(LocalDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionSpecCompliant, 1))); + + // Remove simulcast as a sender and set it up as a receiver. + RemoveSimulcast(offer.get()); + AddRequestToReceiveSimulcast(layers, offer.get()); + std::string error; + EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; + EXPECT_THAT(RemoteDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionSpecCompliant, 1))); + + auto transceiver = remote->pc()->GetTransceivers()[0]; + transceiver->SetDirection(RtpTransceiverDirection::kSendRecv); + EXPECT_TRUE(remote->CreateAnswerAndSetAsLocal()); + EXPECT_THAT(LocalDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionSpecCompliant, 2))); +} + +// Checks that a spec-compliant simulcast offer that is rejected is logged. +TEST_F(PeerConnectionSimulcastMetricsTests, RejectedSimulcastIsLogged) { + auto local = CreatePeerConnectionWrapper(); + auto remote = CreatePeerConnectionWrapper(); + auto layers = CreateLayers({"1", "2", "3"}, true); + AddTransceiver(local.get(), layers); + auto offer = local->CreateOfferAndSetAsLocal(); + EXPECT_THAT(LocalDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionSpecCompliant, 1))); + RemoveSimulcast(offer.get()); + std::string error; + EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; + EXPECT_THAT(RemoteDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 1))); + + auto answer = remote->CreateAnswerAndSetAsLocal(); + EXPECT_THAT(LocalDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 1), + Pair(kSimulcastApiVersionSpecCompliant, 1))); + EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; + EXPECT_THAT(RemoteDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 2))); +} + +// Checks the logged metrics when legacy munging simulcast technique is used. +TEST_F(PeerConnectionSimulcastMetricsTests, LegacySimulcastIsLogged) { + auto local = CreatePeerConnectionWrapper(); + auto remote = CreatePeerConnectionWrapper(); + auto layers = CreateLayers(0, true); + AddTransceiver(local.get(), layers); + auto offer = local->CreateOffer(); + // Munge the SDP to set up legacy simulcast. + const std::string end_line = "\r\n"; + std::string sdp; + offer->ToString(&sdp); + rtc::StringBuilder builder(sdp); + builder << "a=ssrc:1111 cname:slimshady" << end_line; + builder << "a=ssrc:2222 cname:slimshady" << end_line; + builder << "a=ssrc:3333 cname:slimshady" << end_line; + builder << "a=ssrc-group:SIM 1111 2222 3333" << end_line; + + SdpParseError parse_error; + auto sd = + CreateSessionDescription(SdpType::kOffer, builder.str(), &parse_error); + ASSERT_TRUE(sd) << parse_error.line << parse_error.description; + std::string error; + EXPECT_TRUE(local->SetLocalDescription(std::move(sd), &error)) << error; + EXPECT_THAT(LocalDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionLegacy, 1))); + EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; + EXPECT_THAT(RemoteDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 1))); + auto answer = remote->CreateAnswerAndSetAsLocal(); + EXPECT_THAT(LocalDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 1), + Pair(kSimulcastApiVersionLegacy, 1))); + // Legacy simulcast is not signaled in remote description. + EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; + EXPECT_THAT(RemoteDescriptionSamples(), + ElementsAre(Pair(kSimulcastApiVersionNone, 2))); +} + +// Checks that disabling simulcast is logged in the metrics. +TEST_F(PeerConnectionSimulcastMetricsTests, SimulcastDisabledIsLogged) { + auto local = CreatePeerConnectionWrapper(); + auto remote = CreatePeerConnectionWrapper(); + auto layers = CreateLayers({"1", "2", "3"}, true); + AddTransceiver(local.get(), layers); + auto offer = local->CreateOfferAndSetAsLocal(); + RemoveSimulcast(offer.get()); + std::string error; + EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &error)) << error; + auto answer = remote->CreateAnswerAndSetAsLocal(); + EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &error)) << error; + + EXPECT_EQ(1, metrics::NumSamples("WebRTC.PeerConnection.Simulcast.Disabled")); + EXPECT_EQ(1, + metrics::NumEvents("WebRTC.PeerConnection.Simulcast.Disabled", 1)); +} + +// Checks that the disabled metric is not logged if simulcast is not disabled. +TEST_F(PeerConnectionSimulcastMetricsTests, SimulcastDisabledIsNotLogged) { + auto local = CreatePeerConnectionWrapper(); + auto remote = CreatePeerConnectionWrapper(); + auto layers = CreateLayers({"1", "2", "3"}, true); + AddTransceiver(local.get(), layers); + ExchangeOfferAnswer(local.get(), remote.get(), layers); + + EXPECT_EQ(0, metrics::NumSamples("WebRTC.PeerConnection.Simulcast.Disabled")); +} + +const int kMaxLayersInMetricsTest = 8; + +// Checks that the number of send encodings is logged in a metric. +TEST_P(PeerConnectionSimulcastMetricsTests, NumberOfSendEncodingsIsLogged) { + auto local = CreatePeerConnectionWrapper(); + auto num_layers = GetParam(); + auto layers = CreateLayers(num_layers, true); + AddTransceiver(local.get(), layers); + EXPECT_EQ(1, metrics::NumSamples( + "WebRTC.PeerConnection.Simulcast.NumberOfSendEncodings")); + EXPECT_EQ(1, metrics::NumEvents( + "WebRTC.PeerConnection.Simulcast.NumberOfSendEncodings", + num_layers)); +} + +INSTANTIATE_TEST_SUITE_P(NumberOfSendEncodings, + PeerConnectionSimulcastMetricsTests, + ::testing::Range(0, kMaxLayersInMetricsTest)); + } // namespace webrtc