Implement actually decoding frames in VTVideoDecodeAccelerator.

content/common/gpu/media/vt_video_decode_accelerator.cc

Index: content/common/gpu/media/vt_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/vt_video_decode_accelerator.cc b/content/common/gpu/media/vt_video_decode_accelerator.cc
index 7745e6dfb999aacc259f50033d856d2aded7d28a..02e4bcc2b8d802973e79101efe4d725423749aea 100644
--- a/content/common/gpu/media/vt_video_decode_accelerator.cc
+++ b/content/common/gpu/media/vt_video_decode_accelerator.cc
@@ -17,9 +17,14 @@ 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,
@@ -29,21 +34,21 @@ static void OutputThunk(
     CVImageBufferRef image_buffer,
     CMTime presentation_time_stamp,
     CMTime presentation_duration) {
+  // Will be guaranteed to be valid once flush before delete is implemented.

[scherkus (not reviewing), 2014/07/16 18:18:51]

nit: this reads more like a TODO -- is that the case?

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

Done.

   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;
+  intptr_t bitstream_id = reinterpret_cast<intptr_t>(source_frame_refcon);
   CFRetain(image_buffer);

[scherkus (not reviewing), 2014/07/16 18:18:51]

does this call need to be balanced with a CFRelease()/

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

No, that is handled by the destruction of what is now called picture_bindings_
(was retained_images_). I've moved this call to VTVDA::Output and tried to make
it more clear by using a ScopedCFTypeRef.

   vda->Output(bitstream_id, status, info_flags, image_buffer);
 }
 
 VTVideoDecodeAccelerator::VTVideoDecodeAccelerator(CGLContextObj cgl_context)
     : cgl_context_(cgl_context),
-      client_(NULL),
-      decoder_thread_("VTDecoderThread"),
       format_(NULL),
       session_(NULL),
+      frames_pending_decode_(0),

[scherkus (not reviewing), 2014/07/16 18:18:51]

while you increment/decrement this variable, I don't see it being actually used
anywhere

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

It will be used to determine when a request to flush has completed.

