From b8ca2a18a5c1f749c5bc1dacc869fa1cdb6bc0a1 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Wed, 7 Oct 2020 12:47:10 +0200 Subject: [PATCH] count plan-b/unified-plan usage in SDP answers the UMA stats currently do not count services like Hangouts that have "complex" SDP with multiple tracks only in the answer, not in the offer. Note that this changes the definition of the existing metric. BUG=chromium:857004 Change-Id: Ib4520a82f7d94cdd4a307d32846e2d26a5f03b90 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/186701 Reviewed-by: Harald Alvestrand Commit-Queue: Philipp Hancke Cr-Commit-Position: refs/heads/master@{#32355} --- pc/peer_connection.cc | 17 +++++++++++++++-- pc/peer_connection_rtp_unittest.cc | 13 +++++++++++++ pc/sdp_offer_answer.cc | 5 +++-- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index e4edfcce41..6d26f9e62a 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -3009,8 +3009,21 @@ void PeerConnection::ReportSdpFormatReceived( } else if (num_audio_tracks > 0 || num_video_tracks > 0) { format = kSdpFormatReceivedSimple; } - RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpFormatReceived", format, - kSdpFormatReceivedMax); + switch (remote_offer.GetType()) { + case SdpType::kOffer: + // Historically only offers were counted. + RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpFormatReceived", + format, kSdpFormatReceivedMax); + break; + case SdpType::kAnswer: + RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpFormatReceivedAnswer", + format, kSdpFormatReceivedMax); + break; + default: + RTC_LOG(LS_ERROR) << "Can not report SdpFormatReceived for " + << SdpTypeToString(remote_offer.GetType()); + break; + } } void PeerConnection::ReportIceCandidateCollected( diff --git a/pc/peer_connection_rtp_unittest.cc b/pc/peer_connection_rtp_unittest.cc index b1670c1684..eeb44d82a2 100644 --- a/pc/peer_connection_rtp_unittest.cc +++ b/pc/peer_connection_rtp_unittest.cc @@ -1855,6 +1855,19 @@ TEST_F(SdpFormatReceivedTest, ComplexPlanBIsReportedAsComplexPlanB) { ElementsAre(Pair(kSdpFormatReceivedComplexPlanB, 1))); } +TEST_F(SdpFormatReceivedTest, AnswerIsReported) { + auto caller = CreatePeerConnectionWithPlanB(); + caller->AddAudioTrack("audio"); + caller->AddVideoTrack("video"); + auto callee = CreatePeerConnectionWithUnifiedPlan(); + + ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); + ASSERT_TRUE(caller->SetRemoteDescription(callee->CreateAnswer())); + EXPECT_METRIC_THAT( + metrics::Samples("WebRTC.PeerConnection.SdpFormatReceivedAnswer"), + ElementsAre(Pair(kSdpFormatReceivedSimple, 1))); +} + // Sender setups in a call. TEST_P(PeerConnectionRtpTest, CreateTwoSendersWithSameTrack) { diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 6d963bad8f..8b2d5a0b3b 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -2009,8 +2009,9 @@ void SdpOfferAnswerHandler::DoSetRemoteDescription( "Rollback not supported in Plan B")); return; } - if (desc->GetType() == SdpType::kOffer) { - // Report to UMA the format of the received offer. + if (desc->GetType() == SdpType::kOffer || + desc->GetType() == SdpType::kAnswer) { + // Report to UMA the format of the received offer or answer. pc_->ReportSdpFormatReceived(*desc); }