Implement actually decoding frames in VTVideoDecodeAccelerator.

content/common/gpu/media/vt_video_decode_accelerator.cc

 // Copyright 2014 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 <CoreVideo/CoreVideo.h>
 #include <OpenGL/CGLIOSurface.h>
 
 #include "base/bind.h"
 #include "base/thread_task_runner_handle.h"
 #include "content/common/gpu/media/vt_video_decode_accelerator.h"
 #include "media/filters/h264_parser.h"
+#include "ui/gl/scoped_binders.h"
 
 using content_common_gpu_media::kModuleVt;
 using content_common_gpu_media::InitializeStubs;
 using content_common_gpu_media::IsVtInitialized;
 using content_common_gpu_media::StubPathMap;
 
 namespace content {
 
-// Size of length headers prepended to NALUs in MPEG-4 framing. (1, 2, or 4.)
+// Size of NALU length headers in AVCC/MPEG-4 format (can be 1, 2, or 4).
 static const int kNALUHeaderLength = 4;
 
+// We only request 5 picture buffers from the client which are used to hold the
+// decoded samples. These buffers are then reused when the client tells us that
+// it is done with the buffer.
+static const int kNumPictureBuffers = 5;
+
 // Route decoded frame callbacks back into the VTVideoDecodeAccelerator.
 static void OutputThunk(
     void* decompression_output_refcon,
     void* source_frame_refcon,
     OSStatus status,
     VTDecodeInfoFlags info_flags,
     CVImageBufferRef image_buffer,
     CMTime presentation_time_stamp,
     CMTime presentation_duration) {
+  // TODO(sandersd): Implement flush-before-delete to guarantee validity.
   VTVideoDecodeAccelerator* vda =
       reinterpret_cast<VTVideoDecodeAccelerator*>(decompression_output_refcon);
-  int32_t* bitstream_id_ptr = reinterpret_cast<int32_t*>(source_frame_refcon);
-  int32_t bitstream_id = *bitstream_id_ptr;
-  delete bitstream_id_ptr;
-  CFRetain(image_buffer);
-  vda->Output(bitstream_id, status, info_flags, image_buffer);
+  intptr_t bitstream_id = reinterpret_cast<intptr_t>(source_frame_refcon);

[Pawel Osciak, 2014/08/02 00:28:14]

Would it work to cast to int32_t directly?

[sandersd (OOO until July 31), 2014/08/02 01:43:13]

Done.

+  vda->Output(bitstream_id, status, image_buffer);
+}
+
+VTVideoDecodeAccelerator::DecodedFrame::DecodedFrame(
+    uint32_t bitstream_id,
+    CVImageBufferRef image_buffer)
+    : bitstream_id(bitstream_id),
+      image_buffer(image_buffer) {
+}
+
+VTVideoDecodeAccelerator::DecodedFrame::~DecodedFrame() {
 }
 
 VTVideoDecodeAccelerator::VTVideoDecodeAccelerator(CGLContextObj cgl_context)
     : cgl_context_(cgl_context),
       client_(NULL),
-      decoder_thread_("VTDecoderThread"),
       format_(NULL),
       session_(NULL),
-      weak_this_factory_(this) {
+      gpu_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      weak_this_factory_(this),
+      decoder_thread_("VTDecoderThread") {
   callback_.decompressionOutputCallback = OutputThunk;
   callback_.decompressionOutputRefCon = this;
 }
 
 VTVideoDecodeAccelerator::~VTVideoDecodeAccelerator() {
 }
 
 bool VTVideoDecodeAccelerator::Initialize(
     media::VideoCodecProfile profile,
     Client* client) {
   DCHECK(CalledOnValidThread());
   client_ = client;
 
   // Only H.264 is supported.
   if (profile < media::H264PROFILE_MIN || profile > media::H264PROFILE_MAX)
     return false;
 
   // TODO(sandersd): Move VideoToolbox library loading to sandbox startup;
   // until then, --no-sandbox is required.
   if (!IsVtInitialized()) {
-    StubPathMap paths;
     // CoreVideo is also required, but the loader stops after the first
     // path is loaded. Instead we rely on the transitive dependency from
     // VideoToolbox to CoreVideo.
     // TODO(sandersd): Fallback to PrivateFrameworks for VideoToolbox.
+    StubPathMap paths;
     paths[kModuleVt].push_back(FILE_PATH_LITERAL(
         "/System/Library/Frameworks/VideoToolbox.framework/VideoToolbox"));
     if (!InitializeStubs(paths))
       return false;
   }
 
   // Spawn a thread to handle parsing and calling VideoToolbox.
   if (!decoder_thread_.Start())
     return false;
 
   // Note that --ignore-gpu-blacklist is still required to get here.
   return true;
 }
 
 // TODO(sandersd): Proper error reporting instead of CHECKs.
 void VTVideoDecodeAccelerator::ConfigureDecoder(
     const std::vector<const uint8_t*>& nalu_data_ptrs,
     const std::vector<size_t>& nalu_data_sizes) {
+  DCHECK(decoder_thread_.message_loop_proxy()->BelongsToCurrentThread());
+  // Construct a new format description from the parameter sets.
+  // TODO(sandersd): Replace this with custom code to support OS X < 10.9.
   format_.reset();
   CHECK(!CMVideoFormatDescriptionCreateFromH264ParameterSets(
       kCFAllocatorDefault,
       nalu_data_ptrs.size(),      // parameter_set_count
       &nalu_data_ptrs.front(),    // &parameter_set_pointers
       &nalu_data_sizes.front(),   // &parameter_set_sizes
       kNALUHeaderLength,          // nal_unit_header_length
-      format_.InitializeInto()
-  ));
+      format_.InitializeInto()));
+  CMVideoDimensions coded_dimensions =
+      CMVideoFormatDescriptionGetDimensions(format_);
 
-  // TODO(sandersd): Check if the size has changed and handle picture requests.
-  CMVideoDimensions coded_size = CMVideoFormatDescriptionGetDimensions(format_);
-  coded_size_.SetSize(coded_size.width, coded_size.height);
-
+  // Prepare VideoToolbox configuration dictionaries.
   base::ScopedCFTypeRef<CFMutableDictionaryRef> decoder_config(
       CFDictionaryCreateMutable(
           kCFAllocatorDefault,
           1,  // capacity
           &kCFTypeDictionaryKeyCallBacks,
           &kCFTypeDictionaryValueCallBacks));
 
   CFDictionarySetValue(
       decoder_config,
       // kVTVideoDecoderSpecification_EnableHardwareAcceleratedVideoDecoder
       CFSTR("EnableHardwareAcceleratedVideoDecoder"),
       kCFBooleanTrue);
 
   base::ScopedCFTypeRef<CFMutableDictionaryRef> image_config(
       CFDictionaryCreateMutable(
           kCFAllocatorDefault,
           4,  // capacity
           &kCFTypeDictionaryKeyCallBacks,
           &kCFTypeDictionaryValueCallBacks));
 
-  // TODO(sandersd): ARGB for video that is not 4:2:0.
-  int32_t pixel_format = '2vuy';
 #define CFINT(i) CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &i)
+  // TODO(sandersd): RGBA option for 4:4:4 video.
+  int32_t pixel_format = kCVPixelFormatType_422YpCbCr8;
   base::ScopedCFTypeRef<CFNumberRef> cf_pixel_format(CFINT(pixel_format));
-  base::ScopedCFTypeRef<CFNumberRef> cf_width(CFINT(coded_size.width));
-  base::ScopedCFTypeRef<CFNumberRef> cf_height(CFINT(coded_size.height));
+  base::ScopedCFTypeRef<CFNumberRef> cf_width(CFINT(coded_dimensions.width));
+  base::ScopedCFTypeRef<CFNumberRef> cf_height(CFINT(coded_dimensions.height));
 #undef CFINT
   CFDictionarySetValue(
       image_config, kCVPixelBufferPixelFormatTypeKey, cf_pixel_format);
   CFDictionarySetValue(image_config, kCVPixelBufferWidthKey, cf_width);
   CFDictionarySetValue(image_config, kCVPixelBufferHeightKey, cf_height);
   CFDictionarySetValue(
       image_config, kCVPixelBufferOpenGLCompatibilityKey, kCFBooleanTrue);
 
-  // TODO(sandersd): Skip if the session is compatible.
-  // TODO(sandersd): Flush frames when resetting.
+  // TODO(sandersd): Check if the session is already compatible.
+  // TODO(sandersd): Flush.
   session_.reset();
   CHECK(!VTDecompressionSessionCreate(
       kCFAllocatorDefault,
       format_,              // video_format_description
       decoder_config,       // video_decoder_specification
       image_config,         // destination_image_buffer_attributes
       &callback_,           // output_callback
-      session_.InitializeInto()
-  ));
-  DVLOG(2) << "Created VTDecompressionSession";
+      session_.InitializeInto()));
+
+  // If the size has changed, trigger a request for new picture buffers.

