Add |texture_target_| and |pixel_format_| to media::PictureBuffer

media/filters/gpu_video_decoder.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/filters/gpu_video_decoder.h"
 
 #include <algorithm>
 #include <array>
 #include <utility>
 
@@ skipping 102 matching lines @@
 
 GpuVideoDecoder::GpuVideoDecoder(GpuVideoAcceleratorFactories* factories,
                                  const RequestSurfaceCB& request_surface_cb,
                                  MediaLog* media_log)
     : needs_bitstream_conversion_(false),
       factories_(factories),
       request_surface_cb_(request_surface_cb),
       media_log_(media_log),
       vda_initialized_(false),
       state_(kNormal),
-      decoder_texture_target_(0),
-      pixel_format_(PIXEL_FORMAT_UNKNOWN),
       next_picture_buffer_id_(0),
       next_bitstream_buffer_id_(0),
       available_pictures_(0),
       needs_all_picture_buffers_to_decode_(false),
       supports_deferred_initialization_(false),
       requires_texture_copy_(false),
       cdm_id_(CdmContext::kInvalidCdmId),
       weak_factory_(this) {
   DCHECK(factories_);
 }
@@ skipping 389 matching lines @@
                                             VideoPixelFormat format,
                                             uint32_t textures_per_buffer,
                                             const gfx::Size& size,
                                             uint32_t texture_target) {
   DVLOG(3) << "ProvidePictureBuffers(" << count << ", "
            << size.width() << "x" << size.height() << ")";
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
 
   std::vector<uint32_t> texture_ids;
   std::vector<gpu::Mailbox> texture_mailboxes;
-  decoder_texture_target_ = texture_target;
 
   if (format == PIXEL_FORMAT_UNKNOWN) {
     format = IsOpaque(config_.format()) ? PIXEL_FORMAT_XRGB : PIXEL_FORMAT_ARGB;
   }
 
-  // TODO(jbauman): Move decoder_texture_target_ and pixel_format_ to the
-  // picture buffer. http://crbug.com/614789
-  if ((pixel_format_ != PIXEL_FORMAT_UNKNOWN) && (pixel_format_ != format)) {
+  if (!factories_->CreateTextures(count * textures_per_buffer, size,
+                                  &texture_ids, &texture_mailboxes,
+                                  texture_target)) {
     NotifyError(VideoDecodeAccelerator::PLATFORM_FAILURE);
     return;
   }
-
-  pixel_format_ = format;
-  if (!factories_->CreateTextures(count * textures_per_buffer, size,
-                                  &texture_ids, &texture_mailboxes,
-                                  decoder_texture_target_)) {
-    NotifyError(VideoDecodeAccelerator::PLATFORM_FAILURE);
-    return;
-  }
   sync_token_ = factories_->CreateSyncToken();
   DCHECK_EQ(count * textures_per_buffer, texture_ids.size());
   DCHECK_EQ(count * textures_per_buffer, texture_mailboxes.size());
 
   if (!vda_)
     return;
 
   std::vector<PictureBuffer> picture_buffers;
   size_t index = 0;
   for (size_t i = 0; i < count; ++i) {
     PictureBuffer::TextureIds ids;
     std::vector<gpu::Mailbox> mailboxes;
     for (size_t j = 0; j < textures_per_buffer; j++) {
       ids.push_back(texture_ids[index]);
       mailboxes.push_back(texture_mailboxes[index]);
       index++;
     }
 
-    picture_buffers.push_back(
-        PictureBuffer(next_picture_buffer_id_++, size, ids, mailboxes));
+    picture_buffers.push_back(PictureBuffer(next_picture_buffer_id_++, size,
+                                            ids, texture_target, format,
+                                            mailboxes));
     bool inserted = assigned_picture_buffers_.insert(std::make_pair(
         picture_buffers.back().id(), picture_buffers.back())).second;
     DCHECK(inserted);
   }
 
   available_pictures_ += count;
 
   vda_->AssignPictureBuffers(picture_buffers);

[liberato (no reviews please), 2017/05/11 16:24:40]

the picture buffer received by GPU clients will not have the texture target nor
format filled in properly; they're not sent via IPC in
gpu_video_decode_accelerator_host.cc (AssignPictureBuffers).

it works as written, since nothing expects that those fields exist currently. 
it's still a little unexpected.

to be fair, |size| isn't sent in the existing code, either.

would you mind adding all three to the IPC?  it's a very rare message, so the
extra IPC overhead to minimize surprises seems worth it to me.

[Chandan, 2017/05/12 13:47:51]

Sure. I have added these fields to the IPC. However, they are unused on the
client side. Some work left there.
I feel we could have restricted this CL to only adding pixel_format and
texture_target to PictureBuffer.

 }
 
 void GpuVideoDecoder::DismissPictureBuffer(int32_t id) {
   DVLOG(3) << "DismissPictureBuffer(" << id << ")";
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
 
   PictureBufferMap::iterator it = assigned_picture_buffers_.find(id);
   if (it == assigned_picture_buffers_.end()) {
     NOTREACHED() << "Missing picture buffer: " << id;
     return;
@@ skipping 50 matching lines @@
 
   if (!picture.visible_rect().IsEmpty()) {
     visible_rect = picture.visible_rect();
   }
   if (!gfx::Rect(pb.size()).Contains(visible_rect)) {
     LOG(WARNING) << "Visible size " << visible_rect.ToString()
                  << " is larger than coded size " << pb.size().ToString();
     visible_rect = gfx::Rect(pb.size());
   }
 
-  DCHECK(decoder_texture_target_);
+  DCHECK(pb.texture_target());
 
   gpu::MailboxHolder mailbox_holders[VideoFrame::kMaxPlanes];
   for (size_t i = 0; i < pb.client_texture_ids().size(); ++i) {
     mailbox_holders[i] = gpu::MailboxHolder(pb.texture_mailbox(i), sync_token_,
-                                            decoder_texture_target_);
+                                            pb.texture_target());
   }
 
   scoped_refptr<VideoFrame> frame(VideoFrame::WrapNativeTextures(
-      pixel_format_, mailbox_holders,
+      pb.pixel_format(), mailbox_holders,
       // Always post ReleaseMailbox to avoid deadlock with the compositor when
       // releasing video frames on the media thread; http://crbug.com/710209.
       BindToCurrentLoop(base::Bind(
           &GpuVideoDecoder::ReleaseMailbox, weak_factory_.GetWeakPtr(),
           factories_, picture.picture_buffer_id(), pb.client_texture_ids())),
       pb.size(), visible_rect, natural_size, timestamp));
   if (!frame) {
     DLOG(ERROR) << "Create frame failed for: " << picture.picture_buffer_id();
     NotifyError(VideoDecodeAccelerator::PLATFORM_FAILURE);
     return;
@@ skipping 218 matching lines @@
   }
   return false;
 }
 
 void GpuVideoDecoder::DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent()
     const {
   DCHECK(factories_->GetTaskRunner()->BelongsToCurrentThread());
 }
 
 }  // namespace media