cc: Make VideoResourceUpdater use CopyToResource instead of SetPixels.

cc: Make VideoResourceUpdater use CopyToResource instead of SetPixels.

This makes video stop relying on the TextureUploader class. I've ported
the stride-handling code from TextureUploader to the
VideoResourceUpdater class, since it is the only place that will make
use for it for now.

R=enne
BUG=454575
Committed: https://crrev.com/e3f994390b28d440bacfa88031077761a29f1854
Cr-Commit-Position: refs/heads/master@{#317171}

[enne (OOO), 2015-02-19 19:53:01]

The CQ bit was checked by enne@chromium.org

[enne (OOO), 2015-02-19 19:53:03]

lgtm

https://codereview.chromium.org/935383004/diff/1/cc/resources/video_resource_...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/935383004/diff/1/cc/resources/video_resource_...
cc/resources/video_resource_updater.cc:322: // TODO(reveman): Can use of
GpuMemoryBuffers here to improve performance.
s/of//

[enne (OOO), 2015-02-19 19:53:17]

The CQ bit was unchecked by enne@chromium.org

[danakj, 2015-02-19 19:55:10]

New patchsets have been uploaded after l-g-t-m from enne@chromium.org

[danakj, 2015-02-19 19:55:12]

The CQ bit was checked by danakj@chromium.org

[danakj, 2015-02-19 19:55:34]

The CQ bit was unchecked by danakj@chromium.org

[danakj, 2015-02-19 19:55:43]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2015-02-19 19:56:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/935383004/20001

[ericrk, 2015-02-19 19:57:40]

ericrk@chromium.org changed reviewers:
+ ericrk@chromium.org

[ericrk, 2015-02-19 19:57:41]

lgtm as well.

https://codereview.chromium.org/935383004/diff/1/cc/resources/video_resource_...
File cc/resources/video_resource_updater.h (right):

https://codereview.chromium.org/935383004/diff/1/cc/resources/video_resource_...
cc/resources/video_resource_updater.h:134: // TODO(danakj): Use std::array when
we can.
std::array is compile-time fixed size.

Maybe std::vector once C++11 std::vector::data() is available?

[danakj, 2015-02-19 20:46:21]

https://codereview.chromium.org/935383004/diff/1/cc/resources/video_resource_...
File cc/resources/video_resource_updater.h (right):

https://codereview.chromium.org/935383004/diff/1/cc/resources/video_resource_...
cc/resources/video_resource_updater.h:134: // TODO(danakj): Use std::array when
we can.
On 2015/02/19 19:57:41, ericrk wrote:
> std::array is compile-time fixed size.
> 
> Maybe std::vector once C++11 std::vector::data() is available?

Oh! Hrm. I could just use vector now with some casts then..

[danakj, 2015-02-19 20:46:27]

The CQ bit was unchecked by danakj@chromium.org

[danakj, 2015-02-19 20:48:36]

Switched it to a vector.

[danakj, 2015-02-19 20:48:39]

New patchsets have been uploaded after l-g-t-m from ericrk@chromium.org

[danakj, 2015-02-19 20:48:42]

The CQ bit was checked by danakj@chromium.org

[danakj, 2015-02-19 20:49:04]

Patchset #3 (id:40001) has been deleted

[danakj, 2015-02-19 22:29:00]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2015-02-19 22:29:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/935383004/60001

[commit-bot: I haz the power, 2015-02-19 23:40:11]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-02-19 23:41:44]

Patchset 3 (id:??) landed as
https://crrev.com/e3f994390b28d440bacfa88031077761a29f1854
Cr-Commit-Position: refs/heads/master@{#317171}

[DaleCurtis, 2015-02-20 03:21:27]

dalecurtis@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2015-02-20 03:21:28]

https://codereview.chromium.org/935383004/diff/60001/cc/resources/video_resou...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/935383004/diff/60001/cc/resources/video_resou...
cc/resources/video_resource_updater.cc:349: memcpy(dst, src,
resource_size_pixels.width() * bytes_per_pixel);
Does this mean we're doing a memcpy for every frame within system memory and
then another copy from the system memory into the gpu?

[danakj, 2015-02-20 03:34:03]

