media: Let VideoFrame carry more than one native texture.

media/base/video_frame.h

Index: media/base/video_frame.h
diff --git a/media/base/video_frame.h b/media/base/video_frame.h
index cebed0af11a47aea5537b92346d97cebae1ef05f..d64cd5c03bb856cfe6e16c2132766d54dbcd7627 100644
--- a/media/base/video_frame.h
+++ b/media/base/video_frame.h
@@ -11,6 +11,7 @@
 #include "base/md5.h"
 #include "base/memory/shared_memory.h"
 #include "base/synchronization/lock.h"
+#include "gpu/command_buffer/common/mailbox_holder.h"
 #include "media/base/buffers.h"
 #include "media/base/video_frame_metadata.h"
 #include "ui/gfx/geometry/rect.h"
@@ -21,10 +22,6 @@
 #include "base/mac/scoped_cftyperef.h"
 #endif
 
-namespace gpu {
-struct MailboxHolder;
-}  // namespace gpu
-
 namespace media {
 
 class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
@@ -69,6 +66,15 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
     FORMAT_MAX = YV12HD,  // Must always be equal to largest entry logged.
   };
 
+  // Defines the number and the pixel format of the textures in the mailbox
+  // holders.

[reveman, 2015/05/03 16:13:27]

hm, this defines the subsampling factor and internal format as needed to use
these textures in a shader. The actual pixel format does not need to be known.

[Daniele Castagna, 2015/05/04 15:00:15]

Changed the comment.

