When creating an answer, takes the codec preference from the offer.
This change is based on 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."
BUG=2868
TEST=unit tests and manually with munge-sdp test
R=juberti@google.com
Review URL: https://webrtc-codereview.appspot.com/14589004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@6514 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/media/base/codec.cc b/talk/media/base/codec.cc
index bc7401a..c6f0ea5 100644
--- a/talk/media/base/codec.cc
+++ b/talk/media/base/codec.cc
@@ -38,6 +38,7 @@
namespace cricket {
static const int kMaxStaticPayloadId = 95;
+const int kMaxPayloadId = 127;
bool FeedbackParam::operator==(const FeedbackParam& other) const {
return _stricmp(other.id().c_str(), id().c_str()) == 0 &&
diff --git a/talk/media/base/codec.h b/talk/media/base/codec.h
index 863e289..8619620 100644
--- a/talk/media/base/codec.h
+++ b/talk/media/base/codec.h
@@ -39,6 +39,8 @@
typedef std::map<std::string, std::string> CodecParameterMap;
+extern const int kMaxPayloadId;
+
class FeedbackParam {
public:
FeedbackParam(const std::string& id, const std::string& param)
diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc
index 006975f..a5b1eb0 100644
--- a/talk/session/media/mediasession.cc
+++ b/talk/session/media/mediasession.cc
@@ -775,6 +775,11 @@
negotiated.SetParam(kCodecParamAssociatedPayloadType, 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);
}
}
diff --git a/talk/session/media/mediasession_unittest.cc b/talk/session/media/mediasession_unittest.cc
index cf492a6..b76cce4 100644
--- a/talk/session/media/mediasession_unittest.cc
+++ b/talk/session/media/mediasession_unittest.cc
@@ -31,6 +31,7 @@
#include "talk/base/gunit.h"
#include "talk/base/fakesslidentity.h"
#include "talk/base/messagedigest.h"
+#include "talk/base/ssladapter.h"
#include "talk/media/base/codec.h"
#include "talk/media/base/testutils.h"
#include "talk/p2p/base/constants.h"
@@ -97,13 +98,13 @@
static const AudioCodec kAudioCodecs2[] = {
AudioCodec(126, "speex", 16000, 22000, 1, 3),
- AudioCodec(127, "iLBC", 8000, 13300, 1, 2),
- AudioCodec(0, "PCMU", 8000, 64000, 1, 1),
+ AudioCodec(0, "PCMU", 8000, 64000, 1, 2),
+ AudioCodec(127, "iLBC", 8000, 13300, 1, 1),
};
static const AudioCodec kAudioCodecsAnswer[] = {
- AudioCodec(102, "iLBC", 8000, 13300, 1, 2),
- AudioCodec(0, "PCMU", 8000, 64000, 1, 1),
+ AudioCodec(102, "iLBC", 8000, 13300, 1, 5),
+ AudioCodec(0, "PCMU", 8000, 64000, 1, 4),
};
static const VideoCodec kVideoCodecs1[] = {
@@ -117,7 +118,7 @@
};
static const VideoCodec kVideoCodecsAnswer[] = {
- VideoCodec(97, "H264", 320, 200, 30, 2)
+ VideoCodec(97, "H264", 320, 200, 30, 1)
};
static const DataCodec kDataCodecs1[] = {
@@ -196,6 +197,14 @@
tdf2_.set_identity(&id2_);
}
+ static void SetUpTestCase() {
+ talk_base::InitializeSSL();
+ }
+
+ static void TearDownTestCase() {
+ talk_base::CleanupSSL();
+ }
+
// Create a video StreamParamsVec object with:
// - one video stream with 3 simulcast streams and FEC,
StreamParamsVec CreateComplexVideoStreamParamsVec() {
@@ -1379,10 +1388,12 @@
// The expected audio codecs are the common audio codecs from the first
// offer/answer exchange plus the audio codecs only |f2_| offer, sorted in
// preference order.
+ // TODO(wu): |updated_offer| should not include the codec
+ // (i.e. |kAudioCodecs2[0]|) the other side doesn't support.
const AudioCodec kUpdatedAudioCodecOffer[] = {
- kAudioCodecs2[0],
kAudioCodecsAnswer[0],
kAudioCodecsAnswer[1],
+ kAudioCodecs2[0],
};
// The expected video codecs are the common video codecs from the first
diff --git a/talk/session/media/mediasessionclient.cc b/talk/session/media/mediasessionclient.cc
index a5a652c..2ada987 100644
--- a/talk/session/media/mediasessionclient.cc
+++ b/talk/session/media/mediasessionclient.cc
@@ -373,6 +373,7 @@
ParseError* error) {
AudioContentDescription* audio = new AudioContentDescription();
+ int preference = kMaxPayloadId;
if (content_elem->FirstElement()) {
for (const buzz::XmlElement* codec_elem =
content_elem->FirstNamed(QN_GINGLE_AUDIO_PAYLOADTYPE);
@@ -380,6 +381,7 @@
codec_elem = codec_elem->NextNamed(QN_GINGLE_AUDIO_PAYLOADTYPE)) {
AudioCodec codec;
if (ParseGingleAudioCodec(codec_elem, &codec)) {
+ codec.preference = preference--;
audio->AddCodec(codec);
}
}
@@ -406,12 +408,14 @@
ParseError* error) {
VideoContentDescription* video = new VideoContentDescription();
+ int preference = kMaxPayloadId;
for (const buzz::XmlElement* codec_elem =
content_elem->FirstNamed(QN_GINGLE_VIDEO_PAYLOADTYPE);
codec_elem != NULL;
codec_elem = codec_elem->NextNamed(QN_GINGLE_VIDEO_PAYLOADTYPE)) {
VideoCodec codec;
if (ParseGingleVideoCodec(codec_elem, &codec)) {
+ codec.preference = preference--;
video->AddCodec(codec);
}
}
@@ -571,6 +575,7 @@
FeedbackParams content_feedback_params;
ParseFeedbackParams(content_elem, &content_feedback_params);
+ int preference = kMaxPayloadId;
for (const buzz::XmlElement* payload_elem =
content_elem->FirstNamed(QN_JINGLE_RTP_PAYLOADTYPE);
payload_elem != NULL;
@@ -578,6 +583,7 @@
AudioCodec codec;
if (ParseJingleAudioCodec(payload_elem, &codec)) {
AddFeedbackParams(content_feedback_params, &codec.feedback_params);
+ codec.preference = preference--;
audio->AddCodec(codec);
}
}
@@ -611,6 +617,7 @@
FeedbackParams content_feedback_params;
ParseFeedbackParams(content_elem, &content_feedback_params);
+ int preference = kMaxPayloadId;
for (const buzz::XmlElement* payload_elem =
content_elem->FirstNamed(QN_JINGLE_RTP_PAYLOADTYPE);
payload_elem != NULL;
@@ -618,6 +625,7 @@
VideoCodec codec;
if (ParseJingleVideoCodec(payload_elem, &codec)) {
AddFeedbackParams(content_feedback_params, &codec.feedback_params);
+ codec.preference = preference--;
video->AddCodec(codec);
}
}
@@ -681,6 +689,7 @@
FeedbackParams content_feedback_params;
ParseFeedbackParams(content_elem, &content_feedback_params);
+ int preference = kMaxPayloadId;
for (const buzz::XmlElement* payload_elem =
content_elem->FirstNamed(QN_JINGLE_RTP_PAYLOADTYPE);
payload_elem != NULL;
@@ -688,6 +697,7 @@
DataCodec codec;
if (ParseJingleDataCodec(payload_elem, &codec)) {
AddFeedbackParams(content_feedback_params, &codec.feedback_params);
+ codec.preference = preference--;
data->AddCodec(codec);
}
}
diff --git a/talk/session/media/mediasessionclient_unittest.cc b/talk/session/media/mediasessionclient_unittest.cc
index 7cb1ec9..3f3c4fa 100644
--- a/talk/session/media/mediasessionclient_unittest.cc
+++ b/talk/session/media/mediasessionclient_unittest.cc
@@ -32,6 +32,7 @@
#include "talk/base/logging.h"
#include "talk/base/scoped_ptr.h"
#include "talk/media/base/fakemediaengine.h"
+#include "talk/media/base/testutils.h"
#include "talk/media/devices/fakedevicemanager.h"
#include "talk/p2p/base/constants.h"
#include "talk/p2p/client/basicportallocator.h"
@@ -74,6 +75,29 @@
cricket::AudioCodec(106, "telephone-event", 8000, 0, 1, 1)
};
+// The codecs that our FakeMediaEngine will support with a different order of
+// supported codecs.
+static const cricket::AudioCodec kAudioCodecsDifferentPreference[] = {
+ cricket::AudioCodec(104, "ISAC", 32000, -1, 1, 17),
+ cricket::AudioCodec(97, "IPCMWB", 16000, 80000, 1, 14),
+ cricket::AudioCodec(9, "G722", 16000, 64000, 1, 13),
+ cricket::AudioCodec(119, "ISACLC", 16000, 40000, 1, 16),
+ cricket::AudioCodec(103, "ISAC", 16000, -1, 1, 18),
+ cricket::AudioCodec(99, "speex", 16000, 22000, 1, 15),
+ cricket::AudioCodec(100, "EG711U", 8000, 64000, 1, 9),
+ cricket::AudioCodec(101, "EG711A", 8000, 64000, 1, 8),
+ cricket::AudioCodec(0, "PCMU", 8000, 64000, 1, 7),
+ cricket::AudioCodec(8, "PCMA", 8000, 64000, 1, 6),
+ cricket::AudioCodec(102, "iLBC", 8000, 13300, 1, 12),
+ cricket::AudioCodec(3, "GSM", 8000, 13000, 1, 10),
+ cricket::AudioCodec(98, "speex", 8000, 11000, 1, 11),
+ cricket::AudioCodec(126, "CN", 32000, 0, 1, 5),
+ cricket::AudioCodec(105, "CN", 16000, 0, 1, 4),
+ cricket::AudioCodec(13, "CN", 8000, 0, 1, 3),
+ cricket::AudioCodec(117, "red", 8000, 0, 1, 2),
+ cricket::AudioCodec(106, "telephone-event", 8000, 0, 1, 1)
+};
+
static const cricket::VideoCodec kVideoCodecs[] = {
cricket::VideoCodec(96, "H264-SVC", 320, 200, 30, 1)
};
@@ -270,123 +294,6 @@
" </jingle> " \
"</iq> ");
-// Initiate string with a different order of supported codecs.
-// Should accept the supported ones, but with our desired order.
-const std::string kGingleInitiateDifferentPreference(
- "<iq xmlns='jabber:client' from='me@domain.com/resource' " \
- " to='user@domain.com/resource' type='set' id='123'> " \
- " <session xmlns='http://www.google.com/session' type='initiate'" \
- " id='abcdef' initiator='me@domain.com/resource'> " \
- " <description xmlns='http://www.google.com/session/phone'> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='104' name='ISAC' clockrate='32000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='97' name='IPCMWB' clockrate='16000' bitrate='80000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='9' name='G722' clockrate='16000' bitrate='64000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='119' name='ISACLC' clockrate='16000' bitrate='40000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='103' name='ISAC' clockrate='16000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='99' name='speex' clockrate='16000' bitrate='22000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='100' name='EG711U' clockrate='8000' bitrate='64000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='101' name='EG711A' clockrate='8000' bitrate='64000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='0' name='PCMU' clockrate='8000' bitrate='64000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='8' name='PCMA' clockrate='8000' bitrate='64000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='102' name='iLBC' clockrate='8000' bitrate='13300' />" \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='3' name='GSM' clockrate='8000' bitrate='13000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='98' name='speex' clockrate='8000' bitrate='11000' />" \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='126' name='CN' clockrate='32000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='105' name='CN' clockrate='16000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='13' name='CN' clockrate='8000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='117' name='red' clockrate='8000' /> " \
- " <payload-type xmlns='http://www.google.com/session/phone' " \
- " id='106' name='telephone-event' clockrate='8000' /> " \
- " </description> " \
- " </session> " \
- "</iq> ");
-
-const std::string kJingleInitiateDifferentPreference(
- "<iq xmlns='jabber:client' from='me@domain.com/resource' " \
- " to='user@domain.com/resource' type='set' id='123'> " \
- " <jingle xmlns='urn:xmpp:jingle:1' action='session-initiate' " \
- " sid='abcdef' initiator='me@domain.com/resource'> " \
- " <content name='test audio'> " \
- " <description xmlns='urn:xmpp:jingle:apps:rtp:1' media='audio'> " \
- " <payload-type id='104' name='ISAC' clockrate='32000'/> " \
- " <payload-type " \
- " id='97' name='IPCMWB' clockrate='16000'> " \
- " <parameter name='bitrate' value='80000'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='9' name='G722' clockrate='16000'> " \
- " <parameter name='bitrate' value='64000'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='119' name='ISACLC' clockrate='16000'> " \
- " <parameter name='bitrate' value='40000'/> " \
- " </payload-type> " \
- " <payload-type id='103' name='ISAC' clockrate='16000'/> " \
- " <payload-type " \
- " id='99' name='speex' clockrate='16000'> " \
- " <parameter name='bitrate' value='22000'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='100' name='EG711U' clockrate='8000'> " \
- " <parameter name='bitrate' value='64000'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='101' name='EG711A' clockrate='8000'> " \
- " <parameter name='bitrate' value='64000'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='0' name='PCMU' clockrate='8000'> " \
- " <parameter name='bitrate' value='64000'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='8' name='PCMA' clockrate='8000'> " \
- " <parameter name='bitrate' value='64000'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='102' name='iLBC' clockrate='8000'> " \
- " <parameter name='bitrate' value='13300'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='3' name='GSM' clockrate='8000'> " \
- " <parameter name='bitrate' value='13000'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='98' name='speex' clockrate='8000'> " \
- " <parameter name='bitrate' value='11000'/> " \
- " </payload-type> " \
- " <payload-type " \
- " id='126' name='CN' clockrate='32000' /> " \
- " <payload-type " \
- " id='105' name='CN' clockrate='16000' /> " \
- " <payload-type " \
- " id='13' name='CN' clockrate='8000' /> " \
- " <payload-type " \
- " id='117' name='red' clockrate='8000' /> " \
- " <payload-type " \
- " id='106' name='telephone-event' clockrate='8000' /> " \
- " </description> " \
- " <transport xmlns=\"http://www.google.com/transport/p2p\"/> " \
- " </content> " \
- " </jingle> " \
- "</iq> ");
-
const std::string kJingleInitiateWithRtcpFb(
"<iq xmlns='jabber:client' from='me@domain.com/resource' " \
" to='user@domain.com/resource' type='set' id='123'> " \
@@ -2793,6 +2700,8 @@
expected_data_fb_params_ = params_nack;
}
+ cricket::FakeMediaEngine* fme() { return fme_; }
+
private:
void OnSendStanza(cricket::SessionManager* manager,
const buzz::XmlElement* stanza) {
@@ -2949,11 +2858,15 @@
test->TestHasAllSupportedAudioCodecs(elem.get());
}
+// Changes the codecs that our FakeMediaEngine will support with a different
+// preference order than the incoming offer.
+// Verifies the answer accepts the preference order of the remote peer.
TEST(MediaSessionTest, JingleGoodInitiateDifferentPreferenceAudioCodecs) {
talk_base::scoped_ptr<MediaSessionClientTest> test(JingleTest());
+ test->fme()->SetAudioCodecs(MAKE_VECTOR(kAudioCodecsDifferentPreference));
talk_base::scoped_ptr<buzz::XmlElement> elem;
test->TestGoodIncomingInitiate(
- kJingleInitiateDifferentPreference, AudioCallOptions(), elem.use());
+ kJingleInitiate, AudioCallOptions(), elem.use());
test->TestHasAllSupportedAudioCodecs(elem.get());
}
@@ -3213,11 +3126,15 @@
test->TestHasAllSupportedAudioCodecs(elem.get());
}
+// Changes the codecs that our FakeMediaEngine will support with a different
+// preference order than the incoming offer.
+// Verifies the answer accepts the preference order of the remote peer.
TEST(MediaSessionTest, GingleGoodInitiateDifferentPreferenceAudioCodecs) {
- talk_base::scoped_ptr<buzz::XmlElement> elem;
talk_base::scoped_ptr<MediaSessionClientTest> test(GingleTest());
+ test->fme()->SetAudioCodecs(MAKE_VECTOR(kAudioCodecsDifferentPreference));
+ talk_base::scoped_ptr<buzz::XmlElement> elem;
test->TestGoodIncomingInitiate(
- kGingleInitiateDifferentPreference, AudioCallOptions(), elem.use());
+ kGingleInitiate, AudioCallOptions(), elem.use());
test->TestHasAllSupportedAudioCodecs(elem.get());
}
