Removing `preference` field from `cricket::Codec`.

webrtc/api/webrtcsdp.cc

Index: webrtc/api/webrtcsdp.cc
diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc
index 93d4acc6c3fad5fd2e2612ff22af02de2554baeb..61a4edf7f6a6886d74dfab8967b9e7cf20428a0c 100644
--- a/webrtc/api/webrtcsdp.cc
+++ b/webrtc/api/webrtcsdp.cc
@@ -269,7 +269,7 @@ static bool ParseContent(const std::string& message,
                          const MediaType media_type,
                          int mline_index,
                          const std::string& protocol,
-                         const std::vector<int>& codec_preference,
+                         const std::vector<int>& payload_types,
                          size_t* pos,
                          std::string* content_name,
                          MediaContentDescription* media_desc,
@@ -287,7 +287,7 @@ static bool ParseCryptoAttribute(const std::string& line,
                                  SdpParseError* error);
 static bool ParseRtpmapAttribute(const std::string& line,
                                  const MediaType media_type,
-                                 const std::vector<int>& codec_preference,
+                                 const std::vector<int>& payload_types,
                                  MediaContentDescription* media_desc,
                                  SdpParseError* error);
 static bool ParseFmtpAttributes(const std::string& line,
@@ -1639,9 +1639,8 @@ bool AddSctpDataCodec(DataContentDescription* media_desc,
                        NULL);
   }
   // Add the SCTP Port number as a pseudo-codec "port" parameter
-  cricket::DataCodec codec_port(
-      cricket::kGoogleSctpDataCodecId, cricket::kGoogleSctpDataCodecName,
-      0);
+  cricket::DataCodec codec_port(cricket::kGoogleSctpDataCodecId,
+                                cricket::kGoogleSctpDataCodecName);
   codec_port.SetParam(cricket::kCodecParamPort, sctp_port);
   LOG(INFO) << "AddSctpDataCodec: Got SCTP Port Number "
             << sctp_port;
@@ -2167,9 +2166,8 @@ void MaybeCreateStaticPayloadAudioCodecs(
   if (!media_desc) {
     return;
   }
-  int preference = static_cast<int>(fmts.size());
+  RTC_DCHECK(media_desc->codecs().empty());
   std::vector<int>::const_iterator it = fmts.begin();
-  bool add_new_codec = false;
   for (; it != fmts.end(); ++it) {
     int payload_type = *it;
     if (!media_desc->HasCodec(payload_type) &&
@@ -2179,14 +2177,8 @@ void MaybeCreateStaticPayloadAudioCodecs(
       int clock_rate = kStaticPayloadAudioCodecs[payload_type].clockrate;
       size_t channels = kStaticPayloadAudioCodecs[payload_type].channels;
       media_desc->AddCodec(cricket::AudioCodec(payload_type, encoding_name,
-                                               clock_rate, 0, channels,
-                                               preference));
-      add_new_codec = true;
+                                               clock_rate, 0, channels));
     }
-    --preference;
-  }
-  if (add_new_codec) {
-    media_desc->SortCodecs();
   }
 }
 
