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

media/filters/ffmpeg_video_decoder.cc

 // Copyright (c) 2012 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 "media/filters/ffmpeg_video_decoder.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/message_loop.h"
@@ skipping 41 matching lines @@
     const base::Callback<MessageLoop*()>& message_loop_cb)
     : message_loop_factory_cb_(message_loop_cb),
       message_loop_(NULL),
       state_(kUninitialized),
       codec_context_(NULL),
       av_frame_(NULL),
       frame_rate_numerator_(0),
       frame_rate_denominator_(0) {
 }
 
+int FFmpegVideoDecoder::AllocBuffer(VideoFrame **ptr)

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

no doco, no unittest, single call-site ==> inline it into the caller.

+{
+  VideoFrame::Format format = PixelFormatToVideoFormat(codec_context_->pix_fmt);
+  int w = codec_context_->width;
+  int h = codec_context_->height;
+  int ret, strides[4];
+  avcodec_align_dimensions(codec_context_, &w, &h);
+  if ((ret = av_image_check_size(w, h, 0, NULL)) < 0)
+    return ret;
+  if ((ret = av_image_fill_linesizes(strides, codec_context_->pix_fmt, w)) < 0)
+    return ret;
+  VideoFrame *buf = new VideoFrame::VideoFrame(format, codec_context_->width,
+                                               codec_context_->height,
+                                               kNoTimestamp(), kNoTimestamp());
+  if ((ret = buf->AllocateAlignedWithStrides(strides, h)) < 0) {
+    free(buf);
+    return ret;
+  }
+  *ptr = buf;
+  return 0;
+}
+
+int FFmpegVideoDecoder::GetVideoBuffer(AVFrame *frame)
+{
+  VideoFrame *buf;
+  int ret = AllocBuffer(&buf);
+  if (ret < 0)
+    return ret;
+
+  buf->AddRef();

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

AddRef() is almost never the right thing to call.
AllocBuffer should store the result of its "new VideoFrame" in a
scoped_refptr<VideoFrame>, and return that type.

When you need to pass it to C, call .release() on the scoped_refptr.

+  frame->opaque        = buf;

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

This sort of ascii-art alignment isn't adding anything, IMO.

+  frame->type          = FF_BUFFER_TYPE_USER;
+  frame->pkt_pts       = codec_context_->pkt ? codec_context_->pkt->pts : AV_NOPTS_VALUE;
+  frame->width         = codec_context_->width;
+  frame->height        = codec_context_->height;
+  frame->format        = codec_context_->pix_fmt;
+
+  for (int i = 0; i < (buf->format() == media::VideoFrame::RGB32 ? 1 : 3); i++) {

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

I think you're assuming either RGB32 or YV12/YV16.  In that case precede this
for loop with
DCHECK(buf->format() == VideoFrame::YV12 || buf->format() == VideoFrame::YV16 ||
       buf->format() == VideoFrame::RGB32) << buf->format();

       

+    frame->base[i]     =

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

Un-style-ish.  Repeat buf->data(i)

+    frame->data[i]     = buf->data(i);
+    frame->linesize[i] = buf->stride(i);
+  }
+
+  return 0;
+}
+
+static int c_GetVideoBuffer(AVCodecContext *s, AVFrame *frame)

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

the "c_" prefix is un-style-ish.

