diff --git a/webrtc/api/jsepsessiondescription.cc b/webrtc/api/jsepsessiondescription.cc index eb776c86fa..ee0a8e14eb 100644 --- a/webrtc/api/jsepsessiondescription.cc +++ b/webrtc/api/jsepsessiondescription.cc @@ -57,7 +57,6 @@ const int JsepSessionDescription::kMaxVideoCodecHeight = 1280; const int JsepSessionDescription::kMaxVideoCodecWidth = 1920; const int JsepSessionDescription::kMaxVideoCodecHeight = 1080; #endif -const int JsepSessionDescription::kDefaultVideoCodecPreference = 1; SessionDescriptionInterface* CreateSessionDescription(const std::string& type, const std::string& sdp, diff --git a/webrtc/api/jsepsessiondescription.h b/webrtc/api/jsepsessiondescription.h index 9a0d873204..ca39b8a675 100644 --- a/webrtc/api/jsepsessiondescription.h +++ b/webrtc/api/jsepsessiondescription.h @@ -72,7 +72,6 @@ class JsepSessionDescription : public SessionDescriptionInterface { static const char kDefaultVideoCodecName[]; static const int kMaxVideoCodecWidth; static const int kMaxVideoCodecHeight; - static const int kDefaultVideoCodecPreference; private: rtc::scoped_ptr description_; diff --git a/webrtc/api/jsepsessiondescription_unittest.cc b/webrtc/api/jsepsessiondescription_unittest.cc index 3d87513731..8f9fc5427f 100644 --- a/webrtc/api/jsepsessiondescription_unittest.cc +++ b/webrtc/api/jsepsessiondescription_unittest.cc @@ -48,11 +48,11 @@ static cricket::SessionDescription* CreateCricketSessionDescription() { scoped_ptr video( new cricket::VideoContentDescription()); - audio->AddCodec(cricket::AudioCodec(103, "ISAC", 16000, 0, 0, 0)); + audio->AddCodec(cricket::AudioCodec(103, "ISAC", 16000, 0, 0)); desc->AddContent(cricket::CN_AUDIO, cricket::NS_JINGLE_RTP, audio.release()); - video->AddCodec(cricket::VideoCodec(120, "VP8", 640, 480, 30, 0)); + video->AddCodec(cricket::VideoCodec(120, "VP8", 640, 480, 30)); desc->AddContent(cricket::CN_VIDEO, cricket::NS_JINGLE_RTP, video.release()); diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc index 90e0007628..8497f4c5fa 100644 --- a/webrtc/api/webrtcsdp.cc +++ b/webrtc/api/webrtcsdp.cc @@ -15,6 +15,7 @@ #include #include #include +#include #include #include "webrtc/api/jsepicecandidate.h" @@ -269,7 +270,7 @@ static bool ParseContent(const std::string& message, const MediaType media_type, int mline_index, const std::string& protocol, - const std::vector& codec_preference, + const std::vector& payload_types, size_t* pos, std::string* content_name, MediaContentDescription* media_desc, @@ -287,7 +288,7 @@ static bool ParseCryptoAttribute(const std::string& line, SdpParseError* error); static bool ParseRtpmapAttribute(const std::string& line, const MediaType media_type, - const std::vector& codec_preference, + const std::vector& payload_types, MediaContentDescription* media_desc, SdpParseError* error); static bool ParseFmtpAttributes(const std::string& line, @@ -1651,9 +1652,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; @@ -2179,9 +2179,8 @@ void MaybeCreateStaticPayloadAudioCodecs( if (!media_desc) { return; } - int preference = static_cast(fmts.size()); + RTC_DCHECK(media_desc->codecs().empty()); std::vector::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 @@ 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(); } } @@ -2207,7 +2200,7 @@ static C* ParseContentDescription(const std::string& message, const MediaType media_type, int mline_index, const std::string& protocol, - const std::vector& codec_preference, + const std::vector& payload_types, size_t* pos, std::string* content_name, TransportDescription* transport, @@ -2228,14 +2221,28 @@ 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::unordered_map 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(payload_types.size() + 1); + for (int pt : payload_types) { + payload_type_preferences[pt] = preference--; + } + std::vector 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 @@ bool ParseMediaDescription(const std::string& message, std::string protocol = fields[2]; // - std::vector codec_preference; + std::vector 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 @@ 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); } } @@ -2302,20 +2309,17 @@ bool ParseMediaDescription(const std::string& message, std::string content_name; if (HasAttribute(line, kMediaTypeVideo)) { content.reset(ParseContentDescription( - 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( - 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( - 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 @@ bool ParseContent(const std::string& message, const MediaType media_type, int mline_index, const std::string& protocol, - const std::vector& codec_preference, + const std::vector& payload_types, size_t* pos, std::string* content_name, MediaContentDescription* media_desc, @@ -2535,7 +2539,7 @@ bool ParseContent(const std::string& message, if (media_type == cricket::MEDIA_TYPE_AUDIO) { MaybeCreateStaticPayloadAudioCodecs( - codec_preference, static_cast(media_desc)); + payload_types, static_cast(media_desc)); } // The media level "ice-ufrag" and "ice-pwd". @@ -2670,8 +2674,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)) { @@ -2927,9 +2931,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). @@ -2939,15 +2946,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(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 @@ void UpdateCodec(int payload_type, const std::string& name, int width, codec.width = width; codec.height = height; codec.framerate = framerate; - codec.preference = preference; AddOrReplaceCodec(video_desc, codec); } bool ParseRtpmapAttribute(const std::string& line, const MediaType media_type, - const std::vector& codec_preference, + const std::vector& payload_types, MediaContentDescription* media_desc, SdpParseError* error) { std::vector fields; @@ -2986,12 +2994,8 @@ bool ParseRtpmapAttribute(const std::string& line, return false; } - // Set the preference order depending on the order of the pl type in the - // 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 " << " of the m-line: " << line; return true; @@ -3021,7 +3025,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, indicates the number @@ -3049,12 +3053,11 @@ bool ParseRtpmapAttribute(const std::string& line, AudioContentDescription* audio_desc = static_cast(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(media_desc); - data_desc->AddCodec(cricket::DataCodec(payload_type, encoding_name, - preference)); + data_desc->AddCodec(cricket::DataCodec(payload_type, encoding_name)); } return true; } diff --git a/webrtc/api/webrtcsdp_unittest.cc b/webrtc/api/webrtcsdp_unittest.cc index 3734ac5b86..2b63c798b0 100644 --- a/webrtc/api/webrtcsdp_unittest.cc +++ b/webrtc/api/webrtcsdp_unittest.cc @@ -1030,10 +1030,10 @@ class WebRtcSdpTest : public testing::Test { "inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32", "dummy_session_params")); audio->set_protocol(cricket::kMediaProtocolSavpf); - AudioCodec opus(111, "opus", 48000, 0, 2, 3); + AudioCodec opus(111, "opus", 48000, 0, 2); audio->AddCodec(opus); - audio->AddCodec(AudioCodec(103, "ISAC", 16000, 32000, 1, 2)); - audio->AddCodec(AudioCodec(104, "ISAC", 32000, 56000, 1, 1)); + audio->AddCodec(AudioCodec(103, "ISAC", 16000, 32000, 1)); + audio->AddCodec(AudioCodec(104, "ISAC", 32000, 56000, 1)); return audio; } @@ -1049,8 +1049,7 @@ class WebRtcSdpTest : public testing::Test { VideoCodec(120, JsepSessionDescription::kDefaultVideoCodecName, JsepSessionDescription::kMaxVideoCodecWidth, JsepSessionDescription::kMaxVideoCodecHeight, - JsepSessionDescription::kDefaultVideoCodecFramerate, - JsepSessionDescription::kDefaultVideoCodecPreference)); + JsepSessionDescription::kDefaultVideoCodecFramerate)); return video; } @@ -1400,7 +1399,7 @@ class WebRtcSdpTest : public testing::Test { data_desc_ = data.get(); data_desc_->set_protocol(cricket::kMediaProtocolDtlsSctp); DataCodec codec(cricket::kGoogleSctpDataCodecId, - cricket::kGoogleSctpDataCodecName, 0); + cricket::kGoogleSctpDataCodecName); codec.SetParam(cricket::kCodecParamPort, kDefaultSctpPort); data_desc_->AddCodec(codec); desc_.AddContent(kDataContentName, NS_JINGLE_DRAFT_SCTP, data.release()); @@ -1413,7 +1412,7 @@ class WebRtcSdpTest : public testing::Test { new DataContentDescription()); data_desc_ = data.get(); - data_desc_->AddCodec(DataCodec(101, "google-data", 1)); + data_desc_->AddCodec(DataCodec(101, "google-data")); StreamParams data_stream; data_stream.id = kDataChannelMsid; data_stream.cname = kDataChannelCname; @@ -1995,8 +1994,8 @@ TEST_F(WebRtcSdpTest, SerializeWithSctpDataChannelAndNewPort) { jsep_desc.description()->GetContentDescriptionByName(kDataContentName)); const int kNewPort = 1234; - cricket::DataCodec codec( - cricket::kGoogleSctpDataCodecId, cricket::kGoogleSctpDataCodecName, 0); + cricket::DataCodec codec(cricket::kGoogleSctpDataCodecId, + cricket::kGoogleSctpDataCodecName); codec.SetParam(cricket::kCodecParamPort, kNewPort); dcdesc->AddOrReplaceCodec(codec); @@ -2177,10 +2176,11 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutRtpmap) { static_cast( jdesc.description()->GetContentDescriptionByName(cricket::CN_AUDIO)); AudioCodecs ref_codecs; - // The codecs in the AudioContentDescription will be sorted by preference. - ref_codecs.push_back(AudioCodec(0, "PCMU", 8000, 0, 1, 3)); - ref_codecs.push_back(AudioCodec(18, "G729", 16000, 0, 1, 2)); - ref_codecs.push_back(AudioCodec(103, "ISAC", 16000, 32000, 1, 1)); + // The codecs in the AudioContentDescription should be in the same order as + // the payload types (s) on the m= line. + ref_codecs.push_back(AudioCodec(0, "PCMU", 8000, 0, 1)); + ref_codecs.push_back(AudioCodec(18, "G729", 16000, 0, 1)); + ref_codecs.push_back(AudioCodec(103, "ISAC", 16000, 32000, 1)); EXPECT_EQ(ref_codecs, audio->codecs()); } diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index 28f93dfd71..feb1518db0 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -450,8 +450,8 @@ class WebRtcSessionTest void InitWithDtmfCodec() { // Add kTelephoneEventCodec for dtmf test. - const cricket::AudioCodec kTelephoneEventCodec( - 106, "telephone-event", 8000, 0, 1, 0); + const cricket::AudioCodec kTelephoneEventCodec(106, "telephone-event", 8000, + 0, 1); std::vector codecs; codecs.push_back(kTelephoneEventCodec); media_engine_->SetAudioCodecs(codecs); @@ -1313,8 +1313,8 @@ class WebRtcSessionTest // Adds CN codecs to FakeMediaEngine and MediaDescriptionFactory. void AddCNCodecs() { - const cricket::AudioCodec kCNCodec1(102, "CN", 8000, 0, 1, 0); - const cricket::AudioCodec kCNCodec2(103, "CN", 16000, 0, 1, 0); + const cricket::AudioCodec kCNCodec1(102, "CN", 8000, 0, 1); + const cricket::AudioCodec kCNCodec2(103, "CN", 16000, 0, 1); // Add kCNCodec for dtmf test. std::vector codecs = media_engine_->audio_codecs();; diff --git a/webrtc/media/base/codec.cc b/webrtc/media/base/codec.cc index 287de0cdbf..9e54d2971a 100644 --- a/webrtc/media/base/codec.cc +++ b/webrtc/media/base/codec.cc @@ -71,12 +71,10 @@ bool FeedbackParams::HasDuplicateEntries() const { return false; } -Codec::Codec(int id, const std::string& name, int clockrate, int preference) - : id(id), name(name), clockrate(clockrate), preference(preference) { -} +Codec::Codec(int id, const std::string& name, int clockrate) + : id(id), name(name), clockrate(clockrate) {} -Codec::Codec() : id(0), clockrate(0), preference(0) { -} +Codec::Codec() : id(0), clockrate(0) {} Codec::Codec(const Codec& c) = default; @@ -86,7 +84,6 @@ Codec& Codec::operator=(const Codec& c) { this->id = c.id; // id is reserved in objective-c name = c.name; clockrate = c.clockrate; - preference = c.preference; params = c.params; feedback_params = c.feedback_params; return *this; @@ -94,8 +91,7 @@ Codec& Codec::operator=(const Codec& c) { bool Codec::operator==(const Codec& c) const { return this->id == c.id && // id is reserved in objective-c - name == c.name && clockrate == c.clockrate && - preference == c.preference && params == c.params && + name == c.name && clockrate == c.clockrate && params == c.params && feedback_params == c.feedback_params; } @@ -150,12 +146,8 @@ AudioCodec::AudioCodec(int id, const std::string& name, int clockrate, int bitrate, - size_t channels, - int preference) - : Codec(id, name, clockrate, preference), - bitrate(bitrate), - channels(channels) { -} + size_t channels) + : Codec(id, name, clockrate), bitrate(bitrate), channels(channels) {} AudioCodec::AudioCodec() : Codec(), bitrate(0), channels(0) { } @@ -193,14 +185,14 @@ bool AudioCodec::Matches(const AudioCodec& codec) const { std::string AudioCodec::ToString() const { std::ostringstream os; os << "AudioCodec[" << id << ":" << name << ":" << clockrate << ":" << bitrate - << ":" << channels << ":" << preference << "]"; + << ":" << channels << "]"; return os.str(); } std::string VideoCodec::ToString() const { std::ostringstream os; os << "VideoCodec[" << id << ":" << name << ":" << width << ":" << height - << ":" << framerate << ":" << preference << "]"; + << ":" << framerate << "]"; return os.str(); } @@ -208,20 +200,17 @@ VideoCodec::VideoCodec(int id, const std::string& name, int width, int height, - int framerate, - int preference) - : Codec(id, name, kVideoCodecClockrate, preference), + int framerate) + : Codec(id, name, kVideoCodecClockrate), width(width), height(height), - framerate(framerate) { -} + framerate(framerate) {} VideoCodec::VideoCodec(int id, const std::string& name) - : Codec(id, name, kVideoCodecClockrate, 0), + : Codec(id, name, kVideoCodecClockrate), width(0), height(0), - framerate(0) { -} + framerate(0) {} VideoCodec::VideoCodec() : Codec(), width(0), height(0), framerate(0) { clockrate = kVideoCodecClockrate; @@ -244,7 +233,7 @@ bool VideoCodec::operator==(const VideoCodec& c) const { VideoCodec VideoCodec::CreateRtxCodec(int rtx_payload_type, int associated_payload_type) { - VideoCodec rtx_codec(rtx_payload_type, kRtxCodecName, 0, 0, 0, 0); + VideoCodec rtx_codec(rtx_payload_type, kRtxCodecName, 0, 0, 0); rtx_codec.SetParam(kCodecParamAssociatedPayloadType, associated_payload_type); return rtx_codec; } @@ -291,9 +280,8 @@ bool VideoCodec::ValidateCodecFormat() const { return true; } -DataCodec::DataCodec(int id, const std::string& name, int preference) - : Codec(id, name, kDataCodecClockrate, preference) { -} +DataCodec::DataCodec(int id, const std::string& name) + : Codec(id, name, kDataCodecClockrate) {} DataCodec::DataCodec() : Codec() { clockrate = kDataCodecClockrate; diff --git a/webrtc/media/base/codec.h b/webrtc/media/base/codec.h index 7476c02694..c79a3fafc8 100644 --- a/webrtc/media/base/codec.h +++ b/webrtc/media/base/codec.h @@ -64,12 +64,11 @@ struct Codec { int id; std::string name; int clockrate; - int preference; CodecParameterMap params; FeedbackParams feedback_params; // Creates a codec with the given parameters. - Codec(int id, const std::string& name, int clockrate, int preference); + Codec(int id, const std::string& name, int clockrate); // Creates an empty codec. Codec(); Codec(const Codec& c); @@ -92,10 +91,6 @@ struct Codec { bool HasFeedbackParam(const FeedbackParam& param) const; void AddFeedbackParam(const FeedbackParam& param); - static bool Preferable(const Codec& first, const Codec& other) { - return first.preference > other.preference; - } - // Filter |this| feedbacks params such that only those shared by both |this| // and |other| are kept. void IntersectFeedbackParams(const Codec& other); @@ -118,8 +113,7 @@ struct AudioCodec : public Codec { const std::string& name, int clockrate, int bitrate, - size_t channels, - int preference); + size_t channels); // Creates an empty codec. AudioCodec(); AudioCodec(const AudioCodec& c); @@ -128,10 +122,6 @@ struct AudioCodec : public Codec { // Indicates if this codec is compatible with the specified codec. bool Matches(const AudioCodec& codec) const; - static bool Preferable(const AudioCodec& first, const AudioCodec& other) { - return first.preference > other.preference; - } - std::string ToString() const; AudioCodec& operator=(const AudioCodec& c); @@ -153,18 +143,13 @@ struct VideoCodec : public Codec { const std::string& name, int width, int height, - int framerate, - int preference); + int framerate); VideoCodec(int id, const std::string& name); // Creates an empty codec. VideoCodec(); VideoCodec(const VideoCodec& c); ~VideoCodec() = default; - static bool Preferable(const VideoCodec& first, const VideoCodec& other) { - return first.preference > other.preference; - } - std::string ToString() const; VideoCodec& operator=(const VideoCodec& c); @@ -193,7 +178,7 @@ struct VideoCodec : public Codec { }; struct DataCodec : public Codec { - DataCodec(int id, const std::string& name, int preference); + DataCodec(int id, const std::string& name); DataCodec(); DataCodec(const DataCodec& c); diff --git a/webrtc/media/base/codec_unittest.cc b/webrtc/media/base/codec_unittest.cc index 88d49632d0..26a6ddae18 100644 --- a/webrtc/media/base/codec_unittest.cc +++ b/webrtc/media/base/codec_unittest.cc @@ -26,7 +26,7 @@ class CodecTest : public testing::Test { }; TEST_F(CodecTest, TestCodecOperators) { - Codec c0(96, "D", 1000, 0); + Codec c0(96, "D", 1000); c0.SetParam("a", 1); Codec c1 = c0; @@ -49,36 +49,30 @@ TEST_F(CodecTest, TestCodecOperators) { c1.clockrate = 2000; EXPECT_TRUE(c0 != c1); - c1 = c0; - c1.preference = 1; - EXPECT_TRUE(c0 != c1); - c1 = c0; c1.SetParam("a", 2); EXPECT_TRUE(c0 != c1); Codec c5; - Codec c6(0, "", 0, 0); + Codec c6(0, "", 0); EXPECT_TRUE(c5 == c6); } TEST_F(CodecTest, TestAudioCodecOperators) { - AudioCodec c0(96, "A", 44100, 20000, 2, 3); - AudioCodec c1(95, "A", 44100, 20000, 2, 3); - AudioCodec c2(96, "x", 44100, 20000, 2, 3); - AudioCodec c3(96, "A", 48000, 20000, 2, 3); - AudioCodec c4(96, "A", 44100, 10000, 2, 3); - AudioCodec c5(96, "A", 44100, 20000, 1, 3); - AudioCodec c6(96, "A", 44100, 20000, 2, 1); + AudioCodec c0(96, "A", 44100, 20000, 2); + AudioCodec c1(95, "A", 44100, 20000, 2); + AudioCodec c2(96, "x", 44100, 20000, 2); + AudioCodec c3(96, "A", 48000, 20000, 2); + AudioCodec c4(96, "A", 44100, 10000, 2); + AudioCodec c5(96, "A", 44100, 20000, 1); EXPECT_TRUE(c0 != c1); EXPECT_TRUE(c0 != c2); EXPECT_TRUE(c0 != c3); EXPECT_TRUE(c0 != c4); EXPECT_TRUE(c0 != c5); - EXPECT_TRUE(c0 != c6); AudioCodec c7; - AudioCodec c8(0, "", 0, 0, 0, 0); + AudioCodec c8(0, "", 0, 0, 0); AudioCodec c9 = c0; EXPECT_TRUE(c8 == c7); EXPECT_TRUE(c9 != c7); @@ -103,61 +97,59 @@ TEST_F(CodecTest, TestAudioCodecOperators) { TEST_F(CodecTest, TestAudioCodecMatches) { // Test a codec with a static payload type. - AudioCodec c0(95, "A", 44100, 20000, 1, 3); - EXPECT_TRUE(c0.Matches(AudioCodec(95, "", 44100, 20000, 1, 0))); - EXPECT_TRUE(c0.Matches(AudioCodec(95, "", 44100, 20000, 0, 0))); - EXPECT_TRUE(c0.Matches(AudioCodec(95, "", 44100, 0, 0, 0))); - EXPECT_TRUE(c0.Matches(AudioCodec(95, "", 0, 0, 0, 0))); - EXPECT_FALSE(c0.Matches(AudioCodec(96, "", 44100, 20000, 1, 0))); - EXPECT_FALSE(c0.Matches(AudioCodec(95, "", 55100, 20000, 1, 0))); - EXPECT_FALSE(c0.Matches(AudioCodec(95, "", 44100, 30000, 1, 0))); - EXPECT_FALSE(c0.Matches(AudioCodec(95, "", 44100, 20000, 2, 0))); - EXPECT_FALSE(c0.Matches(AudioCodec(95, "", 55100, 30000, 2, 0))); + AudioCodec c0(95, "A", 44100, 20000, 1); + EXPECT_TRUE(c0.Matches(AudioCodec(95, "", 44100, 20000, 1))); + EXPECT_TRUE(c0.Matches(AudioCodec(95, "", 44100, 20000, 0))); + EXPECT_TRUE(c0.Matches(AudioCodec(95, "", 44100, 0, 0))); + EXPECT_TRUE(c0.Matches(AudioCodec(95, "", 0, 0, 0))); + EXPECT_FALSE(c0.Matches(AudioCodec(96, "", 44100, 20000, 1))); + EXPECT_FALSE(c0.Matches(AudioCodec(95, "", 55100, 20000, 1))); + EXPECT_FALSE(c0.Matches(AudioCodec(95, "", 44100, 30000, 1))); + EXPECT_FALSE(c0.Matches(AudioCodec(95, "", 44100, 20000, 2))); + EXPECT_FALSE(c0.Matches(AudioCodec(95, "", 55100, 30000, 2))); // Test a codec with a dynamic payload type. - AudioCodec c1(96, "A", 44100, 20000, 1, 3); - EXPECT_TRUE(c1.Matches(AudioCodec(96, "A", 0, 0, 0, 0))); - EXPECT_TRUE(c1.Matches(AudioCodec(97, "A", 0, 0, 0, 0))); - EXPECT_TRUE(c1.Matches(AudioCodec(96, "a", 0, 0, 0, 0))); - EXPECT_TRUE(c1.Matches(AudioCodec(97, "a", 0, 0, 0, 0))); - EXPECT_FALSE(c1.Matches(AudioCodec(95, "A", 0, 0, 0, 0))); - EXPECT_FALSE(c1.Matches(AudioCodec(96, "", 44100, 20000, 2, 0))); - EXPECT_FALSE(c1.Matches(AudioCodec(96, "A", 55100, 30000, 1, 0))); + AudioCodec c1(96, "A", 44100, 20000, 1); + EXPECT_TRUE(c1.Matches(AudioCodec(96, "A", 0, 0, 0))); + EXPECT_TRUE(c1.Matches(AudioCodec(97, "A", 0, 0, 0))); + EXPECT_TRUE(c1.Matches(AudioCodec(96, "a", 0, 0, 0))); + EXPECT_TRUE(c1.Matches(AudioCodec(97, "a", 0, 0, 0))); + EXPECT_FALSE(c1.Matches(AudioCodec(95, "A", 0, 0, 0))); + EXPECT_FALSE(c1.Matches(AudioCodec(96, "", 44100, 20000, 2))); + EXPECT_FALSE(c1.Matches(AudioCodec(96, "A", 55100, 30000, 1))); // Test a codec with a dynamic payload type, and auto bitrate. - AudioCodec c2(97, "A", 16000, 0, 1, 3); + AudioCodec c2(97, "A", 16000, 0, 1); // Use default bitrate. - EXPECT_TRUE(c2.Matches(AudioCodec(97, "A", 16000, 0, 1, 0))); - EXPECT_TRUE(c2.Matches(AudioCodec(97, "A", 16000, 0, 0, 0))); + EXPECT_TRUE(c2.Matches(AudioCodec(97, "A", 16000, 0, 1))); + EXPECT_TRUE(c2.Matches(AudioCodec(97, "A", 16000, 0, 0))); // Use explicit bitrate. - EXPECT_TRUE(c2.Matches(AudioCodec(97, "A", 16000, 32000, 1, 0))); + EXPECT_TRUE(c2.Matches(AudioCodec(97, "A", 16000, 32000, 1))); // Backward compatibility with clients that might send "-1" (for default). - EXPECT_TRUE(c2.Matches(AudioCodec(97, "A", 16000, -1, 1, 0))); + EXPECT_TRUE(c2.Matches(AudioCodec(97, "A", 16000, -1, 1))); // Stereo doesn't match channels = 0. - AudioCodec c3(96, "A", 44100, 20000, 2, 3); - EXPECT_TRUE(c3.Matches(AudioCodec(96, "A", 44100, 20000, 2, 3))); - EXPECT_FALSE(c3.Matches(AudioCodec(96, "A", 44100, 20000, 1, 3))); - EXPECT_FALSE(c3.Matches(AudioCodec(96, "A", 44100, 20000, 0, 3))); + AudioCodec c3(96, "A", 44100, 20000, 2); + EXPECT_TRUE(c3.Matches(AudioCodec(96, "A", 44100, 20000, 2))); + EXPECT_FALSE(c3.Matches(AudioCodec(96, "A", 44100, 20000, 1))); + EXPECT_FALSE(c3.Matches(AudioCodec(96, "A", 44100, 20000, 0))); } TEST_F(CodecTest, TestVideoCodecOperators) { - VideoCodec c0(96, "V", 320, 200, 30, 3); - VideoCodec c1(95, "V", 320, 200, 30, 3); - VideoCodec c2(96, "x", 320, 200, 30, 3); - VideoCodec c3(96, "V", 120, 200, 30, 3); - VideoCodec c4(96, "V", 320, 100, 30, 3); - VideoCodec c5(96, "V", 320, 200, 10, 3); - VideoCodec c6(96, "V", 320, 200, 30, 1); + VideoCodec c0(96, "V", 320, 200, 30); + VideoCodec c1(95, "V", 320, 200, 30); + VideoCodec c2(96, "x", 320, 200, 30); + VideoCodec c3(96, "V", 120, 200, 30); + VideoCodec c4(96, "V", 320, 100, 30); + VideoCodec c5(96, "V", 320, 200, 10); EXPECT_TRUE(c0 != c1); EXPECT_TRUE(c0 != c2); EXPECT_TRUE(c0 != c3); EXPECT_TRUE(c0 != c4); EXPECT_TRUE(c0 != c5); - EXPECT_TRUE(c0 != c6); VideoCodec c7; - VideoCodec c8(0, "", 0, 0, 0, 0); + VideoCodec c8(0, "", 0, 0, 0); VideoCodec c9 = c0; EXPECT_TRUE(c8 == c7); EXPECT_TRUE(c9 != c7); @@ -182,34 +174,34 @@ TEST_F(CodecTest, TestVideoCodecOperators) { TEST_F(CodecTest, TestVideoCodecMatches) { // Test a codec with a static payload type. - VideoCodec c0(95, "V", 320, 200, 30, 3); - EXPECT_TRUE(c0.Matches(VideoCodec(95, "", 640, 400, 15, 0))); - EXPECT_FALSE(c0.Matches(VideoCodec(96, "", 320, 200, 30, 0))); + VideoCodec c0(95, "V", 320, 200, 30); + EXPECT_TRUE(c0.Matches(VideoCodec(95, "", 640, 400, 15))); + EXPECT_FALSE(c0.Matches(VideoCodec(96, "", 320, 200, 30))); // Test a codec with a dynamic payload type. - VideoCodec c1(96, "V", 320, 200, 30, 3); - EXPECT_TRUE(c1.Matches(VideoCodec(96, "V", 640, 400, 15, 0))); - EXPECT_TRUE(c1.Matches(VideoCodec(97, "V", 640, 400, 15, 0))); - EXPECT_TRUE(c1.Matches(VideoCodec(96, "v", 640, 400, 15, 0))); - EXPECT_TRUE(c1.Matches(VideoCodec(97, "v", 640, 400, 15, 0))); - EXPECT_FALSE(c1.Matches(VideoCodec(96, "", 320, 200, 30, 0))); - EXPECT_FALSE(c1.Matches(VideoCodec(95, "V", 640, 400, 15, 0))); + VideoCodec c1(96, "V", 320, 200, 30); + EXPECT_TRUE(c1.Matches(VideoCodec(96, "V", 640, 400, 15))); + EXPECT_TRUE(c1.Matches(VideoCodec(97, "V", 640, 400, 15))); + EXPECT_TRUE(c1.Matches(VideoCodec(96, "v", 640, 400, 15))); + EXPECT_TRUE(c1.Matches(VideoCodec(97, "v", 640, 400, 15))); + EXPECT_FALSE(c1.Matches(VideoCodec(96, "", 320, 200, 30))); + EXPECT_FALSE(c1.Matches(VideoCodec(95, "V", 640, 400, 15))); } TEST_F(CodecTest, TestDataCodecMatches) { // Test a codec with a static payload type. - DataCodec c0(95, "D", 0); - EXPECT_TRUE(c0.Matches(DataCodec(95, "", 0))); - EXPECT_FALSE(c0.Matches(DataCodec(96, "", 0))); + DataCodec c0(95, "D"); + EXPECT_TRUE(c0.Matches(DataCodec(95, ""))); + EXPECT_FALSE(c0.Matches(DataCodec(96, ""))); // Test a codec with a dynamic payload type. - DataCodec c1(96, "D", 3); - EXPECT_TRUE(c1.Matches(DataCodec(96, "D", 0))); - EXPECT_TRUE(c1.Matches(DataCodec(97, "D", 0))); - EXPECT_TRUE(c1.Matches(DataCodec(96, "d", 0))); - EXPECT_TRUE(c1.Matches(DataCodec(97, "d", 0))); - EXPECT_FALSE(c1.Matches(DataCodec(96, "", 0))); - EXPECT_FALSE(c1.Matches(DataCodec(95, "D", 0))); + DataCodec c1(96, "D"); + EXPECT_TRUE(c1.Matches(DataCodec(96, "D"))); + EXPECT_TRUE(c1.Matches(DataCodec(97, "D"))); + EXPECT_TRUE(c1.Matches(DataCodec(96, "d"))); + EXPECT_TRUE(c1.Matches(DataCodec(97, "d"))); + EXPECT_FALSE(c1.Matches(DataCodec(96, ""))); + EXPECT_FALSE(c1.Matches(DataCodec(95, "D"))); } TEST_F(CodecTest, TestSetParamGetParamAndRemoveParam) { @@ -254,10 +246,10 @@ TEST_F(CodecTest, TestIntersectFeedbackParams) { TEST_F(CodecTest, TestGetCodecType) { // Codec type comparison should be case insenstive on names. - const VideoCodec codec(96, "V", 320, 200, 30, 3); - const VideoCodec rtx_codec(96, "rTx", 320, 200, 30, 3); - const VideoCodec ulpfec_codec(96, "ulpFeC", 320, 200, 30, 3); - const VideoCodec red_codec(96, "ReD", 320, 200, 30, 3); + const VideoCodec codec(96, "V", 320, 200, 30); + const VideoCodec rtx_codec(96, "rTx", 320, 200, 30); + const VideoCodec ulpfec_codec(96, "ulpFeC", 320, 200, 30); + const VideoCodec red_codec(96, "ReD", 320, 200, 30); EXPECT_EQ(VideoCodec::CODEC_VIDEO, codec.GetCodecType()); EXPECT_EQ(VideoCodec::CODEC_RTX, rtx_codec.GetCodecType()); EXPECT_EQ(VideoCodec::CODEC_ULPFEC, ulpfec_codec.GetCodecType()); @@ -275,7 +267,7 @@ TEST_F(CodecTest, TestCreateRtxCodec) { } TEST_F(CodecTest, TestValidateCodecFormat) { - const VideoCodec codec(96, "V", 320, 200, 30, 3); + const VideoCodec codec(96, "V", 320, 200, 30); ASSERT_TRUE(codec.ValidateCodecFormat()); // Accept 0-127 as payload types. diff --git a/webrtc/media/base/fakemediaengine.h b/webrtc/media/base/fakemediaengine.h index 64f4adca50..fbd8f4c275 100644 --- a/webrtc/media/base/fakemediaengine.h +++ b/webrtc/media/base/fakemediaengine.h @@ -702,7 +702,7 @@ class FakeVoiceEngine : public FakeBaseEngine { : output_volume_(-1) { // Add a fake audio codec. Note that the name must not be "" as there are // sanity checks against that. - codecs_.push_back(AudioCodec(101, "fake_audio_codec", 0, 0, 1, 0)); + codecs_.push_back(AudioCodec(101, "fake_audio_codec", 0, 0, 1)); } rtc::scoped_refptr GetAudioState() const { return rtc::scoped_refptr(); @@ -763,7 +763,7 @@ class FakeVideoEngine : public FakeBaseEngine { FakeVideoEngine() : capture_(false) { // Add a fake video codec. Note that the name must not be "" as there are // sanity checks against that. - codecs_.push_back(VideoCodec(0, "fake_video_codec", 0, 0, 0, 0)); + codecs_.push_back(VideoCodec(0, "fake_video_codec", 0, 0, 0)); } void Init() {} bool SetOptions(const VideoOptions& options) { diff --git a/webrtc/media/base/rtpdataengine.cc b/webrtc/media/base/rtpdataengine.cc index ae36174555..4b6647c649 100644 --- a/webrtc/media/base/rtpdataengine.cc +++ b/webrtc/media/base/rtpdataengine.cc @@ -36,8 +36,7 @@ static const size_t kMaxSrtpHmacOverhead = 16; RtpDataEngine::RtpDataEngine() { data_codecs_.push_back( - DataCodec(kGoogleRtpDataCodecId, - kGoogleRtpDataCodecName, 0)); + DataCodec(kGoogleRtpDataCodecId, kGoogleRtpDataCodecName)); SetTiming(new rtc::Timing()); } @@ -92,7 +91,7 @@ void RtpClock::Tick(double now, int* seq_num, uint32_t* timestamp) { } const DataCodec* FindUnknownCodec(const std::vector& codecs) { - DataCodec data_codec(kGoogleRtpDataCodecId, kGoogleRtpDataCodecName, 0); + DataCodec data_codec(kGoogleRtpDataCodecId, kGoogleRtpDataCodecName); std::vector::const_iterator iter; for (iter = codecs.begin(); iter != codecs.end(); ++iter) { if (!iter->Matches(data_codec)) { @@ -103,7 +102,7 @@ const DataCodec* FindUnknownCodec(const std::vector& codecs) { } const DataCodec* FindKnownCodec(const std::vector& codecs) { - DataCodec data_codec(kGoogleRtpDataCodecId, kGoogleRtpDataCodecName, 0); + DataCodec data_codec(kGoogleRtpDataCodecId, kGoogleRtpDataCodecName); std::vector::const_iterator iter; for (iter = codecs.begin(); iter != codecs.end(); ++iter) { if (iter->Matches(data_codec)) { diff --git a/webrtc/media/base/videoengine_unittest.h b/webrtc/media/base/videoengine_unittest.h index 491149c7e2..394b9c9a8e 100644 --- a/webrtc/media/base/videoengine_unittest.h +++ b/webrtc/media/base/videoengine_unittest.h @@ -151,7 +151,7 @@ class VideoMediaChannelTest : public testing::Test, } bool SetOneCodec(int pt, const char* name, int w, int h, int fr) { - return SetOneCodec(cricket::VideoCodec(pt, name, w, h, fr, 0)); + return SetOneCodec(cricket::VideoCodec(pt, name, w, h, fr)); } bool SetOneCodec(const cricket::VideoCodec& codec) { cricket::VideoFormat capture_format(codec.width, codec.height, diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index b593027dbc..4914d9f51d 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -180,7 +180,7 @@ void AddDefaultFeedbackParams(VideoCodec* codec) { static VideoCodec MakeVideoCodecWithDefaultFeedbackParams(int payload_type, const char* name) { VideoCodec codec(payload_type, name, kDefaultVideoMaxWidth, - kDefaultVideoMaxHeight, kDefaultVideoMaxFramerate, 0); + kDefaultVideoMaxHeight, kDefaultVideoMaxFramerate); AddDefaultFeedbackParams(&codec); return codec; } @@ -646,12 +646,9 @@ std::vector WebRtcVideoEngine2::GetSupportedCodecs() const { const int kExternalVideoPayloadTypeBase = 120; size_t payload_type = kExternalVideoPayloadTypeBase + i; RTC_DCHECK(payload_type < 128); - VideoCodec codec(static_cast(payload_type), - codecs[i].name, - codecs[i].max_width, - codecs[i].max_height, - codecs[i].max_fps, - 0); + VideoCodec codec(static_cast(payload_type), codecs[i].name, + codecs[i].max_width, codecs[i].max_height, + codecs[i].max_fps); AddDefaultFeedbackParams(&codec); supported_codecs.push_back(codec); @@ -739,17 +736,7 @@ bool WebRtcVideoChannel2::ReceiveCodecsHaveChanged( }; std::sort(before.begin(), before.end(), comparison); std::sort(after.begin(), after.end(), comparison); - for (size_t i = 0; i < before.size(); ++i) { - // For the same reason that we sort the codecs, we also ignore the - // preference. We don't want a preference change on the receive - // side to cause recreation of the stream. - before[i].codec.preference = 0; - after[i].codec.preference = 0; - if (before[i] != after[i]) { - return true; - } - } - return false; + return before != after; } bool WebRtcVideoChannel2::GetChangedSendParameters( diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 7865102159..d765f8232e 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -31,16 +31,16 @@ namespace { static const int kDefaultQpMax = 56; static const int kDefaultFramerate = 30; -static const cricket::VideoCodec kVp8Codec720p(100, "VP8", 1280, 720, 30, 0); -static const cricket::VideoCodec kVp8Codec360p(100, "VP8", 640, 360, 30, 0); -static const cricket::VideoCodec kVp8Codec270p(100, "VP8", 480, 270, 30, 0); +static const cricket::VideoCodec kVp8Codec720p(100, "VP8", 1280, 720, 30); +static const cricket::VideoCodec kVp8Codec360p(100, "VP8", 640, 360, 30); +static const cricket::VideoCodec kVp8Codec270p(100, "VP8", 480, 270, 30); -static const cricket::VideoCodec kVp8Codec(100, "VP8", 640, 400, 30, 0); -static const cricket::VideoCodec kVp9Codec(101, "VP9", 640, 400, 30, 0); -static const cricket::VideoCodec kH264Codec(102, "H264", 640, 400, 30, 0); +static const cricket::VideoCodec kVp8Codec(100, "VP8", 640, 400, 30); +static const cricket::VideoCodec kVp9Codec(101, "VP9", 640, 400, 30); +static const cricket::VideoCodec kH264Codec(102, "H264", 640, 400, 30); -static const cricket::VideoCodec kRedCodec(116, "red", 0, 0, 0, 0); -static const cricket::VideoCodec kUlpfecCodec(117, "ulpfec", 0, 0, 0, 0); +static const cricket::VideoCodec kRedCodec(116, "red", 0, 0, 0); +static const cricket::VideoCodec kUlpfecCodec(117, "ulpfec", 0, 0, 0); static const uint8_t kRedRtxPayloadType = 125; @@ -852,15 +852,15 @@ WEBRTC_BASE_TEST(SendsLowerResolutionOnSmallerFrames); WEBRTC_BASE_TEST(MultipleSendStreams); TEST_F(WebRtcVideoChannel2BaseTest, SendAndReceiveVp8Vga) { - SendAndReceive(cricket::VideoCodec(100, "VP8", 640, 400, 30, 0)); + SendAndReceive(cricket::VideoCodec(100, "VP8", 640, 400, 30)); } TEST_F(WebRtcVideoChannel2BaseTest, SendAndReceiveVp8Qvga) { - SendAndReceive(cricket::VideoCodec(100, "VP8", 320, 200, 30, 0)); + SendAndReceive(cricket::VideoCodec(100, "VP8", 320, 200, 30)); } TEST_F(WebRtcVideoChannel2BaseTest, SendAndReceiveVp8SvcQqvga) { - SendAndReceive(cricket::VideoCodec(100, "VP8", 160, 100, 30, 0)); + SendAndReceive(cricket::VideoCodec(100, "VP8", 160, 100, 30)); } TEST_F(WebRtcVideoChannel2BaseTest, TwoStreamsSendAndReceive) { @@ -2278,7 +2278,7 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithoutFec) { TEST_F(WebRtcVideoChannel2Test, SetSendCodecRejectsRtxWithoutAssociatedPayloadType) { cricket::VideoSendParameters parameters; - cricket::VideoCodec rtx_codec(96, "rtx", 0, 0, 0, 0); + cricket::VideoCodec rtx_codec(96, "rtx", 0, 0, 0); parameters.codecs.push_back(rtx_codec); EXPECT_FALSE(channel_->SetSendParameters(parameters)) << "RTX codec without associated payload type should be rejected."; @@ -2510,6 +2510,26 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsAcceptAllValidPayloadTypes) { } } +// Test that setting the a different set of codecs but with an identical front +// codec doesn't result in the stream being recreated. +// This may happen when a subsequent negotiation includes fewer codecs, as a +// result of one of the codecs being rejected. +TEST_F(WebRtcVideoChannel2Test, + SetSendCodecsIdenticalFirstCodecDoesntRecreateStream) { + cricket::VideoSendParameters parameters1; + parameters1.codecs.push_back(kVp8Codec); + parameters1.codecs.push_back(kVp9Codec); + EXPECT_TRUE(channel_->SetSendParameters(parameters1)); + + AddSendStream(); + EXPECT_EQ(1, fake_call_->GetNumCreatedSendStreams()); + + cricket::VideoSendParameters parameters2; + parameters2.codecs.push_back(kVp8Codec); + EXPECT_TRUE(channel_->SetSendParameters(parameters2)); + EXPECT_EQ(1, fake_call_->GetNumCreatedSendStreams()); +} + TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithOnlyVp8) { cricket::VideoRecvParameters parameters; parameters.codecs.push_back(kVp8Codec); @@ -2520,7 +2540,7 @@ TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithOnlyVp8) { TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithRtx) { cricket::VideoRecvParameters parameters; parameters.codecs.push_back(kVp8Codec); - cricket::VideoCodec rtx_codec(96, "rtx", 0, 0, 0, 0); + cricket::VideoCodec rtx_codec(96, "rtx", 0, 0, 0); parameters.codecs.push_back(rtx_codec); EXPECT_FALSE(channel_->SetRecvParameters(parameters)) << "RTX codec without associated payload should be rejected."; @@ -2532,7 +2552,7 @@ TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithRtx) { parameters.codecs[1].SetParam("apt", kVp8Codec.id); EXPECT_TRUE(channel_->SetRecvParameters(parameters)); - cricket::VideoCodec rtx_codec2(97, "rtx", 0, 0, 0, 0); + cricket::VideoCodec rtx_codec2(97, "rtx", 0, 0, 0); rtx_codec2.SetParam("apt", rtx_codec.id); parameters.codecs.push_back(rtx_codec2); @@ -2562,7 +2582,7 @@ TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsAcceptDefaultCodecs) { TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsRejectUnsupportedCodec) { cricket::VideoRecvParameters parameters; parameters.codecs.push_back(kVp8Codec); - parameters.codecs.push_back(VideoCodec(101, "WTF3", 640, 400, 30, 0)); + parameters.codecs.push_back(VideoCodec(101, "WTF3", 640, 400, 30)); EXPECT_FALSE(channel_->SetRecvParameters(parameters)); } @@ -2630,10 +2650,10 @@ TEST_F(WebRtcVideoChannel2Test, EXPECT_TRUE(channel_->SetRecvParameters(parameters)); } -// Test that setting the same codecs but with a different order and preference +// Test that setting the same codecs but with a different order // doesn't result in the stream being recreated. TEST_F(WebRtcVideoChannel2Test, - SetRecvCodecsDifferentOrderAndPreferenceDoesntRecreateStream) { + SetRecvCodecsDifferentOrderDoesntRecreateStream) { cricket::VideoRecvParameters parameters1; parameters1.codecs.push_back(kVp8Codec); parameters1.codecs.push_back(kRedCodec); @@ -2645,7 +2665,6 @@ TEST_F(WebRtcVideoChannel2Test, cricket::VideoRecvParameters parameters2; parameters2.codecs.push_back(kRedCodec); parameters2.codecs.push_back(kVp8Codec); - parameters2.codecs[1].preference += 1; EXPECT_TRUE(channel_->SetRecvParameters(parameters2)); EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams()); } @@ -2830,7 +2849,7 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationStats) { channel_->SetSource(kSsrcs3[0], &video_capturer_vga); EXPECT_TRUE(video_capturer_vga.CaptureFrame()); - cricket::VideoCodec send_codec(100, "VP8", 640, 480, 30, 0); + cricket::VideoCodec send_codec(100, "VP8", 640, 480, 30); cricket::VideoSendParameters parameters; parameters.codecs.push_back(send_codec); EXPECT_TRUE(channel_->SetSendParameters(parameters)); @@ -2903,7 +2922,7 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationAndBandwidthStats) { channel_->SetSource(kSsrcs3[0], &video_capturer_vga); EXPECT_TRUE(video_capturer_vga.CaptureFrame()); - cricket::VideoCodec send_codec(100, "VP8", 640, 480, 30, 0); + cricket::VideoCodec send_codec(100, "VP8", 640, 480, 30); cricket::VideoSendParameters parameters; parameters.codecs.push_back(send_codec); EXPECT_TRUE(channel_->SetSendParameters(parameters)); diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index cef7cdf2be..46119311bc 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -250,31 +250,28 @@ class WebRtcVoiceCodecs final { // list and add a test which verifies VoE supports the listed codecs. static std::vector SupportedCodecs() { std::vector result; - for (webrtc::CodecInst voe_codec : webrtc::acm2::RentACodec::Database()) { - // Change the sample rate of G722 to 8000 to match SDP. - MaybeFixupG722(&voe_codec, 8000); - // Skip uncompressed formats. - if (IsCodec(voe_codec, kL16CodecName)) { - continue; - } - - const CodecPref* pref = NULL; - for (size_t j = 0; j < arraysize(kCodecPrefs); ++j) { - if (IsCodec(voe_codec, kCodecPrefs[j].name) && - kCodecPrefs[j].clockrate == voe_codec.plfreq && - kCodecPrefs[j].channels == voe_codec.channels) { - pref = &kCodecPrefs[j]; - break; + // Iterate first over our preferred codecs list, so that the results are + // added in order of preference. + for (size_t i = 0; i < arraysize(kCodecPrefs); ++i) { + const CodecPref* pref = &kCodecPrefs[i]; + for (webrtc::CodecInst voe_codec : webrtc::acm2::RentACodec::Database()) { + // Change the sample rate of G722 to 8000 to match SDP. + MaybeFixupG722(&voe_codec, 8000); + // Skip uncompressed formats. + if (IsCodec(voe_codec, kL16CodecName)) { + continue; } - } - if (pref) { - // Use the payload type that we've configured in our pref table; - // use the offset in our pref table to determine the sort order. - AudioCodec codec( - pref->payload_type, voe_codec.plname, voe_codec.plfreq, - voe_codec.rate, voe_codec.channels, - static_cast(arraysize(kCodecPrefs)) - (pref - kCodecPrefs)); + if (!IsCodec(voe_codec, pref->name) || + pref->clockrate != voe_codec.plfreq || + pref->channels != voe_codec.channels) { + // Not a match. + continue; + } + + AudioCodec codec(pref->payload_type, voe_codec.plname, voe_codec.plfreq, + voe_codec.rate, voe_codec.channels); + LOG(LS_INFO) << "Adding supported codec: " << ToString(codec); if (IsCodec(codec, kIsacCodecName)) { // Indicate auto-bitrate in signaling. codec.bitrate = 0; @@ -297,12 +294,8 @@ class WebRtcVoiceCodecs final { // when they can be set to values other than the default. } result.push_back(codec); - } else { - LOG(LS_INFO) << "[Unused] " << ToString(voe_codec); } } - // Make sure they are in local preference order. - std::sort(result.begin(), result.end(), &AudioCodec::Preferable); return result; } @@ -312,7 +305,7 @@ class WebRtcVoiceCodecs final { // Change the sample rate of G722 to 8000 to match SDP. MaybeFixupG722(&voe_codec, 8000); AudioCodec codec(voe_codec.pltype, voe_codec.plname, voe_codec.plfreq, - voe_codec.rate, voe_codec.channels, 0); + voe_codec.rate, voe_codec.channels); bool multi_rate = IsCodecMultiRate(voe_codec); // Allow arbitrary rates for ISAC to be specified. if (multi_rate) { diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index 1ed493b6d0..e17aea34b8 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -33,16 +33,19 @@ using testing::StrictMock; namespace { -const cricket::AudioCodec kPcmuCodec(0, "PCMU", 8000, 64000, 1, 0); -const cricket::AudioCodec kIsacCodec(103, "ISAC", 16000, 32000, 1, 0); -const cricket::AudioCodec kOpusCodec(111, "opus", 48000, 64000, 2, 0); -const cricket::AudioCodec kG722CodecVoE(9, "G722", 16000, 64000, 1, 0); -const cricket::AudioCodec kG722CodecSdp(9, "G722", 8000, 64000, 1, 0); -const cricket::AudioCodec kRedCodec(117, "red", 8000, 0, 1, 0); -const cricket::AudioCodec kCn8000Codec(13, "CN", 8000, 0, 1, 0); -const cricket::AudioCodec kCn16000Codec(105, "CN", 16000, 0, 1, 0); -const cricket::AudioCodec kTelephoneEventCodec(106, "telephone-event", 8000, 0, - 1, 0); +const cricket::AudioCodec kPcmuCodec(0, "PCMU", 8000, 64000, 1); +const cricket::AudioCodec kIsacCodec(103, "ISAC", 16000, 32000, 1); +const cricket::AudioCodec kOpusCodec(111, "opus", 48000, 64000, 2); +const cricket::AudioCodec kG722CodecVoE(9, "G722", 16000, 64000, 1); +const cricket::AudioCodec kG722CodecSdp(9, "G722", 8000, 64000, 1); +const cricket::AudioCodec kRedCodec(117, "red", 8000, 0, 1); +const cricket::AudioCodec kCn8000Codec(13, "CN", 8000, 0, 1); +const cricket::AudioCodec kCn16000Codec(105, "CN", 16000, 0, 1); +const cricket::AudioCodec kTelephoneEventCodec(106, + "telephone-event", + 8000, + 0, + 1); const uint32_t kSsrc1 = 0x99; const uint32_t kSsrc2 = 2; const uint32_t kSsrc3 = 3; @@ -483,19 +486,14 @@ TEST_F(WebRtcVoiceEngineTestFake, CreateChannel) { } // Tests that the list of supported codecs is created properly and ordered -// correctly -TEST_F(WebRtcVoiceEngineTestFake, CodecPreference) { +// correctly (such that opus appears first). +TEST_F(WebRtcVoiceEngineTestFake, CodecOrder) { const std::vector& codecs = engine_->codecs(); ASSERT_FALSE(codecs.empty()); EXPECT_STRCASEEQ("opus", codecs[0].name.c_str()); EXPECT_EQ(48000, codecs[0].clockrate); EXPECT_EQ(2, codecs[0].channels); EXPECT_EQ(64000, codecs[0].bitrate); - int pref = codecs[0].preference; - for (size_t i = 1; i < codecs.size(); ++i) { - EXPECT_GT(pref, codecs[i].preference); - pref = codecs[i].preference; - } } TEST_F(WebRtcVoiceEngineTestFake, OpusSupportsTransportCc) { @@ -579,7 +577,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRecvCodecsUnsupportedCodec) { EXPECT_TRUE(SetupChannel()); cricket::AudioRecvParameters parameters; parameters.codecs.push_back(kIsacCodec); - parameters.codecs.push_back(cricket::AudioCodec(127, "XYZ", 32000, 0, 1, 0)); + parameters.codecs.push_back(cricket::AudioCodec(127, "XYZ", 32000, 0, 1)); EXPECT_FALSE(channel_->SetRecvParameters(parameters)); } @@ -3410,55 +3408,55 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) { TEST(WebRtcVoiceEngineTest, HasCorrectCodecs) { // Check codecs by name. EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "OPUS", 48000, 0, 2, 0), nullptr)); + cricket::AudioCodec(96, "OPUS", 48000, 0, 2), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "ISAC", 16000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "ISAC", 16000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "ISAC", 32000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "ISAC", 32000, 0, 1), nullptr)); // Check that name matching is case-insensitive. EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "ILBC", 8000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "ILBC", 8000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "iLBC", 8000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "iLBC", 8000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "PCMU", 8000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "PCMU", 8000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "PCMA", 8000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "PCMA", 8000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "G722", 8000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "G722", 8000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "red", 8000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "red", 8000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "CN", 32000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "CN", 32000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "CN", 16000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "CN", 16000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "CN", 8000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "CN", 8000, 0, 1), nullptr)); EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(96, "telephone-event", 8000, 0, 1, 0), nullptr)); + cricket::AudioCodec(96, "telephone-event", 8000, 0, 1), nullptr)); // Check codecs with an id by id. EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(0, "", 8000, 0, 1, 0), nullptr)); // PCMU + cricket::AudioCodec(0, "", 8000, 0, 1), nullptr)); // PCMU EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(8, "", 8000, 0, 1, 0), nullptr)); // PCMA + cricket::AudioCodec(8, "", 8000, 0, 1), nullptr)); // PCMA EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(9, "", 8000, 0, 1, 0), nullptr)); // G722 + cricket::AudioCodec(9, "", 8000, 0, 1), nullptr)); // G722 EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(13, "", 8000, 0, 1, 0), nullptr)); // CN + cricket::AudioCodec(13, "", 8000, 0, 1), nullptr)); // CN // Check sample/bitrate matching. EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(0, "PCMU", 8000, 64000, 1, 0), nullptr)); + cricket::AudioCodec(0, "PCMU", 8000, 64000, 1), nullptr)); // Check that bad codecs fail. EXPECT_FALSE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(99, "ABCD", 0, 0, 1, 0), nullptr)); + cricket::AudioCodec(99, "ABCD", 0, 0, 1), nullptr)); EXPECT_FALSE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(88, "", 0, 0, 1, 0), nullptr)); + cricket::AudioCodec(88, "", 0, 0, 1), nullptr)); EXPECT_FALSE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(0, "", 0, 0, 2, 0), nullptr)); + cricket::AudioCodec(0, "", 0, 0, 2), nullptr)); EXPECT_FALSE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(0, "", 5000, 0, 1, 0), nullptr)); + cricket::AudioCodec(0, "", 5000, 0, 1), nullptr)); EXPECT_FALSE(cricket::WebRtcVoiceEngine::ToCodecInst( - cricket::AudioCodec(0, "", 0, 5000, 1, 0), nullptr)); + cricket::AudioCodec(0, "", 0, 5000, 1), nullptr)); // Verify the payload id of common audio codecs, including CN, ISAC, and G722. cricket::WebRtcVoiceEngine engine(nullptr); diff --git a/webrtc/media/sctp/sctpdataengine.cc b/webrtc/media/sctp/sctpdataengine.cc index ba3b8f2408..b7462de879 100644 --- a/webrtc/media/sctp/sctpdataengine.cc +++ b/webrtc/media/sctp/sctpdataengine.cc @@ -283,7 +283,7 @@ SctpDataEngine::SctpDataEngine() { } usrsctp_engines_count++; - cricket::DataCodec codec(kGoogleSctpDataCodecId, kGoogleSctpDataCodecName, 0); + cricket::DataCodec codec(kGoogleSctpDataCodecId, kGoogleSctpDataCodecName); codec.SetParam(kCodecParamPort, kSctpDefaultPort); codecs_.push_back(codec); } diff --git a/webrtc/pc/channel_unittest.cc b/webrtc/pc/channel_unittest.cc index d84076ec2d..5993a69f78 100644 --- a/webrtc/pc/channel_unittest.cc +++ b/webrtc/pc/channel_unittest.cc @@ -48,12 +48,12 @@ using cricket::StreamParams; using cricket::TransportChannel; using rtc::WindowId; -static const cricket::AudioCodec kPcmuCodec(0, "PCMU", 64000, 8000, 1, 0); -static const cricket::AudioCodec kPcmaCodec(8, "PCMA", 64000, 8000, 1, 0); -static const cricket::AudioCodec kIsacCodec(103, "ISAC", 40000, 16000, 1, 0); -static const cricket::VideoCodec kH264Codec(97, "H264", 640, 400, 30, 0); -static const cricket::VideoCodec kH264SvcCodec(99, "H264-SVC", 320, 200, 15, 0); -static const cricket::DataCodec kGoogleDataCodec(101, "google-data", 0); +static const cricket::AudioCodec kPcmuCodec(0, "PCMU", 64000, 8000, 1); +static const cricket::AudioCodec kPcmaCodec(8, "PCMA", 64000, 8000, 1); +static const cricket::AudioCodec kIsacCodec(103, "ISAC", 40000, 16000, 1); +static const cricket::VideoCodec kH264Codec(97, "H264", 640, 400, 30); +static const cricket::VideoCodec kH264SvcCodec(99, "H264-SVC", 320, 200, 15); +static const cricket::DataCodec kGoogleDataCodec(101, "google-data"); static const uint32_t kSsrc1 = 0x1111; static const uint32_t kSsrc2 = 0x2222; static const uint32_t kSsrc3 = 0x3333; diff --git a/webrtc/pc/channelmanager_unittest.cc b/webrtc/pc/channelmanager_unittest.cc index 1eb09198d9..8cb066b25d 100644 --- a/webrtc/pc/channelmanager_unittest.cc +++ b/webrtc/pc/channelmanager_unittest.cc @@ -22,14 +22,12 @@ namespace cricket { static const AudioCodec kAudioCodecs[] = { - AudioCodec(97, "voice", 1, 2, 3, 0), - AudioCodec(111, "OPUS", 48000, 32000, 2, 0), + AudioCodec(97, "voice", 1, 2, 3), AudioCodec(111, "OPUS", 48000, 32000, 2), }; static const VideoCodec kVideoCodecs[] = { - VideoCodec(99, "H264", 100, 200, 300, 0), - VideoCodec(100, "VP8", 100, 200, 300, 0), - VideoCodec(96, "rtx", 100, 200, 300, 0), + VideoCodec(99, "H264", 100, 200, 300), + VideoCodec(100, "VP8", 100, 200, 300), VideoCodec(96, "rtx", 100, 200, 300), }; class ChannelManagerTest : public testing::Test { @@ -198,7 +196,7 @@ TEST_F(ChannelManagerTest, GetSetOutputVolume) { TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { std::vector codecs; - const VideoCodec rtx_codec(96, "rtx", 0, 0, 0, 0); + const VideoCodec rtx_codec(96, "rtx", 0, 0, 0); // By default RTX is disabled. cm_->GetSupportedVideoCodecs(&codecs); diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index 382cdbc44f..f5995010b3 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -10,11 +10,12 @@ #include "webrtc/pc/mediasession.h" -#include // For std::find_if. +#include // For std::find_if, std::sort. #include #include #include #include +#include #include #include "webrtc/base/helpers.h" @@ -756,7 +757,6 @@ static bool CreateMediaContentOffer( StreamParamsVec* current_streams, MediaContentDescriptionImpl* offer) { offer->AddCodecs(codecs); - offer->SortCodecs(); if (secure_policy == SEC_REQUIRED) { offer->set_crypto_required(CT_SDES); @@ -817,6 +817,8 @@ static void NegotiateCodecs(const std::vector& local_codecs, std::vector* negotiated_codecs) { for (const C& ours : local_codecs) { C theirs; + // Note that we intentionally only find one matching codec for each of our + // local codecs, in case the remote offer contains duplicate codecs. if (FindMatchingCodec(local_codecs, offered_codecs, ours, &theirs)) { C negotiated = ours; negotiated.IntersectFeedbackParams(theirs); @@ -829,14 +831,23 @@ static void NegotiateCodecs(const std::vector& local_codecs, offered_apt_value); } negotiated.id = theirs.id; - // RFC3264: Although the answerer MAY list the formats in their desired - // order of preference, it is RECOMMENDED that unless there is a - // specific reason, the answerer list formats in the same relative order - // they were present in the offer. - negotiated.preference = theirs.preference; negotiated_codecs->push_back(negotiated); } } + // RFC3264: Although the answerer MAY list the formats in their desired + // order of preference, it is RECOMMENDED that unless there is a + // specific reason, the answerer list formats in the same relative order + // they were present in the offer. + std::unordered_map payload_type_preferences; + int preference = static_cast(offered_codecs.size() + 1); + for (const C& codec : offered_codecs) { + payload_type_preferences[codec.id] = preference--; + } + std::sort(negotiated_codecs->begin(), negotiated_codecs->end(), + [&payload_type_preferences](const C& a, const C& b) { + return payload_type_preferences[a.id] > + payload_type_preferences[b.id]; + }); } // Finds a codec in |codecs2| that matches |codec_to_match|, which is @@ -1049,7 +1060,6 @@ static bool CreateMediaContentAnswer( std::vector negotiated_codecs; NegotiateCodecs(local_codecs, offer->codecs(), &negotiated_codecs); answer->AddCodecs(negotiated_codecs); - answer->SortCodecs(); answer->set_protocol(offer->protocol()); RtpHeaderExtensions negotiated_rtp_extensions; NegotiateRtpHeaderExtensions(local_rtp_extenstions, @@ -1377,8 +1387,8 @@ SessionDescription* MediaSessionDescriptionFactory::CreateAnswer( const SessionDescription* offer, const MediaSessionOptions& options, const SessionDescription* current_description) const { // The answer contains the intersection of the codecs in the offer with the - // codecs we support, ordered by our local preference. As indicated by - // XEP-0167, we retain the same payload ids from the offer in the answer. + // codecs we support. As indicated by XEP-0167, we retain the same payload ids + // from the offer in the answer. std::unique_ptr answer(new SessionDescription()); StreamParamsVec current_streams; diff --git a/webrtc/pc/mediasession.h b/webrtc/pc/mediasession.h index ae221554e2..98a1f070d6 100644 --- a/webrtc/pc/mediasession.h +++ b/webrtc/pc/mediasession.h @@ -293,10 +293,9 @@ class MediaContentDescription : public ContentDescription { template class MediaContentDescriptionImpl : public MediaContentDescription { public: - struct PreferenceSort { - bool operator()(C a, C b) { return a.preference > b.preference; } - }; + typedef C CodecType; + // Codecs should be in preference order (most preferred codec first). const std::vector& codecs() const { return codecs_; } void set_codecs(const std::vector& codecs) { codecs_ = codecs; } virtual bool has_codecs() const { return !codecs_.empty(); } @@ -330,9 +329,6 @@ class MediaContentDescriptionImpl : public MediaContentDescription { AddCodec(*codec); } } - void SortCodecs() { - std::sort(codecs_.begin(), codecs_.end(), PreferenceSort()); - } private: std::vector codecs_; diff --git a/webrtc/pc/mediasession_unittest.cc b/webrtc/pc/mediasession_unittest.cc index 1b2aa4eb3b..4c3eb4f01e 100644 --- a/webrtc/pc/mediasession_unittest.cc +++ b/webrtc/pc/mediasession_unittest.cc @@ -75,53 +75,43 @@ using rtc::CS_AES_CM_128_HMAC_SHA1_32; using rtc::CS_AES_CM_128_HMAC_SHA1_80; static const AudioCodec kAudioCodecs1[] = { - AudioCodec(103, "ISAC", 16000, -1, 1, 6), - AudioCodec(102, "iLBC", 8000, 13300, 1, 5), - AudioCodec(0, "PCMU", 8000, 64000, 1, 4), - AudioCodec(8, "PCMA", 8000, 64000, 1, 3), - AudioCodec(117, "red", 8000, 0, 1, 2), - AudioCodec(107, "CN", 48000, 0, 1, 1) -}; + AudioCodec(103, "ISAC", 16000, -1, 1), + AudioCodec(102, "iLBC", 8000, 13300, 1), + AudioCodec(0, "PCMU", 8000, 64000, 1), + AudioCodec(8, "PCMA", 8000, 64000, 1), + AudioCodec(117, "red", 8000, 0, 1), + AudioCodec(107, "CN", 48000, 0, 1)}; static const AudioCodec kAudioCodecs2[] = { - AudioCodec(126, "speex", 16000, 22000, 1, 3), - AudioCodec(0, "PCMU", 8000, 64000, 1, 2), - AudioCodec(127, "iLBC", 8000, 13300, 1, 1), + AudioCodec(126, "speex", 16000, 22000, 1), + AudioCodec(0, "PCMU", 8000, 64000, 1), + AudioCodec(127, "iLBC", 8000, 13300, 1), }; static const AudioCodec kAudioCodecsAnswer[] = { - AudioCodec(102, "iLBC", 8000, 13300, 1, 5), - AudioCodec(0, "PCMU", 8000, 64000, 1, 4), + AudioCodec(102, "iLBC", 8000, 13300, 1), + AudioCodec(0, "PCMU", 8000, 64000, 1), }; static const VideoCodec kVideoCodecs1[] = { - VideoCodec(96, "H264-SVC", 320, 200, 30, 2), - VideoCodec(97, "H264", 320, 200, 30, 1) -}; + VideoCodec(96, "H264-SVC", 320, 200, 30), + VideoCodec(97, "H264", 320, 200, 30)}; static const VideoCodec kVideoCodecs2[] = { - VideoCodec(126, "H264", 320, 200, 30, 2), - VideoCodec(127, "H263", 320, 200, 30, 1) -}; + VideoCodec(126, "H264", 320, 200, 30), + VideoCodec(127, "H263", 320, 200, 30)}; static const VideoCodec kVideoCodecsAnswer[] = { - VideoCodec(97, "H264", 320, 200, 30, 1) -}; + VideoCodec(97, "H264", 320, 200, 30)}; -static const DataCodec kDataCodecs1[] = { - DataCodec(98, "binary-data", 2), - DataCodec(99, "utf8-text", 1) -}; +static const DataCodec kDataCodecs1[] = {DataCodec(98, "binary-data"), + DataCodec(99, "utf8-text")}; -static const DataCodec kDataCodecs2[] = { - DataCodec(126, "binary-data", 2), - DataCodec(127, "utf8-text", 1) -}; +static const DataCodec kDataCodecs2[] = {DataCodec(126, "binary-data"), + DataCodec(127, "utf8-text")}; -static const DataCodec kDataCodecsAnswer[] = { - DataCodec(98, "binary-data", 2), - DataCodec(99, "utf8-text", 1) -}; +static const DataCodec kDataCodecsAnswer[] = {DataCodec(98, "binary-data"), + DataCodec(99, "utf8-text")}; static const RtpHeaderExtension kAudioRtpExtension1[] = { RtpHeaderExtension("urn:ietf:params:rtp-hdrext:ssrc-audio-level", 8), @@ -1576,11 +1566,9 @@ TEST_F(MediaSessionDescriptionFactoryTest, EXPECT_EQ(expected_codecs, vcd->codecs()); - // Now, make sure we get same result, except for the preference order, - // if |f2_| creates an updated offer even though the default payload types - // are different from |f1_|. - expected_codecs[0].preference = f1_codecs[1].preference; - + // Now, make sure we get same result (except for the order) if |f2_| creates + // an updated offer even though the default payload types between |f1_| and + // |f2_| are different. std::unique_ptr updated_offer( f2_.CreateOffer(opts, answer.get())); ASSERT_TRUE(updated_offer); @@ -1700,7 +1688,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, RtxWithoutApt) { opts.recv_audio = false; std::vector f1_codecs = MAKE_VECTOR(kVideoCodecs1); // This creates RTX without associated payload type parameter. - AddRtxCodec(VideoCodec(126, cricket::kRtxCodecName, 0, 0, 0, 0), &f1_codecs); + AddRtxCodec(VideoCodec(126, cricket::kRtxCodecName, 0, 0, 0), &f1_codecs); f1_.set_video_codecs(f1_codecs); std::vector f2_codecs = MAKE_VECTOR(kVideoCodecs2); @@ -1848,7 +1836,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, SimSsrcsGenerateMultipleRtxSsrcs) { // Use a single real codec, and then add RTX for it. std::vector f1_codecs; - f1_codecs.push_back(VideoCodec(97, "H264", 320, 200, 30, 1)); + f1_codecs.push_back(VideoCodec(97, "H264", 320, 200, 30)); AddRtxCodec(VideoCodec::CreateRtxCodec(125, 97), &f1_codecs); f1_.set_video_codecs(f1_codecs);