Populate sdp_fmtp_line and channels of RTCCodecStats

Change RtpCodecCapability::parameters and RtpCodecParameters::parameters
to map from unordered_map to get welldefined FMTP lines.

Bug: webrtc:7061
Change-Id: Ie61f76bbab915d72369e36e3f40ea11838827940
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168190
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30512}

api/rtp_parameters.h

@@ -13,8 +13,8 @@
 
 #include <stdint.h>
 
+#include <map>
 #include <string>
-#include <unordered_map>
 #include <vector>
 
 #include "absl/types/optional.h"
@@ -157,12 +157,12 @@ struct RTC_EXPORT RtpCodecCapability {
   // Contrary to ORTC, these parameters are named using all lowercase strings.
   // This helps make the mapping to SDP simpler, if an application is using SDP.
   // Boolean values are represented by the string "1".
-  std::unordered_map<std::string, std::string> parameters;
+  std::map<std::string, std::string> parameters;
 
   // Codec-specific parameters that may optionally be signaled to the remote
   // party.
   // TODO(deadbeef): Not implemented.
-  std::unordered_map<std::string, std::string> options;
+  std::map<std::string, std::string> options;
 
   // Maximum number of temporal layer extensions supported by this codec.
   // For example, a value of 1 indicates that 2 total layers are supported.
@@ -500,7 +500,7 @@ struct RTC_EXPORT RtpCodecParameters {
   // Contrary to ORTC, these parameters are named using all lowercase strings.
   // This helps make the mapping to SDP simpler, if an application is using SDP.
   // Boolean values are represented by the string "1".
-  std::unordered_map<std::string, std::string> parameters;
+  std::map<std::string, std::string> parameters;
 
   bool operator==(const RtpCodecParameters& o) const {
     return name == o.name && kind == o.kind && payload_type == o.payload_type &&

api/stats/rtcstats_objects.h

@@ -118,9 +118,7 @@ class RTC_EXPORT RTCCodecStats final : public RTCStats {
   RTCStatsMember<uint32_t> payload_type;
   RTCStatsMember<std::string> mime_type;
   RTCStatsMember<uint32_t> clock_rate;
-  // TODO(hbos): Collect and populate this value. https://bugs.webrtc.org/7061
   RTCStatsMember<uint32_t> channels;
-  // TODO(hbos): Collect and populate this value. https://bugs.webrtc.org/7061
   RTCStatsMember<std::string> sdp_fmtp_line;
 };
 

pc/rtc_stats_collector.cc

@@ -10,6 +10,7 @@
 
 #include "pc/rtc_stats_collector.h"
 
+#include <map>
 #include <memory>
 #include <string>
 #include <utility>
@@ -24,6 +25,7 @@
 #include "p2p/base/port.h"
 #include "pc/peer_connection.h"
 #include "pc/rtc_stats_traversal.h"
+#include "pc/webrtc_sdp.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/strings/string_builder.h"
 #include "rtc_base/time_utils.h"
@@ -235,6 +237,14 @@ std::unique_ptr<RTCCodecStats> CodecStatsFromRtpCodecParameters(
   if (codec_params.clock_rate) {
     codec_stats->clock_rate = static_cast<uint32_t>(*codec_params.clock_rate);
   }
+  if (codec_params.num_channels) {
+    codec_stats->channels = *codec_params.num_channels;
+  }
+
+  rtc::StringBuilder fmtp;
+  if (WriteFmtpParameters(codec_params.parameters, &fmtp)) {
+    codec_stats->sdp_fmtp_line = fmtp.Release();
+  }
   return codec_stats;
 }
 

pc/rtc_stats_collector_unittest.cc

@@ -813,6 +813,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) {
   inbound_audio_codec.kind = cricket::MEDIA_TYPE_AUDIO;
   inbound_audio_codec.name = "opus";
   inbound_audio_codec.clock_rate = 1337;
+  inbound_audio_codec.num_channels = 1;
+  inbound_audio_codec.parameters = {{"minptime", "10"}, {"useinbandfec", "1"}};
   voice_media_info.receive_codecs.insert(
       std::make_pair(inbound_audio_codec.payload_type, inbound_audio_codec));
 
@@ -821,6 +823,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) {
   outbound_audio_codec.kind = cricket::MEDIA_TYPE_AUDIO;
   outbound_audio_codec.name = "isac";
   outbound_audio_codec.clock_rate = 1338;
+  outbound_audio_codec.num_channels = 2;
   voice_media_info.send_codecs.insert(
       std::make_pair(outbound_audio_codec.payload_type, outbound_audio_codec));
 
@@ -835,6 +838,9 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) {
   inbound_video_codec.kind = cricket::MEDIA_TYPE_VIDEO;
   inbound_video_codec.name = "H264";
   inbound_video_codec.clock_rate = 1339;
+  inbound_video_codec.parameters = {{"level-asymmetry-allowed", "1"},
+                                    {"packetization-mode", "1"},
+                                    {"profile-level-id", "42001f"}};
   video_media_info.receive_codecs.insert(
       std::make_pair(inbound_video_codec.payload_type, inbound_video_codec));
 
@@ -856,18 +862,23 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) {
   expected_inbound_audio_codec.payload_type = 1;
   expected_inbound_audio_codec.mime_type = "audio/opus";
   expected_inbound_audio_codec.clock_rate = 1337;
+  expected_inbound_audio_codec.channels = 1;
+  expected_inbound_audio_codec.sdp_fmtp_line = "minptime=10;useinbandfec=1";
 
   RTCCodecStats expected_outbound_audio_codec("RTCCodec_AudioMid_Outbound_2",
                                               report->timestamp_us());
   expected_outbound_audio_codec.payload_type = 2;
   expected_outbound_audio_codec.mime_type = "audio/isac";
   expected_outbound_audio_codec.clock_rate = 1338;
+  expected_outbound_audio_codec.channels = 2;
 
   RTCCodecStats expected_inbound_video_codec("RTCCodec_VideoMid_Inbound_3",
                                              report->timestamp_us());
   expected_inbound_video_codec.payload_type = 3;
   expected_inbound_video_codec.mime_type = "video/H264";
   expected_inbound_video_codec.clock_rate = 1339;
+  expected_inbound_video_codec.sdp_fmtp_line =
+      "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f";
 
   RTCCodecStats expected_outbound_video_codec("RTCCodec_VideoMid_Outbound_4",
                                               report->timestamp_us());

pc/rtc_stats_integrationtest.cc

@@ -445,8 +445,14 @@ class RTCStatsReportVerifier {
     verifier.TestMemberIsDefined(codec.payload_type);
     verifier.TestMemberIsDefined(codec.mime_type);
     verifier.TestMemberIsPositive<uint32_t>(codec.clock_rate);
-    verifier.TestMemberIsUndefined(codec.channels);
-    verifier.TestMemberIsUndefined(codec.sdp_fmtp_line);
+
+    if (codec.mime_type->rfind("audio", 0) == 0)
+      verifier.TestMemberIsPositive<uint32_t>(codec.channels);
+    else
+      verifier.TestMemberIsUndefined(codec.channels);
+
+    // sdp_fmtp_line is an optional field.
+    verifier.MarkMemberTested(codec.sdp_fmtp_line, true);
     return verifier.ExpectAllMembersSuccessfullyTested();
   }
 

pc/rtp_parameters_conversion.cc

@@ -164,7 +164,7 @@ RTCErrorOr<C> ToCricketCodec(const RtpCodecParameters& codec) {
     }
     cricket_codec.AddFeedbackParam(result.MoveValue());
   }
-  cricket_codec.params.insert(codec.parameters.begin(), codec.parameters.end());
+  cricket_codec.params = codec.parameters;
   return std::move(cricket_codec);
 }
 
@@ -366,8 +366,7 @@ RtpCodecParameters ToRtpCodecParameters(const C& cricket_codec) {
     }
   }
   ToRtpCodecParametersTypeSpecific(cricket_codec, &codec_param);