[Pawel Osciak, 2014/08/02 00:28:14]

Nit: I'd CHECK(coded_size_.empty()) in case you get a resolution change
mid-stream, but up to you. Not a finished product anyway :)

[sandersd (OOO until July 31), 2014/08/02 01:43:13]

This should be the correct behavior for a mid-stream size change (except for
flushing which is not implemented yet).

[Pawel Osciak, 2014/08/03 04:26:30]

coded_size_.empty() would mean not mid-stream, but beginning of stream. So
basically CHECK that this is not a mid-stream change.

[sandersd (OOO until July 31), 2014/08/04 19:24:20]

Right, but my intention is that this code will execute at any place in the
stream, and this is the correct implementation for that. For now, it's the call
to ConfigureDecoder() that is restricted.

+  gfx::Size new_coded_size(coded_dimensions.width, coded_dimensions.height);
+  if (coded_size_ != new_coded_size) {
+    coded_size_ = new_coded_size;
+    gpu_task_runner_->PostTask(FROM_HERE, base::Bind(
+        &VTVideoDecodeAccelerator::SizeChangedTask,
+        weak_this_factory_.GetWeakPtr(),
+        coded_size_));;
+  }
 }
 
 void VTVideoDecodeAccelerator::Decode(const media::BitstreamBuffer& bitstream) {
   DCHECK(CalledOnValidThread());
   decoder_thread_.message_loop_proxy()->PostTask(FROM_HERE, base::Bind(
       &VTVideoDecodeAccelerator::DecodeTask, base::Unretained(this),
       bitstream));
 }
 
 void VTVideoDecodeAccelerator::DecodeTask(
     const media::BitstreamBuffer bitstream) {
   DCHECK(decoder_thread_.message_loop_proxy()->BelongsToCurrentThread());
 
   // Map the bitstream buffer.
   base::SharedMemory memory(bitstream.handle(), true);
   size_t size = bitstream.size();
   CHECK(memory.Map(size));
   const uint8_t* buf = static_cast<uint8_t*>(memory.memory());
 
-  // Locate relevant NALUs in the buffer.
+  // NALUs are stored with Annex B format in the bitstream buffer (start codes),
+  // but VideoToolbox expects AVCC/MPEG-4 format (length headers), so we must
+  // rewrite the data.
+  //
+  // 1. Locate relevant NALUs and compute the size of the translated data.
+  //    Also record any parameter sets for VideoToolbox initialization.
   size_t data_size = 0;
   std::vector<media::H264NALU> nalus;
   std::vector<const uint8_t*> config_nalu_data_ptrs;
   std::vector<size_t> config_nalu_data_sizes;
   parser_.SetStream(buf, size);
   media::H264NALU nalu;
   while (true) {
     media::H264Parser::Result result = parser_.AdvanceToNextNALU(&nalu);
     if (result == media::H264Parser::kEOStream)
       break;
     CHECK_EQ(result, media::H264Parser::kOk);

[Pawel Osciak, 2014/08/02 00:28:14]

In general this means that any bad stream provided by the user/from the wild can
crash GPU process. We have to handle it as an error properly, so at least please
add a TODO for this.

[sandersd (OOO until July 31), 2014/08/02 01:43:14]

Done.

+    // TODO(sandersd): Check that these are only at the start.
     if (nalu.nal_unit_type == media::H264NALU::kSPS ||
         nalu.nal_unit_type == media::H264NALU::kPPS ||
         nalu.nal_unit_type == media::H264NALU::kSPSExt) {
+      DVLOG(2) << "Parameter set " << nalu.nal_unit_type;
       config_nalu_data_ptrs.push_back(nalu.data);
       config_nalu_data_sizes.push_back(nalu.size);
+    } else {
+      nalus.push_back(nalu);
+      data_size += kNALUHeaderLength + nalu.size;
     }
-    nalus.push_back(nalu);
-    // Each NALU will have a 4-byte length header prepended.
-    data_size += kNALUHeaderLength + nalu.size;
   }
 
-  if (!config_nalu_data_ptrs.empty())
+  // 2. Initialize VideoToolbox.
+  // TODO(sandersd): Reinitialize when there are new parameter sets.
+  if (!session_)
     ConfigureDecoder(config_nalu_data_ptrs, config_nalu_data_sizes);

[Pawel Osciak, 2014/08/02 00:28:14]

What if there were no SPS and/or PPS in this bitstream buffer?

[sandersd (OOO until July 31), 2014/08/02 01:43:14]

This code is knowingly wrong in that case. The logic will be corrected at the
same time as parameter set changes.

 
-  // TODO(sandersd): Rewrite slice NALU headers and send for decoding.
+  // 3. Allocate a memory-backed CMBlockBuffer for the translated data.
+  base::ScopedCFTypeRef<CMBlockBufferRef> data;
+  CHECK(!CMBlockBufferCreateWithMemoryBlock(
+      kCFAllocatorDefault,
+      NULL,                 // &memory_block
+      data_size,            // block_length
+      kCFAllocatorDefault,  // block_allocator
+      NULL,                 // &custom_block_source
+      0,                    // offset_to_data
+      data_size,            // data_length
+      0,                    // flags
+      data.InitializeInto()));
+
+  // 4. Copy NALU data, inserting length headers.
+  size_t offset = 0;
+  for (size_t i = 0; i < nalus.size(); i++) {

[Pawel Osciak, 2014/08/02 00:28:14]

Do we require at least one NALU to be there? Maybe we need to check for that if
so...

[sandersd (OOO until July 31), 2014/08/02 01:43:14]

If we don't get any NALUs, it means something has gone wrong at a lower layer
already. I'm not sure what VT will do if we give it 0 NALUs, but either failing
or not failing is fine.

+    media::H264NALU& nalu = nalus[i];
+    uint8_t header[4] = {0xff & nalu.size >> 24,

[Pawel Osciak, 2014/08/02 00:28:14]

Please base::HostToNet32(checked_cast<uint32_t>(nalu.size)).

[sandersd (OOO until July 31), 2014/08/02 01:43:14]

I have a deep personal aversion to byte swapping, but we'll try it your way ;-)

Done.

+                         0xff & nalu.size >> 16,
+                         0xff & nalu.size >> 8,
+                         0xff & nalu.size};
+    CHECK(!CMBlockBufferReplaceDataBytes(header, data, offset, 4));

[Pawel Osciak, 2014/08/02 00:28:14]

s/4/kNALUHeaderLength/

[sandersd (OOO until July 31), 2014/08/02 01:43:13]

Done.

+    offset += 4;

[Pawel Osciak, 2014/08/02 00:28:14]

ditto

[sandersd (OOO until July 31), 2014/08/02 01:43:14]

Done.

+    CHECK(!CMBlockBufferReplaceDataBytes(nalu.data, data, offset, nalu.size));
+    offset += nalu.size;
+  }
+
+  // 5. Package the data for VideoToolbox and request decoding.
+  base::ScopedCFTypeRef<CMSampleBufferRef> frame;
+  CHECK(!CMSampleBufferCreate(
+      kCFAllocatorDefault,
+      data,                 // data_buffer
+      true,                 // data_ready
+      NULL,                 // make_data_ready_callback
+      NULL,                 // make_data_ready_refcon
+      format_,              // format_description
+      1,                    // num_samples
+      0,                    // num_sample_timing_entries
+      NULL,                 // &sample_timing_array
+      0,                    // num_sample_size_entries
+      NULL,                 // &sample_size_array
+      frame.InitializeInto()));
+
+  VTDecodeFrameFlags decode_flags =

[Pawel Osciak, 2014/08/02 00:28:14]

Please comment what this does.

[sandersd (OOO until July 31), 2014/08/02 01:43:13]

Done.

+      kVTDecodeFrame_EnableAsynchronousDecompression |
+      kVTDecodeFrame_EnableTemporalProcessing;
+
+  intptr_t bitstream_id = bitstream.id();
+  CHECK(!VTDecompressionSessionDecodeFrame(
+      session_,
+      frame,                                  // sample_buffer
+      decode_flags,                           // decode_flags
+      reinterpret_cast<void*>(bitstream_id),  // source_frame_refcon
+      NULL));                                 // &info_flags_out
 }
 
 // This method may be called on any VideoToolbox thread.
 void VTVideoDecodeAccelerator::Output(
     int32_t bitstream_id,
     OSStatus status,
-    VTDecodeInfoFlags info_flags,
     CVImageBufferRef image_buffer) {
-  // TODO(sandersd): Store the frame in a queue.
-  CFRelease(image_buffer);
+  CHECK(!status);
+  CHECK_EQ(CFGetTypeID(image_buffer), CVPixelBufferGetTypeID());
+  CFRetain(image_buffer);

[Pawel Osciak, 2014/08/02 00:28:14]

TODO for later, I guess if we get killed before this is reaches OutputTask or we
early return somewhere on the way, or before it is returned via
ReusePictureBuffer, we'll leak image_buffer? If so, we need a TODO to keep track
of those... And also clean up the lists of decoded frames properly on
destruction of this object?

By the way, did we just lose CFRelease(image_buffer) from the code? How it it
released once it's no longer used? Does CVImageBufferRef work as scoper? But
then we wouldn't use CFRetain...?

[sandersd (OOO until July 31), 2014/08/02 01:43:14]

This is the standard mode for CoreFramework APIs; they pass the object and then
decrement the refcount when the callback returns. What's going on here is that
the ScopedCFTypeRef in DecodedFrame is operating in the default mode of ASSUME,
which means that it assumes ownership without increasing the refcount.

The image will be free'd in the case that the callback is discarded, when that
ScopedCFTypeRef is deleted.

+  gpu_task_runner_->PostTask(FROM_HERE, base::Bind(
+      &VTVideoDecodeAccelerator::OutputTask,
+      weak_this_factory_.GetWeakPtr(),
+      DecodedFrame(bitstream_id, image_buffer)));
+}
+
+void VTVideoDecodeAccelerator::OutputTask(DecodedFrame frame) {
+  DCHECK(CalledOnValidThread());
+  decoded_frames_.push(frame);
+  SendPictures();
+}
+
+void VTVideoDecodeAccelerator::SizeChangedTask(gfx::Size coded_size) {

[Pawel Osciak, 2014/08/02 00:28:14]

DCHECK(CalledOnValidThread()); ?

[sandersd (OOO until July 31), 2014/08/02 01:43:14]

Done.

+  texture_size_ = coded_size;
+  // TODO(sandersd): Dismiss existing picture buffers.

[Pawel Osciak, 2014/08/02 00:28:14]

And todo hold the decode thread while we replace pictures.

[sandersd (OOO until July 31), 2014/08/02 01:43:14]

This one is going to be a rather tricky dance, but I'm not certain that
SizeChangedTask() will be involved in it directly. The core functionality will
be a condition variable that blocks inside ConfigureDecoder() until all pending
images have been sent by SendPictures().

+  client_->ProvidePictureBuffers(
+      kNumPictureBuffers, texture_size_, GL_TEXTURE_RECTANGLE_ARB);
 }
 
 void VTVideoDecodeAccelerator::AssignPictureBuffers(
     const std::vector<media::PictureBuffer>& pictures) {
   DCHECK(CalledOnValidThread());
+
+  for (size_t i = 0; i < pictures.size(); i++) {
+    available_picture_ids_.push(pictures[i].id());

[Pawel Osciak, 2014/08/02 00:28:14]

check these are empty first just in case maybe...

[sandersd (OOO until July 31), 2014/08/02 01:43:13]

Done.

+    texture_ids_[pictures[i].id()] = pictures[i].texture_id();
+  }
+
+  // Pictures are not marked as uncleared until this method returns. They will

[Pawel Osciak, 2014/08/02 00:28:14]

I don't understand this. How are they marked by the act of returning from this
method?

[sandersd (OOO until July 31), 2014/08/02 01:43:13]

https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu...

+  // become broken if they are used before that happens.
+  gpu_task_runner_->PostTask(FROM_HERE, base::Bind(
+      &VTVideoDecodeAccelerator::SendPictures,
+      weak_this_factory_.GetWeakPtr()));
 }
 
 void VTVideoDecodeAccelerator::ReusePictureBuffer(int32_t picture_id) {
   DCHECK(CalledOnValidThread());
+  DCHECK_EQ(CFGetRetainCount(picture_bindings_[picture_id]), 1);
+  picture_bindings_.erase(picture_id);
+  available_picture_ids_.push(picture_id);
+  SendPictures();
+}
+
+void VTVideoDecodeAccelerator::SendPictures() {
+  DCHECK(CalledOnValidThread());
+  if (available_picture_ids_.empty() || decoded_frames_.empty())
+    return;
+
+  CGLContextObj prev_context = CGLGetCurrentContext();

[Pawel Osciak, 2014/08/02 00:28:14]

Looks like we have a gfx::ScopedCGLSetCurrentContext, could we use it?

[sandersd (OOO until July 31), 2014/08/02 01:43:14]

Done.

+  CHECK(!CGLSetCurrentContext(cgl_context_));
+  glEnable(GL_TEXTURE_RECTANGLE_ARB);
+
+  while (!available_picture_ids_.empty() && !decoded_frames_.empty()) {
+    int32_t picture_id = available_picture_ids_.front();
+    available_picture_ids_.pop();
+    DecodedFrame frame = decoded_frames_.front();
+    decoded_frames_.pop();
+    IOSurfaceRef surface = CVPixelBufferGetIOSurface(frame.image_buffer);
+
+    gfx::ScopedTextureBinder
+        texture_binder(GL_TEXTURE_RECTANGLE_ARB, texture_ids_[picture_id]);
+    CHECK(!CGLTexImageIOSurface2D(
+        cgl_context_,                 // ctx
+        GL_TEXTURE_RECTANGLE_ARB,     // target
+        GL_RGB,                       // internal_format
+        texture_size_.width(),        // width
+        texture_size_.height(),       // height
+        GL_YCBCR_422_APPLE,           // format
+        GL_UNSIGNED_SHORT_8_8_APPLE,  // type
+        surface,                      // io_surface
+        0));                          // plane
+
+    picture_bindings_[picture_id] = frame.image_buffer;
+    client_->PictureReady(media::Picture(picture_id, frame.bitstream_id));
+    client_->NotifyEndOfBitstreamBuffer(frame.bitstream_id);
+  }
+
+  glDisable(GL_TEXTURE_RECTANGLE_ARB);
+  CHECK(!CGLSetCurrentContext(prev_context));
 }
 
 void VTVideoDecodeAccelerator::Flush() {
   DCHECK(CalledOnValidThread());
   // TODO(sandersd): Trigger flush, sending frames.
 }
 
 void VTVideoDecodeAccelerator::Reset() {
   DCHECK(CalledOnValidThread());
   // TODO(sandersd): Trigger flush, discarding frames.
 }
 
 void VTVideoDecodeAccelerator::Destroy() {
   DCHECK(CalledOnValidThread());
   // TODO(sandersd): Trigger flush, discarding frames, and wait for them.
   delete this;
 }
 
 bool VTVideoDecodeAccelerator::CanDecodeOnIOThread() {
   return false;
 }
 
 }  // namespace content