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

media/gpu/ipc/client/gpu_video_decode_accelerator_host.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/gpu/ipc/client/gpu_video_decode_accelerator_host.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ skipping 33 matching lines @@
 
 bool GpuVideoDecodeAcceleratorHost::OnMessageReceived(const IPC::Message& msg) {
   DCHECK(CalledOnValidThread());
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(GpuVideoDecodeAcceleratorHost, msg)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_InitializationComplete,
                         OnInitializationComplete)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_BitstreamBufferProcessed,
                         OnBitstreamBufferProcessed)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_ProvidePictureBuffers,
-                        OnProvidePictureBuffer)
+                        OnProvidePictureBuffers)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_PictureReady,
                         OnPictureReady)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_FlushDone, OnFlushDone)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_ResetDone, OnResetDone)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_ErrorNotification,
                         OnNotifyError)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_DismissPictureBuffer,
                         OnDismissPictureBuffer)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
@@ skipping 58 matching lines @@
 }
 
 void GpuVideoDecodeAcceleratorHost::AssignPictureBuffers(
     const std::vector<PictureBuffer>& buffers) {
   DCHECK(CalledOnValidThread());
   if (!channel_)
     return;
   // Rearrange data for IPC command.
   std::vector<int32_t> buffer_ids;
   std::vector<PictureBuffer::TextureIds> texture_ids;
-  for (uint32_t i = 0; i < buffers.size(); i++) {
+  uint32_t texture_target = 0;
+  VideoPixelFormat format = PIXEL_FORMAT_UNKNOWN;
+  gfx::Size size;
+
+  uint32_t buffers_size = buffers.size();
+  if (buffers_size) {
+    texture_target = buffers[0].texture_target();

[dcheng, 2017/05/18 14:15:41]

Is it possible (via something malicious) that the buffers don't all have
matching attributes? In particular, size. This code can send a message from the
renderer to the GPU right? Does the GPU service check all these parameters for
correctness?

[Chandan, 2017/05/18 19:38:53]

Can you pls elaborate a bit on 'matching attributes'?
I assume you are referring to GPU process by 'GPU'. Then, yes.
These extra parameters are passed on to the underlying decoder. 
However, these are unused currently. Their correctness will be checked may be
when we actually switch picture buffer formats mid-stream.
liberato@, your views on this?

[liberato (no reviews please), 2017/05/18 20:07:01]

hrm, it's a good point that i hadn't considerd.

i suppose that one could remove it from IPC and fill it in on the GPU side,
based on the most recently requested parameters.  that's what size was doing
before, i think.

if one does that, then we avoid the security issue but still get buffers with
correct metadata on the gpu side.

[Chandan, 2017/05/18 20:36:36]

Ok. I will remove these 3 extra params from IPC then and cache format, target
similar to what was done for size?

+    format = buffers[0].pixel_format();
+    size = buffers[0].size();
+  }
+
+  for (uint32_t i = 0; i < buffers_size; i++) {
     const PictureBuffer& buffer = buffers[i];
     if (buffer.size() != picture_buffer_dimensions_) {
       DLOG(ERROR) << "buffer.size() invalid: expected "
                   << picture_buffer_dimensions_.ToString() << ", got "
                   << buffer.size().ToString();
       PostNotifyError(INVALID_ARGUMENT);
       return;
     }
     texture_ids.push_back(buffer.client_texture_ids());
     buffer_ids.push_back(buffer.id());
   }
+
   Send(new AcceleratedVideoDecoderMsg_AssignPictureBuffers(
-      decoder_route_id_, buffer_ids, texture_ids));
+      decoder_route_id_, buffer_ids, texture_ids, texture_target, format,
+      size));
 }
 
 void GpuVideoDecodeAcceleratorHost::ReusePictureBuffer(
     int32_t picture_buffer_id) {
   DCHECK(CalledOnValidThread());
   if (!channel_)
     return;
   Send(new AcceleratedVideoDecoderMsg_ReusePictureBuffer(decoder_route_id_,
                                                          picture_buffer_id));
 }
@@ skipping 61 matching lines @@
     client_->NotifyInitializationComplete(success);
 }
 
 void GpuVideoDecodeAcceleratorHost::OnBitstreamBufferProcessed(
     int32_t bitstream_buffer_id) {
   DCHECK(CalledOnValidThread());
   if (client_)
     client_->NotifyEndOfBitstreamBuffer(bitstream_buffer_id);
 }
 
-void GpuVideoDecodeAcceleratorHost::OnProvidePictureBuffer(
+void GpuVideoDecodeAcceleratorHost::OnProvidePictureBuffers(
     uint32_t num_requested_buffers,
     VideoPixelFormat format,
     uint32_t textures_per_buffer,
     const gfx::Size& dimensions,
     uint32_t texture_target) {
   DCHECK(CalledOnValidThread());
   picture_buffer_dimensions_ = dimensions;
 
   const int kMaxVideoPlanes = 4;
   if (textures_per_buffer > kMaxVideoPlanes) {
@@ skipping 48 matching lines @@
   weak_this_factory_.InvalidateWeakPtrs();
 
   // Client::NotifyError() may Destroy() |this|, so calling it needs to be the
   // last thing done on this stack!
   VideoDecodeAccelerator::Client* client = nullptr;
   std::swap(client, client_);
   client->NotifyError(static_cast<VideoDecodeAccelerator::Error>(error));
 }
 
 }  // namespace media