diff --git a/pc/peer_connection_simulcast_unittest.cc b/pc/peer_connection_simulcast_unittest.cc index 1adb8a52e0..6b6a96c473 100644 --- a/pc/peer_connection_simulcast_unittest.cc +++ b/pc/peer_connection_simulcast_unittest.cc @@ -221,15 +221,6 @@ class PeerConnectionSimulcastMetricsTests 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"); - } }; #endif @@ -614,156 +605,6 @@ TEST_F(PeerConnectionSimulcastTests, SimulcastSldModificationRejected) { } #if RTC_METRICS_ENABLED -// -// 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->SetDirectionWithError(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; diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 4874444eae..a9e45d03b1 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -109,13 +109,6 @@ const char kSdpWithoutSdesCrypto[] = "Called with SDP without SDES crypto."; const char kSessionError[] = "Session error code: "; const char kSessionErrorDesc[] = "Session error description: "; -// UMA metric names. -const char kSimulcastVersionApplyLocalDescription[] = - "WebRTC.PeerConnection.Simulcast.ApplyLocalDescription"; -const char kSimulcastVersionApplyRemoteDescription[] = - "WebRTC.PeerConnection.Simulcast.ApplyRemoteDescription"; -const char kSimulcastDisabled[] = "WebRTC.PeerConnection.Simulcast.Disabled"; - // The length of RTCP CNAMEs. static const int kRtcpCnameLength = 16; @@ -201,34 +194,6 @@ std::string GetStreamIdsString(rtc::ArrayView stream_ids) { return output; } -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.media_description()) { - continue; - } - has_spec_compliant |= content.media_description()->HasSimulcast(); - for (const StreamParams& sp : content.media_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); - } -} - const ContentInfo* FindTransceiverMSection( RtpTransceiver* transceiver, const SessionDescriptionInterface* session_description) { @@ -1671,10 +1636,6 @@ RTCError SdpOfferAnswerHandler::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. @@ -1962,10 +1923,6 @@ RTCError SdpOfferAnswerHandler::ReplaceRemoteDescription( const cricket::SessionDescription* session_desc = remote_description()->description(); - // Report statistics about any use of simulcast. - ReportSimulcastApiVersion(kSimulcastVersionApplyRemoteDescription, - *session_desc); - // NOTE: This will perform a BlockingCall() to the network thread. return transport_controller_s()->SetRemoteDescription(sdp_type, session_desc); } @@ -3842,7 +3799,6 @@ SdpOfferAnswerHandler::AssociateTransceiver( if (SimulcastIsRejected(old_local_content, *media_desc, pc_->GetCryptoOptions() .srtp.enable_encrypted_rtp_header_extensions)) { - RTC_HISTOGRAM_BOOLEAN(kSimulcastDisabled, true); RTCError error = DisableSimulcastInSender(transceiver->internal()->sender_internal()); if (!error.ok()) {