generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

When creating an answer, takes the codec preference from the offer.

Browse Source 
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
This commit is contained in:
wu@webrtc.org 2014-06-20 20:57:42 +00:00
parent a24d366e1c
commit ff1b1bf094
6 changed files with 72 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
talk/media/base/codec.cc
View File

@ -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 &&

2
talk/media/base/codec.h
View File

@ -39,6 +39,8 @@ namespace cricket {
typedef std::map<std::string, std::string> CodecParameterMap;
extern const int kMaxPayloadId;
class FeedbackParam {
public:
FeedbackParam(const std::string& id, const std::string& param)

5
talk/session/media/mediasession.cc
View File

@ -775,6 +775,11 @@ static void NegotiateCodecs(const std::vector<C>& local_codecs,
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);
}
}

23
talk/session/media/mediasession_unittest.cc
View File

@ -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 kAudioCodecs1[] = {
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 kVideoCodecs2[] = {
};
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 @@ class MediaSessionDescriptionFactoryTest : public testing::Test {
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 @@ TEST_F(MediaSessionDescriptionFactoryTest,
// 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

10
talk/session/media/mediasessionclient.cc
View File

@ -373,6 +373,7 @@ bool ParseGingleAudioContent(const buzz::XmlElement* content_elem,
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 @@ bool ParseGingleAudioContent(const buzz::XmlElement* content_elem,
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 @@ bool ParseGingleVideoContent(const buzz::XmlElement* content_elem,
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 @@ bool ParseJingleAudioContent(const buzz::XmlElement* content_elem,
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 @@ bool ParseJingleAudioContent(const buzz::XmlElement* content_elem,
AudioCodec codec;
if (ParseJingleAudioCodec(payload_elem, &codec)) {
AddFeedbackParams(content_feedback_params, &codec.feedback_params);
codec.preference = preference--;
audio->AddCodec(codec);
}
}
@ -611,6 +617,7 @@ bool ParseJingleVideoContent(const buzz::XmlElement* content_elem,
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 @@ bool ParseJingleVideoContent(const buzz::XmlElement* content_elem,
VideoCodec codec;
if (ParseJingleVideoCodec(payload_elem, &codec)) {
AddFeedbackParams(content_feedback_params, &codec.feedback_params);
codec.preference = preference--;
video->AddCodec(codec);
}
}
@ -681,6 +689,7 @@ bool ParseJingleRtpDataContent(const buzz::XmlElement* content_elem,
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 @@ bool ParseJingleRtpDataContent(const buzz::XmlElement* content_elem,
DataCodec codec;
if (ParseJingleDataCodec(payload_elem, &codec)) {
AddFeedbackParams(content_feedback_params, &codec.feedback_params);
codec.preference = preference--;
data->AddCodec(codec);
}
}

157
talk/session/media/mediasessionclient_unittest.cc
View File

@ -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 @@ static const cricket::AudioCodec kAudioCodecs[] = {
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 @@ const std::string kJingleInitiate(
" </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 @@ class MediaSessionClientTest : public sigslot::has_slots<> {
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(MediaSessionTest, JingleGoodInitiateAllSupportedAudioCodecs) {
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(MediaSessionTest, GingleGoodInitiateAllSupportedAudioCodecsWithCrypto) {
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());
}