Fix broken draw/upload paths from videos to 2D canvas and WebGL.

Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case by
reverting to CopyTextureCHROMIUM in most cases, and allocating the
texture to the correct sizes in others.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2791813003
Cr-Original-Commit-Position: refs/heads/master@{#463925}
Committed: https://chromium.googlesource.com/chromium/src/+/0cc4c62a163318c06636916b7e2b2c6a56f16b74
Review-Url: https://codereview.chromium.org/2791813003
Cr-Commit-Position: refs/heads/master@{#464253}
Committed: https://chromium.googlesource.com/chromium/src/+/0986e62026891425cb209b61092d091d44ec8812

[Ken Russell (switch to Gerrit), 2017-04-01 04:11:04]

Description was changed from

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case.

Also revert to using CopyTextureCHROMIUM for a common case on macOS
for efficiency.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll.

BUG=701060
==========

to

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case.

Also revert to using CopyTextureCHROMIUM for a common case on macOS
for efficiency.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll.

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Ken Russell (switch to Gerrit), 2017-04-01 04:16:24]

Description was changed from

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case.

Also revert to using CopyTextureCHROMIUM for a common case on macOS
for efficiency.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll.

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case.

Also revert to using CopyTextureCHROMIUM for a common case on macOS
for efficiency.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Ken Russell (switch to Gerrit), 2017-04-01 04:32:23]

kbr@chromium.org changed reviewers:
+ dalecurtis@chromium.org, sandersd@chromium.org

[Ken Russell (switch to Gerrit), 2017-04-01 04:32:24]

dalecurtis or sandersd: please review. Thanks.

Note I'm not sure this fix is correct in all cases. I think the required
semantic may be to stretch the source video to cover the destination. However,
we don't have a primitive to do that yet. More tests will be added, and any bugs
fixed.

junov: FYI. I incorporated the 2D Canvas test for this case into the WebGL
conformance test I've submitted in
https://github.com/KhronosGroup/WebGL/pull/2359 and which will be rolled in
shortly. The reason that was done is I'm not sure if there are any other
configurations on the waterfall which test hardware decoded video. I'd also be
happy to roll in the new test before this, and have this CL start running the
new test.

[Ken Russell (switch to Gerrit), 2017-04-01 04:32:30]

The CQ bit was checked by kbr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-01 04:32:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-01 06:00:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-01 06:00:49]

Dry run: This issue passed the CQ dry run.

[DaleCurtis, 2017-04-03 18:40:21]

VideoResourceUpdater just always uses the coded size and
GpuMemoryBufferVideoFramePool uses the slice of coded_size defined by
visible_rect. Any reason we don't want to do one of those things here?

[sandersd (OOO until July 31), 2017-04-03 18:56:21]

https://codereview.chromium.org/2791813003/diff/40001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/40001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:172: // incorrect.
This wording is misleading; it implies that normal usages are errors. We track
all three values (|coded_size|, |visible_rect|, and |natural_size|) so that we
can account for all of the valid situations:

  - |coded_size| is the actual pixels encoded by the encoder. For H.264 and VPx
this is a multiple of the macroblock size. Encoders generally expand the source
material to this size by smearing the edge pixels.

  - |visible_rect| is the region of the encoded pixels that represent the source
material (cropping away the smeared pixels). The origin may not be (0, 0), in
particular if rotation is part of the encoding pipeline. In practice it is
always aligned to *some* corner, but this isn't a technical requirement.

  - |natural_size| is the intended display size. This is the source of truth for
the pixel aspect ratio of the content. There are no analog sources that use a
1:1 pixel aspect ratio, so it is common that |natural_size| does not match
|visible_rect| size. It is also normal to leave |natural_size| unchanged when
switching stream resolutions.

Natural size is the only size that users (or web developers) interact with in
most situations.

When mapping on to a texture, the size of that texture isn't usually relevant,
just that the sampled region is the interior of the |visible_rect|.

[Ken Russell (switch to Gerrit), 2017-04-03 20:36:26]

On 2017/04/03 18:40:21, DaleCurtis wrote:
> VideoResourceUpdater just always uses the coded size and
> GpuMemoryBufferVideoFramePool uses the slice of coded_size defined by
> visible_rect. Any reason we don't want to do one of those things here?

The necessary knowledge isn't available at the point where it's needed. The
texture is allocated here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/webgl/...

