Don't cache video codec list in VideoEngine2.

A WebRtcVideoEngine2 object seems to be reused between PeerConnections,
which means that the field trial added in
https://codereview.webrtc.org/2511703002/ may not activate/deactivate
as intended between calls. This CL removes the caching of video codecs,
which gets rid of this problem.

BUG=webrtc:5654

Review-Url: https://codereview.webrtc.org/2521393004
Cr-Commit-Position: refs/heads/master@{#15265}

webrtc/media/base/mediaengine.h

@@ -74,7 +74,7 @@ class MediaEngineInterface {
   virtual const std::vector<AudioCodec>& audio_send_codecs() = 0;
   virtual const std::vector<AudioCodec>& audio_recv_codecs() = 0;
   virtual RtpCapabilities GetAudioCapabilities() = 0;
-  virtual const std::vector<VideoCodec>& video_codecs() = 0;
+  virtual std::vector<VideoCodec> video_codecs() = 0;
   virtual RtpCapabilities GetVideoCapabilities() = 0;
 
   // Starts AEC dump using existing file, a maximum file size in bytes can be
@@ -147,9 +147,7 @@ class CompositeMediaEngine : public MediaEngineInterface {
   virtual RtpCapabilities GetAudioCapabilities() {
     return voice_.GetCapabilities();
   }
-  virtual const std::vector<VideoCodec>& video_codecs() {
-    return video_.codecs();
-  }
+  virtual std::vector<VideoCodec> video_codecs() { return video_.codecs(); }
   virtual RtpCapabilities GetVideoCapabilities() {
     return video_.GetCapabilities();
   }

webrtc/media/engine/webrtcvideoengine2.cc

@@ -497,7 +497,6 @@ WebRtcVideoEngine2::WebRtcVideoEngine2()
       external_decoder_factory_(NULL),
       external_encoder_factory_(NULL) {
   LOG(LS_INFO) << "WebRtcVideoEngine2::WebRtcVideoEngine2()";
-  video_codecs_ = GetSupportedCodecs(external_encoder_factory_);
 }
 
 WebRtcVideoEngine2::~WebRtcVideoEngine2() {
@@ -520,8 +519,8 @@ WebRtcVideoChannel2* WebRtcVideoEngine2::CreateChannel(
                                  external_decoder_factory_);
 }
 
-const std::vector<VideoCodec>& WebRtcVideoEngine2::codecs() const {
-  return video_codecs_;
+std::vector<VideoCodec> WebRtcVideoEngine2::codecs() const {
+  return GetSupportedCodecs(external_encoder_factory_);
 }
 
 RtpCapabilities WebRtcVideoEngine2::GetCapabilities() const {
@@ -568,8 +567,6 @@ void WebRtcVideoEngine2::SetExternalEncoderFactory(
     encoder_factory = simulcast_encoder_factory_.get();
   }
   external_encoder_factory_ = encoder_factory;
-
-  video_codecs_ = GetSupportedCodecs(encoder_factory);
 }
 
 // This is a helper function for AppendVideoCodecs below. It will return the

webrtc/media/engine/webrtcvideoengine2.h

@@ -102,7 +102,7 @@ class WebRtcVideoEngine2 {
                                      const MediaConfig& config,
                                      const VideoOptions& options);
 
-  const std::vector<VideoCodec>& codecs() const;
+  std::vector<VideoCodec> codecs() const;
   RtpCapabilities GetCapabilities() const;
 
   // Set a WebRtcVideoDecoderFactory for external decoding. Video engine does
@@ -116,8 +116,6 @@ class WebRtcVideoEngine2 {
       WebRtcVideoEncoderFactory* encoder_factory);
 
  private:
-  std::vector<VideoCodec> video_codecs_;
-
   bool initialized_;
 
   WebRtcVideoDecoderFactory* external_decoder_factory_;

webrtc/media/engine/webrtcvideoengine2_unittest.cc

@@ -79,7 +79,7 @@ bool HasRtxCodec(const std::vector<cricket::VideoCodec>& codecs,
   return false;
 }
 
-static rtc::scoped_refptr<webrtc::VideoFrameBuffer> CreateBlackFrameBuffer(
+rtc::scoped_refptr<webrtc::VideoFrameBuffer> CreateBlackFrameBuffer(
     int width,
     int height) {
   rtc::scoped_refptr<webrtc::I420Buffer> buffer =
@@ -751,6 +751,32 @@ TEST_F(WebRtcVideoEngine2Test, SimulcastDisabledForH264) {
   EXPECT_TRUE(channel->SetVideoSend(ssrcs[0], true, nullptr, nullptr));
 }
 
+// Test that the FlexFEC field trial properly alters the output of
+// WebRtcVideoEngine2::codecs(), for an existing |engine_| object.
+//
+// TODO(brandtr): Remove this test, when the FlexFEC field trial is gone.
+TEST_F(WebRtcVideoEngine2Test,
+       Flexfec03SupportedAsInternalCodecBehindFieldTrial) {
+  auto is_flexfec = [](const VideoCodec& codec) {
+    if (codec.name == "flexfec-03")
+      return true;
+    return false;
+  };
+
+  // FlexFEC is not active without field trial.
+  engine_.Init();
+  const std::vector<VideoCodec> codecs_before = engine_.codecs();
+  EXPECT_EQ(codecs_before.end(), std::find_if(codecs_before.begin(),
+                                              codecs_before.end(), is_flexfec));
+
+  // FlexFEC is active with field trial.
+  webrtc::test::ScopedFieldTrials override_field_trials_(
+      "WebRTC-FlexFEC-03/Enabled/");
+  const std::vector<VideoCodec> codecs_after = engine_.codecs();
+  EXPECT_NE(codecs_after.end(),
+            std::find_if(codecs_after.begin(), codecs_after.end(), is_flexfec));
+}
+
 // Test that external codecs are added to the end of the supported codec list.
 TEST_F(WebRtcVideoEngine2Test, ReportSupportedExternalCodecs) {
   cricket::FakeWebRtcVideoEncoderFactory encoder_factory;
@@ -763,11 +789,31 @@ TEST_F(WebRtcVideoEngine2Test, ReportSupportedExternalCodecs) {
   cricket::VideoCodec internal_codec = codecs.front();
   cricket::VideoCodec external_codec = codecs.back();
 
-  // The external codec will appear at last.
+  // The external codec will appear last in the vector.
   EXPECT_EQ("VP8", internal_codec.name);
   EXPECT_EQ("FakeExternalCodec", external_codec.name);
 }
 
+// Test that an external codec that was added after the engine was initialized
+// does show up in the codec list after it was added.
+TEST_F(WebRtcVideoEngine2Test, ReportSupportedExternalCodecsWithAddedCodec) {
+  // Set up external encoder factory with first codec, and initialize engine.
+  cricket::FakeWebRtcVideoEncoderFactory encoder_factory;
+  encoder_factory.AddSupportedVideoCodecType("FakeExternalCodec1");
+  engine_.SetExternalEncoderFactory(&encoder_factory);
+  engine_.Init();
+
+  // The first external codec will appear last in the vector.
+  std::vector<cricket::VideoCodec> codecs_before(engine_.codecs());
+  EXPECT_EQ("FakeExternalCodec1", codecs_before.back().name);
+
+  // Add second codec.
+  encoder_factory.AddSupportedVideoCodecType("FakeExternalCodec2");
+  std::vector<cricket::VideoCodec> codecs_after(engine_.codecs());
+  EXPECT_EQ(codecs_before.size() + 1, codecs_after.size());
+  EXPECT_EQ("FakeExternalCodec2", codecs_after.back().name);
+}
+
 TEST_F(WebRtcVideoEngine2Test, RegisterExternalDecodersIfSupported) {
   cricket::FakeWebRtcVideoDecoderFactory decoder_factory;
   decoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8);

webrtc/pc/channelmanager.cc

@@ -141,13 +141,13 @@ void ChannelManager::GetSupportedVideoCodecs(
     std::vector<VideoCodec>* codecs) const {
   codecs->clear();
 
-  std::vector<VideoCodec>::const_iterator it;
-  for (it = media_engine_->video_codecs().begin();
-      it != media_engine_->video_codecs().end(); ++it) {
-    if (!enable_rtx_ && _stricmp(kRtxCodecName, it->name.c_str()) == 0) {
+  std::vector<VideoCodec> video_codecs = media_engine_->video_codecs();
+  for (const auto& video_codec : video_codecs) {
+    if (!enable_rtx_ &&
+        _stricmp(kRtxCodecName, video_codec.name.c_str()) == 0) {
       continue;
     }
-    codecs->push_back(*it);
+    codecs->push_back(video_codec);
   }
 }