+{

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

opening braces go on previous line (here and elsewhere)

+  FFmpegVideoDecoder *vd = (FFmpegVideoDecoder *) s->opaque;

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

This isn't used until l.114, so it can be done there.

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

use static_cast<>() instead of c-style casts
(here and elsewhere)

+  if (!(s->flags & CODEC_FLAG_EMU_EDGE)) {

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

When will this be true?
(can you drop the "then" clause?  ditto for l.121 below).

+    return avcodec_default_get_buffer(s, frame);
+  } else {
+    return vd->GetVideoBuffer(frame);
+  }
+}
+
+static void c_ReleaseVideoBuffer(AVCodecContext *s, AVFrame *frame)
+{
+  if (!(s->flags & CODEC_FLAG_EMU_EDGE)) {
+    avcodec_default_release_buffer(s, frame);
+  } else {
+    VideoFrame *buf = (media::VideoFrame *) frame->opaque;
+    buf->Release();
+    memset(frame->data, 0, sizeof(frame->data));

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

why? (also, do you really mean to only set the first 4/8 bytes to 0?)

+    frame->opaque = NULL;
+  }
+}
+
 void FFmpegVideoDecoder::Initialize(const scoped_refptr<DemuxerStream>& stream,
                                     const PipelineStatusCB& status_cb,
                                     const StatisticsCB& statistics_cb) {
   if (!message_loop_) {
     message_loop_ = message_loop_factory_cb_.Run();
     message_loop_factory_cb_.Reset();
 
     message_loop_->PostTask(FROM_HERE, base::Bind(
         &FFmpegVideoDecoder::Initialize, this,
         stream, status_cb, statistics_cb));
@@ skipping 23 matching lines @@
 
   // Initialize AVCodecContext structure.
   codec_context_ = avcodec_alloc_context();
   VideoDecoderConfigToAVCodecContext(config, codec_context_);
 
   // Enable motion vector search (potentially slow), strong deblocking filter
   // for damaged macroblocks, and set our error detection sensitivity.
   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 = c_GetVideoBuffer;

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

Depending on ffmpeg dispatches these callbacks, it may be possible to assign to
them the result of a base::Bind() which would allow you to drop the file-static
trampolines.

+  codec_context_->release_buffer = c_ReleaseVideoBuffer;
 
   AVCodec* codec = avcodec_find_decoder(codec_context_->codec_id);
   if (!codec) {
     status_cb.Run(PIPELINE_ERROR_DECODE);
     return;
   }
 
   if (avcodec_open2(codec_context_, codec, NULL) < 0) {
     status_cb.Run(PIPELINE_ERROR_DECODE);
     return;
@@ skipping 234 matching lines @@
   // The decoder is in a bad state and not decoding correctly.
   // Checking for NULL avoids a crash in CopyPlane().
   if (!av_frame_->data[VideoFrame::kYPlane] ||
       !av_frame_->data[VideoFrame::kUPlane] ||
       !av_frame_->data[VideoFrame::kVPlane]) {
     LOG(ERROR) << "Video frame was produced yet has invalid frame data.";
     *video_frame = NULL;
     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";
-    return false;
+  if (!(codec_context_->flags & CODEC_FLAG_EMU_EDGE)) {

[Ami GONE FROM CHROMIUM, 2012/05/26 22:52:34]

Drop this test here and elsewhere?

+    // 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";
+      return false;
+    }
+  } else {
+    *video_frame = (media::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
   // follows:
   //   fps = 1 / time_base
   //
   //   duration = (1 / fps) + (repeat_pict) / (2 * fps)
   //            = (2 + repeat_pict) / (2 * fps)
   //            = (2 + repeat_pict) / (2 * (1 / time_base))
   DCHECK_LE(av_frame_->repeat_pict, 2);  // Sanity check.
   AVRational doubled_time_base;
   doubled_time_base.num = frame_rate_denominator_;
   doubled_time_base.den = frame_rate_numerator_ * 2;
 
   (*video_frame)->SetTimestamp(
       base::TimeDelta::FromMicroseconds(av_frame_->reordered_opaque));
   (*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;
+  if (!(codec_context_->flags & CODEC_FLAG_EMU_EDGE)) {
+    // 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);
   }
 
-  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;
 }
 
 void FFmpegVideoDecoder::DeliverFrame(
     const scoped_refptr<VideoFrame>& video_frame) {
   // Reset the callback before running to protect against reentrancy.
   base::ResetAndReturn(&read_cb_).Run(kOk, video_frame);
 }
 
 void FFmpegVideoDecoder::ReleaseFFmpegResources() {
@@ skipping 12 matching lines @@
 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