Implement software fallback for PPB_VideoDecoder.

content/renderer/pepper/video_decoder_proxy.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_PROXY_H_
+#define CONTENT_RENDERER_PEPPER_VIDEO_DECODER_PROXY_H_
+
+#include <queue>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/containers/hash_tables.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"
+#include "media/base/video_decoder.h"
+#include "media/base/video_decoder_config.h"
+
+#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.

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

I think it is the case that this class is meant to be a compatibility shim
between a client that wants to talk to a VDA (PepperVideoDecoderHost) doesn't
have one, so this class cooks up a media::VideoDecoder for it.
You might want to say that in this comment, somehow :)

Although, wouldn't it simplify the host significantly if this class actually
implemented VDA, so the host could hold either the GVDAH or VDP in the same
scoped_ptr<media::VideoDecodeAccelerator> decoder_;
member?

[bbudge, 2014/06/06 02:03:44]

It would definitely simplify the host if this was an adapter that implemented
VDA around a VD. I considered this. The two APIs are tantalizingly close but
there are a few differences that make it difficult. The first is that
VDA::Decode takes a shm handle, which is difficult to map to the address that VD
needs to create its DecodeBuffer. A lesser problem is that VDP has to create
textures that alias the plugin's, so it creates mailboxes, but there is no way
to pass these back via the VDA::Client interface. I could define some methods so
VDP could set or query to get missing information but this is messy. Shared
memory handles make poor keys.

+// Instances should be constructed, used, and destructed on the main (render)
+// thread.
+class VideoDecoderProxy {

[dmichael (off chromium), 2014/06/05 23:00:43]

I can see why you used "Proxy", but I'm so used to that mostly referring to a
proxy for something in another process in Chromium.

This is basically Adapter pattern, so maybe VideoDecoderAdapter? (Or Wrapper, or
Helper, or ??)

WDYT?

[bbudge, 2014/06/06 02:03:43]

Adapter it is. Done.

+ public:
+  explicit VideoDecoderProxy(PepperVideoDecoderHost* host);
+
+  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();
+
+ protected:

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

why not private?

[bbudge, 2014/06/06 02:03:43]

Done.

+  // Do not delete directly; use Destroy() or own it with a scoped_ptr, which
+  // will Destroy() it properly by default.
+  ~VideoDecoderProxy();
+
+ private:
+  enum State {
+    UNINITIALIZED,
+    DECODING,
+    FLUSHING,
+    RESETTING,
+  };
+
+  struct PendingDecode {
+    PendingDecode(uint32_t decode_id,
+                  const scoped_refptr<media::DecoderBuffer>& buffer);
+    ~PendingDecode();
+
+    uint32_t decode_id;

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

Here and below, make const-able members const?

[bbudge, 2014/06/06 02:03:44]

Done.

+    scoped_refptr<media::DecoderBuffer> buffer;
+  };
+
+  struct PendingFrame {
+    PendingFrame(uint32_t decode_id, const gfx::Size& size);
+    ~PendingFrame();
+
+    uint32_t decode_id;
+    gfx::Size size;
+    std::vector<uint8_t> pixels;

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

rgba_pixels ?
(or comment on their colorspace)

[bbudge, 2014/06/06 02:03:43]

Changed to argb_pixels, to match the conversion function I call.

It's confusing to me that the API promises GL_BGRA pixels. Did I get this wrong?
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/c/pp_codecs....

+  };
+
+  // This class is constructed on the main (render) thread, but used and
+  // destructed on the media thread.
+  class Delegate {

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

optional: here and above, can fwd-declare inner classes here and fully-declare
in the .cc file.

[bbudge, 2014/06/06 02:03:44]

Done.

+   public:
+    Delegate(const base::WeakPtr<VideoDecoderProxy>& proxy);

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

explicit

[bbudge, 2014/06/06 02:03:43]

Done.

+    ~Delegate();

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

private b/c Destroy?

[bbudge, 2014/06/06 02:03:43]

Destroy is a misleading name here, since it doesn't call the dtor. Changing the
name to Stop().

+
+    void Initialize(scoped_ptr<media::VideoDecoder> decoder,

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

decoder seems like a strange thing to pass to Initialize instead of to the ctor.
It could even be new'd _in_ the ctor (and then media_message_loop is implicit as
MessageLoopProxy::current()).

[bbudge, 2014/06/06 02:03:43]

Yeah, that works out well. Thanks.

+                    media::VideoDecoderConfig config);
+    void ReceiveBuffer(uint32_t decode_id,

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

I get that you want to distinguish between a caller's notion of "Decode" (which
this is) and the Decode call you make from this class (which is what Decode()
below is) but ReceiveBuffer is confusingly passive.
WDYT of QueueDecode() for this and DrainDecode() for the next?

[bbudge, 2014/06/06 02:03:43]

I eliminated the second method, and renamed this method Decode. See the
implementation for how it works.

+                       scoped_refptr<media::DecoderBuffer> buffer);
+    void Decode();
+    void Reset();
+    void Destroy();
+
+   private:
+    void OnPipelineStatus(media::PipelineStatus status);
+    void ConvertFrame(uint32_t decode_id,

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

"convert"?  From what to what?
doco please (here and elsewhere where not obvious from name/context)

[bbudge, 2014/06/06 02:03:43]

I changed some other method names, and now OnDecodeComplete seems clearest.
Done.

+                      media::VideoDecoder::Status status,
+                      const scoped_refptr<media::VideoFrame>& frame);
+    void OnResetComplete();
+
+    base::WeakPtr<VideoDecoderProxy> proxy_;

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