@@ -2195,7 +2187,7 @@ static C* ParseContentDescription(const std::string& message,
                                   const MediaType media_type,
                                   int mline_index,
                                   const std::string& protocol,
-                                  const std::vector<int>& codec_preference,
+                                  const std::vector<int>& payload_types,
                                   size_t* pos,
                                   std::string* content_name,
                                   TransportDescription* transport,
@@ -2216,14 +2208,21 @@ static C* ParseContentDescription(const std::string& message,
       ASSERT(false);
       break;
   }
-  if (!ParseContent(message, media_type, mline_index, protocol,
-                    codec_preference, pos, content_name,
-                    media_desc, transport, candidates, error)) {
+  if (!ParseContent(message, media_type, mline_index, protocol, payload_types,
+                    pos, content_name, media_desc, transport, candidates,
+                    error)) {
     delete media_desc;
     return NULL;
   }
   // Sort the codecs according to the m-line fmt list.
-  media_desc->SortCodecs();
+  std::vector<typename C::CodecType> codecs = media_desc->codecs();
+  std::sort(codecs.begin(), codecs.end(), [payload_types](
+                                              const typename C::CodecType& a,
+                                              const typename C::CodecType& b) {
+    return std::find(payload_types.begin(), payload_types.end(), a.id) <
+           std::find(payload_types.begin(), payload_types.end(), b.id);
+  });
+  media_desc->set_codecs(codecs);

[pthatcher1, 2016/04/11 21:53:40]

It would be nice if this weren't O(n^2), but I guess that's at most 127^2.

Maybe say something like "It's O(n^2), but n <= 127, it's not so bad.  If we
ever support more than 127 PTs (which is very unlikely), then we'd want to
revisit this."

[Taylor Brandstetter, 2016/04/12 00:18:00]

It's actually O(N^2 log N) :)
I fixed this pretty easily by making a map of payload types to preferences
though.

   return media_desc;
 }
 
@@ -2262,7 +2261,7 @@ bool ParseMediaDescription(const std::string& message,
     std::string protocol = fields[2];
 
     // <fmt>
-    std::vector<int> codec_preference;
+    std::vector<int> payload_types;
     if (IsRtp(protocol)) {
       for (size_t j = 3 ; j < fields.size(); ++j) {
         // TODO(wu): Remove when below bug is fixed.
@@ -2275,7 +2274,7 @@ bool ParseMediaDescription(const std::string& message,
         if (!GetPayloadTypeFromString(line, fields[j], &pl, error)) {
           return false;
         }
-        codec_preference.push_back(pl);
+        payload_types.push_back(pl);
       }
     }
 
@@ -2290,20 +2289,17 @@ bool ParseMediaDescription(const std::string& message,
     std::string content_name;
     if (HasAttribute(line, kMediaTypeVideo)) {
       content.reset(ParseContentDescription<VideoContentDescription>(
-                    message, cricket::MEDIA_TYPE_VIDEO, mline_index, protocol,
-                    codec_preference, pos, &content_name,
-                    &transport, candidates, error));
+          message, cricket::MEDIA_TYPE_VIDEO, mline_index, protocol,
+          payload_types, pos, &content_name, &transport, candidates, error));
     } else if (HasAttribute(line, kMediaTypeAudio)) {
       content.reset(ParseContentDescription<AudioContentDescription>(
-                    message, cricket::MEDIA_TYPE_AUDIO, mline_index, protocol,
-                    codec_preference, pos, &content_name,
-                    &transport, candidates, error));
+          message, cricket::MEDIA_TYPE_AUDIO, mline_index, protocol,
+          payload_types, pos, &content_name, &transport, candidates, error));
     } else if (HasAttribute(line, kMediaTypeData)) {
       DataContentDescription* data_desc =
           ParseContentDescription<DataContentDescription>(
-                    message, cricket::MEDIA_TYPE_DATA, mline_index, protocol,
-                    codec_preference, pos, &content_name,
-                    &transport, candidates, error);
+              message, cricket::MEDIA_TYPE_DATA, mline_index, protocol,
+              payload_types, pos, &content_name, &transport, candidates, error);
       content.reset(data_desc);
 
       int p;