-  codec_param.parameters.insert(cricket_codec.params.begin(),
-                                cricket_codec.params.end());
+  codec_param.parameters = cricket_codec.params;
   return codec_param;
 }
 

pc/webrtc_sdp.cc

@@ -1785,24 +1785,6 @@ void WriteFmtpParameter(const std::string& parameter_name,
   *os << parameter_name << kSdpDelimiterEqual << parameter_value;
 }
 
-void WriteFmtpParameters(const cricket::CodecParameterMap& parameters,
-                         rtc::StringBuilder* os) {
-  bool first = true;
-  for (const auto& entry : parameters) {
-    const std::string& key = entry.first;
-    const std::string& value = entry.second;
-    // Parameters are a semicolon-separated list, no spaces.
-    // The list is separated from the header by a space.
-    if (first) {
-      *os << kSdpDelimiterSpace;
-      first = false;
-    } else {
-      *os << kSdpDelimiterSemicolon;
-    }
-    WriteFmtpParameter(key, value, os);
-  }
-}
-
 bool IsFmtpParam(const std::string& name) {
   // RFC 4855, section 3 specifies the mapping of media format parameters to SDP
   // parameters. Only ptime, maxptime, channels and rate are placed outside of
@@ -1811,31 +1793,35 @@ bool IsFmtpParam(const std::string& name) {
   return name != kCodecParamPTime && name != kCodecParamMaxPTime;
 }
 
-// Retreives fmtp parameters from |params|, which may contain other parameters
-// as well, and puts them in |fmtp_parameters|.
-void GetFmtpParams(const cricket::CodecParameterMap& params,
-                   cricket::CodecParameterMap* fmtp_parameters) {
-  for (const auto& entry : params) {
+bool WriteFmtpParameters(const cricket::CodecParameterMap& parameters,
+                         rtc::StringBuilder* os) {
+  bool empty = true;
+  const char* delimiter = "";  // No delimiter before first parameter.
+  for (const auto& entry : parameters) {
     const std::string& key = entry.first;
     const std::string& value = entry.second;
+
     if (IsFmtpParam(key)) {
-      (*fmtp_parameters)[key] = value;
+      *os << delimiter;
+      // A semicolon before each subsequent parameter.
+      delimiter = kSdpDelimiterSemicolon;
+      WriteFmtpParameter(key, value, os);
+      empty = false;
     }
   }
+
+  return !empty;
 }
 
 template <class T>
 void AddFmtpLine(const T& codec, std::string* message) {
-  cricket::CodecParameterMap fmtp_parameters;
-  GetFmtpParams(codec.params, &fmtp_parameters);
-  if (fmtp_parameters.empty()) {
-    // No need to add an fmtp if it will have no (optional) parameters.
-    return;
-  }
   rtc::StringBuilder os;
   WriteFmtpHeader(codec.id, &os);
-  WriteFmtpParameters(fmtp_parameters, &os);
-  AddLine(os.str(), message);
+  os << kSdpDelimiterSpace;
+  // Create FMTP line and check that it's nonempty.
+  if (WriteFmtpParameters(codec.params, &os)) {
+    AddLine(os.str(), message);
+  }
   return;
 }
 

pc/webrtc_sdp.h

@@ -22,12 +22,17 @@
 
 #include <string>
 
+#include "media/base/codec.h"
 #include "rtc_base/system/rtc_export.h"
 
 namespace cricket {
 class Candidate;
 }  // namespace cricket
 
+namespace rtc {
+class StringBuilder;
+}  // namespace rtc
+
 namespace webrtc {
 class IceCandidateInterface;
 class JsepIceCandidate;
@@ -95,6 +100,13 @@ RTC_EXPORT bool ParseCandidate(const std::string& message,
                                SdpParseError* error,
                                bool is_raw);
 
+// Generates an FMTP line based on |parameters|. Please note that some
+// parameters are not considered to be part of the FMTP line, see the function
+// IsFmtpParam(). Returns true if the set of FMTP parameters is nonempty, false
+// otherwise.
+bool WriteFmtpParameters(const cricket::CodecParameterMap& parameters,
+                         rtc::StringBuilder* os);
+
 }  // namespace webrtc
 
 #endif  // PC_WEBRTC_SDP_H_