Use local codec parameters in the answer.

Previously when creating an answer, we used the codecs from the remote list
to get correctly mapped codec id, but the side effect is that we also used
the remote codec parameters which is wrong.

With this change, we use the local codecs with local parameters and the
|NegotiateCodecs| method will take care of the codec id mapping.

Bug: webrtc:8550
Change-Id: Ib7b2fe5f6ed3a72c58cf5df8bf2c1d00476c141c
Reviewed-on: https://webrtc-review.googlesource.com/25285
Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20843}

pc/mediasession.cc

@@ -2119,15 +2119,14 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer(
     }
   }
   // Add other supported audio codecs.
-  AudioCodec found_codec;
   for (const AudioCodec& codec : supported_audio_codecs) {
     if (FindMatchingCodec<AudioCodec>(supported_audio_codecs, audio_codecs,
-                                      codec, &found_codec) &&
+                                      codec, nullptr) &&
         !FindMatchingCodec<AudioCodec>(supported_audio_codecs, filtered_codecs,
                                        codec, nullptr)) {
-      // Use the |found_codec| from |audio_codecs| because it has the correctly
-      // mapped payload type.
-      filtered_codecs.push_back(found_codec);
+      // We should use the local codec with local parameters and the codec id
+      // would be correctly mapped in |NegotiateCodecs|.
+      filtered_codecs.push_back(codec);
     }
   }
 
@@ -2203,15 +2202,14 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer(
     }
   }
   // Add other supported video codecs.
-  VideoCodec found_codec;
   for (const VideoCodec& codec : video_codecs_) {
     if (FindMatchingCodec<VideoCodec>(video_codecs_, video_codecs, codec,
-                                      &found_codec) &&
+                                      nullptr) &&
         !FindMatchingCodec<VideoCodec>(video_codecs_, filtered_codecs, codec,
                                        nullptr)) {
-      // Use the |found_codec| from |video_codecs| because it has the correctly
-      // mapped payload type.
-      filtered_codecs.push_back(found_codec);
+      // We should use the local codec with local parameters and the codec id
+      // would be correctly mapped in |NegotiateCodecs|.
+      filtered_codecs.push_back(codec);
     }
   }
 

pc/mediasession_unittest.cc

@@ -3408,6 +3408,64 @@ TEST_F(MediaSessionDescriptionFactoryTest,
   EXPECT_EQ(video_codecs, vcd2->codecs());
 }
 
+// Test that when creating an answer, the codecs use local parameters instead of
+// the remote ones.
+TEST_F(MediaSessionDescriptionFactoryTest, CreateAnswerWithLocalCodecParams) {
+  const std::string audio_param_name = "audio_param";
+  const std::string audio_value1 = "audio_v1";
+  const std::string audio_value2 = "audio_v2";
+  const std::string video_param_name = "video_param";
+  const std::string video_value1 = "video_v1";
+  const std::string video_value2 = "video_v2";
+
+  auto audio_codecs1 = MAKE_VECTOR(kAudioCodecs1);
+  auto audio_codecs2 = MAKE_VECTOR(kAudioCodecs1);
+  auto video_codecs1 = MAKE_VECTOR(kVideoCodecs1);
+  auto video_codecs2 = MAKE_VECTOR(kVideoCodecs1);
+
+  // Set the parameters for codecs.
+  audio_codecs1[0].SetParam(audio_param_name, audio_value1);
+  video_codecs1[0].SetParam(video_param_name, video_value1);
+  audio_codecs2[0].SetParam(audio_param_name, audio_value2);
+  video_codecs2[0].SetParam(video_param_name, video_value2);
+
+  f1_.set_audio_codecs(audio_codecs1, audio_codecs1);
+  f1_.set_video_codecs(video_codecs1);
+  f2_.set_audio_codecs(audio_codecs2, audio_codecs2);
+  f2_.set_video_codecs(video_codecs2);
+
+  MediaSessionOptions opts;
+  AddMediaSection(MEDIA_TYPE_AUDIO, "audio", cricket::MD_SENDRECV, kActive,
+                  &opts);
+  AddMediaSection(MEDIA_TYPE_VIDEO, "video", cricket::MD_SENDRECV, kActive,
+                  &opts);
+
+  std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, nullptr));
+  ASSERT_TRUE(offer);
+  auto offer_acd =
+      static_cast<AudioContentDescription*>(offer->contents()[0].description);
+  auto offer_vcd =
+      static_cast<VideoContentDescription*>(offer->contents()[1].description);
+  std::string value;
+  EXPECT_TRUE(offer_acd->codecs()[0].GetParam(audio_param_name, &value));
+  EXPECT_EQ(audio_value1, value);
+  EXPECT_TRUE(offer_vcd->codecs()[0].GetParam(video_param_name, &value));
+  EXPECT_EQ(video_value1, value);
+
+  std::unique_ptr<SessionDescription> answer(
+      f2_.CreateAnswer(offer.get(), opts, nullptr));
+  ASSERT_TRUE(answer);
+  auto answer_acd =
+      static_cast<AudioContentDescription*>(answer->contents()[0].description);
+  auto answer_vcd =
+      static_cast<VideoContentDescription*>(answer->contents()[1].description);
+  // Use the parameters from the local codecs.
+  EXPECT_TRUE(answer_acd->codecs()[0].GetParam(audio_param_name, &value));
+  EXPECT_EQ(audio_value2, value);
+  EXPECT_TRUE(answer_vcd->codecs()[0].GetParam(video_param_name, &value));
+  EXPECT_EQ(video_value2, value);
+}
+
 class MediaProtocolTest : public ::testing::TestWithParam<const char*> {
  public:
   MediaProtocolTest() : f1_(&tdf1_), f2_(&tdf2_) {