+  enum TextureFormat {
+    TEXTURE_NONE = 0,        // This frame doesn't contain any native texture.

[reveman, 2015/05/03 16:13:27]

I think the code would be cleaner if this enum contained only valid values and
texture_format_ field should be ignored if format is not NATIVE_TEXTURE. e.g.
the IsValidConfig change goes away.

[Daniele Castagna, 2015/05/04 15:00:15]

Done.

[DaleCurtis, 2015/05/04 16:00:14]

Hmm, I think that's fine so long as we still initialize it to RGBA or something.
Otherwise I prefer the DCHECK_NE(NONE in places to avoid waiting for valgrind to
catch an uninitialized value.

+    TEXTURE_RGBA = 1,        // One RGBA texture.

[DaleCurtis, 2015/05/02 18:21:33]

Doesn't android use ARGB or something?

[reveman, 2015/05/03 16:13:27]

I think the idea is that this just describes the internal format as far as a
shader needs to know. TEXTURE_RGBA, means it's one texture with
red/green/blue/alpha channels but the pixel layout is unknown.

[Daniele Castagna, 2015/05/04 15:00:15]

Agreed.

+    TEXTURE_YUV_R8R8R8 = 2,  // Three R8 textures one per YUV channel.

[reveman, 2015/05/03 16:13:27]

s/YUV_R8R8R8/YUV_420/ as that would make it clear the UV planes have a
subsampling factor of 2 and the actual resolution (R8R8R8) of each plane should
not be useful at this level.

[Daniele Castagna, 2015/05/04 15:00:15]

Done.

+    TEXTURE_FORMAT_MAX = TEXTURE_YUV_R8R8R8,
+  };
+
   // Returns the name of a Format as a string.
   static std::string FormatToString(Format format);
 
@@ -88,7 +94,9 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
   // Call prior to CreateFrame to ensure validity of frame configuration. Called
   // automatically by VideoDecoderConfig::IsValidConfig().
   // TODO(scherkus): VideoDecoderConfig shouldn't call this method
-  static bool IsValidConfig(Format format, const gfx::Size& coded_size,
+  static bool IsValidConfig(Format format,
+                            TextureFormat texture_format,
+                            const gfx::Size& coded_size,
                             const gfx::Rect& visible_rect,
                             const gfx::Size& natural_size);
 
@@ -96,14 +104,12 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
   // destroyed.
   typedef base::Callback<void(uint32)> ReleaseMailboxCB;
 
-  // Wraps a native texture of the given parameters with a VideoFrame.  The
-  // backing of the VideoFrame is held in the mailbox held by |mailbox_holder|,
-  // and |mailbox_holder_release_cb| will be called with |mailbox_holder| as the
-  // argument when the VideoFrame is to be destroyed.
-  // |read_pixels_cb| may be used to do (slow!) readbacks from the
-  // texture to main memory.
+  // Wraps a native texture of the given parameters with a VideoFrame.
+  // The backing of the VideoFrame is held in the mailbox held by
+  // |mailbox_holder|, and |mailbox_holder_release_cb| will be called with
+  // a syncpoint as the argument when the VideoFrame is to be destroyed.
   static scoped_refptr<VideoFrame> WrapNativeTexture(
-      scoped_ptr<gpu::MailboxHolder> mailbox_holder,
+      const gpu::MailboxHolder& mailbox_holder,
       const ReleaseMailboxCB& mailbox_holder_release_cb,
       const gfx::Size& coded_size,
       const gfx::Rect& visible_rect,
@@ -111,6 +117,20 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
       base::TimeDelta timestamp,
       bool allow_overlay);
 
+  // Wraps a set of native textures representing YUV data with a VideoFrame.
+  // |mailbox_holders_release_cb| will be called with a syncpoint as the
+  // argument when the VideoFrame is to be destroyed.
+  static scoped_refptr<VideoFrame> WrapYUVNativeTextures(

[reveman, 2015/05/03 16:13:27]

WrapYUV420NativeTextures? Or pass the texture format as a parameter. Or why
don't we just change the WrapNativeTexture function above into a
"WrapNativeTextures" function that take an array of mailbox holders and a
texture format parameter?

[Daniele Castagna, 2015/05/04 15:00:15]

Renamed to WrapYUV420NativeTextures.
I decided not to change WrapNativeTextures to take an array of mailboxes and a
texture format since all the other wrap functions seem to be for a specific
video format. The videoframe format is never passed from the outside but always
determined by one of this wrap function, so keeping WrapYUV420NativeTextures
seems to fit better with the rest of the class.

+      const gpu::MailboxHolder& y_mailbox_holder,
+      const gpu::MailboxHolder& u_mailbox_holder,
+      const gpu::MailboxHolder& v_mailbox_holder,
+      const ReleaseMailboxCB& mailbox_holders_release_cb,
+      const gfx::Size& coded_size,
+      const gfx::Rect& visible_rect,
+      const gfx::Size& natural_size,
+      base::TimeDelta timestamp,
+      bool allow_overlay);
+
   // Wraps packed image data residing in a memory buffer with a VideoFrame.
   // The image data resides in |data| and is assumed to be packed tightly in a
   // buffer of logical dimensions |coded_size| with the appropriate bit depth
@@ -217,6 +237,8 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
 
   static size_t NumPlanes(Format format);
 
+  static size_t NumTextures(TextureFormat texture_format);
+
   // Returns the required allocation size for a (tightly packed) frame of the
   // given coded size and format.
   static size_t AllocationSize(Format format, const gfx::Size& coded_size);
@@ -253,6 +275,8 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
 
   Format format() const { return format_; }
 
+  TextureFormat texture_format() const { return texture_format_; }
+
   const gfx::Size& coded_size() const { return coded_size_; }
   const gfx::Rect& visible_rect() const { return visible_rect_; }
   const gfx::Size& natural_size() const { return natural_size_; }
@@ -278,10 +302,10 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
   const uint8* visible_data(size_t plane) const;
   uint8* visible_data(size_t plane);
 
-  // Returns the mailbox holder of the native texture wrapped by this frame.
+  // Returns a mailbox holder for a given texture.
   // Only valid to call if this is a NATIVE_TEXTURE frame. Before using the
   // mailbox, the caller must wait for the included sync point.
-  const gpu::MailboxHolder* mailbox_holder() const;
+  const gpu::MailboxHolder& mailbox_holder(size_t texture) const;
 
   // Returns the shared-memory handle, if present
   base::SharedMemoryHandle shared_memory_handle() const;
@@ -349,7 +373,8 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
              const gfx::Size& coded_size,
              const gfx::Rect& visible_rect,
              const gfx::Size& natural_size,
-             scoped_ptr<gpu::MailboxHolder> mailbox_holder,
+             const gpu::MailboxHolder mailbox_holders[kMaxPlanes],

[reveman, 2015/05/03 16:13:27]

Pass by reference instead of value.

[Daniele Castagna, 2015/05/04 15:00:15]

Done.

git cl format seems to be confused by the syntax though.

+             TextureFormat texture_format,
              base::TimeDelta timestamp,
              bool end_of_stream);
   virtual ~VideoFrame();
@@ -359,6 +384,9 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
   // Frame format.
   const Format format_;
 
+  // Format of the native textures associated with this frames.

[reveman, 2015/05/03 16:13:27]

nit: s/frames/frame/

[Daniele Castagna, 2015/05/04 15:00:15]

Done.

+  const TextureFormat texture_format_;
+
   // Width and height of the video frame, in pixels. This must include pixel
   // data for the whole image; i.e. for YUV formats with subsampled chroma
   // planes, in the case that the visible portion of the image does not line up
@@ -383,9 +411,9 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
   // Array of data pointers to each plane.
   uint8* data_[kMaxPlanes];
 
-  // Native texture mailbox, if this is a NATIVE_TEXTURE frame.
-  const scoped_ptr<gpu::MailboxHolder> mailbox_holder_;
-  ReleaseMailboxCB mailbox_holder_release_cb_;
+  // Native texture mailboxes, if this is a NATIVE_TEXTURE frame.
+  gpu::MailboxHolder mailbox_holders_[kMaxPlanes];
+  ReleaseMailboxCB mailbox_holders_release_cb_;
 
   // Shared memory handle, if this frame was allocated from shared memory.
   base::SharedMemoryHandle shared_memory_handle_;