Setup encoders inexpensively before first frame.

Modifies WebRtcVideoSendStream to use a default width/height of 16px.
This significantly reduces SetRemoteDescription time under
WebRtcVideoEngine2. Also preventing (expensive) reconfigurations due to
incoming frames when the channel is not sending yet.

Tests have been modified to generate a frame before expecting a certain
encoder size to have been configured.

Also adding tracing to WebRtcVideoSendStream::InputFrame as it can lead
to reconfigurations of the encoder which is expensive and it should show
up in chrome://tracing.

BUG=1788
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/42369004

Cr-Commit-Position: refs/heads/master@{#8381}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8381 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc
index 4ad0d31..cb025f2 100644
--- a/talk/media/webrtc/webrtcvideoengine2.cc
+++ b/talk/media/webrtc/webrtcvideoengine2.cc
@@ -1396,6 +1396,7 @@
 void WebRtcVideoChannel2::WebRtcVideoSendStream::InputFrame(
     VideoCapturer* capturer,
     const VideoFrame* frame) {
+  TRACE_EVENT0("webrtc", "WebRtcVideoSendStream::InputFrame");
   LOG(LS_VERBOSE) << "InputFrame: " << frame->GetWidth() << "x"
                   << frame->GetHeight();
   // Lock before copying, can be called concurrently when swapping input source.
@@ -1408,6 +1409,12 @@
                        "configured, dropping.";
     return;
   }
+
+  // Not sending, abort early to prevent expensive reconfigurations while
+  // setting up codecs etc.
+  if (!sending_)
+    return;
+
   if (format_.width == 0) {  // Dropping frames.
     assert(format_.height == 0);
     LOG(LS_VERBOSE) << "VideoFormat 0x0 set, Dropping frame.";
@@ -1585,16 +1592,10 @@
 void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions(
     const VideoCodecSettings& codec_settings,
     const VideoOptions& options) {
-  if (last_dimensions_.width == -1) {
-    last_dimensions_.width = codec_settings.codec.width;
-    last_dimensions_.height = codec_settings.codec.height;
-    last_dimensions_.is_screencast = false;
-  }
   parameters_.encoder_config =
       CreateVideoEncoderConfig(last_dimensions_, codec_settings.codec);
-  if (parameters_.encoder_config.streams.empty()) {
+  if (parameters_.encoder_config.streams.empty())
     return;
-  }
 
   format_ = VideoFormat(codec_settings.codec.width,
                         codec_settings.codec.height,
diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h
index 35ef592..d94c1bb 100644
--- a/talk/media/webrtc/webrtcvideoengine2.h
+++ b/talk/media/webrtc/webrtcvideoengine2.h
@@ -319,7 +319,9 @@
     };
 
     struct Dimensions {
-      Dimensions() : width(-1), height(-1), is_screencast(false) {}
+      // Use low width/height to make encoder creation (before first frame)
+      // cheap.
+      Dimensions() : width(16), height(16), is_screencast(false) {}
       int width;
       int height;
       bool is_screencast;
diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
index 061126d..d9eb7d5 100644
--- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc
+++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
@@ -572,13 +572,18 @@
   EXPECT_EQ(cricket::CS_RUNNING,
             capturer.Start(capturer.GetSupportedFormats()->front()));
   EXPECT_TRUE(capturer.CaptureFrame());
-
   EXPECT_TRUE_WAIT(encoder_factory.encoders()[0]->GetNumEncodedFrames() > 0,
                    kTimeout);
 
-  // Setting codecs of the same type should not reallocate the encoder.
+  // Sending one frame will have reallocated the encoder since input size
+  // changes from a small default to the actual frame width/height.
+  int num_created_encoders = encoder_factory.GetNumCreatedEncoders();
+  EXPECT_EQ(num_created_encoders, 2);
+
+  // Setting codecs of the same type should not reallocate any encoders
+  // (expecting a no-op).
   EXPECT_TRUE(channel->SetSendCodecs(codecs));
-  EXPECT_EQ(1, encoder_factory.GetNumCreatedEncoders());
+  EXPECT_EQ(num_created_encoders, encoder_factory.GetNumCreatedEncoders());
 
   // Remove stream previously added to free the external encoder instance.
   EXPECT_TRUE(channel->RemoveSendStream(kSsrc));
@@ -626,6 +631,12 @@
       channel->AddSendStream(CreateSimStreamParams("cname", ssrcs)));
   EXPECT_TRUE(channel->SetSend(true));
 
+  cricket::FakeVideoCapturer capturer;
+  EXPECT_TRUE(channel->SetCapturer(ssrcs.front(), &capturer));
+  EXPECT_EQ(cricket::CS_RUNNING,
+            capturer.Start(capturer.GetSupportedFormats()->front()));
+  EXPECT_TRUE(capturer.CaptureFrame());
+
   EXPECT_GT(encoder_factory.encoders().size(), 1u);
 
   // Verify that encoders are configured for simulcast through adapter
@@ -638,6 +649,8 @@
     EXPECT_GT(codec_settings.width, prev_width);
     prev_width = codec_settings.width;
   }
+
+  EXPECT_TRUE(channel->SetCapturer(ssrcs.front(), NULL));
 }
 
 TEST_F(WebRtcVideoEngine2Test, ChannelWithExternalH264CanChangeToInternalVp8) {
@@ -1728,8 +1741,17 @@
   std::vector<VideoCodec> codecs;
   codecs.push_back(kVp8Codec720p);
   ASSERT_TRUE(channel_->SetSendCodecs(codecs));
+  channel_->SetSend(true);
 
-  std::vector<webrtc::VideoStream> streams = AddSendStream()->GetVideoStreams();
+  FakeVideoSendStream* stream = AddSendStream();
+
+  cricket::FakeVideoCapturer capturer;
+  EXPECT_TRUE(channel_->SetCapturer(last_ssrc_, &capturer));
+  EXPECT_EQ(cricket::CS_RUNNING,
+            capturer.Start(capturer.GetSupportedFormats()->front()));
+  EXPECT_TRUE(capturer.CaptureFrame());
+
+  std::vector<webrtc::VideoStream> streams = stream->GetVideoStreams();
   EXPECT_EQ(kVp8Codec720p.width, streams[0].width);
   EXPECT_EQ(kVp8Codec720p.height, streams[0].height);
 
@@ -1739,6 +1761,7 @@
   streams = fake_call_->GetVideoSendStreams()[0]->GetVideoStreams();
   EXPECT_EQ(kVp8Codec360p.width, streams[0].width);
   EXPECT_EQ(kVp8Codec360p.height, streams[0].height);
+  EXPECT_TRUE(channel_->SetCapturer(last_ssrc_, NULL));
 }
 
 TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithBitrates) {
@@ -2078,7 +2101,7 @@
 };
 
 class WebRtcVideoChannel2SimulcastTest : public WebRtcVideoEngine2SimulcastTest,
