Don't cache video codec list in VideoEngine2.

webrtc/media/engine/webrtcvideoengine2_unittest.cc

 /*
  *  Copyright (c) 2004 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include <algorithm>
 #include <list>
 #include <map>
 #include <memory>
+#include <unordered_set>
 #include <vector>
 
 #include "webrtc/base/arraysize.h"
 #include "webrtc/base/gunit.h"
 #include "webrtc/base/stringutils.h"
 #include "webrtc/common_video/h264/profile_level_id.h"
 #include "webrtc/logging/rtc_event_log/rtc_event_log.h"
 #include "webrtc/media/base/testutils.h"
 #include "webrtc/media/base/videoengine_unittest.h"
 #include "webrtc/media/engine/fakewebrtccall.h"
 #include "webrtc/media/engine/fakewebrtcvideoengine.h"
 #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"
+#include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h"
 #include "webrtc/test/field_trial.h"
 #include "webrtc/video_encoder.h"
 
 using webrtc::RtpExtension;
 
 namespace {
 static const int kDefaultQpMax = 56;
 
 static const uint8_t kRedRtxPayloadType = 125;
 
@@ skipping 32 matching lines @@
     if (cricket::CodecNamesEq(codec.name.c_str(), "rtx") &&
         codec.GetParam(cricket::kCodecParamAssociatedPayloadType,
                        &associated_payload_type) &&
         associated_payload_type == payload_type) {
       return true;
     }
   }
   return false;
 }
 
 static rtc::scoped_refptr<webrtc::VideoFrameBuffer> CreateBlackFrameBuffer(

[perkj_webrtc, 2016/11/28 09:20:06]

can we remove static here. (not your cl I know)

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

Done.

     int width,
     int height) {
   rtc::scoped_refptr<webrtc::I420Buffer> buffer =
       webrtc::I420Buffer::Create(width, height);
   buffer->SetToBlack();
   return buffer;
 }
 
 void VerifySendStreamHasRtxTypes(const webrtc::VideoSendStream::Config& config,
                                  const std::map<int, int>& rtx_types) {
   std::map<int, int>::const_iterator it;
   it = rtx_types.find(config.encoder_settings.payload_type);
   EXPECT_TRUE(it != rtx_types.end() &&
               it->second == config.rtp.rtx.payload_type);
 
   if (config.rtp.ulpfec.red_rtx_payload_type != -1) {
     it = rtx_types.find(config.rtp.ulpfec.red_payload_type);
     EXPECT_TRUE(it != rtx_types.end() &&
                 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 {
 class WebRtcVideoEngine2Test : public ::testing::Test {
  public:
   WebRtcVideoEngine2Test() : WebRtcVideoEngine2Test("") {}
   explicit WebRtcVideoEngine2Test(const char* field_trials)
       : override_field_trials_(field_trials),
         call_(webrtc::Call::Create(webrtc::Call::Config(&event_log_))),
         engine_() {
@@ skipping 629 matching lines @@
   EXPECT_TRUE(capturer.CaptureFrame());
 
   ASSERT_EQ(1u, encoder_factory.encoders().size());
   FakeWebRtcVideoEncoder* encoder = encoder_factory.encoders()[0];
   ASSERT_TRUE(encoder_factory.encoders()[0]->WaitForInitEncode());
   EXPECT_EQ(webrtc::kVideoCodecH264, encoder->GetCodecSettings().codecType);
   EXPECT_EQ(1u, encoder->GetCodecSettings().numberOfSimulcastStreams);
   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) {

[perkj_webrtc, 2016/11/28 09:20:06]

The fix is for  different codecs when the engine has been reset right? 
So can we write a test that Gets the codecs and then reinitialise the engine
with the  experiment set and reinitialise the engine and get the codecs again.

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

Done. I did not reinitialize the engine in the test however, as the field trial
change is effective without reinitializing the engine.

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

[magjed_webrtc, 2016/11/28 10:31:31]

nit: I think it would be slightly cleaner to write it like:
EXPECT_EQ(1u, codec_names.count("VP8"));

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

Agree. I'm used to the find()/end() style after working with std::set's, but
since this is a hash table, count() is nicer.

+  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());
+}
+
+class WebRtcVideoEngine2FlexfecTest : public WebRtcVideoEngine2Test {
+ public:
+  WebRtcVideoEngine2FlexfecTest()
+      : WebRtcVideoEngine2Test("WebRTC-FlexFEC-03/Enabled/") {}
+};
+
+// 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 external codecs are added to the end of the supported codec list.
 TEST_F(WebRtcVideoEngine2Test, ReportSupportedExternalCodecs) {
   cricket::FakeWebRtcVideoEncoderFactory encoder_factory;
   encoder_factory.AddSupportedVideoCodecType("FakeExternalCodec");
   engine_.SetExternalEncoderFactory(&encoder_factory);
   engine_.Init();
 
   std::vector<cricket::VideoCodec> codecs(engine_.codecs());
   ASSERT_GE(codecs.size(), 2u);
   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_F(WebRtcVideoEngine2Test, RegisterExternalDecodersIfSupported) {
   cricket::FakeWebRtcVideoDecoderFactory decoder_factory;
   decoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8);
   cricket::VideoRecvParameters parameters;
   parameters.codecs.push_back(GetEngineCodec("VP8"));
 
@@ skipping 3214 matching lines @@
 
 TEST_F(WebRtcVideoChannel2SimulcastTest, SetSendCodecsWith3SimulcastStreams) {
   VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 3);
 }
 
 // Test that we normalize send codec format size in simulcast.
 TEST_F(WebRtcVideoChannel2SimulcastTest, SetSendCodecsWithOddSizeInSimulcast) {
   VerifySimulcastSettings(cricket::VideoCodec("VP8"), 541, 271, 2, 2);
 }
 }  // namespace cricket