+      gpu_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      decoder_thread_("VTDecoderThread"),
       weak_this_factory_(this) {
   callback_.decompressionOutputCallback = OutputThunk;
   callback_.decompressionOutputRefCon = this;
@@ -56,7 +61,9 @@ bool VTVideoDecodeAccelerator::Initialize(
     media::VideoCodecProfile profile,
     Client* client) {
   DCHECK(CalledOnValidThread());
-  client_ = client;
+
+  weak_client_factory_.reset(
+      new base::WeakPtrFactory<media::VideoDecodeAccelerator::Client>(client));
 
   // Only H.264 is supported.
   if (profile < media::H264PROFILE_MIN || profile > media::H264PROFILE_MAX)
@@ -88,6 +95,8 @@ bool VTVideoDecodeAccelerator::Initialize(
 void VTVideoDecodeAccelerator::ConfigureDecoder(
     const std::vector<const uint8_t*>& nalu_data_ptrs,
     const std::vector<size_t>& nalu_data_sizes) {

[scherkus (not reviewing), 2014/07/16 18:18:51]

add DCHECK(decoder_thread_.message_loop_proxy()->BelongsToCurrentThread());

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

Done.

+  // 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,
@@ -95,13 +104,11 @@ void VTVideoDecodeAccelerator::ConfigureDecoder(
       &nalu_data_ptrs.front(),    // &parameter_set_pointers
       &nalu_data_sizes.front(),   // &parameter_set_sizes
       kNALUHeaderLength,          // nal_unit_header_length
-      format_.InitializeInto()
-  ));
-
-  // 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);
+      format_.InitializeInto()));
+  CMVideoDimensions coded_dimensions =
+      CMVideoFormatDescriptionGetDimensions(format_);
 
+  // Prepare VideoToolbox configuration dictionaries.
   base::ScopedCFTypeRef<CFMutableDictionaryRef> decoder_config(
       CFDictionaryCreateMutable(
           kCFAllocatorDefault,
@@ -126,8 +133,8 @@ void VTVideoDecodeAccelerator::ConfigureDecoder(
   int32_t pixel_format = '2vuy';

[scherkus (not reviewing), 2014/07/16 18:18:51]

this is used in multiple places -- can you make this a constant at the top of
this file?

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

Done.

 #define CFINT(i) CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &i)
   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);
@@ -136,8 +143,8 @@ void VTVideoDecodeAccelerator::ConfigureDecoder(
   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,
@@ -145,9 +152,20 @@ void VTVideoDecodeAccelerator::ConfigureDecoder(
       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, request new picture buffers.
+  gfx::Size new_coded_size(coded_dimensions.width, coded_dimensions.height);
+  if (coded_size_ != new_coded_size) {
+    coded_size_ = new_coded_size;
+    // TODO(sandersd): Dismiss existing picture buffers.
+    gpu_task_runner_->PostTask(FROM_HERE, base::Bind(
+        &media::VideoDecodeAccelerator::Client::ProvidePictureBuffers,
+        weak_client_factory_->GetWeakPtr(),
+        kNumPictureBuffers,
+        coded_size_,
+        GL_TEXTURE_RECTANGLE_ARB));
+  }
 }
 
 void VTVideoDecodeAccelerator::Decode(const media::BitstreamBuffer& bitstream) {
@@ -167,7 +185,12 @@ void VTVideoDecodeAccelerator::DecodeTask(
   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 (3-byte start
+  // codes), but VideoToolbox expects AVCC/MPEG-4 format (length headers), so we
+  // must to 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;
@@ -179,21 +202,81 @@ void VTVideoDecodeAccelerator::DecodeTask(
     if (result == media::H264Parser::kEOStream)
       break;
     CHECK_EQ(result, media::H264Parser::kOk);
+    // 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);
 
-  // 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++) {
+    media::H264NALU& nalu = nalus[i];
+    uint8_t header[4] = {0xff & nalu.size >> 24,
+                         0xff & nalu.size >> 16,
+                         0xff & nalu.size >> 8,
+                         0xff & nalu.size};
+    CHECK(!CMBlockBufferReplaceDataBytes(header, data, offset, 4));
+    offset += 4;
+    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()));
+
+  // kVTDecodeFrame_EnableAsynchronousDecompression |

[scherkus (not reviewing), 2014/07/16 18:18:51]

does it make send to define these somewhere?

perhaps in the fragment header file along w/ the other forward declarations?

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

Done.

+  // kVTDecodeFrame_EnableTemporalProcessing
+  VTDecodeFrameFlags decode_flags = (1<<0) | (1<<3);

[scherkus (not reviewing), 2014/07/16 18:18:51]

spaces around binary operators

(1 << 0) | (1 << 3)

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

Done.

+
+  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
+
+  base::AutoLock lock(lock_);
+  frames_pending_decode_++;
 }
 
 // This method may be called on any VideoToolbox thread.
@@ -202,17 +285,90 @@ void VTVideoDecodeAccelerator::Output(
     OSStatus status,
     VTDecodeInfoFlags info_flags,
     CVImageBufferRef image_buffer) {
-  // TODO(sandersd): Store the frame in a queue.
-  CFRelease(image_buffer);
+  CHECK(!status);
+  CHECK(CFGetTypeID(image_buffer) == CVPixelBufferGetTypeID());
+  CHECK(CVPixelBufferGetPixelFormatType(image_buffer) == '2vuy');
+  {
+    base::AutoLock lock(lock_);
+    decoded_frames_.push(DecodedFrame(bitstream_id, image_buffer));

[scherkus (not reviewing), 2014/07/16 18:18:51]

do we need locking around decoded_frames_?

alternatively we can include the DecodedFrame in the posted task to
SendPictures() and avoid locking for the entirety of that method


relating to my comment above about frames_pending_decode_ ... if we don't need
that either we can remove all locking

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

I've implemented a task-posting version of this. It's much more refcount-heavy,
but it does avoid doing anything expensive inside a lock. Probably a much better
plan where the OpenGL API is involved.

Most of the excess refcounting can be removed through clever use of references
and maybe a little bit of C++11, if necessary.

+    frames_pending_decode_--;
+  }
+  gpu_task_runner_->PostTask(FROM_HERE, base::Bind(
+      &VTVideoDecodeAccelerator::SendPictures,
+      weak_this_factory_.GetWeakPtr()));
 }
 
 void VTVideoDecodeAccelerator::AssignPictureBuffers(
     const std::vector<media::PictureBuffer>& pictures) {
   DCHECK(CalledOnValidThread());
+
+  for (size_t i = 0; i < pictures.size(); i++) {
+    picture_ids_.push(pictures[i].id());
+    texture_ids_[pictures[i].id()] = pictures[i].texture_id();
+  }
+
+  // Pictures are not marked as uncleared until this method returns. They will
+  // 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());
+
+  retained_images_.erase(picture_id);
+  picture_ids_.push(picture_id);
+
+  gpu_task_runner_->PostTask(FROM_HERE, base::Bind(
+      &VTVideoDecodeAccelerator::SendPictures,
+      weak_this_factory_.GetWeakPtr()));
+}
+
+void VTVideoDecodeAccelerator::SendPictures() {
+  DCHECK(CalledOnValidThread());
+  base::AutoLock lock(lock_);
+  if (!picture_ids_.empty() && !decoded_frames_.empty()) {
+    CGLContextObj prev_context = CGLGetCurrentContext();
+    CHECK(!CGLSetCurrentContext(cgl_context_));
+    glEnable(GL_TEXTURE_RECTANGLE_ARB);
+
+    while (!picture_ids_.empty() && !decoded_frames_.empty()) {
+      DecodedFrame frame = decoded_frames_.front(); decoded_frames_.pop();

[scherkus (not reviewing), 2014/07/16 18:18:51]

put the pop() calls on separate lines

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

Done.

+      IOSurfaceRef surface = CVPixelBufferGetIOSurface(frame.image_buffer);
+      int32_t picture_id = picture_ids_.front(); picture_ids_.pop();

[scherkus (not reviewing), 2014/07/16 18:18:51]

ditto

[sandersd (OOO until July 31), 2014/07/17 00:22:22]

Done.

+      int32_t texture_id = texture_ids_[picture_id];
+
+      glBindTexture(GL_TEXTURE_RECTANGLE_ARB, texture_id);
+      CHECK(!CGLTexImageIOSurface2D(
+          cgl_context_,                 // ctx
+          GL_TEXTURE_RECTANGLE_ARB,     // target
+          GL_RGB,                       // internal_format
+          coded_size_.width(),          // width
+          coded_size_.height(),         // height
+          GL_YCBCR_422_APPLE,           // format
+          GL_UNSIGNED_SHORT_8_8_APPLE,  // type
+          surface,                      // io_surface
+          0));                          // plane
+      glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0);
+
+      // The image buffer must be retained as it now backs the texture.
+      retained_images_[picture_id].reset(frame.image_buffer);
+
+      gpu_task_runner_->PostTask(FROM_HERE, base::Bind(
+          &media::VideoDecodeAccelerator::Client::PictureReady,
+          weak_client_factory_->GetWeakPtr(),
+          media::Picture(picture_id, frame.bitstream_id)));
+
+      gpu_task_runner_->PostTask(FROM_HERE, base::Bind(
+          &media::VideoDecodeAccelerator::Client::NotifyEndOfBitstreamBuffer,
+          weak_client_factory_->GetWeakPtr(),
+          frame.bitstream_id));
+    }
+
+    glDisable(GL_TEXTURE_RECTANGLE_ARB);
+    CHECK(!CGLSetCurrentContext(prev_context));
+  }
 }
 
 void VTVideoDecodeAccelerator::Flush() {
@@ -228,6 +384,7 @@ void VTVideoDecodeAccelerator::Reset() {
 void VTVideoDecodeAccelerator::Destroy() {
   DCHECK(CalledOnValidThread());
   // TODO(sandersd): Trigger flush, discarding frames, and wait for them.
+  // TODO(sandersd): Make sure decoded_frames_ is empty.
   delete this;
 }