Reland "Fix HTML5 video blurry"

Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes: Skip failed gpu pixel tests until baseline is created.

TBR=dalecurtis@chromium.org,ccameron@chromium.org,dpranke@chromium.org,enne@chromium.org,junov@chromium.org
BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/4bfcc05d589ea7c232c4e207380135c1cf28e3d7
Cr-Commit-Position: refs/heads/master@{#429849}

[dshwang, 2016-10-31 17:45:46]

Description was changed from

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes:

BUG=615325
==========

to

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes:

BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[inactive_dshwang_plz_cc_intel, 2016-10-31 17:45:51]

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

[commit-bot: I haz the power, 2016-10-31 17:46:26]

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, 2016-10-31 19:25:32]

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

[commit-bot: I haz the power, 2016-10-31 19:25:33]

Dry run: This issue passed the CQ dry run.

[dshwang, 2016-11-02 18:47:08]

Description was changed from

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes:

BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes: Skip failed gpu pixel tests until baseline is created.

BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[inactive_dshwang_plz_cc_intel, 2016-11-02 18:47:30]

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

[dshwang, 2016-11-02 18:48:09]

dongseong.hwang@intel.com changed reviewers:
+ enne@chromium.org, kbr@chromium.org

[commit-bot: I haz the power, 2016-11-02 18:48:24]

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, 2016-11-02 18:51:59]

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

[commit-bot: I haz the power, 2016-11-02 18:52:01]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dshwang, 2016-11-02 18:57:26]

kbr@, could you review content/test/gpu/gpu_tests ?
https://codereview.chromium.org/2418173002/ was reverted due to gpu.fyi bot
fail, because slightly different pixel value on the edge of canvas.
https://build.chromium.org/p/chromium.gpu.fyi/builders/Android%20Release%20%2...

enne@, could you review overall again?

Let me keep eyes on gpu bot.
Note: following failure is not related to this CL, because this CL doesn't touch
TextureQuad with blackground. I cannot reporduce it using my Win 8.1 laptop.
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%2...

[inactive_dshwang_plz_cc_intel, 2016-11-02 19:03:19]

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

[commit-bot: I haz the power, 2016-11-02 19:04:08]

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, 2016-11-02 19:16:12]

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

[commit-bot: I haz the power, 2016-11-02 19:16:13]

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

[dshwang, 2016-11-02 19:23:13]

Patchset #3 (id:40001) has been deleted

[inactive_dshwang_plz_cc_intel, 2016-11-02 19:23:20]

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

[commit-bot: I haz the power, 2016-11-02 19:24:08]

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, 2016-11-02 20:25:12]

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

[commit-bot: I haz the power, 2016-11-02 20:25:13]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ken Russell (switch to Gerrit), 2016-11-02 20:41:43]

In principle the pixel_test expectation changes look OK, but it looks like
you'll need some Mac-specific expectations. Please also be prepared to remove
the Fail expectations quickly, as otherwise we will lose significant test
coverage for a long period of time.

There are still cc_unittests failures. Please re-send the review once the bots
are green. Thanks.

[inactive_dshwang_plz_cc_intel, 2016-11-03 16:18:05]

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

[commit-bot: I haz the power, 2016-11-03 16:18:51]

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, 2016-11-03 16:21:19]

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