@@ -2510,7 +2506,7 @@ bool ParseContent(const std::string& message,
                   const MediaType media_type,
                   int mline_index,
                   const std::string& protocol,
-                  const std::vector<int>& codec_preference,
+                  const std::vector<int>& payload_types,
                   size_t* pos,
                   std::string* content_name,
                   MediaContentDescription* media_desc,
@@ -2523,7 +2519,7 @@ bool ParseContent(const std::string& message,
 
   if (media_type == cricket::MEDIA_TYPE_AUDIO) {
     MaybeCreateStaticPayloadAudioCodecs(
-        codec_preference, static_cast<AudioContentDescription*>(media_desc));
+        payload_types, static_cast<AudioContentDescription*>(media_desc));
   }
 
   // The media level "ice-ufrag" and "ice-pwd".
@@ -2658,8 +2654,8 @@ bool ParseContent(const std::string& message,
           return false;
         }
       } else if (HasAttribute(line, kAttributeRtpmap)) {
-        if (!ParseRtpmapAttribute(line, media_type, codec_preference,
-                                  media_desc, error)) {
+        if (!ParseRtpmapAttribute(line, media_type, payload_types, media_desc,
+                                  error)) {
           return false;
         }
       } else if (HasAttribute(line, kCodecParamMaxPTime)) {
@@ -2915,9 +2911,12 @@ bool ParseCryptoAttribute(const std::string& line,
 }
 
 // Updates or creates a new codec entry in the audio description with according
-// to |name|, |clockrate|, |bitrate|, |channels| and |preference|.
-void UpdateCodec(int payload_type, const std::string& name, int clockrate,
-                 int bitrate, size_t channels, int preference,
+// to |name|, |clockrate|, |bitrate|, and |channels|.
+void UpdateCodec(int payload_type,
+                 const std::string& name,
+                 int clockrate,
+                 int bitrate,
+                 size_t channels,
                  AudioContentDescription* audio_desc) {
   // Codec may already be populated with (only) optional parameters
   // (from an fmtp).
@@ -2927,15 +2926,17 @@ void UpdateCodec(int payload_type, const std::string& name, int clockrate,
   codec.clockrate = clockrate;
   codec.bitrate = bitrate;
   codec.channels = channels;
-  codec.preference = preference;
   AddOrReplaceCodec<AudioContentDescription, cricket::AudioCodec>(audio_desc,
                                                                   codec);
 }
 
 // Updates or creates a new codec entry in the video description according to
-// |name|, |width|, |height|, |framerate| and |preference|.
-void UpdateCodec(int payload_type, const std::string& name, int width,
-                 int height, int framerate, int preference,
+// |name|, |width|, |height|, and |framerate|.
+void UpdateCodec(int payload_type,
+                 const std::string& name,
+                 int width,
+                 int height,
+                 int framerate,
                  VideoContentDescription* video_desc) {
   // Codec may already be populated with (only) optional parameters
   // (from an fmtp).
@@ -2945,14 +2946,13 @@ void UpdateCodec(int payload_type, const std::string& name, int width,
   codec.width = width;
   codec.height = height;
   codec.framerate = framerate;
-  codec.preference = preference;
   AddOrReplaceCodec<VideoContentDescription, cricket::VideoCodec>(video_desc,
                                                                   codec);
 }
 
 bool ParseRtpmapAttribute(const std::string& line,
                           const MediaType media_type,
-                          const std::vector<int>& codec_preference,
+                          const std::vector<int>& payload_types,
                           MediaContentDescription* media_desc,
                           SdpParseError* error) {
   std::vector<std::string> fields;
@@ -2974,12 +2974,8 @@ bool ParseRtpmapAttribute(const std::string& line,
     return false;
   }
 
-  // Set the preference order depending on the order of the pl type in the
-  // <fmt> of the m-line.
-  const int preference = codec_preference.end() -
-      std::find(codec_preference.begin(), codec_preference.end(),
-                payload_type);
-  if (preference == 0) {
+  if (std::find(payload_types.begin(), payload_types.end(), payload_type) ==
+      payload_types.end()) {
     LOG(LS_WARNING) << "Ignore rtpmap line that did not appear in the "
                     << "<fmt> of the m-line: " << line;
     return true;
@@ -3009,7 +3005,7 @@ bool ParseRtpmapAttribute(const std::string& line,
                 JsepSessionDescription::kMaxVideoCodecWidth,
                 JsepSessionDescription::kMaxVideoCodecHeight,
                 JsepSessionDescription::kDefaultVideoCodecFramerate,
-                preference, video_desc);
+                video_desc);
   } else if (media_type == cricket::MEDIA_TYPE_AUDIO) {
     // RFC 4566
     // For audio streams, <encoding parameters> indicates the number
@@ -3037,12 +3033,11 @@ bool ParseRtpmapAttribute(const std::string& line,
     AudioContentDescription* audio_desc =
         static_cast<AudioContentDescription*>(media_desc);
     UpdateCodec(payload_type, encoding_name, clock_rate, bitrate, channels,
-                preference, audio_desc);
+                audio_desc);
   } else if (media_type == cricket::MEDIA_TYPE_DATA) {
     DataContentDescription* data_desc =
         static_cast<DataContentDescription*>(media_desc);
-    data_desc->AddCodec(cricket::DataCodec(payload_type, encoding_name,
-                                           preference));
+    data_desc->AddCodec(cricket::DataCodec(payload_type, encoding_name));
   }
   return true;
 }