Add texture target field to video frame (for use by native textures).

media/base/video_frame.cc

 // Copyright (c) 2012 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.
 
 #include "media/base/video_frame.h"
 
+#include <GL/gl.h>

[Ami GONE FROM CHROMIUM, 2012/02/22 04:51:00]

Can drop this if you take my suggestion below.

+
 #include "base/logging.h"
 #include "media/base/video_util.h"
 
 namespace media {
 
 // static
 scoped_refptr<VideoFrame> VideoFrame::CreateFrame(
     VideoFrame::Format format,
     size_t width,
     size_t height,
@@ skipping 33 matching lines @@
 scoped_refptr<VideoFrame> VideoFrame::WrapNativeTexture(
     uint32 texture_id,
     size_t width,
     size_t height,
     base::TimeDelta timestamp,
     base::TimeDelta duration,
     const base::Closure& no_longer_needed) {
   scoped_refptr<VideoFrame> frame(
       new VideoFrame(NATIVE_TEXTURE, width, height, timestamp, duration));
   frame->texture_id_ = texture_id;
+  frame->texture_target_ = GL_TEXTURE_2D;

[Ami GONE FROM CHROMIUM, 2012/02/22 04:51:00]

This smells wrong to me (b/c what's 2Dlicious about this class/method?).  I
think at the very least mention of GL_TEXTURE_2D belongs one level higher (in
the caller, GpuVideoDecoder), but more likely it belongs even higher than that. 


The magic of TEXTURE_2D is introduced in
RendererGpuVideoDecoderFactories::AsyncCreateTextures, so ISTM *that's* where
the texture_target should start being specified from.  Following that logic
through would have media::GpuVideoDecoder::Factories::CreateTextures also return
the target (assuming we can use a single target for all returned textures).  A
purist approach would preserve the target alongside the texture_id in
media::PictureBuffer but that seems like overkill to me (both because it would
require plumbing through pepper as well, and because it is constant per video
element).  I think a reasonable compromise is to have CreateTextures return the
textures' target in an additional out param, and have GpuVideoDecoder remember
the target for later calls to WrapNativeTexture().  WDYT?

(a side benefit of this scheme is that everyone but
RendererGpuVideoDecoderFactories gets to treat the target as an opaque uint32 so
you don't have to think too hard about what may include <GL/gl.h> :))

   frame->texture_no_longer_needed_ = no_longer_needed;
   return frame;
 }
 
 // static
 scoped_refptr<VideoFrame> VideoFrame::CreateEmptyFrame() {
   return new VideoFrame(
       VideoFrame::EMPTY, 0, 0, base::TimeDelta(), base::TimeDelta());
 }
 
@@ skipping 61 matching lines @@
 }
 
 VideoFrame::VideoFrame(VideoFrame::Format format,
                        size_t width,
                        size_t height,
                        base::TimeDelta timestamp,
                        base::TimeDelta duration)
     : format_(format),
       width_(width),
       height_(height),
-      texture_id_(0) {
+      texture_id_(0),
+      texture_target_(0) {
   SetTimestamp(timestamp);
   SetDuration(duration);
   memset(&strides_, 0, sizeof(strides_));
   memset(&data_, 0, sizeof(data_));
 }
 
 VideoFrame::~VideoFrame() {
   if (format_ == NATIVE_TEXTURE && !texture_no_longer_needed_.is_null()) {
     texture_no_longer_needed_.Run();
     texture_no_longer_needed_.Reset();
@@ skipping 89 matching lines @@
 uint8* VideoFrame::data(size_t plane) const {
   DCHECK(IsValidPlane(plane));
   return data_[plane];
 }
 
 uint32 VideoFrame::texture_id() const {
   DCHECK_EQ(format_, NATIVE_TEXTURE);
   return texture_id_;
 }
 
+uint32 VideoFrame::texture_target() const {
+  DCHECK_EQ(format_, NATIVE_TEXTURE);
+  return texture_target_;
+}
+
 bool VideoFrame::IsEndOfStream() const {
   return format_ == VideoFrame::EMPTY;
 }
 
 }  // namespace media