Removing `preference` field from `cricket::Codec`.
This field only existed as an implementation detail for getting the
codecs sorted, so it doesn't need to be in the public interface.
It cluttered the code and undesirably affected codec comparisons,
causing the video encoder to be reconfigured if a codec's preference
changed but nothing else did.
BUG=webrtc:5690
Review URL: https://codereview.webrtc.org/1845673002
Cr-Commit-Position: refs/heads/master@{#12349}
diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc
index 90e0007..8497f4c 100644
--- a/webrtc/api/webrtcsdp.cc
+++ b/webrtc/api/webrtcsdp.cc
@@ -15,6 +15,7 @@
#include <stdio.h>
#include <algorithm>
#include <string>
+#include <unordered_map>
#include <vector>
#include "webrtc/api/jsepicecandidate.h"
@@ -269,7 +270,7 @@
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 +288,7 @@
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,
@@ -1651,9 +1652,8 @@
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;
@@ -2179,9 +2179,8 @@
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) &&
@@ -2191,14 +2190,8 @@
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();
}
}
@@ -2207,7 +2200,7 @@
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,
@@ -2228,14 +2221,28 @@
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::unordered_map<int, int> payload_type_preferences;
+ // "size + 1" so that the lowest preference payload type has a preference of
+ // 1, which is greater than the default (0) for payload types not in the fmt
+ // list.
+ int preference = static_cast<int>(payload_types.size() + 1);
+ for (int pt : payload_types) {
+ payload_type_preferences[pt] = preference--;
+ }
+ std::vector<typename C::CodecType> codecs = media_desc->codecs();
+ std::sort(codecs.begin(), codecs.end(), [&payload_type_preferences](
+ const typename C::CodecType& a,
+ const typename C::CodecType& b) {
+ return payload_type_preferences[a.id] > payload_type_preferences[b.id];
+ });
+ media_desc->set_codecs(codecs);
return media_desc;
}
@@ -2274,7 +2281,7 @@
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.
@@ -2287,7 +2294,7 @@
if (!GetPayloadTypeFromString(line, fields[j], &pl, error)) {
return false;
}
- codec_preference.push_back(pl);
+ payload_types.push_back(pl);
}
}
@@ -2302,20 +2309,17 @@
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;
@@ -2522,7 +2526,7 @@
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,
@@ -2535,7 +2539,7 @@
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".
@@ -2670,8 +2674,8 @@
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)) {
@@ -2927,9 +2931,12 @@
}
// 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).
@@ -2939,15 +2946,17 @@
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).
@@ -2957,14 +2966,13 @@
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;
@@ -2986,12 +2994,8 @@
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;
@@ -3021,7 +3025,7 @@
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
@@ -3049,12 +3053,11 @@
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;
}