Change PeerConnection stats interface to be more flexible

This removes the SessionStats object and replaces it with two methods on PeerConnection: GetTransportNamesByMid and GetTransportStatsByNames for use by the stats collectors. These methods are more flexible and can cover cases where there are more than one video/audio channel. Bug: webrtc:8764 Change-Id: Id400cc548fc43675462ff6175a7fa9c9f4fd5948 Reviewed-on: https://webrtc-review.googlesource.com/47244 Commit-Queue: Steve Anton <[email protected]> Reviewed-by: Taylor Brandstetter <[email protected]> Cr-Commit-Position: refs/heads/master@{#21921}
cneyoo · Feb 6, 2018 · 5dfde18 · 5dfde18
1 parent 8234ead
commit 5dfde18
Show file tree

Hide file tree

Showing 12 changed files with 180 additions and 210 deletions.
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
@@ -4760,39 +4760,48 @@ bool PeerConnection::ReadyToSendData() const {
          sctp_ready_to_send_data_;
 }
 
-std::unique_ptr<SessionStats> PeerConnection::GetSessionStats_s() {
-  RTC_DCHECK(signaling_thread()->IsCurrent());
-  ChannelNamePairs channel_name_pairs;
-  if (voice_channel()) {
-    channel_name_pairs.voice = ChannelNamePair(
-        voice_channel()->content_name(), voice_channel()->transport_name());
-  }
-  if (video_channel()) {
-    channel_name_pairs.video = ChannelNamePair(
-        video_channel()->content_name(), video_channel()->transport_name());
+std::map<std::string, std::string> PeerConnection::GetTransportNamesByMid()
+    const {
+  std::map<std::string, std::string> transport_names_by_mid;
+  for (auto transceiver : transceivers_) {
+    cricket::BaseChannel* channel = transceiver->internal()->channel();
+    if (channel) {
+      transport_names_by_mid[channel->content_name()] =
+          channel->transport_name();
+    }
   }
-  if (rtp_data_channel()) {
-    channel_name_pairs.data =
-        ChannelNamePair(rtp_data_channel()->content_name(),
-                        rtp_data_channel()->transport_name());
+  if (rtp_data_channel_) {
+    transport_names_by_mid[rtp_data_channel_->content_name()] =
+        rtp_data_channel_->transport_name();
   }
   if (sctp_transport_) {
-    RTC_DCHECK(sctp_content_name_);
-    RTC_DCHECK(sctp_transport_name_);
-    channel_name_pairs.data =
-        ChannelNamePair(*sctp_content_name_, *sctp_transport_name_);
+    transport_names_by_mid[*sctp_content_name_] = *sctp_transport_name_;
   }
-  return GetSessionStats(channel_name_pairs);
+  return transport_names_by_mid;
 }
 
-std::unique_ptr<SessionStats> PeerConnection::GetSessionStats(
-    const ChannelNamePairs& channel_name_pairs) {
-  if (network_thread()->IsCurrent()) {
-    return GetSessionStats_n(channel_name_pairs);
+std::map<std::string, cricket::TransportStats>
+PeerConnection::GetTransportStatsByNames(
+    const std::set<std::string>& transport_names) {
+  if (!network_thread()->IsCurrent()) {
+    return network_thread()
+        ->Invoke<std::map<std::string, cricket::TransportStats>>(
+            RTC_FROM_HERE,
+            [&] { return GetTransportStatsByNames(transport_names); });
   }
-  return network_thread()->Invoke<std::unique_ptr<SessionStats>>(
-      RTC_FROM_HERE,
-      rtc::Bind(&PeerConnection::GetSessionStats_n, this, channel_name_pairs));
+  std::map<std::string, cricket::TransportStats> transport_stats_by_name;
+  for (const std::string& transport_name : transport_names) {
+    cricket::TransportStats transport_stats;
+    bool success =
+        transport_controller_->GetStats(transport_name, &transport_stats);
+    if (success) {
+      transport_stats_by_name[transport_name] = std::move(transport_stats);
+    } else {
+      RTC_LOG(LS_ERROR) << "Failed to get transport stats for transport_name="
+                        << transport_name;
+    }
+  }
+  return transport_stats_by_name;
 }
 
 bool PeerConnection::GetLocalCertificate(
@@ -5270,26 +5279,6 @@ Call::Stats PeerConnection::GetCallStats() {
   }
 }
 
-std::unique_ptr<SessionStats> PeerConnection::GetSessionStats_n(
-    const ChannelNamePairs& channel_name_pairs) {
-  RTC_DCHECK(network_thread()->IsCurrent());
-  std::unique_ptr<SessionStats> session_stats(new SessionStats());
-  for (const auto channel_name_pair :
-       {&channel_name_pairs.voice, &channel_name_pairs.video,
-        &channel_name_pairs.data}) {
-    if (*channel_name_pair) {
-      cricket::TransportStats transport_stats;
-      if (!transport_controller_->GetStats((*channel_name_pair)->transport_name,
-                                           &transport_stats)) {
-        return nullptr;
-      }
-      session_stats->transport_stats[(*channel_name_pair)->transport_name] =
-          std::move(transport_stats);
-    }
-  }
-  return session_stats;
-}
-
 bool PeerConnection::CreateSctpTransport_n(const std::string& content_name,
                                            const std::string& transport_name) {
   RTC_DCHECK(network_thread()->IsCurrent());

diff --git a/pc/peerconnection.h b/pc/peerconnection.h
@@ -253,9 +253,9 @@ class PeerConnection : public PeerConnectionInternal,
     return sctp_transport_name_;
   }
 
-  std::unique_ptr<SessionStats> GetSessionStats_s() override;
-  std::unique_ptr<SessionStats> GetSessionStats(
-      const ChannelNamePairs& channel_name_pairs) override;
+  std::map<std::string, std::string> GetTransportNamesByMid() const override;
+  std::map<std::string, cricket::TransportStats> GetTransportStatsByNames(
+      const std::set<std::string>& transport_names) override;
   Call::Stats GetCallStats() override;
 
   bool GetLocalCertificate(
@@ -812,9 +812,6 @@ class PeerConnection : public PeerConnectionInternal,
   bool CreateDataChannel(const std::string& mid,
                          const std::string& transport_name);
 
-  std::unique_ptr<SessionStats> GetSessionStats_n(
-      const ChannelNamePairs& channel_name_pairs);
-
   bool CreateSctpTransport_n(const std::string& content_name,
                              const std::string& transport_name);
   // For bundling.

diff --git a/pc/peerconnectioninternal.h b/pc/peerconnectioninternal.h
@@ -13,6 +13,7 @@
 
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -22,27 +23,6 @@
 
 namespace webrtc {
 
-// Statistics for all the transports of the session.
-// TODO(pthatcher): Think of a better name for this.  We already have
-// a TransportStats in transport.h.  Perhaps TransportsStats?
-struct SessionStats {
-  std::map<std::string, cricket::TransportStats> transport_stats;
-};
-
-struct ChannelNamePair {
-  ChannelNamePair(const std::string& content_name,
-                  const std::string& transport_name)
-      : content_name(content_name), transport_name(transport_name) {}
-  std::string content_name;
-  std::string transport_name;
-};
-
-struct ChannelNamePairs {
-  rtc::Optional<ChannelNamePair> voice;
-  rtc::Optional<ChannelNamePair> video;
-  rtc::Optional<ChannelNamePair> data;
-};
-
 // Internal interface for extra PeerConnection methods.
 class PeerConnectionInternal : public PeerConnectionInterface {
  public:
@@ -80,16 +60,13 @@ class PeerConnectionInternal : public PeerConnectionInterface {
   virtual rtc::Optional<std::string> sctp_content_name() const = 0;
   virtual rtc::Optional<std::string> sctp_transport_name() const = 0;
 
-  // Returns stats for all channels of all transports.
-  // This avoids exposing the internal structures used to track them.
-  // The parameterless version creates |ChannelNamePairs| from |voice_channel|,
-  // |video_channel| and |voice_channel| if available - this requires it to be
-  // called on the signaling thread - and invokes the other |GetStats|. The
-  // other |GetStats| can be invoked on any thread; if not invoked on the
-  // network thread a thread hop will happen.
-  virtual std::unique_ptr<SessionStats> GetSessionStats_s() = 0;
-  virtual std::unique_ptr<SessionStats> GetSessionStats(
-      const ChannelNamePairs& channel_name_pairs) = 0;
+  // Returns a map from MID to transport name for all active media sections.
+  virtual std::map<std::string, std::string> GetTransportNamesByMid() const = 0;
+
+  // Returns a map from transport name to transport stats for all given
+  // transport names.
+  virtual std::map<std::string, cricket::TransportStats>
+  GetTransportStatsByNames(const std::set<std::string>& transport_names) = 0;
 
   virtual Call::Stats GetCallStats() = 0;