This only knows about HTMLVideoElement::videoWidth and videoHeight, which are
the natural size (which is all that's available from the PipelineMetadata.)

Only once a video frame is available do we know the coded_size. We don't have
that at the Blink level.

I could change this logic again so that the lowest level code (inside
skcanvas_video_renderer.cc) allocates the destination texture in this case. This
will mean changing the semantics of
WebMediaPlayer::copyVideoTextureToPlatformTexture again. Would you prefer I go
in that direction instead?

[Ken Russell (switch to Gerrit), 2017-04-03 20:43:32]

https://codereview.chromium.org/2791813003/diff/40001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/40001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:172: // incorrect.
On 2017/04/03 18:56:21, sandersd wrote:
> This wording is misleading; it implies that normal usages are errors. We track
> all three values (|coded_size|, |visible_rect|, and |natural_size|) so that we
> can account for all of the valid situations:
> 
>   - |coded_size| is the actual pixels encoded by the encoder. For H.264 and
VPx
> this is a multiple of the macroblock size. Encoders generally expand the
source
> material to this size by smearing the edge pixels.
> 
>   - |visible_rect| is the region of the encoded pixels that represent the
source
> material (cropping away the smeared pixels). The origin may not be (0, 0), in
> particular if rotation is part of the encoding pipeline. In practice it is
> always aligned to *some* corner, but this isn't a technical requirement.
> 
>   - |natural_size| is the intended display size. This is the source of truth
for
> the pixel aspect ratio of the content. There are no analog sources that use a
> 1:1 pixel aspect ratio, so it is common that |natural_size| does not match
> |visible_rect| size. It is also normal to leave |natural_size| unchanged when
> switching stream resolutions.
> 
> Natural size is the only size that users (or web developers) interact with in
> most situations.
> 
> When mapping on to a texture, the size of that texture isn't usually relevant,
> just that the sampled region is the interior of the |visible_rect|.

Thanks for these clarifications. Sorry for the misleading wording; I'm not an
expert in this area.

The basic problem is that in the previous fix for http://crbug.com/672895 I
changed the code so that the Blink layer allocates the destination texture for
video-to-WebGL-texture copies, and I see now that that can't really be done
because the required sizing information isn't available at that level.

I'll restructure this fix so that
WebMediaPlayer::copyVideoTextureToPlatformTexture reallocates the destination
texture at the correct size.

[sandersd (OOO until July 31), 2017-04-03 21:19:10]

https://codereview.chromium.org/2791813003/diff/40001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/40001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:172: // incorrect.
> I'll restructure this fix so that
> WebMediaPlayer::copyVideoTextureToPlatformTexture reallocates the
> destination texture at the correct size.

Reading the WebGL spec, I'm not certain what it actually wants. It says 'The
first pixel... corresponds to the upper left corner of the source', which
implies that |visible_rect| cropping is done. It also says 'The width and height
of the texture are set to the width and height of the uploaded frame'; which is
ambiguous with regards to pixel aspect ratio.

Ideally it would be written to allow for optimizations in the common cases, but
at the very least the spec should be more clear on the topic of scaling.

Based on this text, I do agree that Firefox is probably wrong to report a height
different from 1080, even though 1088 is a reasonable way to interpret the
'height of the uploaded frame'. I am curious though how they avoided garbage
along one edge if they are uploading the entire coded frame.

I'm assuming there is no path for us to manipulate the UVs to fake this and
potentially use our existing textures in the fast path? (Is this what Firefox is
doing?)

[Ken Russell (switch to Gerrit), 2017-04-07 07:01:46]

Description was changed from

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case.

Also revert to using CopyTextureCHROMIUM for a common case on macOS
for efficiency.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case by
reverting to CopyTextureCHROMIUM in most cases, and allocating the
texture to the correct sizes in others.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Ken Russell (switch to Gerrit), 2017-04-07 07:03:29]

dalecurtis, sandersd: please take another look.

zmo, kainino, qiankun.miao: FYI.

Note: working on rolling in this new test in
https://codereview.chromium.org/2798083005 so that this CL can remove the
failure expectations.

[Ken Russell (switch to Gerrit), 2017-04-07 07:05:55]

The CQ bit was checked by kbr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 07:06:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Russell (switch to Gerrit), 2017-04-07 07:16:00]

https://codereview.chromium.org/2791813003/diff/40001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/40001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:172: // incorrect.
On 2017/04/03 21:19:10, sandersd wrote:
> > I'll restructure this fix so that
> > WebMediaPlayer::copyVideoTextureToPlatformTexture reallocates the
> > destination texture at the correct size.
> 
> Reading the WebGL spec, I'm not certain what it actually wants. It says 'The
> first pixel... corresponds to the upper left corner of the source', which
> implies that |visible_rect| cropping is done. It also says 'The width and
height
> of the texture are set to the width and height of the uploaded frame'; which
is
> ambiguous with regards to pixel aspect ratio.
> 
> Ideally it would be written to allow for optimizations in the common cases,
but
> at the very least the spec should be more clear on the topic of scaling.

Agreed that the spec needs to be clarified. There wasn't enough expertise in the
WebGL working group to realize the spec was lacking in this area. The language
around the first pixel is intended only to clarify whether, in the uploaded
texture, the Y axis goes "up" or "down".

