Report UMA metrics for received SDP format

This change adds UMA stats that record the format of the remote offered
SDP. There are three classifications for the SDP format:
- Simple: No more than one audio and one video. Should be compatible
    with both Plan B and Unified Plan endpoints.
- ComplexPlanB: More than one audio or more than one video in the Plan B
    format (e.g., one audio mline with multiple streams).
- ComplexUnifiedPlan: More than one audio or more than one video in the
    Unified Plan format (e.g., two audio mlines).

Bug: chromium:811683
Change-Id: If46394edfa6a812ef313d632e27ec27516c18867
Reviewed-on: https://webrtc-review.googlesource.com/57220
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22315}

api/fakemetricsobserver.cc

@@ -61,4 +61,27 @@ int FakeMetricsObserver::GetHistogramSample(
   return histogram_samples_[type];
 }
 
+bool FakeMetricsObserver::ExpectOnlySingleEnumCount(
+    PeerConnectionEnumCounterType type,
+    int counter) const {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  if (counters_.size() <= static_cast<size_t>(type)) {
+    // If a counter has not been allocated then there has been no call to
+    // |IncrementEnumCounter| so all the values are 0.
+    return false;
+  }
+  bool pass = true;
+  if (GetEnumCounter(type, counter) != 1) {
+    RTC_LOG(LS_ERROR) << "Expected single count for counter: " << counter;
+    pass = false;
+  }
+  for (const auto& entry : counters_[type]) {
+    if (entry.first != counter && entry.second > 0) {
+      RTC_LOG(LS_ERROR) << "Expected no count for counter: " << entry.first;
+      pass = false;
+    }
+  }
+  return pass;
+}
+
 }  // namespace webrtc

api/fakemetricsobserver.h