https://codereview.chromium.org/935383004/diff/60001/cc/resources/video_resou...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/935383004/diff/60001/cc/resources/video_resou...
cc/resources/video_resource_updater.cc:349: memcpy(dst, src,
resource_size_pixels.width() * bytes_per_pixel);
On 2015/02/20 03:21:28, DaleCurtis wrote:
> Does this mean we're doing a memcpy for every frame within system memory and
> then another copy from the system memory into the gpu?

When there's a stride, yes. It's matching the current behaviour, so I don't
expect any regressions, but yah. If we can use GpuMemoryBuffers (the TODO above)
we can win a lot.

[DaleCurtis, 2015-02-20 18:03:39]

https://codereview.chromium.org/935383004/diff/60001/cc/resources/video_resou...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/935383004/diff/60001/cc/resources/video_resou...
cc/resources/video_resource_updater.cc:349: memcpy(dst, src,
resource_size_pixels.width() * bytes_per_pixel);
On 2015/02/20 03:34:03, danakj wrote:
> On 2015/02/20 03:21:28, DaleCurtis wrote:
> > Does this mean we're doing a memcpy for every frame within system memory and
> > then another copy from the system memory into the gpu?
> 
> When there's a stride, yes. It's matching the current behaviour, so I don't
> expect any regressions, but yah. If we can use GpuMemoryBuffers (the TODO
above)
> we can win a lot.

Thanks for the explanation. There's always a stride w/ software decoding :)
Looking at the GpuMemoryBufferTypes it seems to only support unpacked 8bpp
r,g,b,a; so seemingly we'd have to do YUV conversion into that format which
might lose some of the benefits.

Is there a bug with more information on how a GpuMemoryBuffer might be used
here?

[reveman, 2015-02-20 18:13:31]

On 2015/02/20 at 18:03:39, dalecurtis wrote:
>
https://codereview.chromium.org/935383004/diff/60001/cc/resources/video_resou...
> File cc/resources/video_resource_updater.cc (right):
> 
>
https://codereview.chromium.org/935383004/diff/60001/cc/resources/video_resou...
> cc/resources/video_resource_updater.cc:349: memcpy(dst, src,
resource_size_pixels.width() * bytes_per_pixel);
> On 2015/02/20 03:34:03, danakj wrote:
> > On 2015/02/20 03:21:28, DaleCurtis wrote:
> > > Does this mean we're doing a memcpy for every frame within system memory
and
> > > then another copy from the system memory into the gpu?
> > 
> > When there's a stride, yes. It's matching the current behaviour, so I don't
> > expect any regressions, but yah. If we can use GpuMemoryBuffers (the TODO
above)
> > we can win a lot.
> 
> Thanks for the explanation. There's always a stride w/ software decoding :)
Looking at the GpuMemoryBufferTypes it seems to only support unpacked 8bpp
r,g,b,a; so seemingly we'd have to do YUV conversion into that format which
might lose some of the benefits.
> 
> Is there a bug with more information on how a GpuMemoryBuffer might be used
here?

There are two possible solutions here. In both cases we'd create a
GpuMemoryBuffer for each YUV plane and that gives us 0-copy transfer of the
planes to the graphics hardware when supported (it will perform an implicit
texture upload when not supported). In some cases the driver allows us to create
a RGB texture from these planes directly and the color space conversion is
handled automatically. That's solution 1. Solution 2 is binding each
GpuMemoryBuffer to a separate texture and doing the color space conversion using
a shader. Solution 1 would be as efficient as gets. Solution 2 would work with
all drivers/hardware but be a bit less efficient.

[DaleCurtis, 2015-02-20 18:17:27]

Awesome, thanks again for the discussion. Let me know if you need some bandwidth
to help get that done. My OKR this quarter is smooth video playback, reducing
texture upload cost for hi-res HFR software playback would definitely make it
more likely that we can deliver a frame on time.

[reveman, 2015-02-20 20:23:02]

On 2015/02/20 at 18:17:27, dalecurtis wrote:
> Awesome, thanks again for the discussion. Let me know if you need some
bandwidth to help get that done. My OKR this quarter is smooth video playback,
reducing texture upload cost for hi-res HFR software playback would definitely
make it more likely that we can deliver a frame on time.

crbug.com/439520 is about adding YUV support to the GpuMemoryBuffer framework.