Simplify VideoEncoder interface.

This should also avoid assert in the linked bug.

BUG=284775
R=wez@chromium.org

Review URL: https://codereview.chromium.org/23477059

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@223152 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 17 insertions, 32 deletions

diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc
index 6d424244..67f4a95 100644
--- a/remoting/host/client_session.cc
+++ b/remoting/host/client_session.cc
@@ -426,9 +426,7 @@ scoped_ptr<VideoEncoder> ClientSession::CreateVideoEncoder(
     const protocol::SessionConfig& config) {
   const protocol::ChannelConfig& video_config = config.video_config();
 
-  if (video_config.codec == protocol::ChannelConfig::CODEC_VERBATIM) {
-    return scoped_ptr<VideoEncoder>(new remoting::VideoEncoderVerbatim());
-  } else if (video_config.codec == protocol::ChannelConfig::CODEC_VP8) {
+  if (video_config.codec == protocol::ChannelConfig::CODEC_VP8) {
     return scoped_ptr<VideoEncoder>(new remoting::VideoEncoderVp8());
   }
 
diff --git a/remoting/host/video_scheduler.cc b/remoting/host/video_scheduler.cc
index 883166b..5c95c7a 100644
--- a/remoting/host/video_scheduler.cc
+++ b/remoting/host/video_scheduler.cc
@@ -249,11 +249,8 @@ void VideoScheduler::SendVideoPacket(scoped_ptr<VideoPacket> packet) {
   if (!video_stub_)
     return;
 
-  base::Closure callback;
-  if ((packet->flags() & VideoPacket::LAST_PARTITION) != 0)
-    callback = base::Bind(&VideoScheduler::VideoFrameSentCallback, this);
-
-  video_stub_->ProcessVideoPacket(packet.Pass(), callback);
+  video_stub_->ProcessVideoPacket(
+      packet.Pass(), base::Bind(&VideoScheduler::VideoFrameSentCallback, this));
 }
 
 void VideoScheduler::VideoFrameSentCallback() {
@@ -286,7 +283,6 @@ void VideoScheduler::EncodeFrame(
   // If there is nothing to encode then send an empty keep-alive packet.
   if (!frame || frame->updated_region().is_empty()) {
     scoped_ptr<VideoPacket> packet(new VideoPacket());
-    packet->set_flags(VideoPacket::LAST_PARTITION);
     packet->set_client_sequence_number(sequence_number);
     network_task_runner_->PostTask(
         FROM_HERE, base::Bind(&VideoScheduler::SendVideoPacket, this,
@@ -295,25 +291,16 @@ void VideoScheduler::EncodeFrame(
     return;
   }
 
-  encoder_->Encode(
-      frame.get(), base::Bind(&VideoScheduler::EncodedDataAvailableCallback,
-                              this, sequence_number));
-  capture_task_runner_->DeleteSoon(FROM_HERE, frame.release());
-}
-
-void VideoScheduler::EncodedDataAvailableCallback(
-    int64 sequence_number,
-    scoped_ptr<VideoPacket> packet) {
-  DCHECK(encode_task_runner_->BelongsToCurrentThread());
-
+  scoped_ptr<VideoPacket> packet = encoder_->Encode(*frame);
   packet->set_client_sequence_number(sequence_number);
 
-  bool last = (packet->flags() & VideoPacket::LAST_PACKET) != 0;
-  if (last) {
-    scheduler_.RecordEncodeTime(
-        base::TimeDelta::FromMilliseconds(packet->encode_time_ms()));
-  }
+  // Destroy the frame before sending |packet| because SendVideoPacket() may
+  // trigger another frame to be captured, and the screen capturer expects the
+  // old frame to be freed by then.
+  frame.reset();
 
+  scheduler_.RecordEncodeTime(
+      base::TimeDelta::FromMilliseconds(packet->encode_time_ms()));
   network_task_runner_->PostTask(
       FROM_HERE, base::Bind(&VideoScheduler::SendVideoPacket, this,
                             base::Passed(&packet)));
diff --git a/remoting/host/video_scheduler_unittest.cc b/remoting/host/video_scheduler_unittest.cc
index 2dd7afb..25567d2 100644
--- a/remoting/host/video_scheduler_unittest.cc
+++ b/remoting/host/video_scheduler_unittest.cc
@@ -37,8 +37,7 @@ namespace {
 
 ACTION(FinishEncode) {
   scoped_ptr<VideoPacket> packet(new VideoPacket());
-  packet->set_flags(VideoPacket::LAST_PACKET | VideoPacket::LAST_PARTITION);
-  arg1.Run(packet.Pass());
+  return packet.release();
 }
 
 ACTION(FinishSend) {
@@ -55,9 +54,11 @@ class MockVideoEncoder : public VideoEncoder {
   MockVideoEncoder();
   virtual ~MockVideoEncoder();
 
-  MOCK_METHOD2(Encode, void(
-      const webrtc::DesktopFrame* frame,
-      const DataAvailableCallback& data_available_callback));
+  scoped_ptr<VideoPacket> Encode(
+      const webrtc::DesktopFrame& frame) {
+    return scoped_ptr<VideoPacket>(EncodePtr(frame));
+  }
+  MOCK_METHOD1(EncodePtr, VideoPacket*(const webrtc::DesktopFrame& frame));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockVideoEncoder);
@@ -158,7 +159,6 @@ TEST_F(VideoSchedulerTest, StartAndStop) {
 
   frame_.reset(new webrtc::BasicDesktopFrame(
       webrtc::DesktopSize(kWidth, kHeight)));
-  webrtc::DesktopFrame* frame_ptr = frame_.get();
 
   // First the capturer is called.
   Expectation capturer_capture = EXPECT_CALL(*capturer, Capture(_))
@@ -166,7 +166,7 @@ TEST_F(VideoSchedulerTest, StartAndStop) {
       .WillRepeatedly(Invoke(this, &VideoSchedulerTest::OnCaptureFrame));
 
   // Expect the encoder be called.
-  EXPECT_CALL(*encoder_, Encode(frame_ptr, _))
+  EXPECT_CALL(*encoder_, EncodePtr(_))
       .WillRepeatedly(FinishEncode());
 
   // By default delete the arguments when ProcessVideoPacket is received.