@@ -35,6 +35,11 @@ class FakeMetricsObserver : public MetricsObserverInterface {
   int GetEnumCounter(PeerConnectionEnumCounterType type, int counter) const;
   int GetHistogramSample(PeerConnectionMetricsName type) const;
 
+  // Returns true if and only if there is a count of 1 for the given counter and
+  // a count of 0 for all other counters of the given enum type.
+  bool ExpectOnlySingleEnumCount(PeerConnectionEnumCounterType type,
+                                 int counter) const;
+
  protected:
   ~FakeMetricsObserver() {}
 

api/umametrics.h

@@ -41,6 +41,7 @@ enum PeerConnectionEnumCounterType {
   kEnumCounterSdpSemanticRequested,
   kEnumCounterSdpSemanticNegotiated,
   kEnumCounterKeyProtocolMediaType,
+  kEnumCounterSdpFormatReceived,
   kPeerConnectionEnumCounterMax
 };
 
@@ -146,6 +147,24 @@ enum SdpSemanticNegotiated {
   kSdpSemanticNegotiatedMax
 };
 
+// Metric which records the format of the received SDP for tracking how much the
+// difference between Plan B and Unified Plan affect users.
+enum SdpFormatReceived {
+  // No audio or video tracks. This is worth special casing since it seems to be
+  // the most common scenario (data-channel only).
+  kSdpFormatReceivedNoTracks,
+  // No more than one audio and one video track. Should be compatible with both
+  // Plan B and Unified Plan endpoints.
+  kSdpFormatReceivedSimple,
+  // More than one audio track or more than one video track in the Plan B format
+  // (e.g., one audio media section with multiple streams).
+  kSdpFormatReceivedComplexPlanB,
+  // More than one audio track or more than one video track in the Unified Plan
+  // format (e.g., two audio media sections).
+  kSdpFormatReceivedComplexUnifiedPlan,
+  kSdpFormatReceivedMax
+};
+
 class MetricsObserverInterface : public rtc::RefCountInterface {
  public:
   // |type| is the type of the enum counter to be incremented. |counter|

pc/peerconnection.cc

@@ -2105,6 +2105,11 @@ void PeerConnection::SetRemoteDescription(
   }
   RTC_DCHECK(remote_description());
 
+  if (type == SdpType::kOffer) {
+    // Report to UMA the format of the received offer.
+    ReportSdpFormatReceived(*remote_description());
+  }
+
   if (type == SdpType::kAnswer) {
     // TODO(deadbeef): We already had to hop to the network thread for
     // MaybeStartGathering...
@@ -5761,6 +5766,39 @@ std::string PeerConnection::GetSessionErrorMsg() {
   return desc.str();
 }
 
+void PeerConnection::ReportSdpFormatReceived(
+    const SessionDescriptionInterface& remote_offer) {
+  if (!uma_observer_) {
+    return;
+  }
+  int num_audio_mlines = 0;
+  int num_video_mlines = 0;
+  int num_audio_tracks = 0;
+  int num_video_tracks = 0;
+  for (const ContentInfo& content : remote_offer.description()->contents()) {
+    cricket::MediaType media_type = content.media_description()->type();
+    int num_tracks = std::max(
+        1, static_cast<int>(content.media_description()->streams().size()));
+    if (media_type == cricket::MEDIA_TYPE_AUDIO) {
+      num_audio_mlines += 1;
+      num_audio_tracks += num_tracks;
+    } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
+      num_video_mlines += 1;
+      num_video_tracks += num_tracks;
+    }
+  }
+  SdpFormatReceived format = kSdpFormatReceivedNoTracks;
+  if (num_audio_mlines > 1 || num_video_mlines > 1) {
+    format = kSdpFormatReceivedComplexUnifiedPlan;
+  } else if (num_audio_tracks > 1 || num_video_tracks > 1) {
+    format = kSdpFormatReceivedComplexPlanB;
+  } else if (num_audio_tracks > 0 || num_video_tracks > 0) {
+    format = kSdpFormatReceivedSimple;
+  }
+  uma_observer_->IncrementEnumCounter(kEnumCounterSdpFormatReceived, format,
+                                      kSdpFormatReceivedMax);
+}
+
 void PeerConnection::ReportNegotiatedSdpSemantics(
     const SessionDescriptionInterface& answer) {
   if (!uma_observer_) {

pc/peerconnection.h

@@ -851,6 +851,9 @@ class PeerConnection : public PeerConnectionInternal,
   const char* SessionErrorToString(SessionError error) const;
   std::string GetSessionErrorMsg();
 
+  // Report the UMA metric SdpFormatReceived for the given remote offer.
+  void ReportSdpFormatReceived(const SessionDescriptionInterface& remote_offer);
+
   // Report inferred negotiated SDP semantics from a local/remote answer to the
   // UMA observer.
   void ReportNegotiatedSdpSemantics(const SessionDescriptionInterface& answer);

pc/peerconnection_rtp_unittest.cc

@@ -13,10 +13,10 @@
 
 #include "api/audio_codecs/builtin_audio_decoder_factory.h"
 #include "api/audio_codecs/builtin_audio_encoder_factory.h"
-#include "api/fakemetricsobserver.h"
 #include "api/jsep.h"
 #include "api/mediastreaminterface.h"
 #include "api/peerconnectioninterface.h"
+#include "api/umametrics.h"
 #include "pc/mediastream.h"
 #include "pc/mediastreamtrack.h"
 #include "pc/peerconnectionwrapper.h"
@@ -1030,9 +1030,7 @@ TEST_F(PeerConnectionMsidSignalingTest, UnifiedPlanTalkingToOurself) {
   caller->AddAudioTrack("caller_audio");
   auto callee = CreatePeerConnectionWithUnifiedPlan();
   callee->AddAudioTrack("callee_audio");
-  auto caller_observer =
+  auto caller_observer = caller->RegisterFakeMetricsObserver();
-      new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
-  caller->pc()->RegisterUMAObserver(caller_observer);
 
   ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
 
@@ -1047,18 +1045,8 @@ TEST_F(PeerConnectionMsidSignalingTest, UnifiedPlanTalkingToOurself) {
   EXPECT_EQ(cricket::kMsidSignalingMediaSection,
             answer->description()->msid_signaling());
   // Check that this is counted correctly
-  EXPECT_EQ(1, caller_observer->GetEnumCounter(
+  EXPECT_TRUE(caller_observer->ExpectOnlySingleEnumCount(
-                   webrtc::kEnumCounterSdpSemanticNegotiated,
+      kEnumCounterSdpSemanticNegotiated, kSdpSemanticNegotiatedUnifiedPlan));
-                   webrtc::kSdpSemanticNegotiatedUnifiedPlan));
-  EXPECT_EQ(0, caller_observer->GetEnumCounter(
-                   webrtc::kEnumCounterSdpSemanticNegotiated,
-                   webrtc::kSdpSemanticNegotiatedNone));
-  EXPECT_EQ(0, caller_observer->GetEnumCounter(
-                   webrtc::kEnumCounterSdpSemanticNegotiated,
-                   webrtc::kSdpSemanticNegotiatedPlanB));
-  EXPECT_EQ(0, caller_observer->GetEnumCounter(
-                   webrtc::kEnumCounterSdpSemanticNegotiated,
-                   webrtc::kSdpSemanticNegotiatedMixed));
 }
 
 TEST_F(PeerConnectionMsidSignalingTest, PlanBOfferToUnifiedPlanAnswer) {
@@ -1101,6 +1089,76 @@ TEST_F(PeerConnectionMsidSignalingTest, PureUnifiedPlanToUs) {
             answer->description()->msid_signaling());
 }
 
+// Test that the correct UMA metrics are reported for simple/complex SDP.
+
+class SdpFormatReceivedTest : public PeerConnectionRtpTest {};
+
+#ifdef HAVE_SCTP
+TEST_F(SdpFormatReceivedTest, DataChannelOnlyIsReportedAsNoTracks) {
+  auto caller = CreatePeerConnectionWithUnifiedPlan();
+  caller->CreateDataChannel("dc");
+  auto callee = CreatePeerConnectionWithUnifiedPlan();
+  auto callee_metrics = callee->RegisterFakeMetricsObserver();
+
+  ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
+
+  EXPECT_TRUE(callee_metrics->ExpectOnlySingleEnumCount(
+      kEnumCounterSdpFormatReceived, kSdpFormatReceivedNoTracks));
+}
+#endif  // HAVE_SCTP
+
+TEST_F(SdpFormatReceivedTest, SimpleUnifiedPlanIsReportedAsSimple) {
+  auto caller = CreatePeerConnectionWithUnifiedPlan();
+  caller->AddAudioTrack("audio");
+  caller->AddVideoTrack("video");
+  auto callee = CreatePeerConnectionWithPlanB();
+  auto callee_metrics = callee->RegisterFakeMetricsObserver();
+
+  ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
+
+  EXPECT_TRUE(callee_metrics->ExpectOnlySingleEnumCount(
+      kEnumCounterSdpFormatReceived, kSdpFormatReceivedSimple));
+}
+
+TEST_F(SdpFormatReceivedTest, SimplePlanBIsReportedAsSimple) {
+  auto caller = CreatePeerConnectionWithPlanB();
+  caller->AddVideoTrack("video");  // Video only.
+  auto callee = CreatePeerConnectionWithUnifiedPlan();
+  auto callee_metrics = callee->RegisterFakeMetricsObserver();
+
+  ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
+
+  EXPECT_TRUE(callee_metrics->ExpectOnlySingleEnumCount(
+      kEnumCounterSdpFormatReceived, kSdpFormatReceivedSimple));
+}
+
+TEST_F(SdpFormatReceivedTest, ComplexUnifiedIsReportedAsComplexUnifiedPlan) {
+  auto caller = CreatePeerConnectionWithUnifiedPlan();
+  caller->AddAudioTrack("audio1");
+  caller->AddAudioTrack("audio2");
+  caller->AddVideoTrack("video");
+  auto callee = CreatePeerConnectionWithPlanB();
+  auto callee_metrics = callee->RegisterFakeMetricsObserver();
+
+  ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
+
+  EXPECT_TRUE(callee_metrics->ExpectOnlySingleEnumCount(
+      kEnumCounterSdpFormatReceived, kSdpFormatReceivedComplexUnifiedPlan));
+}
+
+TEST_F(SdpFormatReceivedTest, ComplexPlanBIsReportedAsComplexPlanB) {
+  auto caller = CreatePeerConnectionWithPlanB();
+  caller->AddVideoTrack("video1");
+  caller->AddVideoTrack("video2");
+  auto callee = CreatePeerConnectionWithUnifiedPlan();
+  auto callee_metrics = callee->RegisterFakeMetricsObserver();
+
+  ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
+
+  EXPECT_TRUE(callee_metrics->ExpectOnlySingleEnumCount(
+      kEnumCounterSdpFormatReceived, kSdpFormatReceivedComplexPlanB));
+}
+
 // Sender setups in a call.
 
 class PeerConnectionSenderTest : public PeerConnectionRtpTest {};

pc/peerconnectionwrapper.cc

@@ -322,4 +322,12 @@ PeerConnectionWrapper::GetStats() {
   return callback->report();
 }
 
+rtc::scoped_refptr<FakeMetricsObserver>
+PeerConnectionWrapper::RegisterFakeMetricsObserver() {
+  RTC_DCHECK(!fake_metrics_observer_);
+  fake_metrics_observer_ = new rtc::RefCountedObject<FakeMetricsObserver>();
+  pc_->RegisterUMAObserver(fake_metrics_observer_);
+  return fake_metrics_observer_;
+}
+
 }  // namespace webrtc

pc/peerconnectionwrapper.h

@@ -16,6 +16,7 @@
 #include <string>
 #include <vector>
 
+#include "api/fakemetricsobserver.h"
 #include "api/peerconnectioninterface.h"
 #include "pc/test/mockpeerconnectionobservers.h"
 #include "rtc_base/function_view.h"
@@ -170,6 +171,10 @@ class PeerConnectionWrapper {
   // report. If GetStats() fails, this method returns null and fails the test.
   rtc::scoped_refptr<const RTCStatsReport> GetStats();
 
+  // Creates a new FakeMetricsObserver and registers it with the PeerConnection
+  // as the UMA observer.
+  rtc::scoped_refptr<FakeMetricsObserver> RegisterFakeMetricsObserver();
+
  private:
   std::unique_ptr<SessionDescriptionInterface> CreateSdp(
       rtc::FunctionView<void(CreateSessionDescriptionObserver*)> fn,
@@ -180,6 +185,7 @@ class PeerConnectionWrapper {
   rtc::scoped_refptr<PeerConnectionFactoryInterface> pc_factory_;
   std::unique_ptr<MockPeerConnectionObserver> observer_;
   rtc::scoped_refptr<PeerConnectionInterface> pc_;
+  rtc::scoped_refptr<FakeMetricsObserver> fake_metrics_observer_;
 };
 
 }  // namespace webrtc