-                                        public WebRtcCallFactory {
+                                         public WebRtcCallFactory {
  public:
   WebRtcVideoChannel2SimulcastTest() : fake_call_(NULL) {}
 
@@ -2117,6 +2140,16 @@
 
     FakeVideoSendStream* stream =
         AddSendStream(CreateSimStreamParams("cname", ssrcs));
+    // Send a full-size frame to trigger a stream reconfiguration to use all
+    // expected simulcast layers.
+    cricket::FakeVideoCapturer capturer;
+    EXPECT_TRUE(channel_->SetCapturer(ssrcs.front(), &capturer));
+    EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(cricket::VideoFormat(
+                                       codec.width, codec.height,
+                                       cricket::VideoFormat::FpsToInterval(30),
+                                       cricket::FOURCC_I420)));
+    channel_->SetSend(true);
+    EXPECT_TRUE(capturer.CaptureFrame());
 
     std::vector<webrtc::VideoStream> video_streams = stream->GetVideoStreams();
     ASSERT_EQ(expected_num_streams, video_streams.size());
@@ -2160,6 +2193,7 @@
       EXPECT_EQ(expected_streams[i].temporal_layer_thresholds_bps,
                 video_streams[i].temporal_layer_thresholds_bps);
     }
+    EXPECT_TRUE(channel_->SetCapturer(ssrcs.front(), NULL));
   }
 
   FakeVideoSendStream* AddSendStream() {
diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
index 15b6023..4035412 100644
--- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
@@ -126,6 +126,11 @@
 }
 
 int SimulcastEncoderAdapter::Release() {
+  // TODO(pbos): Keep the last encoder instance but call ::Release() on it, then
+  // re-use this instance in ::InitEncode(). This means that changing
+  // resolutions doesn't require reallocation of the first encoder, but only
+  // reinitialization, which makes sense. Then Destroy this instance instead in
+  // ~SimulcastEncoderAdapter().
   while (!streaminfos_.empty()) {
     VideoEncoder* encoder = streaminfos_.back().encoder;
     factory_->Destroy(encoder);