Implement software fallback for PPB_VideoDecoder.

content/renderer/pepper/video_decoder_adapter.h

+// Copyright (c) 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.
+
+#ifndef CONTENT_RENDERER_PEPPER_VIDEO_DECODER_ADAPTER_H_
+#define CONTENT_RENDERER_PEPPER_VIDEO_DECODER_ADAPTER_H_
+
+#include <queue>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/containers/hash_tables.h"
+#include "base/memory/linked_ptr.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop/message_loop_proxy.h"
+#include "gpu/command_buffer/common/mailbox.h"
+#include "media/base/decoder_buffer.h"

[dmichael (off chromium), 2014/06/06 17:24:01]

nit: This could probably be a forward reference

[bbudge, 2014/06/07 00:31:09]

Done.

+#include "media/base/video_decoder.h"
+#include "media/base/video_decoder_config.h"

[dmichael (off chromium), 2014/06/06 17:24:00]

nit: ditto (I think even by-value params can be forward declared)

[bbudge, 2014/06/07 00:31:09]

Can't, it's an enum. I was able to fwd decl VideoDecoder though.

+
+#include "ppapi/c/pp_codecs.h"
+
+namespace gpu {
+namespace gles2 {
+class GLES2Interface;
+}
+}
+
+namespace webkit {
+namespace gpu {
+class ContextProviderWebContext;
+}
+}
+
+namespace content {
+
+class PepperVideoDecoderHost;
+
+// This class proxies calls to a media::VideoDecoder on the media thread.
+// Instances should be constructed, used, and destructed on the main (render)
+// thread.
+class VideoDecoderAdapter {

[Ami GONE FROM CHROMIUM, 2014/06/06 17:14:34]

Pro-tip for the future: if you address all the comments on a file
_before_ renaming it, then upload a second patchset to rietveld, then do
the rename and upload a third patchset, then your reviewer can
effectively review comments & your responses using the side-by-side
PS#1->PS#2 view, and take on faith that the only change in PS#2->PS#3 is
the rename.  When you upload a single PS#2 that contains both all your
responses/edits and the rename, rietveld provides no view that lets the
reviewer see a side-by-side view of PS#1 (with comments!) comparing to
PS#2 :(

[Ami GONE FROM CHROMIUM, 2014/06/06 17:14:34]

I'll let you & dmichael make a final call here but IMO VideoDecodeAdapter is a
particularly unfortunate choice: 
- It appreviates to VDA which is already a term-of-art in this precise area of
the codebase
- It implies that a VideoDecoder will be passed in, to be adapted (which is
untrue since the VD is new'd as part of the impl internally)
- It doesn't say what the VD is being Adapter'd _to_.

If it was up to me I think I'd name this something more obviously
double-take-inducing, since there are only ever going to be few call-sites and
the thing that this class does is atypical.

VideoDecoderBasedPseudoVDA
VDALikeVideoDecoderWrapper
etc.

Like I said, up to you & dmichael.
(and moot if you're able to take my suggestion below to make this a true VDA)

[bbudge, 2014/06/07 00:31:09]

OK, how about VideoDecoderShim?

[bbudge, 2014/06/07 00:31:09]

Sorry, I didn't realize that. Thanks for the pro-tip.

+ public:
+  explicit VideoDecoderAdapter(PepperVideoDecoderHost* host);

[Ami GONE FROM CHROMIUM, 2014/06/06 17:14:34]

Why?  Isn't it as simple as 
SharedMemory shm(handle, true);
...use shm->Map()'s return value?
Just because _this_ class implements VDA doesn't mean that it is restricted to
VDA::Client's interface for talking back to the host.

[bbudge, 2014/06/07 00:31:09]

The adapter class now implements media::VDA.
Done.

+
+  void Initialize(media::VideoCodecProfile profile);
+  void Decode(uint32_t decode_id, const uint8_t* buffer, uint32_t size);
+  void AssignTextures(const std::vector<uint32_t>& texture_ids);
+  void RecycleTexture(uint32_t texture_id);
+  void Flush();
+  void Reset();
+  void Destroy();
+
+ private:
+  enum State {
+    UNINITIALIZED,
+    DECODING,
+    FLUSHING,
+    RESETTING,
+    DESTROYED,
+  };
+
+  struct PendingDecode;
+  struct PendingFrame;
+
+  // This class is constructed on the main (render) thread, but used and
+  // destructed on the media thread.

[Ami GONE FROM CHROMIUM, 2014/06/06 17:14:34]

Comment should also say what this is for.

[bbudge, 2014/06/07 00:31:10]

Done.

+  class Delegate {

[Ami GONE FROM CHROMIUM, 2014/06/06 17:14:34]

fwd-declare?

[Ami GONE FROM CHROMIUM, 2014/06/06 17:14:34]

Delegate is used in chrome as a synonym for "Client" and is almost always an
interface.  This seems like an impl detail whose raison d'etre is the need to
trampoline VDAdapter calls from the main thread to the media thread to do the
real work there, and then trampoline the results back, maybe?

If so rename to Impl?

[bbudge, 2014/06/07 00:31:09]

Renamed to VideoDecoderShim::DecoderImpl.

[bbudge, 2014/06/07 00:31:10]

Done.

+   public:
+    explicit Delegate(const base::WeakPtr<VideoDecoderAdapter>& proxy);
+    ~Delegate();
+
+    void Initialize(media::VideoDecoderConfig config);
+    void Decode(uint32_t decode_id, scoped_refptr<media::DecoderBuffer> buffer);
+    void Reset();
+    void Stop();
+
+   private:
+    void OnPipelineStatus(media::PipelineStatus status);
+    void OnDecodeComplete(uint32_t decode_id,
+                          media::VideoDecoder::Status status,
+                          const scoped_refptr<media::VideoFrame>& frame);
+    void OnResetComplete();
+
+    base::WeakPtr<VideoDecoderAdapter> adapter_;

[Ami GONE FROM CHROMIUM, 2014/06/06 17:14:33]

My comment is about the weakptr's thread-binding, not FFVD's.
WeakPtrs can only be dereferenced & invalidated on a single thread, and in that
sense they are bound to that thread.  I find it helpful to document which thread
a weakptr member is bound to.

[dmichael (off chromium), 2014/06/06 17:24:01]

I think Ami's prior comment here was that you should document that adapter_ is
bound to the main thread. In other words, you should never de-reference it on
the media thread. It should only be used to bind callbacks for invoking on the
main thread.

[bbudge, 2014/06/07 00:31:09]

Done.

+    scoped_ptr<media::VideoDecoder> decoder_;
+    scoped_refptr<base::MessageLoopProxy> main_message_loop_;
+    // Queue of decodes waiting for the decoder.
+    typedef std::queue<PendingDecode> PendingDecodeQueue;
+    PendingDecodeQueue pending_decodes_;
+    int max_pending_decodes_;
+    int num_pending_decodes_;
+  };
+
+  // Do not delete directly; use Destroy() or own it with a scoped_ptr, which
+  // will Destroy() it properly by default.
+  ~VideoDecoderAdapter();
+
+  void OnPipelineStatus(media::PipelineStatus status);
+  void OnDecodeComplete(media::VideoDecoder::Status status,
+                        scoped_ptr<PendingFrame> frame);
+  void SendPictures();
+  void OnResetComplete();
+  void DismissTexture(uint32_t texture_id);
+  void DeleteTexture(uint32_t texture_id);
+  // Call this whenever we change GL state that the plugin relies on, such as
+  // creating picture textures.
+  void FlushCommandBuffer();
+
+  scoped_ptr<Delegate> delegate_;
+  State state_;
+
+  PepperVideoDecoderHost* host_;
+  scoped_refptr<base::MessageLoopProxy> media_message_loop_;
+  scoped_refptr<webkit::gpu::ContextProviderWebContext> context_provider_;
+
+  // The current decoded frame size.
+  gfx::Size texture_size_;
+  // Map that takes the plugin's GL texture id to the renderer's GL texture id.
+  typedef base::hash_map<uint32_t, uint32_t> TextureIdMap;
+  TextureIdMap texture_id_map_;
+  // Available textures (these are plugin ids.)
+  std::vector<uint32_t> available_textures_;
+  // Track textures that are no longer needed (these are plugin ids.)
+  typedef base::hash_set<uint32_t> TextureIdSet;
+  TextureIdSet textures_to_dismiss_;
+  // Mailboxes for pending texture requests, to write to plugin's textures.
+  std::vector<gpu::Mailbox> pending_texture_mailboxes_;
+  // Queue of decoded frames that have been converted to RGB and await upload to
+  // a GL texture.
+  typedef std::queue<linked_ptr<PendingFrame> > PendingFrameQueue;
+  PendingFrameQueue pending_frames_;
+  // |num_pending_decodes_| is incremented when we call Decode, and decremented
+  // when we receive a decoded frame. A VideoDecoder always returns a (possibly
+  // empty) frame to signal completion.
+  uint32_t num_pending_decodes_;
+
+  base::WeakPtrFactory<VideoDecoderAdapter> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(VideoDecoderAdapter);
+};
+
+}  // namespace content
+
+namespace base {
+
+template <class T>
+struct DefaultDeleter;
+
+// Specialize DefaultDeleter so that scoped_ptr<content::VideoDecoderAdapter>
+// always uses "Destroy()" instead of trying to use the destructor.
+template <>
+struct DefaultDeleter<content::VideoDecoderAdapter> {
+ public:
+  inline void operator()(void* video_decoder_adapter) const {
+    static_cast<content::VideoDecoderAdapter*>(video_decoder_adapter)
+        ->Destroy();
+  }
+};

[dmichael (off chromium), 2014/06/06 17:24:00]

I still think you can get away without this for VideoDecoderAdapter :). If I'm
wrong, sorry for harping on it. (See comment in Destroy definition)

[bbudge, 2014/06/07 01:01:23]

People who are right can harp. Done.

+
+}  // namespace base
+
+#endif  // CONTENT_RENDERER_PEPPER_VIDEO_DECODER_ADAPTER_H_