Provide a Chrome-owned buffer to FFmpeg for video decoding, instead of

media/filters/ffmpeg_video_decoder.cc

Index: media/filters/ffmpeg_video_decoder.cc
===================================================================
--- media/filters/ffmpeg_video_decoder.cc	(revision 140753)
+++ media/filters/ffmpeg_video_decoder.cc	(working copy)
@@ -61,6 +61,50 @@
       decryptor_(NULL) {
 }
 
+int FFmpegVideoDecoder::GetVideoBuffer(AVFrame *frame) {

[scherkus (not reviewing), 2012/06/14 02:21:33]

pointer w/ type

+  VideoFrame::Format format = PixelFormatToVideoFormat(codec_context_->pix_fmt);
+  if (format == VideoFrame::INVALID)
+    return AVERROR(EINVAL);
+  DCHECK(format == VideoFrame::YV12 || format == VideoFrame::YV16);
+
+  int w = codec_context_->width, h = codec_context_->height, ret;

[scherkus (not reviewing), 2012/06/14 02:21:33]

split each initializer onto each line

s/w/width/
s/h/height/

+  if ((ret = av_image_check_size(w, h, 0, NULL)) < 0)
+    return ret;
+
+  scoped_refptr<VideoFrame> buf = VideoFrame::CreateFrame(format,

[scherkus (not reviewing), 2012/06/14 02:21:33]

drop this to next line + 4-space indent

[scherkus (not reviewing), 2012/06/14 02:21:33]

nit: s/buf/video_frame/

rest of this file uses said convention

+        w, h, 16u, kNoTimestamp(), kNoTimestamp());
+
+  for (int i = 0; i < 3; i++) {
+    frame->base[i] = buf->data(i);
+    frame->data[i] = buf->data(i);
+    frame->linesize[i] = buf->stride(i);
+  }
+
+  frame->opaque = buf.release();
+  frame->type = FF_BUFFER_TYPE_USER;
+  frame->pkt_pts = codec_context_->pkt ? codec_context_->pkt->pts : AV_NOPTS_VALUE;

[scherkus (not reviewing), 2012/06/14 02:21:33]

fix >80 chars

+  frame->width = codec_context_->width;
+  frame->height = codec_context_->height;
+  frame->format = codec_context_->pix_fmt;
+
+  return 0;
+}
+
+static int callbackGetVideoBuffer(AVCodecContext *s, AVFrame *frame) {

[scherkus (not reviewing), 2012/06/14 02:21:33]

pointer w/ type here + below

[scherkus (not reviewing), 2012/06/14 02:21:33]

we don't do camelCase code around here but AllCaps!

I'd drop the "callback" prefix anyway as the name isn't clashing w/ anything

same w/ ReleaseVideoBuffer below

+  FFmpegVideoDecoder *vd = static_cast<FFmpegVideoDecoder *>(s->opaque);
+  return vd->GetVideoBuffer(frame);
+}
+
+static void callbackReleaseVideoBuffer(AVCodecContext *s, AVFrame *frame) {

[scherkus (not reviewing), 2012/06/14 02:21:33]

pointer w/ type here + below

+  // We're releasing the refenence to the buffer allocated in
+  // GetVideoBuffer() here, so the explicit Release() here is
+  // intentional. Would be nice if scoped_refptr::adopt existed.

[scherkus (not reviewing), 2012/06/14 02:21:33]

I'd drop the "Would be nice.." part -- it ain't gonna happen anytime soon :)

+  scoped_refptr<VideoFrame> buf = static_cast<VideoFrame *>(frame->opaque);

[scherkus (not reviewing), 2012/06/14 02:21:33]

s/buf/video_frame

+  buf->Release();
+  memset(frame->data, 0, sizeof(frame->data));
+  frame->opaque = NULL;
+}
+
 void FFmpegVideoDecoder::Initialize(const scoped_refptr<DemuxerStream>& stream,
                                     const PipelineStatusCB& status_cb,
                                     const StatisticsCB& statistics_cb) {
@@ -104,6 +148,10 @@
   codec_context_->error_concealment = FF_EC_GUESS_MVS | FF_EC_DEBLOCK;
   codec_context_->err_recognition = AV_EF_CAREFUL;
   codec_context_->thread_count = GetThreadCount(codec_context_->codec_id);
+  codec_context_->opaque = this;
+  codec_context_->flags |= CODEC_FLAG_EMU_EDGE;
+  codec_context_->get_buffer = callbackGetVideoBuffer;
+  codec_context_->release_buffer = callbackReleaseVideoBuffer;
 
   AVCodec* codec = avcodec_find_decoder(codec_context_->codec_id);
   if (!codec) {
@@ -362,12 +410,11 @@
     return false;
   }
 
-  // We've got a frame! Make sure we have a place to store it.
-  *video_frame = AllocateVideoFrame();
-  if (!(*video_frame)) {
-    LOG(ERROR) << "Failed to allocate video frame";
+  if (!av_frame_->opaque) {
+    LOG(ERROR) << "VideoFrame object associated with frame data not set.";
     return false;
   }
+  *video_frame = static_cast<VideoFrame *>(av_frame_->opaque);
 
   // Determine timestamp and calculate the duration based on the repeat picture
   // count.  According to FFmpeg docs, the total duration can be calculated as
@@ -387,19 +434,6 @@
   (*video_frame)->SetDuration(
       ConvertFromTimeBase(doubled_time_base, 2 + av_frame_->repeat_pict));
 
-  // Copy the frame data since FFmpeg reuses internal buffers for AVFrame
-  // output, meaning the data is only valid until the next
-  // avcodec_decode_video() call.
-  int y_rows = codec_context_->height;
-  int uv_rows = codec_context_->height;
-  if (codec_context_->pix_fmt == PIX_FMT_YUV420P) {
-    uv_rows /= 2;
-  }
-
-  CopyYPlane(av_frame_->data[0], av_frame_->linesize[0], y_rows, *video_frame);
-  CopyUPlane(av_frame_->data[1], av_frame_->linesize[1], uv_rows, *video_frame);
-  CopyVPlane(av_frame_->data[2], av_frame_->linesize[2], uv_rows, *video_frame);
-
   return true;
 }
 
@@ -422,13 +456,4 @@
   }
 }
 
-scoped_refptr<VideoFrame> FFmpegVideoDecoder::AllocateVideoFrame() {
-  VideoFrame::Format format = PixelFormatToVideoFormat(codec_context_->pix_fmt);
-  size_t width = codec_context_->width;
-  size_t height = codec_context_->height;
-
-  return VideoFrame::CreateFrame(format, width, height,
-                                 kNoTimestamp(), kNoTimestamp());
-}
-
 }  // namespace media