MediaStream: Adding VideoTrackRecorder class and unittests

content/renderer/media/video_track_recorder.cc

Index: content/renderer/media/video_track_recorder.cc
diff --git a/content/renderer/media/video_track_recorder.cc b/content/renderer/media/video_track_recorder.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5097a01e34137705a32e68f6e1aed3cbcac36940
--- /dev/null
+++ b/content/renderer/media/video_track_recorder.cc
@@ -0,0 +1,291 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/renderer/media/video_track_recorder.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/threading/thread.h"
+#include "base/time/time.h"
+#include "base/trace_event/trace_event.h"
+#include "content/child/child_process.h"
+#include "media/base/bind_to_current_loop.h"
+#include "media/base/video_frame.h"
+
+extern "C" {
+#define VPX_CODEC_DISABLE_COMPAT 1

[miu, 2015/07/27 22:30:09]

Is this legal?  You're changing a build-time config option, so the libvpx header
files may no longer match the compiled libvpx binary...and crash.

[mcasas, 2015/07/31 11:26:27]

I guess so, is used all over the codebase [1] including
remoting/codec/video_encoder_vpx.cc and 
media/cast/receiver/video_decoder.cc (where you 
landed it [2] :) -- arguably it was a refactor)


[1]
https://code.google.com/p/chromium/codesearch#search/&sq=package:chromium&typ...
[2] https://codereview.chromium.org/225023010

[miu, 2015/08/04 04:06:30]

Weird.  Well, okay I guess.  ;-)