> Based on this text, I do agree that Firefox is probably wrong to report a
height
> different from 1080, even though 1088 is a reasonable way to interpret the
> 'height of the uploaded frame'. I am curious though how they avoided garbage
> along one edge if they are uploading the entire coded frame.

I've come to realize that the video-to-WebGL-texture upload paths are probably
slightly broken in all browsers and that we'll need more tests around videos
with rotations, videos where it's intended that there be a significantly
different visible rectangle, etc.

> I'm assuming there is no path for us to manipulate the UVs to fake this and
> potentially use our existing textures in the fast path? (Is this what Firefox
is
> doing?)

No, it's currently necessary to clip the uploaded video frame when uploading it
to WebGL.

This proposed extension:
https://www.khronos.org/registry/webgl/extensions/proposals/OES_EGL_image_ext...

will provide a more optimized, and zero-copy, upload path. I now realize that
the metadata which it returns must contain the texture size (i.e., coded size),
natural size, and visible rectangle (if I understand everything correctly now).

[commit-bot: I haz the power, 2017-04-07 10:57:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-07 10:57:07]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[Ken Russell (switch to Gerrit), 2017-04-07 18:54:38]

The CQ bit was checked by kbr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 18:55:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sandersd (OOO until July 31), 2017-04-07 19:07:13]

One quick comment, I have not reviewed anything else.

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:173: return
video_frame->natural_size() != video_frame->coded_size();
I'm not sure which check you are intending here. The possible checks are:
  - visible_rect().size() != natural_size() => scaling is required if you want
the result to have 1:1 pixel aspect ratio. (<50% of videos)
  - visible_rect().size() != coded_size() => cropping is required (>50% of
videos)

[Ken Russell (switch to Gerrit), 2017-04-07 19:27:23]

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:173: return
video_frame->natural_size() != video_frame->coded_size();
On 2017/04/07 19:07:13, sandersd wrote:
> I'm not sure which check you are intending here. The possible checks are:
>   - visible_rect().size() != natural_size() => scaling is required if you want
> the result to have 1:1 pixel aspect ratio. (<50% of videos)
>   - visible_rect().size() != coded_size() => cropping is required (>50% of
> videos)

Thanks for the info.

The reason I didn't look at the visible rectangle is that I don't understand its
semantics and we don't have enough test cases.

There isn't a primitive in Chromium's GPU subsystem to copy and stretch a
sub-rectangle of one texture into another: the available primitives are
CopyTextureCHROMIUM and CopySubTextureCHROMIUM. This restricts the possible
fixes for this bug.

The user is currently always going to think that the texture they get from
TexImage2D taking a video element is sized the natural width and height. I think
it's acceptable if we get the aspect ratio wrong.

Copying the smaller of the coded size and natural size into the user's texture
has worked around the bugs seen to date. Should I take the visible rectangle
into account, too? Can that be deferred until I have a test case for that?

[sandersd (OOO until July 31), 2017-04-07 20:58:06]

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:173: return
video_frame->natural_size() != video_frame->coded_size();
On 2017/04/07 19:27:23, Ken Russell wrote:
> On 2017/04/07 19:07:13, sandersd wrote:
> > I'm not sure which check you are intending here. The possible checks are:
> >   - visible_rect().size() != natural_size() => scaling is required if you
want
> > the result to have 1:1 pixel aspect ratio. (<50% of videos)
> >   - visible_rect().size() != coded_size() => cropping is required (>50% of
> > videos)
> 
> Thanks for the info.
> 
> The reason I didn't look at the visible rectangle is that I don't understand
its
> semantics and we don't have enough test cases.
> 
> There isn't a primitive in Chromium's GPU subsystem to copy and stretch a
> sub-rectangle of one texture into another: the available primitives are
> CopyTextureCHROMIUM and CopySubTextureCHROMIUM. This restricts the possible
> fixes for this bug.
> 
> The user is currently always going to think that the texture they get from
> TexImage2D taking a video element is sized the natural width and height. I
think
> it's acceptable if we get the aspect ratio wrong.
> 
> Copying the smaller of the coded size and natural size into the user's texture
> has worked around the bugs seen to date. Should I take the visible rectangle
> into account, too? Can that be deferred until I have a test case for that?

If we don't have the option of scaling, then the only correct option is to clip
to the |visible_rect|, thus returning a texture with a size different from
|natural_size|.

Other choices produce minor visual errors in common cases, significant visual
errors in less common cases, and generally unreliable results for any
computation that isn't visual.

The cases where this texture size distinction can matter to web developers are
small, and it's not clear to me that this implementation is wrong (it does
contradict your recently added test though).

