Check associated payload type when negotiate RTX codecs.
At the moment, only payload name is checked when match two RTX codecs.
This will cause wrong behavior of codec negotiation if multiple RTX codecs
are added.
BUG=
R=pthatcher@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/34189004
Cr-Commit-Position: refs/heads/master@{#8727}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8727 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc
index b2b33fa..be1fe47 100644
--- a/talk/app/webrtc/webrtcsdp.cc
+++ b/talk/app/webrtc/webrtcsdp.cc
@@ -2380,18 +2380,14 @@
}
// Gets the current codec setting associated with |payload_type|. If there
-// is no AudioCodec associated with that payload type it returns an empty codec
+// is no Codec associated with that payload type it returns an empty codec
// with that payload type.
template <class T>
-T GetCodec(const std::vector<T>& codecs, int payload_type) {
- for (typename std::vector<T>::const_iterator codec = codecs.begin();
- codec != codecs.end(); ++codec) {
- if (codec->id == payload_type) {
- return *codec;
- }
+T GetCodecWithPayloadType(const std::vector<T>& codecs, int payload_type) {
+ T ret_val;
+ if (!FindCodecById(codecs, payload_type, &ret_val)) {
+ ret_val.id = payload_type;
}
- T ret_val = T();
- ret_val.id = payload_type;
return ret_val;
}
@@ -2423,7 +2419,8 @@
void UpdateCodec(MediaContentDescription* content_desc, int payload_type,
const cricket::CodecParameterMap& parameters) {
// Codec might already have been populated (from rtpmap).
- U new_codec = GetCodec(static_cast<T*>(content_desc)->codecs(), payload_type);
+ U new_codec = GetCodecWithPayloadType(static_cast<T*>(content_desc)->codecs(),
+ payload_type);
AddParameters(parameters, &new_codec);
AddOrReplaceCodec<T, U>(content_desc, new_codec);
}
@@ -2434,7 +2431,8 @@
void UpdateCodec(MediaContentDescription* content_desc, int payload_type,
const cricket::FeedbackParam& feedback_param) {
// Codec might already have been populated (from rtpmap).
- U new_codec = GetCodec(static_cast<T*>(content_desc)->codecs(), payload_type);
+ U new_codec = GetCodecWithPayloadType(static_cast<T*>(content_desc)->codecs(),
+ payload_type);
AddFeedbackParameter(feedback_param, &new_codec);
AddOrReplaceCodec<T, U>(content_desc, new_codec);
}
@@ -2889,7 +2887,8 @@
AudioContentDescription* audio_desc) {
// Codec may already be populated with (only) optional parameters
// (from an fmtp).
- cricket::AudioCodec codec = GetCodec(audio_desc->codecs(), payload_type);
+ cricket::AudioCodec codec =
+ GetCodecWithPayloadType(audio_desc->codecs(), payload_type);
codec.name = name;
codec.clockrate = clockrate;
codec.bitrate = bitrate;
@@ -2906,7 +2905,8 @@
VideoContentDescription* video_desc) {
// Codec may already be populated with (only) optional parameters
// (from an fmtp).
- cricket::VideoCodec codec = GetCodec(video_desc->codecs(), payload_type);
+ cricket::VideoCodec codec =
+ GetCodecWithPayloadType(video_desc->codecs(), payload_type);
codec.name = name;
codec.width = width;
codec.height = height;
diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc
index ab711ed..9267c4c 100644
--- a/talk/app/webrtc/webrtcsession_unittest.cc
+++ b/talk/app/webrtc/webrtcsession_unittest.cc
@@ -136,6 +136,26 @@
"IceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfrag"
"IceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfrag";
+static const char kSdpWithRtx[] =
+ "v=0\r\n"
+ "o=- 4104004319237231850 2 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "a=msid-semantic: WMS stream1\r\n"
+ "m=video 9 RTP/SAVPF 0 96\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp:9 IN IP4 0.0.0.0\r\n"
+ "a=ice-ufrag:CerjGp19G7wpXwl7\r\n"
+ "a=ice-pwd:cMvOlFvQ6ochez1ZOoC2uBEC\r\n"
+ "a=mid:video\r\n"
+ "a=sendrecv\r\n"
+ "a=rtcp-mux\r\n"
+ "a=crypto:1 AES_CM_128_HMAC_SHA1_80 "
+ "inline:5/4N5CDvMiyDArHtBByUM71VIkguH17ZNoX60GrA\r\n"
+ "a=rtpmap:0 fake_video_codec/90000\r\n"
+ "a=rtpmap:96 rtx/90000\r\n"
+ "a=fmtp:96 apt=0\r\n";
+
// Add some extra |newlines| to the |message| after |line|.
static void InjectAfter(const std::string& line,
const std::string& newlines,
@@ -3546,6 +3566,28 @@
answer = CreateAnswer(NULL);
SetLocalDescriptionWithoutError(answer);
}
+// Tests that RTX codec is removed from the answer when it isn't supported
+// by local side.
+TEST_F(WebRtcSessionTest, TestRtxRemovedByCreateAnswer) {
+ Init();
+ mediastream_signaling_.SendAudioVideoStream1();
+ std::string offer_sdp(kSdpWithRtx);
+
+ SessionDescriptionInterface* offer =
+ CreateSessionDescription(JsepSessionDescription::kOffer, offer_sdp, NULL);
+ EXPECT_TRUE(offer->ToString(&offer_sdp));
+
+ // Offer SDP contains the RTX codec.
+ EXPECT_TRUE(offer_sdp.find("rtx") != std::string::npos);
+ SetRemoteDescriptionWithoutError(offer);
+
+ SessionDescriptionInterface* answer = CreateAnswer(NULL);
+ std::string answer_sdp;
+ answer->ToString(&answer_sdp);
+ // Answer SDP removes the unsupported RTX codec.
+ EXPECT_TRUE(answer_sdp.find("rtx") == std::string::npos);
+ SetLocalDescriptionWithoutError(answer);
+}
// This verifies that the voice channel after bundle has both options from video
// and voice channels.