Don't cache video codec list in VideoEngine2.

webrtc/media/engine/webrtcvideoengine2_unittest.cc

Index: webrtc/media/engine/webrtcvideoengine2_unittest.cc
diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
index c473a4099f98d9838a0a9de28857997253f87141..84beddb38173f5940e660b5f7266ac32628008de 100644
--- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc
+++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
@@ -12,6 +12,7 @@
 #include <list>
 #include <map>
 #include <memory>
+#include <unordered_set>

[perkj_webrtc, 2016/11/28 10:56:05]

remove

[brandtr, 2016/11/28 11:47:50]

Done.

 #include <vector>
 
 #include "webrtc/base/arraysize.h"
@@ -26,6 +27,8 @@
 #include "webrtc/media/engine/simulcast.h"
 #include "webrtc/media/engine/webrtcvideoengine2.h"
 #include "webrtc/media/engine/webrtcvoiceengine.h"
+#include "webrtc/modules/video_coding/codecs/h264/include/h264.h"

[perkj_webrtc, 2016/11/28 10:56:05]

remove

[brandtr, 2016/11/28 11:47:50]

Done.

+#include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h"

[perkj_webrtc, 2016/11/28 10:56:05]

remove

[brandtr, 2016/11/28 11:47:50]

Done.

 #include "webrtc/test/field_trial.h"
 #include "webrtc/video_encoder.h"
 
@@ -78,7 +81,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 =
@@ -100,6 +103,14 @@ void VerifySendStreamHasRtxTypes(const webrtc::VideoSendStream::Config& config,
                 it->second == config.rtp.ulpfec.red_rtx_payload_type);
   }
 }
+
+std::unordered_set<std::string> GetCodecNames(
+    const cricket::WebRtcVideoEngine2& engine) {
+  std::unordered_set<std::string> codec_names;
+  for (const auto& codec : engine.codecs())
+    codec_names.insert(codec.name);
+  return codec_names;
+}
 }  // namespace
 
 namespace cricket {
@@ -749,6 +760,62 @@ TEST_F(WebRtcVideoEngine2Test, SimulcastDisabledForH264) {
   EXPECT_TRUE(channel->SetVideoSend(ssrcs[0], true, nullptr, nullptr));
 }
 
+// Test that internal codecs are added to the codec list.
+// Update this test if the internal codecs are changed!
+TEST_F(WebRtcVideoEngine2Test, ReportSupportedInternalCodecs) {
+  engine_.Init();
+  std::unordered_set<std::string> codec_names = GetCodecNames(engine_);
+
+  size_t expected_num_codecs = 4;
+  EXPECT_NE(codec_names.find("VP8"), codec_names.end());
+  if (webrtc::VP9Encoder::IsSupported()) {
+    EXPECT_NE(codec_names.find("VP9"), codec_names.end());
+    ++expected_num_codecs;
+  }
+  if (webrtc::H264Encoder::IsSupported()) {
+    EXPECT_NE(codec_names.find("H264"), codec_names.end());
+    ++expected_num_codecs;
+  }
+  EXPECT_NE(codec_names.find("rtx"), codec_names.end());
+  EXPECT_NE(codec_names.find("red"), codec_names.end());
+  EXPECT_NE(codec_names.find("ulpfec"), codec_names.end());
+  EXPECT_EQ(expected_num_codecs, codec_names.size());
+}
+
+// TODO(brandtr): Remove this fixture when the FlexFEC field trial is gone.
+class WebRtcVideoEngine2FlexfecTest : public WebRtcVideoEngine2Test {
+ public:
+  WebRtcVideoEngine2FlexfecTest()
+      : WebRtcVideoEngine2Test("WebRTC-FlexFEC-03/Enabled/") {}
+};
+
+// Test that the FlexFEC field trial properly alters the output of
+// WebRtcVideoEngine2::codecs(), for a newly created |engine_| object.
+//
+// TODO(brandtr): Merge with test above, when the FlexFEC field trial is gone.
+TEST_F(WebRtcVideoEngine2FlexfecTest, ReportSupportedInternalCodecs) {
+  engine_.Init();
+  std::unordered_set<std::string> codec_names = GetCodecNames(engine_);
+
+  EXPECT_NE(codec_names.find("flexfec-03"), codec_names.end());
+}
+
+// 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) {

[brandtr, 2016/11/28 10:40:15]

Had I written this test before, the problem addressed in this CL would have been
uncovered earlier...

[perkj_webrtc, 2016/11/28 10:56:05]

great. I think you can remove the first two tests and just keep this.

[brandtr, 2016/11/28 11:47:50]

Done.

+  engine_.Init();
+  std::unordered_set<std::string> codec_names_before = GetCodecNames(engine_);

[perkj_webrtc, 2016/11/28 10:56:05]

please remove GetCodecNames and directy use std::find_if on the returned vector
from engine_.GetCodecs()

[brandtr, 2016/11/28 11:47:50]

Done.

+  EXPECT_EQ(codec_names_before.find("flexfec-03"), codec_names_before.end());
+
+  webrtc::test::ScopedFieldTrials override_field_trials_(
+      "WebRTC-FlexFEC-03/Enabled/");
+  std::unordered_set<std::string> codec_names_after = GetCodecNames(engine_);
+  EXPECT_NE(codec_names_after.find("flexfec-03"), codec_names_after.end());
+}
+
 // Test that external codecs are added to the end of the supported codec list.
 TEST_F(WebRtcVideoEngine2Test, ReportSupportedExternalCodecs) {
   cricket::FakeWebRtcVideoEncoderFactory encoder_factory;
@@ -761,11 +828,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.

[brandtr, 2016/11/28 10:40:15]

This test fails before the change in this CL.

+  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);