[Ken Russell (switch to Gerrit), 2017-04-07 21:15:27]

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:173: return
video_frame->natural_size() != video_frame->coded_size();
On 2017/04/07 20:58:06, sandersd wrote:
> On 2017/04/07 19:27:23, Ken Russell wrote:
> > On 2017/04/07 19:07:13, sandersd wrote:
> > > I'm not sure which check you are intending here. The possible checks are:
> > >   - visible_rect().size() != natural_size() => scaling is required if you
> want
> > > the result to have 1:1 pixel aspect ratio. (<50% of videos)
> > >   - visible_rect().size() != coded_size() => cropping is required (>50% of
> > > videos)
> > 
> > Thanks for the info.
> > 
> > The reason I didn't look at the visible rectangle is that I don't understand
> its
> > semantics and we don't have enough test cases.
> > 
> > There isn't a primitive in Chromium's GPU subsystem to copy and stretch a
> > sub-rectangle of one texture into another: the available primitives are
> > CopyTextureCHROMIUM and CopySubTextureCHROMIUM. This restricts the possible
> > fixes for this bug.
> > 
> > The user is currently always going to think that the texture they get from
> > TexImage2D taking a video element is sized the natural width and height. I
> think
> > it's acceptable if we get the aspect ratio wrong.
> > 
> > Copying the smaller of the coded size and natural size into the user's
texture
> > has worked around the bugs seen to date. Should I take the visible rectangle
> > into account, too? Can that be deferred until I have a test case for that?
> 
> If we don't have the option of scaling, then the only correct option is to
clip
> to the |visible_rect|, thus returning a texture with a size different from
> |natural_size|.
> 
> Other choices produce minor visual errors in common cases, significant visual
> errors in less common cases, and generally unreliable results for any
> computation that isn't visual.

Can the coded size or natural size be smaller (in width or height) than the
visible rectangle?

> The cases where this texture size distinction can matter to web developers are
> small, and it's not clear to me that this implementation is wrong (it does
> contradict your recently added test though).

There were two tests added recently, the one for crbug.com/672895
(conformance2/textures/misc/npot-video-sizing.html) and the one for this bug
(conformance/textures/misc/texture-corner-case-videos.html). I'm pretty sure
texture-corner-case-videos is a correct test, and npot-video-sizing may be
overly strict and somewhat incorrect. Is this what you meant?

If you can think of a way to make npot-video-sizing less fragile, I'll be happy
to encode a new video that exercises things differently. Please send any
suggestions.

[Ken Russell (switch to Gerrit), 2017-04-07 21:36:26]

The CQ bit was checked by kbr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 21:36:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sandersd (OOO until July 31), 2017-04-07 21:41:58]

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:173: return
video_frame->natural_size() != video_frame->coded_size();
> Can the coded size or natural size be smaller (in width or height) than the
> visible rectangle?

Strict rules:
  - Visible rect must be a subset of coded_size.

Common practices:
  - Natural size is scaled to have one dimension match the visible rect size.
  - Natural size is fixed while the coded size may be smaller (and vary over
time).

It is uncommon for natural size to be smaller than the visible rect size in both
dimensions, but there is no rule.

> There were two tests added recently, the one for crbug.com/672895
> (conformance2/textures/misc/npot-video-sizing.html) and the one for this bug
> (conformance/textures/misc/texture-corner-case-videos.html). I'm pretty sure
> texture-corner-case-videos is a correct test, and npot-video-sizing may be
> overly strict and somewhat incorrect. Is this what you meant?
> 
> If you can think of a way to make npot-video-sizing less fragile, I'll be
happy
> to encode a new video that exercises things differently. Please send any
> suggestions.

That sounds correct, although I've not looked at everything that
texture-corner-case-videos.html verifies.

[Ken Russell (switch to Gerrit), 2017-04-07 23:22:08]

kbr@chromium.org changed reviewers:
+ dglazkov@chromium.org, esprehn@chromium.org

[Ken Russell (switch to Gerrit), 2017-04-07 23:22:09]

sandersd@: please take another look.

dglazkov@ or esprehn@: OWNERS review of WebMediaPlayer.h please.

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/80001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:173: return
video_frame->natural_size() != video_frame->coded_size();
On 2017/04/07 21:41:57, sandersd wrote:
> > Can the coded size or natural size be smaller (in width or height) than the
> > visible rectangle?
> 
> Strict rules:
>   - Visible rect must be a subset of coded_size.
> 
> Common practices:
>   - Natural size is scaled to have one dimension match the visible rect size.
>   - Natural size is fixed while the coded size may be smaller (and vary over
> time).
> 
> It is uncommon for natural size to be smaller than the visible rect size in
both
> dimensions, but there is no rule.

OK. Thanks for your feedback. I've revised the fix in this file.

