Implement actually decoding frames in VTVideoDecodeAccelerator.

content/common/gpu/media/vt_video_decode_accelerator.h

Index: content/common/gpu/media/vt_video_decode_accelerator.h
diff --git a/content/common/gpu/media/vt_video_decode_accelerator.h b/content/common/gpu/media/vt_video_decode_accelerator.h
index 08dd8ab8454ca08a1b6fbaa48133643087aa874e..1885fae187ed294b9a92e5c2b258415362a660cb 100644
--- a/content/common/gpu/media/vt_video_decode_accelerator.h
+++ b/content/common/gpu/media/vt_video_decode_accelerator.h
@@ -5,11 +5,16 @@
 #ifndef CONTENT_COMMON_GPU_MEDIA_VT_VIDEO_DECODE_ACCELERATOR_H_
 #define CONTENT_COMMON_GPU_MEDIA_VT_VIDEO_DECODE_ACCELERATOR_H_
 
+#include <map>
+#include <queue>
+
 #include "base/basictypes.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/synchronization/lock.h"
 #include "base/threading/thread.h"
 #include "content/common/gpu/media/vt.h"
 #include "media/filters/h264_parser.h"
@@ -45,38 +50,69 @@ class VTVideoDecodeAccelerator
   virtual void Destroy() OVERRIDE;
   virtual bool CanDecodeOnIOThread() OVERRIDE;
 
-  // Called by VideoToolbox when a frame is decoded.
+  // Called by OutputThunk when VideoToolbox finishes decoding a frame.

[scherkus (not reviewing), 2014/07/17 21:17:26]

nit: we have a convention of adding a "()" suffix to function names referenced
in comments (similar to how we surround variables names with |'s -- makes it
easier to scan and disambiguates functions from types/classes)

[scherkus (not reviewing), 2014/07/17 23:32:51]

FYI not done

[sandersd (OOO until July 31), 2014/07/18 01:05:15]

Done.

   void Output(
       int32_t bitstream_id,
       OSStatus status,
-      VTDecodeInfoFlags info_flags,
       CVImageBufferRef image_buffer);
 
  private:
-  // Configure a VideoToolbox decompression session from parameter set NALUs.
+  struct DecodedFrame {
+    DecodedFrame(uint32_t bitstream_id, CVImageBufferRef image_buffer);
+    ~DecodedFrame();
+
+    int32_t bitstream_id;
+    base::ScopedCFTypeRef<CVImageBufferRef> image_buffer;
+  };
+
+  // Methods for interacting with VideoToolbox. Run on |decoded_thread_|.

[scherkus (not reviewing), 2014/07/17 21:17:26]

typo: decoded -> decoder

[scherkus (not reviewing), 2014/07/17 23:32:51]

FYI not done

[sandersd (OOO until July 31), 2014/07/18 01:05:15]

Done.

   void ConfigureDecoder(
       const std::vector<const uint8_t*>& nalu_data_ptrs,
       const std::vector<size_t>& nalu_data_sizes);
-
-  // Decode a frame of bitstream.
   void DecodeTask(const media::BitstreamBuffer);
 
+  // Methods for interacting with |client_|. Run on |gpu_task_runner_|.
+  void OutputTask(DecodedFrame frame);
+  void SendPictures();
+
+  //
+  // GPU thread state.
+  //
   CGLContextObj cgl_context_;
   media::VideoDecodeAccelerator::Client* client_;
-  base::Thread decoder_thread_;
+  // Texture IDs of pictures.
+  std::map<int32_t, int32_t> texture_ids_;
+  // Pictures ready to be rendered to.
+  std::queue<int32_t> picture_ids_;
+  // Decoded frames ready to render.
+  std::queue<DecodedFrame> decoded_frames_;
+  // Image buffers retained while they are bound to textures.
+  std::map<int32_t, base::ScopedCFTypeRef<CVImageBufferRef>> picture_bindings_;

[scherkus (not reviewing), 2014/07/17 21:17:26]

interesting fact: apparently because we compile with -std=gnu++11 we no longer
need spaces between nested template arguments ">>"

hooray!

[sandersd (OOO until July 31), 2014/07/18 01:05:15]

Acknowledged.

 
-  // Decoder configuration (used only on decoder thread).
+  //
+  // Decoder thread state.
+  //
   VTDecompressionOutputCallbackRecord callback_;
   base::ScopedCFTypeRef<CMFormatDescriptionRef> format_;
   base::ScopedCFTypeRef<VTDecompressionSessionRef> session_;
   media::H264Parser parser_;
   gfx::Size coded_size_;
 
-  // Member variables should appear before the WeakPtrFactory, to ensure
-  // that any WeakPtrs to Controller are invalidated before its members
-  // variable's destructors are executed, rendering them invalid.
+  //
+  // Unprotected shared state (set up and torn down on GPU thread).
+  //
+  scoped_refptr<base::SingleThreadTaskRunner> gpu_task_runner_;
+  scoped_ptr<base::WeakPtrFactory<media::VideoDecodeAccelerator::Client>>
+      weak_client_factory_;
+
+  // Declared second-to-last to prevent any GPU thread tasks from executing once
+  // state starts being destroyed.

[scherkus (not reviewing), 2014/07/17 21:17:26]

the old comment was more accurate

the situation you describe here is impossible (the dtor executes on the GPU
thread so no other tasks can be running until the current task that triggered
destruction yields back to the message loop. at that point all WeakPtrs have
been invalidated and any pending tasks referencing said WeakPtrs will no-op)

[scherkus (not reviewing), 2014/07/17 23:32:51]

FYI not done

[sandersd (OOO until July 31), 2014/07/18 01:05:15]

Ah yes, now that this is correctly used from only the GPU thread, the
destruction order doesn't actually matter anymore (as long as it is destructed
after |decoder_thread_|).

Done.

   base::WeakPtrFactory<VTVideoDecodeAccelerator> weak_this_factory_;
 
+  // Declared last to ensure that all decoder thread tasks complete before any
+  // state starts being destroyed.
+  base::Thread decoder_thread_;
+
   DISALLOW_COPY_AND_ASSIGN(VTVideoDecodeAccelerator);
 };