Begin refactor of AVDA to support zero-copy.

content/common/gpu/media/android_video_decode_accelerator.h

-// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Copyright (c) 2015 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.
 
 #ifndef CONTENT_COMMON_GPU_MEDIA_ANDROID_VIDEO_DECODE_ACCELERATOR_H_
 #define CONTENT_COMMON_GPU_MEDIA_ANDROID_VIDEO_DECODE_ACCELERATOR_H_
 
 #include <list>
 #include <map>
 #include <queue>
 #include <string>
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/threading/thread_checker.h"
 #include "base/timer/timer.h"
 #include "content/common/content_export.h"
-#include "gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 #include "media/base/android/media_codec_bridge.h"
 #include "media/video/video_decode_accelerator.h"
 
 namespace gfx {
 class SurfaceTexture;
 }
 
 namespace content {
-// A VideoDecodeAccelerator implementation for Android.
+// A base class for VideoDecodeAccelerator implementations for Android.
 // This class decodes the input encoded stream by using Android's MediaCodec
 // class. http://developer.android.com/reference/android/media/MediaCodec.html
+// It delegates attaching pictures to PictureBuffers to the client, but
+// otherwise handles the work of transferring data to / from MediaCodec.
 class CONTENT_EXPORT AndroidVideoDecodeAccelerator
     : public media::VideoDecodeAccelerator {
  public:
-  // Does not take ownership of |client| which must outlive |*this|.
-  AndroidVideoDecodeAccelerator(
+
+  // Helper class that provides the BackingStrategy with enough state
+  // to do useful work.
+  class StateProvider {

[watk, 2015/09/04 23:15:16]

I think this is mostly subjective, so take or leave this thought. I wonder if it
would be simpler to inject every dependency (or most dependencies) as parameters
of AssignCurrentSurfaceToPictureBuffer, and delete StateProvider.

* GetSize: pass it in (even if you keep StateProvider, I think this should be
passed in, because it's a per frame thing).
* ThreadChecker: seems like the strategy could create its own
* GetSurfaceTexture: pass a pointer/ref
* GetSurfaceTextureId: pass it in
* GetGlDecoder: could pass it in the constructor of the strategy
* GetMediaCodec: pass a pointer/ref
* PostError: LOG(ERROR) the error and return a media::VDA::Error from
AssignCurrentSurfaceToPictureBuffer

Then you'd also get to remove SetStateProvider, and avda_return_on_failure.h

The parameter list would be quite long, which I feel like is OK. Also, the call
site should be (fairly) readable because the arguments have good names.

[liberato (no reviews please), 2015/09/08 19:50:00]

i had a similar this argument with myself.  i decided to wait until the zero
copy version was in place to make a decision.  i originally went with the
explicit setter now because the strategy does implicitly depend on the
StateProvider staying constant.

for the copying case, it turned out to look rather like there's little stateful
connection between the AVDA and the strategy.  for the zero copy case, we'll
care about frame numbers and outstanding codec buffers and such.  depending on
where that state is held, it'll make it obvious what to do here.

+   public:
+       virtual ~StateProvider() {}
+       virtual const gfx::Size& GetSize() const = 0;
+       virtual const base::ThreadChecker& ThreadChecker() const = 0;
+       virtual gfx::SurfaceTexture* GetSurfaceTexture() const = 0;
+       virtual uint32 GetSurfaceTextureId() const = 0;
+       virtual gpu::gles2::GLES2Decoder* GetGlDecoder() const = 0;
+       virtual media::VideoCodecBridge* GetMediaCodec() = 0;
+
+       // Helper function to report an error condition and stop decodnig.
+       // This will post NotifyError(), and transition to the error state.
+       // It is meant to be called from the RETURN_ON_FAILURE macro.
+       virtual void PostError(const ::tracked_objects::Location& from_here,
+               media::VideoDecodeAccelerator::Error error) = 0;
+
+  };
+
+  // A BackingStrategy is responsible for making a PictureBuffer's texture
+  // contain the image that a MediaCodec decoder buffer tells it to.
+  class BackingStrategy : public base::RefCounted<BackingStrategy> {

[sandersd (OOO until July 31), 2015/09/04 22:12:53]

Why do backingstrategies need to be refcounted?

[liberato (no reviews please), 2015/09/08 19:50:00]

Done.

+   public:
+      virtual ~BackingStrategy(){}
+
+      // Notify about the state provider.
+      virtual void SetStateProvider(StateProvider*) = 0;
+
+      // Called before the AVDA does any Destroy() work.  This will be
+      // the last call that the BackingStrategy receives.
+      virtual void Cleanup() = 0;
+
+      // Return the number of picture buffers that we can support.
+      virtual uint32 GetNumPictureBuffers() const = 0;
+
+      // Return the GL texture target that the PictureBuffer textures use.
+      virtual uint32 GetTextureTarget() const = 0;
+
+      // Use the provided PictureBuffer to hold the current surface.
+      virtual void AssignCurrentSurfaceToPictureBuffer(int32 codec_buffer_index,
+              const media::PictureBuffer&) = 0;
+  };
+
+  // Create an AVDA using the given backing strategy.
+  static AndroidVideoDecodeAccelerator* Create(
       const base::WeakPtr<gpu::gles2::GLES2Decoder> decoder,
-      const base::Callback<bool(void)>& make_context_current);
-
-  // media::VideoDecodeAccelerator implementation.
-  bool Initialize(media::VideoCodecProfile profile, Client* client) override;
-  void Decode(const media::BitstreamBuffer& bitstream_buffer) override;
-  void AssignPictureBuffers(
-      const std::vector<media::PictureBuffer>& buffers) override;
-  void ReusePictureBuffer(int32 picture_buffer_id) override;
-  void Flush() override;
-  void Reset() override;
-  void Destroy() override;
-  bool CanDecodeOnIOThread() override;
+      const base::Callback<bool(void)>& make_context_current,
+      scoped_refptr<BackingStrategy> strategy);
 
   static media::VideoDecodeAccelerator::SupportedProfiles
       GetSupportedProfiles();
 
- private:
-  enum State {
-    NO_ERROR,
-    ERROR,
-  };
-
-  static const base::TimeDelta kDecodePollDelay;
-
-  ~AndroidVideoDecodeAccelerator() override;
-
-  // Configures |media_codec_| with the given codec parameters from the client.
-  bool ConfigureMediaCodec();
-
-  // Sends the current picture on the surface to the client.
-  void SendCurrentSurfaceToClient(int32 bitstream_id);
-
-  // Does pending IO tasks if any. Once this is called, it polls |media_codec_|
-  // until it finishes pending tasks. For the polling, |kDecodePollDelay| is
-  // used.
-  void DoIOTask();
-
-  // Feeds input data to |media_codec_|. This checks
-  // |pending_bitstream_buffers_| and queues a buffer to |media_codec_|.
-  void QueueInput();
-
-  // Dequeues output from |media_codec_| and feeds the decoded frame to the
-  // client.
-  void DequeueOutput();
-
-  // Requests picture buffers from the client.
-  void RequestPictureBuffers();
-
-  // Notifies the client about the availability of a picture.
-  void NotifyPictureReady(const media::Picture& picture);
-
-  // Notifies the client that the input buffer identifed by input_buffer_id has
-  // been processed.
-  void NotifyEndOfBitstreamBuffer(int input_buffer_id);
-
-  // Notifies the client that the decoder was flushed.
-  void NotifyFlushDone();
-
-  // Notifies the client that the decoder was reset.
-  void NotifyResetDone();
-
-  // Notifies about decoding errors.
-  void NotifyError(media::VideoDecodeAccelerator::Error error);
-
-  // Used to DCHECK that we are called on the correct thread.
-  base::ThreadChecker thread_checker_;
-
-  // To expose client callbacks from VideoDecodeAccelerator.
-  Client* client_;
-
-  // Callback to set the correct gl context.
-  base::Callback<bool(void)> make_context_current_;
-
-  // Codec type. Used when we configure media codec.
-  media::VideoCodec codec_;
-
-  // The current state of this class. For now, this is used only for setting
-  // error state.
-  State state_;
-
-  // This map maintains the picture buffers passed to the client for decoding.
-  // The key is the picture buffer id.
-  typedef std::map<int32, media::PictureBuffer> OutputBufferMap;
-  OutputBufferMap output_picture_buffers_;
-
-  // This keeps the free picture buffer ids which can be used for sending
-  // decoded frames to the client.
-  std::queue<int32> free_picture_ids_;
-
-  // Picture buffer ids which have been dismissed and not yet re-assigned.  Used
-  // to ignore ReusePictureBuffer calls that were in flight when the
-  // DismissPictureBuffer call was made.
-  std::set<int32> dismissed_picture_ids_;
-
-  // The low-level decoder which Android SDK provides.
-  scoped_ptr<media::VideoCodecBridge> media_codec_;
-
-  // A container of texture. Used to set a texture to |media_codec_|.
-  scoped_refptr<gfx::SurfaceTexture> surface_texture_;
-
-  // The texture id which is set to |surface_texture_|.
-  uint32 surface_texture_id_;
-
-  // Set to true after requesting picture buffers to the client.
-  bool picturebuffers_requested_;
-
-  // The resolution of the stream.
-  gfx::Size size_;
-
-  // Encoded bitstream buffers to be passed to media codec, queued until an
-  // input buffer is available, along with the time when they were first
-  // enqueued.
-  typedef std::queue<std::pair<media::BitstreamBuffer, base::Time> >
-      PendingBitstreamBuffers;
-  PendingBitstreamBuffers pending_bitstream_buffers_;
-
-  // Keeps track of bitstream ids notified to the client with
-  // NotifyEndOfBitstreamBuffer() before getting output from the bitstream.
-  std::list<int32> bitstreams_notified_in_advance_;
-
-  // Owner of the GL context. Used to restore the context state.
-  base::WeakPtr<gpu::gles2::GLES2Decoder> gl_decoder_;
-
-  // Used for copy the texture from |surface_texture_| to picture buffers.
-  scoped_ptr<gpu::CopyTextureCHROMIUMResourceManager> copier_;
-
-  // Repeating timer responsible for draining pending IO to the codec.
-  base::RepeatingTimer<AndroidVideoDecodeAccelerator> io_timer_;
-
-  // WeakPtrFactory for posting tasks back to |this|.
-  base::WeakPtrFactory<AndroidVideoDecodeAccelerator> weak_this_factory_;
-
   friend class AndroidVideoDecodeAcceleratorTest;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_COMMON_GPU_MEDIA_ANDROID_VIDEO_DECODE_ACCELERATOR_H_