> > There were two tests added recently, the one for crbug.com/672895
> > (conformance2/textures/misc/npot-video-sizing.html) and the one for this bug
> > (conformance/textures/misc/texture-corner-case-videos.html). I'm pretty sure
> > texture-corner-case-videos is a correct test, and npot-video-sizing may be
> > overly strict and somewhat incorrect. Is this what you meant?
> > 
> > If you can think of a way to make npot-video-sizing less fragile, I'll be
> happy
> > to encode a new video that exercises things differently. Please send any
> > suggestions.
> 
> That sounds correct, although I've not looked at everything that
> texture-corner-case-videos.html verifies.

Fingers crossed that the new logic still allows that test to pass.

[Ken Russell (switch to Gerrit), 2017-04-07 23:28:20]

The CQ bit was checked by kbr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 23:29:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sandersd (OOO until July 31), 2017-04-07 23:40:59]

lgtm % nits.

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:174: return
video_frame->visible_rect().size() != video_frame->coded_size();
Might be more clear as Rect(coded_size() == visible_rect().

However, seeing how this is being used, this code is assuming that coded_size ==
texture size. I am not certain that that is always true.

I'll take a look through the various callers and get back.

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:177: gfx::Size
AdjustedVideoTextureSize(const VideoFrame* video_frame) {
Unused?

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:819: DCHECK_LE(dest_rect.height(),
video_frame->coded_size().height());
Perhaps better written as Rect(coded_size()).Contains(dest_rect).

[dglazkov, 2017-04-07 23:42:57]

lgtm

[Ken Russell (switch to Gerrit), 2017-04-07 23:48:18]

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:174: return
video_frame->visible_rect().size() != video_frame->coded_size();
On 2017/04/07 23:40:59, sandersd wrote:
> Might be more clear as Rect(coded_size() == visible_rect().
> 
> However, seeing how this is being used, this code is assuming that coded_size
==
> texture size. I am not certain that that is always true.

WebGL's the only consumer of the code paths where VideoTextureNeedsClipping
returns true. The intent of my earlier fix, and this revision, is to explicitly
make it so that the texture size isn't equal to the coded size.

> I'll take a look through the various callers and get back.

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:177: gfx::Size
AdjustedVideoTextureSize(const VideoFrame* video_frame) {
On 2017/04/07 23:40:59, sandersd wrote:
> Unused?

Fixed in next (current) patch set. I was still in the middle of compiling when I
uploaded this.

[sandersd (OOO until July 31), 2017-04-07 23:51:53]

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:174: return
video_frame->visible_rect().size() != video_frame->coded_size();
On 2017/04/07 23:48:18, Ken Russell wrote:
> On 2017/04/07 23:40:59, sandersd wrote:
> > Might be more clear as Rect(coded_size() == visible_rect().
> > 
> > However, seeing how this is being used, this code is assuming that
coded_size
> ==
> > texture size. I am not certain that that is always true.
> 
> WebGL's the only consumer of the code paths where VideoTextureNeedsClipping
> returns true. The intent of my earlier fix, and this revision, is to
explicitly
> make it so that the texture size isn't equal to the coded size.
> 
> > I'll take a look through the various callers and get back.
> 

My question is about the source texture though. When going through the fast
path, the assumption seems to be that the size of the source texture is the same
as this coded_size?

[Ken Russell (switch to Gerrit), 2017-04-07 23:54:52]

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:174: return
video_frame->visible_rect().size() != video_frame->coded_size();
On 2017/04/07 23:51:53, sandersd wrote:
> On 2017/04/07 23:48:18, Ken Russell wrote:
> > On 2017/04/07 23:40:59, sandersd wrote:
> > > Might be more clear as Rect(coded_size() == visible_rect().
> > > 
> > > However, seeing how this is being used, this code is assuming that
> coded_size
> > ==
> > > texture size. I am not certain that that is always true.
> > 
> > WebGL's the only consumer of the code paths where VideoTextureNeedsClipping
> > returns true. The intent of my earlier fix, and this revision, is to
> explicitly
> > make it so that the texture size isn't equal to the coded size.
> > 
> > > I'll take a look through the various callers and get back.
> > 
> 
> My question is about the source texture though. When going through the fast
> path, the assumption seems to be that the size of the source texture is the
same
> as this coded_size?

Yes, that's my assumption. It appeared to be the case, especially when my
earlier fix failed. Is this incorrect?

[sandersd (OOO until July 31), 2017-04-08 00:01:25]

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:174: return
video_frame->visible_rect().size() != video_frame->coded_size();
On 2017/04/07 23:54:51, Ken Russell wrote:
> On 2017/04/07 23:51:53, sandersd wrote:
> > On 2017/04/07 23:48:18, Ken Russell wrote:
> > > On 2017/04/07 23:40:59, sandersd wrote:
> > > > Might be more clear as Rect(coded_size() == visible_rect().
> > > > 
> > > > However, seeing how this is being used, this code is assuming that
> > coded_size
> > > ==
> > > > texture size. I am not certain that that is always true.
> > > 
> > > WebGL's the only consumer of the code paths where
VideoTextureNeedsClipping
> > > returns true. The intent of my earlier fix, and this revision, is to
> > explicitly
> > > make it so that the texture size isn't equal to the coded size.
> > > 
> > > > I'll take a look through the various callers and get back.
> > > 
> > 
> > My question is about the source texture though. When going through the fast
> > path, the assumption seems to be that the size of the source texture is the
> same
> > as this coded_size?
> 
> Yes, that's my assumption. It appeared to be the case, especially when my
> earlier fix failed. Is this incorrect?

I'm not sure.

I believe it is true when we create textures from software planes (although
there are rounding issues there, I think they apply to coded_size as well).

I am less sure about the hardware decode paths. I *think* that they all allocate
using the coded_size, but it's not inconceivable that there are situations where
over-allocation is possible.

Then there are the strange cases of decoder-allocated textures. I think that's
just on Android now that the Mac path is a GLImage with a software conversion
path.

Android outputs TEXTURE_EXTERNAL textures, I have no idea how they are allocated
or what happens to them through this path.

[sandersd (OOO until July 31), 2017-04-08 00:05:05]

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:174: return
video_frame->visible_rect().size() != video_frame->coded_size();
On 2017/04/08 00:01:25, sandersd wrote:
> On 2017/04/07 23:54:51, Ken Russell wrote:
> > On 2017/04/07 23:51:53, sandersd wrote:
> > > On 2017/04/07 23:48:18, Ken Russell wrote:
> > > > On 2017/04/07 23:40:59, sandersd wrote:
> > > > > Might be more clear as Rect(coded_size() == visible_rect().
> > > > > 
> > > > > However, seeing how this is being used, this code is assuming that
> > > coded_size
> > > > ==
> > > > > texture size. I am not certain that that is always true.
> > > > 
> > > > WebGL's the only consumer of the code paths where
> VideoTextureNeedsClipping
> > > > returns true. The intent of my earlier fix, and this revision, is to
> > > explicitly
> > > > make it so that the texture size isn't equal to the coded size.
> > > > 
> > > > > I'll take a look through the various callers and get back.
> > > > 
> > > 
> > > My question is about the source texture though. When going through the
fast
> > > path, the assumption seems to be that the size of the source texture is
the
> > same
> > > as this coded_size?
> > 
> > Yes, that's my assumption. It appeared to be the case, especially when my
> > earlier fix failed. Is this incorrect?
> 
> I'm not sure.
> 
> I believe it is true when we create textures from software planes (although
> there are rounding issues there, I think they apply to coded_size as well).
> 
> I am less sure about the hardware decode paths. I *think* that they all
allocate
> using the coded_size, but it's not inconceivable that there are situations
where
> over-allocation is possible.
> 
> Then there are the strange cases of decoder-allocated textures. I think that's
> just on Android now that the Mac path is a GLImage with a software conversion
> path.
> 
> Android outputs TEXTURE_EXTERNAL textures, I have no idea how they are
allocated
> or what happens to them through this path.

Upon reflection, I think Android is the only worrying case. The other cases must
match for our regular rendering path to produce correct output.

The Android case does not always have (0,0) -> (1,1) UV mapping for the
coded_size. We need to take its transformation matrix into account, and I don't
recall exactly how that path works.

[sandersd (OOO until July 31), 2017-04-08 00:07:15]

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2791813003/diff/120001/media/renderers/skcanv...
media/renderers/skcanvas_video_renderer.cc:174: return
video_frame->visible_rect().size() != video_frame->coded_size();
On 2017/04/08 00:05:04, sandersd wrote:
> On 2017/04/08 00:01:25, sandersd wrote:
> > On 2017/04/07 23:54:51, Ken Russell wrote:
> > > On 2017/04/07 23:51:53, sandersd wrote:
> > > > On 2017/04/07 23:48:18, Ken Russell wrote:
> > > > > On 2017/04/07 23:40:59, sandersd wrote:
> > > > > > Might be more clear as Rect(coded_size() == visible_rect().
> > > > > > 
> > > > > > However, seeing how this is being used, this code is assuming that
> > > > coded_size
> > > > > ==
> > > > > > texture size. I am not certain that that is always true.
> > > > > 
> > > > > WebGL's the only consumer of the code paths where
> > VideoTextureNeedsClipping
> > > > > returns true. The intent of my earlier fix, and this revision, is to
> > > > explicitly
> > > > > make it so that the texture size isn't equal to the coded size.
> > > > > 
> > > > > > I'll take a look through the various callers and get back.
> > > > > 
> > > > 
> > > > My question is about the source texture though. When going through the
> fast
> > > > path, the assumption seems to be that the size of the source texture is
> the
> > > same
> > > > as this coded_size?
> > > 
> > > Yes, that's my assumption. It appeared to be the case, especially when my
> > > earlier fix failed. Is this incorrect?
> > 
> > I'm not sure.
> > 
> > I believe it is true when we create textures from software planes (although
> > there are rounding issues there, I think they apply to coded_size as well).
> > 
> > I am less sure about the hardware decode paths. I *think* that they all
> allocate
> > using the coded_size, but it's not inconceivable that there are situations
> where
> > over-allocation is possible.
> > 
> > Then there are the strange cases of decoder-allocated textures. I think
that's
> > just on Android now that the Mac path is a GLImage with a software
conversion
> > path.
> > 
> > Android outputs TEXTURE_EXTERNAL textures, I have no idea how they are
> allocated
> > or what happens to them through this path.
> 
> Upon reflection, I think Android is the only worrying case. The other cases
must
> match for our regular rendering path to produce correct output.
> 
> The Android case does not always have (0,0) -> (1,1) UV mapping for the
> coded_size. We need to take its transformation matrix into account, and I
don't
> recall exactly how that path works.

Relevant Android code is here:
https://cs.chromium.org/chromium/src/media/gpu/avda_codec_image.cc?type=cs&sq...

You can tell me whether that goes down any of these paths and whether it is
handled correctly.

[esprehn, 2017-04-08 00:07:51]

esprehn@chromium.org changed reviewers:
- esprehn@chromium.org

[commit-bot: I haz the power, 2017-04-08 01:03:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-08 01:03:10]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[DaleCurtis, 2017-04-10 19:37:09]

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

[DaleCurtis, 2017-04-10 19:37:10]

+liberato who might know of a device where a transform matrix is needed or have
further insight here, here's the copy method for when we have a transform
matrix. I don't know either if that's handled automatically or not.

https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_cop...

[liberato (no reviews please), 2017-04-10 19:57:53]

liberato@chromium.org changed reviewers:
+ tobiasjs@chromium.org

[liberato (no reviews please), 2017-04-10 19:58:42]

On 2017/04/10 19:37:10, DaleCurtis wrote:
> +liberato who might know of a device where a transform matrix is needed or
have
> further insight here, here's the copy method for when we have a transform
> matrix. I don't know either if that's handled automatically or not.
> 
>
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_cop...

it's been a while, but i believe that |coded_size| should be "size if the
transform matrix were applied already" -- i.e., we pre-multiply (i think -- i
can never remember the order :) ) it with whatever the uv coordinates would have
been without an xform.

so, i think that it's correct to ignore from the renderer.  we'll definitely
want to check, though, since it's been tricky to get right.

unfortunately, i don't remember which devices do this.  some samsungs do.  i'll
check my s6 to see if it's non-identity.

+tobiasjs, who did a lot of the work to get transform matrices working.

[Ken Russell (switch to Gerrit), 2017-04-11 02:49:37]

All: thanks for your reviews.

This CL gets the associated code back into a working state and is no worse than
the state of the code before the fix for Issue 672895. It's urgent that this be
merged back to M58 so I'm CQ'ing this now. We can iterate on the solution
further for future releases.

[Ken Russell (switch to Gerrit), 2017-04-11 02:50:08]

The CQ bit was checked by kbr@chromium.org

[Ken Russell (switch to Gerrit), 2017-04-11 02:50:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org, dglazkov@chromium.org
Link to the patchset: https://codereview.chromium.org/2791813003/#ps150001
(title: "Rebased and re-added suppression for Windows NVIDIA.")

[commit-bot: I haz the power, 2017-04-11 02:50:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-11 03:41:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-11 03:41:48]

Try jobs failed on following builders:
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[DaleCurtis, 2017-04-11 18:19:49]

lgtm

[Ken Russell (switch to Gerrit), 2017-04-12 00:23:01]

Test failures should now be addressed. CQ'ing again.

[Ken Russell (switch to Gerrit), 2017-04-12 00:23:08]

The CQ bit was checked by kbr@chromium.org

[Ken Russell (switch to Gerrit), 2017-04-12 00:23:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
dglazkov@chromium.org, dalecurtis@chromium.org, sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/2791813003/#ps210001
(title: "Remove failure expectations on Win/NVIDIA.")

[commit-bot: I haz the power, 2017-04-12 00:23:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 01:23:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 01:23:12]

Try jobs failed on following builders:
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[Ken Russell (switch to Gerrit), 2017-04-12 02:25:45]

The CQ bit was checked by kbr@chromium.org

[Ken Russell (switch to Gerrit), 2017-04-12 02:25:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
dglazkov@chromium.org, dalecurtis@chromium.org, sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/2791813003/#ps230001
(title: "Disallow GPU-GPU copies for GL_RED_INTEGER format textures too.")

[commit-bot: I haz the power, 2017-04-12 02:26:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 03:58:22]

CQ is committing da patch.

Bot data: {"patchset_id": 230001, "attempt_start_ts": 1491963945497200,
"parent_rev": "8354e8c5a28b56b46e05dd85a1dbc14a1d064269", "commit_rev":
"0cc4c62a163318c06636916b7e2b2c6a56f16b74"}

[commit-bot: I haz the power, 2017-04-12 03:59:19]

Description was changed from

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case by
reverting to CopyTextureCHROMIUM in most cases, and allocating the
texture to the correct sizes in others.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case by
reverting to CopyTextureCHROMIUM in most cases, and allocating the
texture to the correct sizes in others.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2791813003
Cr-Commit-Position: refs/heads/master@{#463925}
Committed:
https://chromium.googlesource.com/chromium/src/+/0cc4c62a163318c06636916b7e2b...
==========

[commit-bot: I haz the power, 2017-04-12 03:59:23]

Committed patchset #13 (id:230001) as
https://chromium.googlesource.com/chromium/src/+/0cc4c62a163318c06636916b7e2b...

[Ken Russell (switch to Gerrit), 2017-04-12 16:22:13]

A revert of this CL (patchset #13 id:230001) has been created in
https://codereview.chromium.org/2818493002/ by kbr@chromium.org.

The reason for reverting is: Broke
conformance/extensions/oes-texture-float-with-video.html on Android with NVIDIA
GPUs: http://crbug.com/710874 .
.

[Ken Russell (switch to Gerrit), 2017-04-13 00:12:46]

All: apologies for re-landing this under the same CL -- I usually avoid doing
this -- but the change since the revert is small and I've tested it locally.
CQ'ing patch set 14.

[Ken Russell (switch to Gerrit), 2017-04-13 00:12:56]

Description was changed from

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case by
reverting to CopyTextureCHROMIUM in most cases, and allocating the
texture to the correct sizes in others.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2791813003
Cr-Commit-Position: refs/heads/master@{#463925}
Committed:
https://chromium.googlesource.com/chromium/src/+/0cc4c62a163318c06636916b7e2b...
==========

to

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case by
reverting to CopyTextureCHROMIUM in most cases, and allocating the
texture to the correct sizes in others.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2791813003
Cr-Commit-Position: refs/heads/master@{#463925}
Committed:
https://chromium.googlesource.com/chromium/src/+/0cc4c62a163318c06636916b7e2b...
==========

[Ken Russell (switch to Gerrit), 2017-04-13 00:13:06]

The CQ bit was checked by kbr@chromium.org

[Ken Russell (switch to Gerrit), 2017-04-13 00:13:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
dglazkov@chromium.org, dalecurtis@chromium.org, sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/2791813003/#ps250001
(title: "Disable GPU-GPU copies to floating-point textures on Android.")

[commit-bot: I haz the power, 2017-04-13 00:14:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 01:22:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 01:22:36]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Ken Russell (switch to Gerrit), 2017-04-13 01:41:42]

The CQ bit was checked by kbr@chromium.org

[commit-bot: I haz the power, 2017-04-13 01:42:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 02:35:03]

CQ is committing da patch.

Bot data: {"patchset_id": 250001, "attempt_start_ts": 1492047702373960,
"parent_rev": "88f78352fa3d325662153e2a1e50f46cccce1303", "commit_rev":
"0986e62026891425cb209b61092d091d44ec8812"}

[commit-bot: I haz the power, 2017-04-13 02:35:56]

Description was changed from

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case by
reverting to CopyTextureCHROMIUM in most cases, and allocating the
texture to the correct sizes in others.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2791813003
Cr-Commit-Position: refs/heads/master@{#463925}
Committed:
https://chromium.googlesource.com/chromium/src/+/0cc4c62a163318c06636916b7e2b...
==========

to

==========
Fix broken draw/upload paths from videos to 2D canvas and WebGL.

The fix for Issue 672895 made these video rendering and upload paths
fragile if the video's metadata caused the natural size to differ in
certain ways from the coded size. Make the code robust to this case by
reverting to CopyTextureCHROMIUM in most cases, and allocating the
texture to the correct sizes in others.

Tested with new WebGL conformance test to be incorporated in a
forthcoming roll: https://github.com/KhronosGroup/WebGL/pull/2359 .

BUG=701060
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2791813003
Cr-Original-Commit-Position: refs/heads/master@{#463925}
Committed:
https://chromium.googlesource.com/chromium/src/+/0cc4c62a163318c06636916b7e2b...
Review-Url: https://codereview.chromium.org/2791813003
Cr-Commit-Position: refs/heads/master@{#464253}
Committed:
https://chromium.googlesource.com/chromium/src/+/0986e62026891425cb209b61092d...
==========

[commit-bot: I haz the power, 2017-04-13 02:35:57]

Committed patchset #14 (id:250001) as
https://chromium.googlesource.com/chromium/src/+/0986e62026891425cb209b61092d...