+#include "third_party/libvpx/source/libvpx/vpx/vp8cx.h"
+#include "third_party/libvpx/source/libvpx/vpx/vpx_encoder.h"
+}
+
+using media::VideoFrame;
+using media::VideoFrameMetadata;
+
+namespace content {
+
+namespace {
+
+// Originally from remoting/codec/scoped_vpx_codec.h
+struct VpxCodecDeleter {
+  void operator()(vpx_codec_ctx_t* codec) {
+    if (!codec)
+      return;
+    const vpx_codec_err_t ret = vpx_codec_destroy(codec);
+    DCHECK_EQ(VPX_CODEC_OK, ret) << "Failed to destroy codec";
+    delete codec;
+  }
+};
+
+const vpx_codec_flags_t kNoFlags = 0;
+const uint32_t kDefaultFrameDurationInMilliseconds = 30u;
+
+static scoped_ptr<vpx_image_t> InitVp8Encoding(const gfx::Size& size,
+                                               vpx_codec_ctx_t* codec_context) {
+  const vpx_codec_iface_t* interface = vpx_codec_vp8_cx();
+  scoped_ptr<vpx_codec_enc_cfg_t> codec_config(new vpx_codec_enc_cfg_t);
+  vpx_codec_enc_config_default(interface, codec_config.get(), 0 /* reserved */);
+
+  // Adjust default bit rate to account for the actual size.
+  codec_config->rc_target_bitrate = size.width() * size.height() *
+                                    codec_config->rc_target_bitrate /
+                                    codec_config->g_w / codec_config->g_h;
+  DCHECK(size.width());
+  DCHECK(size.height());
+  codec_config->g_w = size.width();
+  codec_config->g_h = size.height();
+  codec_config->g_pass = VPX_RC_ONE_PASS;
+
+  // Timebase is the smallest interval used by the stream, can be set to the
+  // frame rate or just to milliseconds.
+  codec_config->g_timebase.num = 1;
+  codec_config->g_timebase.den = base::Time::kMillisecondsPerSecond;
+
+  // Let the encoder decide where to place the Keyframes, between min and max.
+  // Due to http://crbug.com/440223, decoding fails after 30,000 non-key
+  // frames, so force an "unnecessary" key-frame every 10,000 frames.
+  codec_config->kf_mode = VPX_KF_AUTO;
+  codec_config->kf_min_dist = 10000;
+  codec_config->kf_max_dist = 10000;
+
+  // Number of frames to consume before producing output.
+  codec_config->g_lag_in_frames = 0;
+
+  const vpx_codec_err_t ret = vpx_codec_enc_init(codec_context, interface,
+                                                 codec_config.get(), kNoFlags);
+  DCHECK_EQ(VPX_CODEC_OK, ret);
+
+  scoped_ptr<vpx_image_t> image(new vpx_image_t());
+  vpx_image_t* const returned_image_ptr =
+      vpx_img_wrap(image.get(), VPX_IMG_FMT_I420, size.width(), size.height(),
+                   1 /* align */, nullptr /* img_data */);
+  DCHECK_EQ(image.get(), returned_image_ptr);
+  return image.Pass();
+}
+
+static double GetFrameRate(const scoped_refptr<VideoFrame>& video_frame) {
+  double frame_rate = 0.0f;
+  base::IgnoreResult(video_frame->metadata()->GetDouble(
+      VideoFrameMetadata::FRAME_RATE, &frame_rate));
+  return frame_rate;
+}
+
+}  // anonymous namespace
+
+// Inner class holding all libvpx configuration and encoding received frames.
+// This class is created and destroyed on its parent's thread: the main render
+// thread. It receives VideoFrames and must Run() the callbacks on IO thread,
+// and finally uses an internal |encoding_thread_| for libvpx interactions,
+// notably for encoding (which might take some time). All this forces a bit of
+// hopping back and forth between threads.
+// Only VP8 is supported for the time being.
+// TODO(mcasas): Consider not getting |io_task_runner| in ctor and instead
+// caching it on first EncodeOnIo().
+class VideoTrackRecorder::VpxEncoder
+    : public base::RefCountedThreadSafe<VpxEncoder> {
+ public:
+  VpxEncoder(const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
+             const OnFirstFrameCB& on_first_frame_callback,
+             const OnEncodedVideoCB& on_encoded_video_callback);
+
+  void EncodeOnIo(const scoped_refptr<VideoFrame>& frame,
+                  const base::TimeTicks& capture_timestamp);
+
+ private:
+  friend class base::RefCountedThreadSafe<VpxEncoder>;
+  virtual ~VpxEncoder();
+
+  void FinishConfigurationOnEncodingThread(const gfx::Size frame_size,
+                                           const base::TimeTicks& timestamp);
+
+  void EncodeOnEncodingThread(bool force_keyframe,
+                              const scoped_refptr<VideoFrame>& frame,
+                              const base::TimeTicks& capture_timestamp);
+
+  void FinishEncodingOnIo(scoped_ptr<std::string> data,
+                          base::TimeDelta timestamp,
+                          bool keyframe);
+
+  // Used to check that we are destroyed on the same thread we were created.
+  base::ThreadChecker main_render_thread_checker_;
+
+  // Render IO thread task runner for posting tasks and DCHECK()s.
+  const scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
+
+  // Configuration happens on first frame arrival. Used on IO thread only.

[miu, 2015/07/27 22:30:09]

This won't work.  MediaStreamVideoSinks must handle variable resolution. 
Suggest you take a look at the way we handle re-init of the encoder when the
resolution changes in media/cast:
https://code.google.com/p/chromium/codesearch#chromium/src/media/cast/sender/...

[mcasas, 2015/07/31 11:26:27]

Done. Shamelessly taken over big chunks of the code there.

+  bool configuration_done_;
+  // Written once on IO thread and used (read) from capture thread.
+  uint64_t track_index_;
+  // Callbacks that should be exercised on IO thread.
+  const OnFirstFrameCB on_first_frame_callback_;
+  const OnEncodedVideoCB on_encoded_video_callback_;
+
+  // Thread for encoding. Active between first frame reception and class
+  // destruction. All variables below this are used in this thread.
+  base::Thread encoding_thread_;
+  // VP8 internal objects: codec configuration and Vpx Image wrapper.
+  scoped_ptr<vpx_codec_ctx_t, VpxCodecDeleter> codec_;
+  scoped_ptr<vpx_image_t> vpx_image_;
+  // Origin of times for frame timestamps.
+  base::TimeTicks timestamp_base_;

[miu, 2015/07/27 22:30:09]

How about first_frame_timestamp_?

[mcasas, 2015/07/31 11:26:27]

Done.

+};
+
+VideoTrackRecorder::VpxEncoder::VpxEncoder(
+    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
+    const OnFirstFrameCB& on_first_frame_callback,
+    const OnEncodedVideoCB& on_encoded_video_callback)
+    : io_task_runner_(io_task_runner),
+      configuration_done_(false),
+      track_index_(0),
+      on_first_frame_callback_(on_first_frame_callback),
+      on_encoded_video_callback_(on_encoded_video_callback),
+      encoding_thread_("EncodingThread"),
+      codec_(new vpx_codec_ctx_t) {
+  DCHECK(!on_first_frame_callback_.is_null());
+  DCHECK(!on_encoded_video_callback_.is_null());
+  DCHECK(!encoding_thread_.IsRunning());
+  encoding_thread_.Start();
+}
+
+VideoTrackRecorder::VpxEncoder::~VpxEncoder() {
+  DCHECK(main_render_thread_checker_.CalledOnValidThread());
+  DCHECK(encoding_thread_.IsRunning());
+  encoding_thread_.Stop();
+}
+
+void VideoTrackRecorder::VpxEncoder::EncodeOnIo(

[miu, 2015/07/27 22:30:09]

Ah!  So the encoding doesn't actually happen on the IO thread?  I'd suggest
changing the naming of the "OnIo" methods.  Suggestions:

  EncodeOnIo() --> StartFrameEncode()
  FinishEncodingOnIo() --> OnFrameEncodeCompleted()

The point is that these are "control logic" methods, not the ones doing actual
work, and that should be reflected in their names.

[mcasas, 2015/07/31 11:26:27]

Good suggestion, done.

+    const scoped_refptr<VideoFrame>& frame,
+    const base::TimeTicks& capture_timestamp) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+
+  bool force_keyframe = false;
+  if (!configuration_done_) {
+    track_index_ = on_first_frame_callback_.Run(frame->coded_size(),

[miu, 2015/07/27 22:30:09]

This must be the visible_rect().size(), not coded_size().  (Here and pretty
everywhere else, FWICT.)

[mcasas, 2015/07/31 11:26:27]

Done.

+                                                GetFrameRate(frame));
+    encoding_thread_.task_runner()->PostTask(FROM_HERE,
+        base::Bind(&VpxEncoder::FinishConfigurationOnEncodingThread,
+                   this, frame->coded_size(), capture_timestamp));
+    force_keyframe = true;
+    configuration_done_ = true;
+  }
+  encoding_thread_.task_runner()->PostTask(FROM_HERE,
+      base::Bind(&VpxEncoder::EncodeOnEncodingThread,
+                 this, force_keyframe, frame, capture_timestamp));
+}
+
+void VideoTrackRecorder::VpxEncoder::FinishConfigurationOnEncodingThread(
+    const gfx::Size frame_size,
+    const base::TimeTicks& timestamp) {
+  DCHECK(encoding_thread_.task_runner()->BelongsToCurrentThread());
+  timestamp_base_ = timestamp;
+
+  vpx_image_ = InitVp8Encoding(frame_size, codec_.get());
+  DCHECK(vpx_image_.get());
+}
+
+void VideoTrackRecorder::VpxEncoder::EncodeOnEncodingThread(
+    bool force_keyframe,

[miu, 2015/07/27 22:30:09]

You don't need to force a key frame.  VP8 will automatically force a KF for the
first encoded frame, and any time the frame resolution changes.

[mcasas, 2015/07/31 11:26:27]

Done.

+    const scoped_refptr<VideoFrame>& frame,
+    const base::TimeTicks& capture_timestamp) {
+  TRACE_EVENT0("video",
+               "VideoTrackRecorder::VpxEncoder::EncodeOnEncodingThread");
+  DCHECK(encoding_thread_.task_runner()->BelongsToCurrentThread());
+
+  DCHECK(frame->visible_data(VideoFrame::kYPlane));
+  vpx_image_->planes[VPX_PLANE_Y] = frame->visible_data(VideoFrame::kYPlane);
+  vpx_image_->planes[VPX_PLANE_U] = frame->visible_data(VideoFrame::kUPlane);
+  vpx_image_->planes[VPX_PLANE_V] = frame->visible_data(VideoFrame::kVPlane);
+  vpx_image_->stride[VPX_PLANE_Y] = frame->stride(VideoFrame::kYPlane);
+  vpx_image_->stride[VPX_PLANE_U] = frame->stride(VideoFrame::kUPlane);
+  vpx_image_->stride[VPX_PLANE_V] = frame->stride(VideoFrame::kVPlane);
+
+  // TODO(mcasas): Use VideoFrameMetadata::FRAME_DURATION if available.

[miu, 2015/07/27 22:30:09]

Quality and target bitrate will be all wrong for many video streams if you use
this hard-coded duration.  Please get rid of this TODO and put in the 3 LOC to
grab this out of the metadata.

When FRAME_DURATION is not set, you can estimate based on the change in
timestamps between frames.  For example:
https://code.google.com/p/chromium/codesearch#chromium/src/media/cast/sender/...

[mcasas, 2015/07/31 11:26:27]

Done.

+  uint32_t duration = kDefaultFrameDurationInMilliseconds;
+  const double frame_rate = GetFrameRate(frame);
+  if (frame_rate)
+    duration = base::Time::kMillisecondsPerSecond / frame_rate;
+  const int timestamp = (capture_timestamp - timestamp_base_).InMilliseconds();
+  const vpx_enc_frame_flags_t flags =
+      kNoFlags | (force_keyframe ?  VPX_EFLAG_FORCE_KF : 0u);
+  const vpx_codec_err_t ret = vpx_codec_encode(codec_.get(), vpx_image_.get(),
+      timestamp, duration, flags, VPX_DL_REALTIME);
+  DCHECK_EQ(ret, VPX_CODEC_OK) << vpx_codec_err_to_string(ret) << ", #"
+                               << vpx_codec_error(codec_.get()) << " -"
+                               << vpx_codec_error_detail(codec_.get());
+
+  scoped_ptr<std::string> data(new std::string);
+  bool keyframe = false;
+  vpx_codec_iter_t iter = NULL;
+  const vpx_codec_cx_pkt_t* pkt = NULL;
+  while ((pkt = vpx_codec_get_cx_data(codec_.get(), &iter)) != NULL) {
+    if (pkt->kind != VPX_CODEC_CX_FRAME_PKT)
+      continue;
+    data->append(static_cast<char*>(pkt->data.frame.buf), pkt->data.frame.sz);

[miu, 2015/07/27 22:30:09]

Use assign() instead of append().  We don't want the string's underlying storage
to be re-allocated under an assumption that it may grow larger.  This may lead
to wasting memory.

[mcasas, 2015/07/31 11:26:27]

Done.

+    keyframe = (pkt->data.frame.flags & VPX_FRAME_IS_KEY) != 0;
+    break;
+  }
+  io_task_runner_->PostTask(FROM_HERE,
+      base::Bind(&VpxEncoder::FinishEncodingOnIo,
+                 this,
+                 base::Passed(&data),
+                 base::TimeDelta::FromMilliseconds(timestamp),
+                 keyframe ));
+}
+
+void VideoTrackRecorder::VpxEncoder::FinishEncodingOnIo(
+    scoped_ptr<std::string> data,
+    base::TimeDelta timestamp,
+    bool keyframe) {
+  DVLOG(1) << (keyframe ? "" : "non ") << "keyframe "
+           << timestamp.InMilliseconds() << " ms - " << data->length() << "B ";
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  on_encoded_video_callback_.Run(track_index_, base::StringPiece(*data),
+                                 timestamp, keyframe);
+}
+
+VideoTrackRecorder::VideoTrackRecorder(
+    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
+    const blink::WebMediaStreamTrack& track,
+    const OnFirstFrameCB& on_first_frame_cb,
+    const OnEncodedVideoCB& on_encoded_video_cb)
+    : track_(track),
+      encoder_(new VpxEncoder(io_task_runner,
+                              on_first_frame_cb,
+                              on_encoded_video_cb)) {
+  DCHECK(main_render_thread_checker_.CalledOnValidThread());
+  DCHECK(track.extraData());
+  AddToVideoTrack(this, base::Bind(&VpxEncoder::EncodeOnIo, encoder_), track_);
+}
+
+VideoTrackRecorder::~VideoTrackRecorder() {
+  DCHECK(main_render_thread_checker_.CalledOnValidThread());
+  RemoveFromVideoTrack(this, track_);
+}
+
+void VideoTrackRecorder::EncodeOnIoForTesting(
+    const scoped_refptr<VideoFrame>& frame,
+    const base::TimeTicks& capture_timestamp) {
+  encoder_->EncodeOnIo(frame, capture_timestamp);
+}
+
+}  // namespace content