Collect VTVideoDecodeAccelerator frames into a work queue

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 698953be7cfc2532873ba49af7522062232c28bf..caae0fd7427d93346a8d8c61cf74d3422c2bd1c0 100644
--- a/content/common/gpu/media/vt_video_decode_accelerator.h
+++ b/content/common/gpu/media/vt_video_decode_accelerator.h
@@ -11,7 +11,7 @@
 #include <queue>
 
 #include "base/mac/scoped_cftyperef.h"
-#include "base/memory/ref_counted.h"
+#include "base/memory/linked_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/threading/thread.h"
@@ -51,85 +51,76 @@ class VTVideoDecodeAccelerator
 
   // Called by OutputThunk() when VideoToolbox finishes decoding a frame.
   void Output(
-      int32_t bitstream_id,
+      void* source_frame_refcon,
       OSStatus status,
       CVImageBufferRef image_buffer);
 
  private:
-  struct DecodedFrame {
-    DecodedFrame(int32_t bitstream_id, CVImageBufferRef image_buffer);
-    ~DecodedFrame();
-
-    int32_t bitstream_id;
-    base::ScopedCFTypeRef<CVImageBufferRef> image_buffer;
+  enum State {
+    STATE_NORMAL,
+    STATE_DESTROYING,
+    STATE_ERROR,
   };
 
-  // Actions are the possible types of pending operations, which are queued
-  // by Flush(), Reset(), and Destroy().
-  enum Action {
-    ACTION_FLUSH,
-    ACTION_RESET,
-    ACTION_DESTROY
+  enum TaskType {
+    TASK_FRAME,
+    TASK_FLUSH,
+    TASK_RESET,
+    TASK_DESTROY,
   };
 
-  // PendingActions contain the |bitstream_id| of a frame that, once decoded and
-  // sent, a particular |action| should be completed at.
-  struct PendingAction {
-    PendingAction(Action action, int32_t bitstream_id);
-    ~PendingAction();
+  struct Frame {
+    Frame(int32_t bitstream_id);
+    ~Frame();
 
-    Action action;
     int32_t bitstream_id;
+    base::ScopedCFTypeRef<CVImageBufferRef> image;
+    // TODO(sandersd): visible_rect.
+    gfx::Size coded_size;
+  };
+
+  struct Task {
+    Task(TaskType type);
+    ~Task();
+
+    TaskType type;
+    linked_ptr<Frame> frame;
   };
 
+  //
   // Methods for interacting with VideoToolbox. Run on |decoder_thread_|.
-  bool ConfigureDecoder(
-      const std::vector<const uint8_t*>& nalu_data_ptrs,
-      const std::vector<size_t>& nalu_data_sizes);
-  void DecodeTask(const media::BitstreamBuffer&);
-  void FlushTask();
-  void DropBitstream(int32_t bitstream_id);
+  //
+  // Set up VideoToolbox using the current SPS and PPS. Returns true or calls
+  // NotifyError() before returning false.
+  bool ConfigureDecoder();
+
+  // Wait for VideoToolbox to output all pending frames. Returns true or calls
+  // NotifyError() before returning false.
+  bool FinishDelayedFrames();
 
+  // |frame| is owned by |pending_frames_|.
+  void DecodeTask(const media::BitstreamBuffer&, Frame* frame);
+  void DecodeDone(Frame* frame);
+
+  //
   // Methods for interacting with |client_|. Run on |gpu_task_runner_|.
-  void OutputTask(DecodedFrame frame);
+  //
   void NotifyError(Error error);
 
-  // Send decoded frames up to and including |up_to_bitstream_id|, and return
-  // the last sent |bitstream_id|.
-  int32_t SendPictures(int32_t up_to_bitstream_id);
-
-  // Internal helper for SendPictures(): Drop frames with no image data up to
-  // a particular bitstream ID, so that if there is still a frame in the queue
-  // when this function returns, it is guaranteed to have image data, and thus
-  // it is time to set up the GPU context. Returns the last bitstream ID that
-  // was dropped, or |last_sent_bitstream_id| if no frames were dropped.
-  int32_t ProcessDroppedFrames(
-      int32_t last_sent_bitstream_id,
-      int32_t up_to_bitstream_id);
-
-  // Internal helper for SendPictures(): Check if the next frame has a size
-  // different from the current picture buffers, and request new ones if so.
-  void ProcessSizeChange();
-
-  // Since VideoToolbox has no reset feature (only flush), and the VDA API
-  // allows Decode() and Flush() calls during a reset operation, it's possible
-  // to have multiple pending actions at once. We handle the fully general case
-  // of an arbitrary sequence of pending actions (in reality, there should
-  // probably be at most one reset and one flush at a time).
-  void QueueAction(Action action);
-
-  // Process queued decoded frames, usually by sending them (unless there
-  // is a pending ACTION_RESET or ACTION_DESTROY, in which case they are
-  // dropped), completing queued actions along the way.
-  void ProcessDecodedFrames();
-
-  // Complete a particular action, by eg. calling NotifyFlushDone().
-  // Warning: Deletes |this| if |action| is ACTION_DESTROY.
-  void CompleteAction(Action action);
-
-  // Complete all actions pending for a particular |bitstream_id|.
-  // Warning: Do not call if there is a pending ACTION_DESTROY.
-  void CompleteActions(int32_t bitstream_id);
+  // |type| is the type of task that the flush will complete, one of TASK_FLUSH,
+  // TASK_RESET, or TASK_DESTROY.
+  void QueueFlush(TaskType type);
+  void FlushTask(TaskType type);
+  void FlushDone(TaskType type);
+
+  // Attempt to make progress on |pending_tasks_|.
+  void ProcessTasks();
+
+  // These methods returns true if the task was completed, false if it couldn't
+  // be completed yet.
+  bool ProcessTask(const Task& task);
+  bool ProcessFrame(const Frame& frame);
+  bool SendFrame(const Frame& frame);
 
   //
   // GPU thread state.
@@ -137,21 +128,24 @@ class VTVideoDecodeAccelerator
   CGLContextObj cgl_context_;
   base::Callback<bool(void)> make_context_current_;
   media::VideoDecodeAccelerator::Client* client_;
+  State state_;
+
+  // Queue of pending flush tasks. This is used to drop frames when a reset
+  // is pending.
+  std::queue<TaskType> pending_flush_tasks_;
 
-  // client_->NotifyError() called.
-  bool has_error_;
+  // Queue of tasks to complete in the GPU thread.
+  std::queue<Task> pending_tasks_;
 
   // Size of assigned picture buffers.
   gfx::Size picture_size_;
 
-  // Queue of actions so that we can quickly discover what the next action will
-  // be; this is useful because we are dropping all frames when the next action
-  // is ACTION_RESET or ACTION_DESTROY.
-  std::queue<PendingAction> pending_actions_;
+  // Queue of frames that have not yet been decoded; maintains ownership of the
+  // Frame objects while they flow through VideoToolbox.
+  std::queue<linked_ptr<Frame>> pending_frames_;

[DaleCurtis, 2014/11/10 21:10:39]

Why queue vs vector ==> ScopedVector instead of linked_ptr?

[sandersd (OOO until July 31), 2014/11/11 18:46:07]

The short answer is that it makes moving ownership from this queue to
|pending_tasks_| simpler. With a scoped_ptr, the task queue items would also
need to be wrapped in smart pointers, so not only are the queue operations
inefficient, there is also a lot of unnecessary boxing.

That said I'm not extremely happy with the ownership story either way, the dance
to pop items of this queue is not a good sign.

 
-  // Queue of bitstreams that have not yet been decoded. This is mostly needed
-  // to be sure we free them all in Destroy().
-  std::queue<int32_t> pending_bitstream_ids_;
+  // Set of assigned bitstream IDs, so that Destroy() can release them all.
+  std::set<int32_t> assigned_bitstream_ids_;
 
   // All picture buffers assigned to us. Used to check if reused picture buffers
   // should be added back to the available list or released. (They are not
@@ -164,9 +158,6 @@ class VTVideoDecodeAccelerator
   // Pictures ready to be rendered to.
   std::vector<int32_t> available_picture_ids_;
 
-  // Decoded frames ready to render.
-  std::queue<DecodedFrame> decoded_frames_;
-
   // Image buffers kept alive while they are bound to pictures.
   std::map<int32_t, base::ScopedCFTypeRef<CVImageBufferRef>> picture_bindings_;
 
@@ -177,6 +168,7 @@ class VTVideoDecodeAccelerator
   base::ScopedCFTypeRef<CMFormatDescriptionRef> format_;
   base::ScopedCFTypeRef<VTDecompressionSessionRef> session_;
   media::H264Parser parser_;
+  gfx::Size coded_size_;
 
   std::vector<uint8_t> last_sps_;
   std::vector<uint8_t> last_spsext_;