Cast: Passing the frame decoded callback to the codec.

This cl cleans up the video receiver from handling the raw
frames, and will align the frame handling of the cast vp8
decoder and the chromium/ffmpeg h264 decoder (to be added).

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

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

media/cast/test/encode_decode_test.cc

@@ -8,7 +8,10 @@
 
 #include <gtest/gtest.h>
 
+#include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
+#include "media/cast/cast_environment.h"
+#include "media/cast/test/fake_task_runner.h"
 #include "media/cast/test/video_utility.h"
 #include "media/cast/video_receiver/codecs/vp8/vp8_decoder.h"
 #include "media/cast/video_sender/codecs/vp8/vp8_encoder.h"
@@ -16,18 +19,65 @@
 namespace media {
 namespace cast {
 
+static const int64 kStartMillisecond = GG_INT64_C(1245);
+static const int kWidth = 1280;
+static const int kHeight = 720;
+static const int kStartbitrate = 4000000;
+static const int kMaxQp = 54;
+static const int kMinQp = 4;
+static const int kMaxFrameRate = 30;
+
 namespace {
-const int kWidth = 1280;
-const int kHeight = 720;
-const int kStartbitrate = 4000000;
-const int kMaxQp = 54;
-const int kMinQp = 4;
-const int kMaxFrameRate = 30;
+class EncodeDecodeTestFrameCallback :
+    public base::RefCountedThreadSafe<EncodeDecodeTestFrameCallback> {
+ public:
+  EncodeDecodeTestFrameCallback()
+      : num_called_(0) {
+    original_frame_.width = kWidth;
+    original_frame_.height = kHeight;
+  }
+
+  void SetFrameStartValue(int start_value) {
+    PopulateVideoFrame(&original_frame_, start_value);
+  }
+
+  void DecodeComplete(scoped_ptr<I420VideoFrame> decoded_frame,
+                      const base::TimeTicks& render_time) {
+    ++num_called_;
+    // Compare frames.
+    // Compare resolution.
+    EXPECT_EQ(original_frame_.width, decoded_frame->width);
+    EXPECT_EQ(original_frame_.height, decoded_frame->height);
+    // Compare data.
+    EXPECT_GT(I420PSNR(original_frame_, *(decoded_frame.get())), 40.0);
+  }
+
+  int num_called() const {
+    return num_called_;
+  }
+
+ protected:
+  virtual ~EncodeDecodeTestFrameCallback() {}
+
+ private:
+  friend class base::RefCountedThreadSafe<EncodeDecodeTestFrameCallback>;
+
+  int num_called_;
+  I420VideoFrame original_frame_;
+};
 }  // namespace
 
 class EncodeDecodeTest : public ::testing::Test {
  protected:
-  EncodeDecodeTest() {
+  EncodeDecodeTest()
+      : task_runner_(new test::FakeTaskRunner(&testing_clock_)),
+        // CastEnvironment will only be used by the vp8 decoder; Enable only the
+        // video decoder and main threads.
+        cast_environment_(new CastEnvironment(&testing_clock_, task_runner_,
+            NULL, NULL, NULL, task_runner_)),
+        test_callback_(new EncodeDecodeTestFrameCallback()) {
+    testing_clock_.Advance(
+        base::TimeDelta::FromMilliseconds(kStartMillisecond));
     encoder_config_.max_number_of_video_buffers_used = 1;
     encoder_config_.number_of_cores = 1;
     encoder_config_.width  = kWidth;
@@ -39,49 +89,46 @@ class EncodeDecodeTest : public ::testing::Test {
     int max_unacked_frames = 1;
     encoder_.reset(new Vp8Encoder(encoder_config_, max_unacked_frames));
     // Initialize to use one core.
-    decoder_.reset(new Vp8Decoder(1));
+    decoder_.reset(new Vp8Decoder(1, cast_environment_));
   }
 
-  virtual void SetUp() {
+  virtual ~EncodeDecodeTest() {}
+
+  virtual void SetUp() OVERRIDE {
     // Create test frame.
     int start_value = 10;  // Random value to start from.
     video_frame_.reset(new I420VideoFrame());
     video_frame_->width = encoder_config_.width;
     video_frame_->height = encoder_config_.height;
     PopulateVideoFrame(video_frame_.get(), start_value);
+    test_callback_->SetFrameStartValue(start_value);
   }
 
-  virtual void TearDown() {
+  virtual void TearDown() OVERRIDE {
     delete [] video_frame_->y_plane.data;
     delete [] video_frame_->u_plane.data;
     delete [] video_frame_->v_plane.data;
   }
 
-  void Compare(const I420VideoFrame& original_image,
-               const I420VideoFrame& decoded_image) {
-    // Compare resolution.
-    EXPECT_EQ(original_image.width, decoded_image.width);
-    EXPECT_EQ(original_image.height, decoded_image.height);
-    // Compare data.
-    EXPECT_GT(I420PSNR(original_image, decoded_image), 40.0);
-  }
-
   VideoSenderConfig encoder_config_;
   scoped_ptr<Vp8Encoder> encoder_;
   scoped_ptr<Vp8Decoder> decoder_;
   scoped_ptr<I420VideoFrame> video_frame_;
+  base::SimpleTestTickClock testing_clock_;
+  scoped_refptr<test::FakeTaskRunner> task_runner_;
+  scoped_refptr<CastEnvironment> cast_environment_;
+  scoped_refptr<EncodeDecodeTestFrameCallback> test_callback_;
 };
 
 TEST_F(EncodeDecodeTest, BasicEncodeDecode) {
   EncodedVideoFrame encoded_frame;
-  I420VideoFrame decoded_frame;
   // Encode frame.
   encoder_->Encode(*(video_frame_.get()), &encoded_frame);
   EXPECT_GT(encoded_frame.data.size(), GG_UINT64_C(0));
   // Decode frame.
-  decoder_->Decode(encoded_frame, &decoded_frame);
-  // Validate data.
-  Compare(*(video_frame_.get()), decoded_frame);
+  decoder_->Decode(&encoded_frame, base::TimeTicks(), base::Bind(
+      &EncodeDecodeTestFrameCallback::DecodeComplete, test_callback_));
+  task_runner_->RunTasks();
 }
 
 }  // namespace cast

media/cast/video_receiver/codecs/vp8/vp8_decoder.cc

@@ -4,14 +4,18 @@
 
 #include "media/cast/video_receiver/codecs/vp8/vp8_decoder.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/message_loop/message_loop.h"
 #include "third_party/libvpx/source/libvpx/vpx/vp8dx.h"
 
 namespace media {
 namespace cast {
 
-Vp8Decoder::Vp8Decoder(int number_of_cores) {
-  decoder_.reset(new vpx_dec_ctx_t());
+Vp8Decoder::Vp8Decoder(int number_of_cores,
+                       scoped_refptr<CastEnvironment> cast_environment)
+    : decoder_(new vpx_dec_ctx_t()),
+      cast_environment_(cast_environment) {
   InitDecode(number_of_cores);
 }
 
@@ -27,18 +31,20 @@ void Vp8Decoder::InitDecode(int number_of_cores) {
   }
 }
 
-bool Vp8Decoder::Decode(const EncodedVideoFrame& input_image,
-                        I420VideoFrame* decoded_frame) {
-  VLOG(1) << "VP8 decode frame:" << static_cast<int>(input_image.frame_id)
-          << " sized:" << input_image.data.size();
+bool Vp8Decoder::Decode(const EncodedVideoFrame* encoded_frame,
+                        const base::TimeTicks render_time,
+                        const VideoFrameDecodedCallback& frame_decoded_cb) {
+  const int frame_id_int = static_cast<int>(encoded_frame->frame_id);
+  VLOG(1) << "VP8 decode frame:" << frame_id_int
+          << " sized:" << encoded_frame->data.size();
 
-  if (input_image.data.empty()) return false;
+  if (encoded_frame->data.empty()) return false;
 
   vpx_codec_iter_t iter = NULL;
   vpx_image_t* img;
   if (vpx_codec_decode(decoder_.get(),
-                       input_image.data.data(),
-                       static_cast<unsigned int>(input_image.data.size()),
+                       encoded_frame->data.data(),
+                       static_cast<unsigned int>(encoded_frame->data.size()),
                        0,
                        1 /* real time*/)) {
     VLOG(1) << "Failed to decode VP8 frame.";
@@ -47,11 +53,12 @@ bool Vp8Decoder::Decode(const EncodedVideoFrame& input_image,
 
   img = vpx_codec_get_frame(decoder_.get(), &iter);
   if (img == NULL) {
-    VLOG(1) << "Skip rendering VP8 frame:"
-            << static_cast<int>(input_image.frame_id);
+    VLOG(1) << "Skip rendering VP8 frame:" << frame_id_int;
     return false;
   }
 
+  scoped_ptr<I420VideoFrame> decoded_frame(new I420VideoFrame());
+
   // The img is only valid until the next call to vpx_codec_decode.
   // Populate the decoded image.
   decoded_frame->width = img->d_w;
@@ -76,6 +83,12 @@ bool Vp8Decoder::Decode(const EncodedVideoFrame& input_image,
   memcpy(decoded_frame->v_plane.data, img->planes[VPX_PLANE_V],
          decoded_frame->v_plane.length);
 
+  // Return frame.
+  VLOG(1) << "Decoded frame " << frame_id_int;
+  // Frame decoded - return frame to the user via callback.
+  cast_environment_->PostTask(CastEnvironment::MAIN, FROM_HERE,
+      base::Bind(frame_decoded_cb, base::Passed(&decoded_frame), render_time));
+
   return true;
 }
 

media/cast/video_receiver/codecs/vp8/vp8_decoder.h

@@ -8,6 +8,8 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "media/cast/cast_config.h"
+#include "media/cast/cast_environment.h"
+#include "media/cast/cast_receiver.h"
 #include "third_party/libvpx/source/libvpx/vpx/vpx_decoder.h"
 
 typedef struct vpx_codec_ctx vpx_dec_ctx_t;
@@ -19,18 +21,24 @@ namespace cast {
 // thread.
 class Vp8Decoder : public base::NonThreadSafe {
  public:
-  explicit Vp8Decoder(int number_of_cores);
+  Vp8Decoder(int number_of_cores,
+             scoped_refptr<CastEnvironment> cast_environment);
   ~Vp8Decoder();
 
-  // Decode encoded image (as a part of a video stream).
-  bool Decode(const EncodedVideoFrame& input_image,
-              I420VideoFrame* decoded_frame);
+  // Decode frame - The decoded frame will be passed via the callback.
+  // Will return false in case of error, and then it's up to the caller to
+  // release the memory.
+  // Ownership of the encoded_frame does not pass to the Vp8Decoder.
+  bool Decode(const EncodedVideoFrame* encoded_frame,
+              const base::TimeTicks render_time,
+              const VideoFrameDecodedCallback& frame_decoded_cb);
 
  private:
   // Initialize the decoder.
   void InitDecode(int number_of_cores);
 
   scoped_ptr<vpx_dec_ctx_t> decoder_;
+  scoped_refptr<CastEnvironment> cast_environment_;
 };
 
 }  // namespace cast

media/cast/video_receiver/video_decoder.cc

@@ -12,13 +12,14 @@
 namespace media {
 namespace cast {
 
-VideoDecoder::VideoDecoder(const VideoReceiverConfig& video_config)
+VideoDecoder::VideoDecoder(const VideoReceiverConfig& video_config,
+                           scoped_refptr<CastEnvironment> cast_environment)
     : codec_(video_config.codec),
       vp8_decoder_() {
   switch (video_config.codec) {
     case kVp8:
       // Initializing to use one core.
-      vp8_decoder_.reset(new Vp8Decoder(1));
+      vp8_decoder_.reset(new Vp8Decoder(1, cast_environment));
       break;
     case kH264:
       NOTIMPLEMENTED();
@@ -31,13 +32,13 @@ VideoDecoder::VideoDecoder(const VideoReceiverConfig& video_config)
 
 VideoDecoder::~VideoDecoder() {}
 
-bool VideoDecoder::DecodeVideoFrame(
-    const EncodedVideoFrame* encoded_frame,
-    const base::TimeTicks render_time,
-    I420VideoFrame* video_frame) {
+bool VideoDecoder::DecodeVideoFrame(const EncodedVideoFrame* encoded_frame,
+                                    const base::TimeTicks render_time,
+                                    const VideoFrameDecodedCallback&
+                                    frame_decoded_cb) {
   DCHECK(encoded_frame->codec == codec_) << "Invalid codec";
   DCHECK_GT(encoded_frame->data.size(), GG_UINT64_C(0)) << "Empty video frame";
-  return vp8_decoder_->Decode(*encoded_frame, video_frame);
+  return vp8_decoder_->Decode(encoded_frame, render_time, frame_decoded_cb);
 }
 
 }  // namespace cast

media/cast/video_receiver/video_decoder.h

@@ -19,14 +19,15 @@ class Vp8Decoder;
 // thread.
 class VideoDecoder : public base::NonThreadSafe {
  public:
-  explicit VideoDecoder(const VideoReceiverConfig& video_config);
+  VideoDecoder(const VideoReceiverConfig& video_config,
+               scoped_refptr<CastEnvironment> cast_environment);
   virtual ~VideoDecoder();
 
-  // Decode a video frame. Decoded (raw) frame will be returned in the
-  // provided video_frame.
+  // Decode a video frame. Decoded (raw) frame will be returned via the
+  // provided callback
   bool DecodeVideoFrame(const EncodedVideoFrame* encoded_frame,
                         const base::TimeTicks render_time,
-                        I420VideoFrame* video_frame);
+                        const VideoFrameDecodedCallback& frame_decoded_cb);
 
  private:
   VideoCodec codec_;

media/cast/video_receiver/video_decoder_unittest.cc

@@ -2,8 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/test/simple_test_tick_clock.h"
+#include "base/time/tick_clock.h"
+#include "media/cast/cast_config.h"
 #include "media/cast/cast_defines.h"
+#include "media/cast/cast_environment.h"
+#include "media/cast/cast_receiver.h"
+#include "media/cast/test/fake_task_runner.h"
 #include "media/cast/video_receiver/video_decoder.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -14,44 +21,70 @@ using testing::_;
 
 // Random frame size for testing.
 const int kFrameSize = 2345;
+static const int64 kStartMillisecond = GG_INT64_C(1245);
+
+namespace {
+class DecodeTestFrameCallback :
+  public base::RefCountedThreadSafe<DecodeTestFrameCallback> {
+ public:
+  DecodeTestFrameCallback() {}
+
+  void DecodeComplete(scoped_ptr<I420VideoFrame> decoded_frame,
+                      const base::TimeTicks& render_time) {}
+ protected:
+  virtual ~DecodeTestFrameCallback() {}
+ private:
+  friend class base::RefCountedThreadSafe<DecodeTestFrameCallback>;
+};
+}  // namespace
 
 class VideoDecoderTest : public ::testing::Test {
  protected:
-  VideoDecoderTest() {
+  VideoDecoderTest()
+    : task_runner_(new test::FakeTaskRunner(&testing_clock_)),
+      cast_environment_(new CastEnvironment(&testing_clock_, task_runner_,
+          task_runner_, task_runner_, task_runner_, task_runner_)),
+      test_callback_(new DecodeTestFrameCallback()) {
     // Configure to vp8.
     config_.codec = kVp8;
     config_.use_external_decoder = false;
-    decoder_.reset(new VideoDecoder(config_));
+    decoder_.reset(new VideoDecoder(config_, cast_environment_));
+    testing_clock_.Advance(
+        base::TimeDelta::FromMilliseconds(kStartMillisecond));
   }
 
   virtual ~VideoDecoderTest() {}
 
   scoped_ptr<VideoDecoder> decoder_;
   VideoReceiverConfig config_;
+  base::SimpleTestTickClock testing_clock_;
+  scoped_refptr<test::FakeTaskRunner> task_runner_;
+  scoped_refptr<CastEnvironment> cast_environment_;
+  scoped_refptr<DecodeTestFrameCallback> test_callback_;
 };
 
 // TODO(pwestin): EXPECT_DEATH tests can not pass valgrind.
 TEST_F(VideoDecoderTest, DISABLED_SizeZero) {
   EncodedVideoFrame encoded_frame;
-  I420VideoFrame video_frame;
   base::TimeTicks render_time;
   encoded_frame.codec = kVp8;
-
   EXPECT_DEATH(
-      decoder_->DecodeVideoFrame(&encoded_frame, render_time, &video_frame),
-     "Empty video frame");
+      decoder_->DecodeVideoFrame(
+          &encoded_frame, render_time,
+          base::Bind(&DecodeTestFrameCallback::DecodeComplete, test_callback_)),
+      "Empty frame");
 }
 
 // TODO(pwestin): EXPECT_DEATH tests can not pass valgrind.
 TEST_F(VideoDecoderTest, DISABLED_InvalidCodec) {
   EncodedVideoFrame encoded_frame;
-  I420VideoFrame video_frame;
   base::TimeTicks render_time;
   encoded_frame.data.assign(kFrameSize, 0);
   encoded_frame.codec = kExternalVideo;
   EXPECT_DEATH(
-      decoder_->DecodeVideoFrame(&encoded_frame, render_time, &video_frame),
-     "Invalid codec");
+      decoder_->DecodeVideoFrame(&encoded_frame, render_time, base::Bind(
+          &DecodeTestFrameCallback::DecodeComplete, test_callback_)),
+      "Invalid codec");
 }
 
 // TODO(pwestin): Test decoding a real frame.

media/cast/video_receiver/video_receiver.cc

@@ -131,7 +131,7 @@ VideoReceiver::VideoReceiver(scoped_refptr<CastEnvironment> cast_environment,
                            video_config.decoder_faster_than_max_frame_rate,
                            max_unacked_frames));
   if (!video_config.use_external_decoder) {
-    video_decoder_.reset(new VideoDecoder(video_config));
+    video_decoder_.reset(new VideoDecoder(video_config, cast_environment));
   }
 
   rtcp_.reset(
@@ -180,23 +180,12 @@ void VideoReceiver::DecodeVideoFrameThread(
   DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::VIDEO_DECODER));
   DCHECK(video_decoder_);
 
-  // TODO(mikhal): Allow the application to allocate this memory.
-  scoped_ptr<I420VideoFrame> video_frame(new I420VideoFrame());
-
-  bool success = video_decoder_->DecodeVideoFrame(encoded_frame.get(),
-      render_time, video_frame.get());
-
-  if (success) {
-    VLOG(1) << "Decoded frame " << static_cast<int>(encoded_frame->frame_id);
-    // Frame decoded - return frame to the user via callback.
-    cast_environment_->PostTask(CastEnvironment::MAIN, FROM_HERE,
-          base::Bind(frame_decoded_callback,
-                     base::Passed(&video_frame), render_time));
-  } else {
+  if (!(video_decoder_->DecodeVideoFrame(encoded_frame.get(), render_time,
+                                        frame_decoded_callback))) {
     // This will happen if we decide to decode but not show a frame.
     cast_environment_->PostTask(CastEnvironment::MAIN, FROM_HERE,
         base::Bind(&VideoReceiver::GetRawVideoFrame,
-            weak_factory_.GetWeakPtr(), frame_decoded_callback));
+                   weak_factory_.GetWeakPtr(), frame_decoded_callback));
   }
 }