[commit-bot: I haz the power, 2016-11-03 16:21:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[inactive_dshwang_plz_cc_intel, 2016-11-03 16:46:22]

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

[commit-bot: I haz the power, 2016-11-03 16:46:45]

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

[dshwang, 2016-11-03 17:20:33]

dongseong.hwang@intel.com changed reviewers:
+ aleksandar.stojiljkovic@intel.com

[dshwang, 2016-11-03 17:47:53]

enne@, could you review overall again?
kbr@, could you review content/test/gpu/gpu_tests ?
aleksandar@, could you review Y16 code change? Check diff between patch set 2
and 3.

On 2016/11/02 20:41:43, Ken Russell wrote:
> In principle the pixel_test expectation changes look OK, but it looks like
> you'll need some Mac-specific expectations. Please also be prepared to remove
> the Fail expectations quickly, as otherwise we will lose significant test
> coverage for a long period of time.

I'll remove Fails as soon as new revision appear in
https://console.cloud.google.com/storage/browser/chromium-gpu-archive/referen...

[Ken Russell (switch to Gerrit), 2016-11-03 18:27:09]

lgtm

https://codereview.chromium.org/2461423002/diff/100001/content/test/gpu/gpu_t...
File content/test/gpu/gpu_tests/pixel_expectations.py (right):

https://codereview.chromium.org/2461423002/diff/100001/content/test/gpu/gpu_t...
content/test/gpu/gpu_tests/pixel_expectations.py:50:
self.Fail('Pixel_2DCanvasWebGL', bug=615325)
Add "TODO(dshwang): remove these after new reference images are generated"

[commit-bot: I haz the power, 2016-11-03 19:04:05]

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

[commit-bot: I haz the power, 2016-11-03 19:04:05]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[aleksandar.stojiljkovic, 2016-11-03 19:57:28]

On 2016/11/03 17:47:53, dshwang wrote:
...
> aleksandar@, could you review Y16 code change? Check diff between patch set 2
Non owner lgtm.

https://codereview.chromium.org/2461423002/diff/100001/cc/resources/video_res...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/2461423002/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:477: input_frame_format ==
media::PIXEL_FORMAT_Y16;
Previous check looks sufficient as there is already check in ResourceFormat
VideoResourceUpdater::YuvResourceFormat(int bits, media::VideoPixelFormat
format) related to disable_one_component_textures and Y16.

[dshwang, 2016-11-04 09:23:27]

Description was changed from

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes: Skip failed gpu pixel tests until baseline is created.

BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes: Skip failed gpu pixel tests until baseline is created.

TBR=dalecurtis@chromium.org,ccameron@chromium.org,dpranke@chromium.org,enne@c...
BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[inactive_dshwang_plz_cc_intel, 2016-11-04 09:23:35]

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

[commit-bot: I haz the power, 2016-11-04 09:23:50]

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

[dshwang, 2016-11-04 09:24:14]

Thank you for review

https://codereview.chromium.org/2461423002/diff/100001/cc/resources/video_res...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/2461423002/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:477: input_frame_format ==
media::PIXEL_FORMAT_Y16;
On 2016/11/03 19:57:28, aleksandar.stojiljkovic wrote:
> Previous check looks sufficient as there is already check in ResourceFormat
> VideoResourceUpdater::YuvResourceFormat(int bits, media::VideoPixelFormat
> format) related to disable_one_component_textures and Y16.

I agree on Previous check looks sufficient, but new code is easier to understand
when we convert to RGBA. In addition, next RG texture CL set Highbit's format to
RGBA but doesn't need conversion.

https://codereview.chromium.org/2461423002/diff/100001/content/test/gpu/gpu_t...
File content/test/gpu/gpu_tests/pixel_expectations.py (right):

https://codereview.chromium.org/2461423002/diff/100001/content/test/gpu/gpu_t...
content/test/gpu/gpu_tests/pixel_expectations.py:50:
self.Fail('Pixel_2DCanvasWebGL', bug=615325)
On 2016/11/03 18:27:09, Ken Russell wrote:
> Add "TODO(dshwang): remove these after new reference images are generated"

Done.

[dshwang, 2016-11-04 09:24:23]

The CQ bit was unchecked by dongseong.hwang@intel.com

[dshwang, 2016-11-04 09:24:24]

The CQ bit was checked by dongseong.hwang@intel.com

[dshwang, 2016-11-04 09:24:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
aleksandar.stojiljkovic@intel.com, kbr@chromium.org
Link to the patchset: https://codereview.chromium.org/2461423002/#ps120001
(title: "add TODO comment")

[commit-bot: I haz the power, 2016-11-04 09:24:38]

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, 2016-11-04 10:49:08]

Description was changed from

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes: Skip failed gpu pixel tests until baseline is created.

TBR=dalecurtis@chromium.org,ccameron@chromium.org,dpranke@chromium.org,enne@c...
BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes: Skip failed gpu pixel tests until baseline is created.

TBR=dalecurtis@chromium.org,ccameron@chromium.org,dpranke@chromium.org,enne@c...
BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-11-04 10:49:10]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-11-04 10:50:26]

Description was changed from

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes: Skip failed gpu pixel tests until baseline is created.

TBR=dalecurtis@chromium.org,ccameron@chromium.org,dpranke@chromium.org,enne@c...
BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Reland "Fix HTML5 video blurry"

Original CL: https://codereview.chromium.org/2418173002/

Revert CL: https://codereview.chromium.org/2463103002/

Changes: Skip failed gpu pixel tests until baseline is created.

TBR=dalecurtis@chromium.org,ccameron@chromium.org,dpranke@chromium.org,enne@c...
BUG=615325
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/4bfcc05d589ea7c232c4e207380135c1cf28e3d7
Cr-Commit-Position: refs/heads/master@{#429849}
==========

[commit-bot: I haz the power, 2016-11-04 10:50:28]

Patchset 6 (id:??) landed as
https://crrev.com/4bfcc05d589ea7c232c4e207380135c1cf28e3d7
Cr-Commit-Position: refs/heads/master@{#429849}

[Mike West, 2016-11-04 12:44:55]

A revert of this CL (patchset #6 id:120001) has been created in
https://codereview.chromium.org/2474073004/ by mkwst@chromium.org.

The reason for reverting is: cc_unittests on Windows-7-SP1 failing on
chromium.win/Win7 Tests (dbg)(1):

RendererPixelTest/2.PremultipliedTextureWithBackground
GLRendererPixelTest.TextureQuadBatching
RendererPixelTest/0.PremultipliedTextureWithBackground
GLRendererPixelTest.NonPremultipliedTextureWithBackground
IntersectingQuadGLPixelTest/0.Y16VideoQuads
VideoGLRendererPixelTest.TwoColorY16Rect
IntersectingQuadGLPixelTest/1.Y16VideoQuads

https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%2...
is the first fail

Looks like this reland is the culprit.

BUG=662385.

[dshwang, 2016-11-04 13:19:57]

On 2016/11/04 12:44:55, Mike West wrote:
> A revert of this CL (patchset #6 id:120001) has been created in
> https://codereview.chromium.org/2474073004/ by mailto:mkwst@chromium.org.
> 
> The reason for reverting is: cc_unittests on Windows-7-SP1 failing on
> chromium.win/Win7 Tests (dbg)(1):
> 
> RendererPixelTest/2.PremultipliedTextureWithBackground
> GLRendererPixelTest.TextureQuadBatching
> RendererPixelTest/0.PremultipliedTextureWithBackground
> GLRendererPixelTest.NonPremultipliedTextureWithBackground
> IntersectingQuadGLPixelTest/0.Y16VideoQuads
> VideoGLRendererPixelTest.TwoColorY16Rect
> IntersectingQuadGLPixelTest/1.Y16VideoQuads
> 
>
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%2...
> is the first fail
> 
> Looks like this reland is the culprit.
> 
> BUG=662385.

thx for reverting. Interesting, I couldn't reproduce it using my Win8.1. I
probably need Win7 to reproduce it.