// Bound to main_message_loop_.

[bbudge, 2014/06/06 02:03:43]

I don't understand this comment. I construct and destruct on the main loop but
FfmpegVideoDecoder doesn't seem to bind to it.

+    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_;
+  };
+
+  void OnPipelineStatus(media::PipelineStatus status);
+  void ReceiveFrame(media::VideoDecoder::Status status,
+                    scoped_ptr<PendingFrame> frame);
+  void SendPictures();
+  void OnResetComplete();
+  void OnDestroyComplete();

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

Not implemented (and thank goodness, because what would that mean??)

[bbudge, 2014/06/06 02:03:43]

Left over from previous patchset. Removed.

+  void DismissTexture(uint32_t texture_id);
+  void DeleteTexture(uint32_t texture_id);
+  void FlushCommandBuffer();

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

doco should say when it is required to call this (and why).

[bbudge, 2014/06/06 02:03:43]

Done.

+
+  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 pending decoded frames. These have been converted to RGB, and
+  // await upload to a GL texture.
+  typedef std::queue<PendingFrame*> PendingFrameQueue;

[Ami GONE FROM CHROMIUM, 2014/06/05 00:06:24]

y no queue<linked_ptr<PF> > ?

[bbudge, 2014/06/06 02:03:43]

Good idea. Done.

+  PendingFrameQueue pending_frames_;
+  uint32_t num_pending_decodes_;
+
+  base::WeakPtrFactory<VideoDecoderProxy> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(VideoDecoderProxy);
+};
+
+}  // namespace content
+
+namespace base {
+
+template <class T>
+struct DefaultDeleter;
+
+// Specialize DefaultDeleter so that scoped_ptr<content::VideoDecoderProxy>
+// always uses "Destroy()" instead of trying to use the destructor.
+template <>
+struct DefaultDeleter<content::VideoDecoderProxy> {
+ public:
+  inline void operator()(void* video_decoder) const {
+    static_cast<content::VideoDecoderProxy*>(video_decoder)->Destroy();
+  }
+};

[dmichael (off chromium), 2014/06/05 23:00:43]

(Note, if you are able to get rid of Destroy and just do stuff in destructors,
then you can get rid of this)

[bbudge, 2014/06/06 02:03:43]

I don't think it's possible for this class.

+
+}  // namespace base
+
+#endif  // CONTENT_RENDERER_PEPPER_VIDEO_DECODER_PROXY_H_