chromeos: decode video into NV12 format instead of RGBA in vaapi decoder

chromeos: decode video into NV12 format instead of RGBA in vaapi decoder

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

There is 2 reasons why NV12 is the most desirable format.
1. The display controller hardware in Intel SoCs support NV12 [1] and YUV422
for hardware overlay.
2. Intel vaapi gpu decoder decodes the encoded video to NV12 first. If we use
YVU420 or YUYV, gpu decoder needs to do additional color conversion or swizzling.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

[1] https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12-display.pdf

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

[dshwang, 2017-02-09 04:17:45]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

BUG=683347
TEST=run relevant unittests, run chrome on
http://www.quirksmode.org/html5/tests/video.html
==========

to

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

BUG=683347
TEST=run relevant unittests, run chrome on
http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=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
==========

[dshwang, 2017-02-09 04:17:47]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

BUG=683347
TEST=run relevant unittests, run chrome on
http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=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

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

BUG=683347
TEST=run relevant unittests, run chrome on
http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[dshwang, 2017-02-09 04:19:50]

dongseong.hwang@intel.com changed reviewers:
+ dcastagna@chromium.org

[dshwang, 2017-02-09 04:19:50]

dcastagna@, as you reviewed in https://codereview.chromium.org/2648633005/, this
part is extracted.

This CL require
cc: UYVY video is not premultiplied rgba
https://codereview.chromium.org/2684073006/
Introduce gfx::BufferFormat::YUYV_422 https://codereview.chromium.org/2689453002
Make OverlayCandidate use gfx::BufferFormat instead of cc::ResourceFormat
https://codereview.chromium.org/2683763003/
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/
https://codereview.chromium.org/2649553003/

[dshwang, 2017-02-09 04:34:55]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

BUG=683347
TEST=run relevant unittests, run chrome on
http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. If enabling experimental
--enable-hardware-overlays,
it saves further power consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=run relevant unittests, run chrome on
http://www.quirksmode.org/html5/tests/video.html with following configurations
* overlay configurations
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
 * no overlay: N/A
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[dshwang, 2017-02-09 04:36:13]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. If enabling experimental
--enable-hardware-overlays,
it saves further power consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=run relevant unittests, run chrome on
http://www.quirksmode.org/html5/tests/video.html with following configurations
* overlay configurations
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
 * no overlay: N/A
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. DrmOverlayValidator checks
overlayable format in runtime.
If enabling experimental --enable-hardware-overlays, it saves further power
consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=run relevant unittests, run chrome on
http://www.quirksmode.org/html5/tests/video.html with following configurations
* overlay configurations
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
 * no overlay: N/A
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Pawel Osciak, 2017-02-14 05:19:55]

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

[Pawel Osciak, 2017-02-14 05:19:56]

Thank you for the patch. Could you please provide details on the following:

- what are the "relevant unittests" you mention in the TEST= line that were run
to test this?
- the description says "no overlay: N/A" for TEST=. Was this change tested
without overlays enabled via flag?

Thank you.

[dshwang, 2017-02-21 22:21:03]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. DrmOverlayValidator checks
overlayable format in runtime.
If enabling experimental --enable-hardware-overlays, it saves further power
consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=run relevant unittests, run chrome on
http://www.quirksmode.org/html5/tests/video.html with following configurations
* overlay configurations
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
 * no overlay: N/A
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. DrmOverlayValidator checks
overlayable format in runtime.
If enabling experimental --enable-hardware-overlays, it saves further power
consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html with
following configurations
* overlay configurations
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
 * no overlay: N/A
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[dshwang, 2017-02-21 22:23:28]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. DrmOverlayValidator checks
overlayable format in runtime.
If enabling experimental --enable-hardware-overlays, it saves further power
consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html with
following configurations
* overlay configurations
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
 * no overlay: N/A
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. DrmOverlayValidator checks
overlayable format in runtime.
If enabling experimental --enable-hardware-overlays, it saves further power
consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html with
following configurations
* overlay configurations
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[dshwang, 2017-02-21 22:23:51]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. DrmOverlayValidator checks
overlayable format in runtime.
If enabling experimental --enable-hardware-overlays, it saves further power
consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html with
following configurations
* overlay configurations
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. DrmOverlayValidator checks
overlayable format in runtime.
If enabling experimental --enable-hardware-overlays, it saves further power
consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html with
following configurations
* overlay configurations
 * without overlay: by default
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[dshwang, 2017-02-21 22:24:44]

Hi Pawel, thank you for reviewing. I changed the description to be more clear.

On 2017/02/14 05:19:56, Pawel Osciak wrote:
> - what are the "relevant unittests" you mention in the TEST= line that were
run
> to test this?

cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture

> - the description says "no overlay: N/A" for TEST=. Was this change tested
> without overlays enabled via flag?

Yes, this change is tested without overlays, which is by default.Th

[dshwang, 2017-06-08 20:55:55]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in YUYV format, instead of
RGBA.

YUYV is 2 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Skylake Acer Chromebook 14 on http://browsertests.herokuapp.com/media/h264.html
* BGRA: 6.6462 W
* YUYV: 6.1729 W
YUYV saves 7.1% power compared to BGRA

In addition, YUYV plane is overlayable in ChromeOS. DrmOverlayValidator checks
overlayable format in runtime.
If enabling experimental --enable-hardware-overlays, it saves further power
consumption.
* BGRA with overlay: 6.1734 W
* YUYV with overlay: 6.0593 W

Say in English
* BGRA with overlay saves 7.1% power compared to BGRA
* YUYV with overlay saves 1.8% power compared to YUYV

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html with
following configurations
* overlay configurations
 * without overlay: by default
 * atomic mode setting: --enable-hardware-overlays --enable-drm-atomic
 * legacy mode setting: --enable-hardware-overlays
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

YUYV is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[dshwang, 2017-06-08 20:58:40]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

YUYV is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[dshwang, 2017-06-08 20:58:58]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

[inactive_dshwang_plz_cc_intel, 2017-06-08 20:59:02]

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

[commit-bot: I haz the power, 2017-06-08 20:59:25]

Dry run: CQ is trying da patch.

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

[dshwang, 2017-06-08 20:59:57]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForHardwarePlanes_YUV422Texture
run samus chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=run samus chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

[dshwang, 2017-06-08 21:05:37]

Hi Pawel and Daniele, could you review again? Now it uses NV12.
This CL is blocked by https://chromium-review.googlesource.com/c/527701/

NV12 has pros and cons compared to YUYV
pros:
* Intel gpu video driver decodes encoded data to NV12 first and then color
conversion to YUYV
* 1.5bytes per pixel instead of 2bytes per pixel
* NV12 code exists already for mac

cons:
* NV12 overlay is available on kernel v4.12. It requires backporting

NV12 is not overlayable now, but YUYV requires redundant color conversion one
more time. I think NV12 is way better, although we need to wait for backporing
for overlay.
I'll share power data soon.

[inactive_dshwang_plz_cc_intel, 2017-06-08 22:11:29]

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

[dshwang, 2017-06-08 22:12:25]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=run samus chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

[commit-bot: I haz the power, 2017-06-08 22:12: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-06-09 00:43:48]

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

[commit-bot: I haz the power, 2017-06-09 00:43:49]

Dry run: This issue passed the CQ dry run.

[inactive_dshwang_plz_cc_intel, 2017-06-12 21:28:31]

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

[commit-bot: I haz the power, 2017-06-12 21:29:04]

Dry run: CQ is trying da patch.

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

[dshwang, 2017-06-12 22:52:33]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

[commit-bot: I haz the power, 2017-06-12 23:47:08]

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

[commit-bot: I haz the power, 2017-06-12 23:47:09]

Dry run: This issue passed the CQ dry run.

[dshwang, 2017-06-14 01:33:27]

dongseong.hwang@intel.com changed reviewers:
+ dnicoara@chromium.org

[dshwang, 2017-06-14 01:35:42]

dnicoara@, could you review?
It switchs RGBA to NV12 for VAAPI gpu video decoding.
NV12 format is supported for all generation Intel SoCs only if the SoCs support
gpu video decoding.
It saves 3.7% power on VP9 and 5.8% power on H264 on Apollolake.

[Daniele Castagna, 2017-06-14 03:04:03]

On 2017/06/14 at 01:35:42, dongseong.hwang wrote:
> dnicoara@, could you review?
> It switchs RGBA to NV12 for VAAPI gpu video decoding.
> NV12 format is supported for all generation Intel SoCs only if the SoCs
support gpu video decoding.
> It saves 3.7% power on VP9 and 5.8% power on H264 on Apollolake.

I think Pawel should have a say in this.
IIRC YUV buffers coming from ARC++ are YVU420, have you tried using that format
instead of NV12? Is there any reason to prefer NV12?

[dshwang, 2017-06-14 20:35:57]

On 2017/06/14 03:04:03, Daniele Castagna wrote:
> On 2017/06/14 at 01:35:42, dongseong.hwang wrote:
> > dnicoara@, could you review?
> > It switchs RGBA to NV12 for VAAPI gpu video decoding.
> > NV12 format is supported for all generation Intel SoCs only if the SoCs
> support gpu video decoding.
> > It saves 3.7% power on VP9 and 5.8% power on H264 on Apollolake.
> 
> I think Pawel should have a say in this.
> IIRC YUV buffers coming from ARC++ are YVU420, have you tried using that
format
> instead of NV12? Is there any reason to prefer NV12?

There is very strong 2 reasons.
1. display controller hardware in Intel SoC support NV12 and YUV422 for hardware
overlay. find "nv12" in
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12...
2. Intel vaapi gpu decoder decodes it to NV12 first. If we use YVU420 or YUYV,
gpu decoder needs to do additional color conversion or swizzling.

3 plane YVU420 is good format but it loses hardware overlay and consume little
more power.

[Daniele Castagna, 2017-06-15 19:21:56]

On 2017/06/14 at 20:35:57, dongseong.hwang wrote:
> On 2017/06/14 03:04:03, Daniele Castagna wrote:
> > On 2017/06/14 at 01:35:42, dongseong.hwang wrote:
> > > dnicoara@, could you review?
> > > It switchs RGBA to NV12 for VAAPI gpu video decoding.
> > > NV12 format is supported for all generation Intel SoCs only if the SoCs
> > support gpu video decoding.
> > > It saves 3.7% power on VP9 and 5.8% power on H264 on Apollolake.
> > 
> > I think Pawel should have a say in this.
> > IIRC YUV buffers coming from ARC++ are YVU420, have you tried using that
format
> > instead of NV12? Is there any reason to prefer NV12?
> 
> There is very strong 2 reasons.
> 1. display controller hardware in Intel SoC support NV12 and YUV422 for
hardware overlay. find "nv12" in
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12...
> 2. Intel vaapi gpu decoder decodes it to NV12 first. If we use YVU420 or YUYV,
gpu decoder needs to do additional color conversion or swizzling.
> 
> 3 plane YVU420 is good format but it loses hardware overlay and consume little
more power.

Got it. Thanks for the explanation. Pawel is aiming at reviewing this CL by the
end of the week.

[Pawel Osciak, 2017-06-16 04:59:42]

Thank you for the patch. I would like to give this some testing first, my
understanding is that in order to do that, I should apply the following set of
the patches in the given order:


1. https://codereview.chromium.org/2684073006/
2. https://codereview.chromium.org/2689453002
3. https://codereview.chromium.org/2683763003/
4. https://codereview.chromium.org/2649553003/
5. This CL.

Unfortunately, they don't apply cleanly. Would you perhaps be able to update
them to apply on ToT please? Thank you!

[dshwang, 2017-06-16 21:04:13]

On 2017/06/16 04:59:42, Pawel Osciak wrote:
> Thank you for the patch. I would like to give this some testing first, my
> understanding is that in order to do that, I should apply the following set of
> the patches in the given order:
> 
> 
> 1. https://codereview.chromium.org/2684073006/
> 2. https://codereview.chromium.org/2689453002
> 3. https://codereview.chromium.org/2683763003/
> 4. https://codereview.chromium.org/2649553003/
> 5. This CL.
> 
> Unfortunately, they don't apply cleanly. Would you perhaps be able to update
> them to apply on ToT please? Thank you!

Hi Pawel,
no this CL doesn't depend on any CLs. This CL can be applied cleanly by itself
in chromium code. However, it requires minigbm change;
https://chromium-review.googlesource.com/c/527701/

Sorry for confusing. 1,2,3,4 were YUYV preparation but after discussion with
Daniele, I made decision to go to NV12 directly, because NV12 is least bandwidth
and overlayable.

[Pawel Osciak, 2017-06-20 08:42:12]

On 2017/06/16 21:04:13, dshwang wrote:
> On 2017/06/16 04:59:42, Pawel Osciak wrote:
> > Thank you for the patch. I would like to give this some testing first, my
> > understanding is that in order to do that, I should apply the following set
of
> > the patches in the given order:
> > 
> > 
> > 1. https://codereview.chromium.org/2684073006/
> > 2. https://codereview.chromium.org/2689453002
> > 3. https://codereview.chromium.org/2683763003/
> > 4. https://codereview.chromium.org/2649553003/
> > 5. This CL.
> > 
> > Unfortunately, they don't apply cleanly. Would you perhaps be able to update
> > them to apply on ToT please? Thank you!
> 
> Hi Pawel,
> no this CL doesn't depend on any CLs. This CL can be applied cleanly by itself
> in chromium code. However, it requires minigbm change;
> https://chromium-review.googlesource.com/c/527701/
> 
> Sorry for confusing. 1,2,3,4 were YUYV preparation but after discussion with
> Daniele, I made decision to go to NV12 directly, because NV12 is least
bandwidth
> and overlayable.

Thank you. Could you explain the status of
https://chromium-review.googlesource.com/c/527701/ please?

It appears that there are still some unsolved issues in order to be able to
retain compatibility with other platforms, as well as software/non-accelerated
playback?
I tried this on Pixel 2/samus and saw failure allocating buffers, and the
resulting software fallback had red tint...

[inactive_dshwang_plz_cc_intel, 2017-06-21 22:16:44]

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

[commit-bot: I haz the power, 2017-06-21 22:17:06]

Dry run: CQ is trying da patch.

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

[dshwang, 2017-06-21 22:18:16]

dongseong.hwang@intel.com changed reviewers:
+ gurchetansingh@chromium.org

[dshwang, 2017-06-21 22:20:51]

On 2017/06/20 08:42:12, Pawel Osciak wrote:
> Thank you. Could you explain the status of
> https://chromium-review.googlesource.com/c/527701/ please?

The CL is blocked by mesa issue, which it can be resolved by
https://chromium-review.googlesource.com/c/378198/
The CL is under review.

> It appears that there are still some unsolved issues in order to be able to
> retain compatibility with other platforms, as well as software/non-accelerated
> playback?
> I tried this on Pixel 2/samus and saw failure allocating buffers, and the
> resulting software fallback had red tint...

Nice catch. The new patch set fixes the red tint issue.
After the minigbm CL is landed, failure allocating buffers won't happen.

[commit-bot: I haz the power, 2017-06-21 23:04:27]

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

[commit-bot: I haz the power, 2017-06-21 23:04:28]

Dry run: 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_...)

[inactive_dshwang_plz_cc_intel, 2017-06-21 23:16:51]

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

[dshwang, 2017-06-21 23:17:06]

Patchset #5 (id:80001) has been deleted

[commit-bot: I haz the power, 2017-06-21 23:17:10]

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-06-22 00:25:23]

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

[commit-bot: I haz the power, 2017-06-22 00:25:24]

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

[dshwang, 2017-06-22 17:25:06]

Hi Pawel, the minigbm patch is landing and the mesa patch was landed.
Would you mind to take a look again?

[dshwang, 2017-06-22 17:28:20]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

There is 2 reasons why NV12 is the best format.
1. The display controller hardware in Intel SoCs support NV12 [1] and YUV422
for hardware overlay.
2. Intel vaapi gpu decoder decodes the encoded video to NV12 first. If we use
YVU420 or YUYV, gpu decoder needs to do additional color conversion or
swizzling.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

[1]
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12...

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

[dshwang, 2017-06-22 17:29:07]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

There is 2 reasons why NV12 is the best format.
1. The display controller hardware in Intel SoCs support NV12 [1] and YUV422
for hardware overlay.
2. Intel vaapi gpu decoder decodes the encoded video to NV12 first. If we use
YVU420 or YUYV, gpu decoder needs to do additional color conversion or
swizzling.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

[1]
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12...

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

There is 2 reasons why NV12 is the most desirable format.
1. The display controller hardware in Intel SoCs support NV12 [1] and YUV422
for hardware overlay.
2. Intel vaapi gpu decoder decodes the encoded video to NV12 first. If we use
YVU420 or YUYV, gpu decoder needs to do additional color conversion or
swizzling.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

[1]
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12...

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

[gurchetansingh, 2017-06-22 23:31:59]

"chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA"

Can you say what this patch does in particular rather?  Does this patch only
work on Intel devices?

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

https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:67: bool is_dual_gmb =
video_frame->mailbox_holder(1).texture_target ==
What is a "dual gmb", one that as more than plane or more than one kernel buffer
backing it?

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
File media/gpu/vaapi_drm_picture.cc (right):

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:111: // crbug.com/683347
Shouldn't you be able to query drm plane resources whether or not this
particular kernel supports scanout?  We'll have certain  kernels (v3.18) that
will not support NV12 and other kernels (v4.4) that will

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:114: gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
What GBM use flags is the NV12 buffer allocated with?

[dshwang, 2017-06-23 01:05:12]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA.

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

There is 2 reasons why NV12 is the most desirable format.
1. The display controller hardware in Intel SoCs support NV12 [1] and YUV422
for hardware overlay.
2. Intel vaapi gpu decoder decodes the encoded video to NV12 first. If we use
YVU420 or YUYV, gpu decoder needs to do additional color conversion or
swizzling.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

[1]
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12...

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

There is 2 reasons why NV12 is the most desirable format.
1. The display controller hardware in Intel SoCs support NV12 [1] and YUV422
for hardware overlay.
2. Intel vaapi gpu decoder decodes the encoded video to NV12 first. If we use
YVU420 or YUYV, gpu decoder needs to do additional color conversion or
swizzling.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

[1]
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12...

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

[dshwang, 2017-06-23 01:07:05]

On 2017/06/22 23:31:59, gurchetansingh wrote:
> Can you say what this patch does in particular rather?  Does this patch only
> work on Intel devices?

Thx for reviewing. Yes, as only Intel uses vaapi gpu decoder, this CL affects
only Intel.

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

https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:67: bool is_dual_gmb =
video_frame->mailbox_holder(1).texture_target ==
On 2017/06/22 23:31:59, gurchetansingh wrote:
> What is a "dual gmb", one that as more than plane or more than one kernel
buffer
> backing it?

According to NV12_DUAL_GMB comment, it consists of One R8 and one RG88 GMB
https://cs.chromium.org/chromium/src/media/renderers/gpu_video_accelerator_fa...

It's used by GpuMemoryBufferVideoFramePool in the case the platform cannot
support real native NV12 GBM.

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
File media/gpu/vaapi_drm_picture.cc (right):

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:111: // crbug.com/683347
On 2017/06/22 23:31:59, gurchetansingh wrote:
> Shouldn't you be able to query drm plane resources whether or not this
> particular kernel supports scanout?  We'll have certain  kernels (v3.18) that
> will not support NV12 and other kernels (v4.4) that will

there is good news and bad news.
bad news: There is a bit messy way. drmModeRmFB() failed with NV12 request.
chromium can try to create NV12 framebuffer in startup time to make it
queryable.

good news: We never use NV12 framebuffer for v3.18. We will enable hardware
overlay only if the kernel version is >=4.4.

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:114: gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
On 2017/06/22 23:31:59, gurchetansingh wrote:
> What GBM use flags is the NV12 buffer allocated with?

gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR.
https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_thread.cc?...

[gurchetansingh, 2017-06-23 03:18:33]

Can you change the subject?  "Decode video into NV12 format instead of RGBA is
vaapi decoder".

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

https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:67: bool is_dual_gmb =
video_frame->mailbox_holder(1).texture_target ==
On 2017/06/23 01:07:05, dshwang wrote:
> On 2017/06/22 23:31:59, gurchetansingh wrote:
> > What is a "dual gmb", one that as more than plane or more than one kernel
> buffer
> > backing it?
> 
> According to NV12_DUAL_GMB comment, it consists of One R8 and one RG88 GMB
>
https://cs.chromium.org/chromium/src/media/renderers/gpu_video_accelerator_fa...
> 
> It's used by GpuMemoryBufferVideoFramePool in the case the platform cannot
> support real native NV12 GBM.

Why is a VideoFrameExternalResources::YUV_RESOURCE returned when  a is_dual_gmb
== true?

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
File media/gpu/vaapi_drm_picture.cc (right):

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:111: // crbug.com/683347
On 2017/06/23 01:07:05, dshwang wrote:
> On 2017/06/22 23:31:59, gurchetansingh wrote:
> > Shouldn't you be able to query drm plane resources whether or not this
> > particular kernel supports scanout?  We'll have certain  kernels (v3.18)
that
> > will not support NV12 and other kernels (v4.4) that will
> 
> there is good news and bad news.
> bad news: There is a bit messy way. drmModeRmFB() failed with NV12 request.
> chromium can try to create NV12 framebuffer in startup time to make it
> queryable.
> 
> good news: We never use NV12 framebuffer for v3.18. We will enable hardware
> overlay only if the kernel version is >=4.4.

I don't understand.  drmModeGetPlaneResources should give you all the
information you need.  Chrome already queries from it (see
hardware_display_plane_manager.cc).  Can you access that info somehow?

The current method is Chrome tries drmModeAddFb with a NV12 framebuffer and it
fails on start up?  Where is this code?  IMO that's much more hacky than
drmModeGetPlaneResources (since drmModeAddFb could fail for a number of
reasons).

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:114: gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
On 2017/06/23 01:07:05, dshwang wrote:
> On 2017/06/22 23:31:59, gurchetansingh wrote:
> > What GBM use flags is the NV12 buffer allocated with?
> 
> gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR.
>
https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_thread.cc?...

That's incorrect.  NV12 is Y-tiled in the upstream kernel, and we composite
(texture) from this buffer.  Are you only allocating LINEAR b/c of the
BO_USE_TEXTURE issue in minigbm?  Can you try the minigbm fix I suggested?

[gurchetansingh, 2017-06-23 03:28:15]

On 2017/06/23 03:18:33, gurchetansingh wrote:
> Can you change the subject?  "Decode video into NV12 format instead of RGBA is
> vaapi decoder".
> 
>
https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
> File cc/resources/video_resource_updater.cc (right):
> 
>
https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
> cc/resources/video_resource_updater.cc:67: bool is_dual_gmb =
> video_frame->mailbox_holder(1).texture_target ==
> On 2017/06/23 01:07:05, dshwang wrote:
> > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > What is a "dual gmb", one that as more than plane or more than one kernel
> > buffer
> > > backing it?
> > 
> > According to NV12_DUAL_GMB comment, it consists of One R8 and one RG88 GMB
> >
>
https://cs.chromium.org/chromium/src/media/renderers/gpu_video_accelerator_fa...
> > 
> > It's used by GpuMemoryBufferVideoFramePool in the case the platform cannot
> > support real native NV12 GBM.
> 
> Why is a VideoFrameExternalResources::YUV_RESOURCE returned when  a
is_dual_gmb
> == true?
> 
>
https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
> File media/gpu/vaapi_drm_picture.cc (right):
> 
>
https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
> media/gpu/vaapi_drm_picture.cc:111: // crbug.com/683347
> On 2017/06/23 01:07:05, dshwang wrote:
> > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > Shouldn't you be able to query drm plane resources whether or not this
> > > particular kernel supports scanout?  We'll have certain  kernels (v3.18)
> that
> > > will not support NV12 and other kernels (v4.4) that will
> > 
> > there is good news and bad news.
> > bad news: There is a bit messy way. drmModeRmFB() failed with NV12 request.
> > chromium can try to create NV12 framebuffer in startup time to make it
> > queryable.
> > 
> > good news: We never use NV12 framebuffer for v3.18. We will enable hardware
> > overlay only if the kernel version is >=4.4.
> 
> I don't understand.  drmModeGetPlaneResources should give you all the
> information you need.  Chrome already queries from it (see
> hardware_display_plane_manager.cc).  Can you access that info somehow?
> 
> The current method is Chrome tries drmModeAddFb with a NV12 framebuffer and it
> fails on start up?  Where is this code?  IMO that's much more hacky than
> drmModeGetPlaneResources (since drmModeAddFb could fail for a number of
> reasons).
> 
>
https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
> media/gpu/vaapi_drm_picture.cc:114:
gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
> On 2017/06/23 01:07:05, dshwang wrote:
> > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > What GBM use flags is the NV12 buffer allocated with?
> > 
> > gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR.
> >
>
https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_thread.cc?...
> 
> That's incorrect.  NV12 is Y-tiled in the upstream kernel, and we composite
> (texture) from this buffer.  Are you only allocating LINEAR b/c of the
> BO_USE_TEXTURE issue in minigbm?  Can you try the minigbm fix I suggested?

Oops my mistake "Decode video into NV12 format instead of RGBA in vaapi
decoder".

[gurchetansingh, 2017-06-23 03:28:16]

On 2017/06/23 03:18:33, gurchetansingh wrote:
> Can you change the subject?  "Decode video into NV12 format instead of RGBA is
> vaapi decoder".
> 
>
https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
> File cc/resources/video_resource_updater.cc (right):
> 
>
https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
> cc/resources/video_resource_updater.cc:67: bool is_dual_gmb =
> video_frame->mailbox_holder(1).texture_target ==
> On 2017/06/23 01:07:05, dshwang wrote:
> > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > What is a "dual gmb", one that as more than plane or more than one kernel
> > buffer
> > > backing it?
> > 
> > According to NV12_DUAL_GMB comment, it consists of One R8 and one RG88 GMB
> >
>
https://cs.chromium.org/chromium/src/media/renderers/gpu_video_accelerator_fa...
> > 
> > It's used by GpuMemoryBufferVideoFramePool in the case the platform cannot
> > support real native NV12 GBM.
> 
> Why is a VideoFrameExternalResources::YUV_RESOURCE returned when  a
is_dual_gmb
> == true?
> 
>
https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
> File media/gpu/vaapi_drm_picture.cc (right):
> 
>
https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
> media/gpu/vaapi_drm_picture.cc:111: // crbug.com/683347
> On 2017/06/23 01:07:05, dshwang wrote:
> > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > Shouldn't you be able to query drm plane resources whether or not this
> > > particular kernel supports scanout?  We'll have certain  kernels (v3.18)
> that
> > > will not support NV12 and other kernels (v4.4) that will
> > 
> > there is good news and bad news.
> > bad news: There is a bit messy way. drmModeRmFB() failed with NV12 request.
> > chromium can try to create NV12 framebuffer in startup time to make it
> > queryable.
> > 
> > good news: We never use NV12 framebuffer for v3.18. We will enable hardware
> > overlay only if the kernel version is >=4.4.
> 
> I don't understand.  drmModeGetPlaneResources should give you all the
> information you need.  Chrome already queries from it (see
> hardware_display_plane_manager.cc).  Can you access that info somehow?
> 
> The current method is Chrome tries drmModeAddFb with a NV12 framebuffer and it
> fails on start up?  Where is this code?  IMO that's much more hacky than
> drmModeGetPlaneResources (since drmModeAddFb could fail for a number of
> reasons).
> 
>
https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
> media/gpu/vaapi_drm_picture.cc:114:
gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
> On 2017/06/23 01:07:05, dshwang wrote:
> > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > What GBM use flags is the NV12 buffer allocated with?
> > 
> > gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR.
> >
>
https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_thread.cc?...
> 
> That's incorrect.  NV12 is Y-tiled in the upstream kernel, and we composite
> (texture) from this buffer.  Are you only allocating LINEAR b/c of the
> BO_USE_TEXTURE issue in minigbm?  Can you try the minigbm fix I suggested?

Oops my mistake "Decode video into NV12 format instead of RGBA in vaapi
decoder".

[dshwang, 2017-06-23 17:31:50]

Description was changed from

==========
chromeos: gpu video decoder on Intel decodes video in NV12 format, instead of
RGBA

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

There is 2 reasons why NV12 is the most desirable format.
1. The display controller hardware in Intel SoCs support NV12 [1] and YUV422
for hardware overlay.
2. Intel vaapi gpu decoder decodes the encoded video to NV12 first. If we use
YVU420 or YUYV, gpu decoder needs to do additional color conversion or
swizzling.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

[1]
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12...

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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

==========
chromeos: decode video into NV12 format instead of RGBA in vaapi decoder

NV12 is 1.5 bytes per pixel, while BGRA is 4 bytes per pixel. It speeds up and
saves power consumption.

There is 2 reasons why NV12 is the most desirable format.
1. The display controller hardware in Intel SoCs support NV12 [1] and YUV422
for hardware overlay.
2. Intel vaapi gpu decoder decodes the encoded video to NV12 first. If we use
YVU420 or YUYV, gpu decoder needs to do additional color conversion or
swizzling.

This CL is based on Johnson Lin's implementaion
https://codereview.chromium.org/2636433003/

Electro (Apollo Lake) on H264 1080p 30FPS video
* BGRA: 7.87 W
* NV12: 7.41 W
NV12 saves 5.8% power compared to BGRA for H264

Electro (Apollo Lake) on VP9 1080p 30FPS video
* BGRA: 8.34 W
* NV12: 8.03 W
NV12 saves 3.7% power compared to BGRA for VP9

[1]
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12...

BUG=683347
TEST=cc_unittests VideoResourceUpdaterTest.CreateForNV12HardwarePlanes
run samus and lars chromeos on http://www.quirksmode.org/html5/tests/video.html
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;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
==========

[dshwang, 2017-06-23 23:25:14]

Gurchetan, thx for reviewing, which give me several TODO. Could you review my
comment again? I think this CL doesn't have required changes to resolve your
comments. I'll handle all of them later when below stack (e.g. kernel, minigbm,
ozone) is ready.

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

https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:67: bool is_dual_gmb =
video_frame->mailbox_holder(1).texture_target ==
On 2017/06/23 03:18:33, gurchetansingh wrote:
> On 2017/06/23 01:07:05, dshwang wrote:
> > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > What is a "dual gmb", one that as more than plane or more than one kernel
> > buffer
> > > backing it?
> > 
> > According to NV12_DUAL_GMB comment, it consists of One R8 and one RG88 GMB
> >
>
https://cs.chromium.org/chromium/src/media/renderers/gpu_video_accelerator_fa...
> > 
> > It's used by GpuMemoryBufferVideoFramePool in the case the platform cannot
> > support real native NV12 GBM.
> 
> Why is a VideoFrameExternalResources::YUV_RESOURCE returned when  a
is_dual_gmb
> == true?

YUV_RESOURCE creates YUVVideoDrawQuad, whose shader lookup y, u, v texture
separately. Of course, it never be overlay candidate.

RGB_RESOURCE creates TextureDrawQuad, whose shader lookup just rgba texture,
because EGL_EXT_image_dma_buf_import convert YUV to RGBA under the hood.
https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_image_dma_buf_imp...

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
File media/gpu/vaapi_drm_picture.cc (right):

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:111: // crbug.com/683347
On 2017/06/23 03:18:33, gurchetansingh wrote:
> On 2017/06/23 01:07:05, dshwang wrote:
> > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > Shouldn't you be able to query drm plane resources whether or not this
> > > particular kernel supports scanout?  We'll have certain  kernels (v3.18)
> that
> > > will not support NV12 and other kernels (v4.4) that will
> > 
> > there is good news and bad news.
> > bad news: There is a bit messy way. drmModeRmFB() failed with NV12 request.
> > chromium can try to create NV12 framebuffer in startup time to make it
> > queryable.
> > 
> > good news: We never use NV12 framebuffer for v3.18. We will enable hardware
> > overlay only if the kernel version is >=4.4.
> 
> I don't understand.  drmModeGetPlaneResources should give you all the
> information you need.  Chrome already queries from it (see
> hardware_display_plane_manager.cc).  Can you access that info somehow?
> 
> The current method is Chrome tries drmModeAddFb with a NV12 framebuffer and it
> fails on start up?  Where is this code?  IMO that's much more hacky than
> drmModeGetPlaneResources (since drmModeAddFb could fail for a number of
> reasons).

Oh, that's good to know. It works. I'll make
ClientNativePixmapFactoryDmabuf::IsConfigurationSupported() reflect the result
of drmModeGetPlaneResources in runtime in another CL.

Note: drmModeGetPlaneResources say the following format list is overlayable in
Apollo Lake.
DRM_FORMAT_C8
DRM_FORMAT_RGB565
DRM_FORMAT_XRGB8888
DRM_FORMAT_XBGR8888
DRM_FORMAT_ARGB8888
DRM_FORMAT_ABGR8888
DRM_FORMAT_XRGB2101010
DRM_FORMAT_XBGR2101010
DRM_FORMAT_YUYV
DRM_FORMAT_YVYU
DRM_FORMAT_UYVY
DRM_FORMAT_VYUY

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:114: gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
On 2017/06/23 03:18:33, gurchetansingh wrote:
> On 2017/06/23 01:07:05, dshwang wrote:
> > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > What GBM use flags is the NV12 buffer allocated with?
> > 
> > gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR.
> >
>
https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_thread.cc?...
> 
> That's incorrect.  NV12 is Y-tiled in the upstream kernel, and we composite
> (texture) from this buffer. 

Does "NV12 is Y-tiled in the upstream kernel" mean that scanout framebuffer need
y-tiling? That's true, but it requires on-going patches about NV12 in upstream
kernel.
https://patchwork.freedesktop.org/series/25377/

> Are you only allocating LINEAR b/c of the
> BO_USE_TEXTURE issue in minigbm? 

It's because vaapi requires linear buffer currently.
gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR to
BO_USE_LINEAR.
If I use tiling buffer, some glitch happen because vaapi lib cannot handle it.
I'll raise this issue to intel media team, but it will take long time. If this
CL is landed, it will help me a lot to provide reproducible test to the team.

> Can you try the minigbm fix I suggested?

Yes, I submitted https://chromium-review.googlesource.com/c/546884/

[gurchetansingh, 2017-06-24 01:50:48]

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

https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:67: bool is_dual_gmb =
video_frame->mailbox_holder(1).texture_target ==
On 2017/06/23 23:25:14, dshwang wrote:
> On 2017/06/23 03:18:33, gurchetansingh wrote:
> > On 2017/06/23 01:07:05, dshwang wrote:
> > > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > > What is a "dual gmb", one that as more than plane or more than one
kernel
> > > buffer
> > > > backing it?
> > > 
> > > According to NV12_DUAL_GMB comment, it consists of One R8 and one RG88 GMB
> > >
> >
>
https://cs.chromium.org/chromium/src/media/renderers/gpu_video_accelerator_fa...
> > > 
> > > It's used by GpuMemoryBufferVideoFramePool in the case the platform cannot
> > > support real native NV12 GBM.
> > 
> > Why is a VideoFrameExternalResources::YUV_RESOURCE returned when  a
> is_dual_gmb
> > == true?
> 
> YUV_RESOURCE creates YUVVideoDrawQuad, whose shader lookup y, u, v texture
> separately. Of course, it never be overlay candidate.
> 
> RGB_RESOURCE creates TextureDrawQuad, whose shader lookup just rgba texture,
> because EGL_EXT_image_dma_buf_import convert YUV to RGBA under the hood.
>
https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_image_dma_buf_imp...

How does whether it's a YUVVideoDrawQuad or a TextureDrawQuad affect the shaders
and EGL import?  Can you point me to the shader code?

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
File media/gpu/vaapi_drm_picture.cc (right):

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:111: // crbug.com/683347
On 2017/06/23 23:25:14, dshwang wrote:
> On 2017/06/23 03:18:33, gurchetansingh wrote:
> > On 2017/06/23 01:07:05, dshwang wrote:
> > > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > > Shouldn't you be able to query drm plane resources whether or not this
> > > > particular kernel supports scanout?  We'll have certain  kernels (v3.18)
> > that
> > > > will not support NV12 and other kernels (v4.4) that will
> > > 
> > > there is good news and bad news.
> > > bad news: There is a bit messy way. drmModeRmFB() failed with NV12
request.
> > > chromium can try to create NV12 framebuffer in startup time to make it
> > > queryable.
> > > 
> > > good news: We never use NV12 framebuffer for v3.18. We will enable
hardware
> > > overlay only if the kernel version is >=4.4.
> > 
> > I don't understand.  drmModeGetPlaneResources should give you all the
> > information you need.  Chrome already queries from it (see
> > hardware_display_plane_manager.cc).  Can you access that info somehow?
> > 
> > The current method is Chrome tries drmModeAddFb with a NV12 framebuffer and
it
> > fails on start up?  Where is this code?  IMO that's much more hacky than
> > drmModeGetPlaneResources (since drmModeAddFb could fail for a number of
> > reasons).
> 
> Oh, that's good to know. It works. I'll make
> ClientNativePixmapFactoryDmabuf::IsConfigurationSupported() reflect the result
> of drmModeGetPlaneResources in runtime in another CL.
> 
> Note: drmModeGetPlaneResources say the following format list is overlayable in
> Apollo Lake.
> DRM_FORMAT_C8
> DRM_FORMAT_RGB565
> DRM_FORMAT_XRGB8888
> DRM_FORMAT_XBGR8888
> DRM_FORMAT_ARGB8888
> DRM_FORMAT_ABGR8888
> DRM_FORMAT_XRGB2101010
> DRM_FORMAT_XBGR2101010
> DRM_FORMAT_YUYV
> DRM_FORMAT_YVYU
> DRM_FORMAT_UYVY
> DRM_FORMAT_VYUY
> 

If NV12 is not overlay-able yet, that's fine.  Let's take what the kernel gives
us and not hard-code.  Where is IsConfigurationSupported called in this part of
the code-base?  Maybe we should think about how we want to wire up the
plane/overlay resource sharing first ... dcastagna@ any opinions?

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:114: gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
On 2017/06/23 23:25:14, dshwang wrote:
> On 2017/06/23 03:18:33, gurchetansingh wrote:
> > On 2017/06/23 01:07:05, dshwang wrote:
> > > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > > What GBM use flags is the NV12 buffer allocated with?
> > > 
> > > gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to
GBM_BO_USE_LINEAR.
> > >
> >
>
https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_thread.cc?...
> > 
> > That's incorrect.  NV12 is Y-tiled in the upstream kernel, and we composite
> > (texture) from this buffer. 
> 
> Does "NV12 is Y-tiled in the upstream kernel" mean that scanout framebuffer
need
> y-tiling? That's true, but it requires on-going patches about NV12 in upstream
> kernel.
> https://patchwork.freedesktop.org/series/25377/
> 
> > Are you only allocating LINEAR b/c of the
> > BO_USE_TEXTURE issue in minigbm? 
> 
> It's because vaapi requires linear buffer currently.
> gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR to
> BO_USE_LINEAR.
> If I use tiling buffer, some glitch happen because vaapi lib cannot handle it.
> I'll raise this issue to intel media team, but it will take long time. If this
> CL is landed, it will help me a lot to provide reproducible test to the team.
> 
> > Can you try the minigbm fix I suggested?
> 
> Yes, I submitted https://chromium-review.googlesource.com/c/546884/

So the buffer is 

1) imported into VAAPI, 
2) a video stream is decoded into the NV12 buffer 
3) A glitch happens -- what kind of a glitch?

I noticed on the Android side tiled YV12 buffers on i915 cause artifacts. Since
one of the goals of overlays is end to end lossless tiled compression, IMO we
should really get to the bottom why this not working.  Can you file a bug
against the VAAPI team if they are Chromium committers?

In regards to test ability, we could just wire up one the drm-tests to use the
video API as well.  I don't want to commit things just so we can test.

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:146: // TODO(dshwang): make it true when cros
kernel supports NV12 overlay.
Should be a runtime decision based on plane resources.

[Daniele Castagna, 2017-06-26 18:06:43]

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

https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:67: bool is_dual_gmb =
video_frame->mailbox_holder(1).texture_target ==
On 2017/06/24 at 01:50:48, gurchetansingh wrote:
> On 2017/06/23 23:25:14, dshwang wrote:
> > On 2017/06/23 03:18:33, gurchetansingh wrote:
> > > On 2017/06/23 01:07:05, dshwang wrote:
> > > > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > > > What is a "dual gmb", one that as more than plane or more than one
kernel
> > > > buffer
> > > > > backing it?
> > > > 
> > > > According to NV12_DUAL_GMB comment, it consists of One R8 and one RG88
GMB
> > > >
> > >
> >
https://cs.chromium.org/chromium/src/media/renderers/gpu_video_accelerator_fa...
> > > > 
> > > > It's used by GpuMemoryBufferVideoFramePool in the case the platform
cannot
> > > > support real native NV12 GBM.
> > > 
> > > Why is a VideoFrameExternalResources::YUV_RESOURCE returned when  a
> > is_dual_gmb
> > > == true?
> > 
> > YUV_RESOURCE creates YUVVideoDrawQuad, whose shader lookup y, u, v texture
> > separately. Of course, it never be overlay candidate.
> > 
> > RGB_RESOURCE creates TextureDrawQuad, whose shader lookup just rgba texture,
> > because EGL_EXT_image_dma_buf_import convert YUV to RGBA under the hood.
> >
https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_image_dma_buf_imp...
> 
> How does whether it's a YUVVideoDrawQuad or a TextureDrawQuad affect the
shaders and EGL import?  Can you point me to the shader code?

The EGL import is done earlier by the video decoder. This code just gets a
VideoFrame with one or more mailboxes then creates the resource (a cc concept)
and then VideoLayerImpl::AppendQuads create the quad based on the resource type.

The shader is generated, you probably want to look at
FragmentShader::GetShaderSource()  in cc/output/shader.cc
Take a look at INPUT_COLOR_SOURCE_YUV_TEXTURES for the yuv bits.

https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:69: return is_dual_gmb ?
VideoFrameExternalResources::YUV_RESOURCE
What if a VideoFrame is YUV with 3 textures? Shouldn't we still return a
YUV_RESOURCE here?

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
File media/gpu/vaapi_drm_picture.cc (right):

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:111: // crbug.com/683347
On 2017/06/24 at 01:50:48, gurchetansingh wrote:
> On 2017/06/23 23:25:14, dshwang wrote:
> > On 2017/06/23 03:18:33, gurchetansingh wrote:
> > > On 2017/06/23 01:07:05, dshwang wrote:
> > > > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > > > Shouldn't you be able to query drm plane resources whether or not this
> > > > > particular kernel supports scanout?  We'll have certain  kernels
(v3.18)
> > > that
> > > > > will not support NV12 and other kernels (v4.4) that will
> > > > 
> > > > there is good news and bad news.
> > > > bad news: There is a bit messy way. drmModeRmFB() failed with NV12
request.
> > > > chromium can try to create NV12 framebuffer in startup time to make it
> > > > queryable.
> > > > 
> > > > good news: We never use NV12 framebuffer for v3.18. We will enable
hardware
> > > > overlay only if the kernel version is >=4.4.
> > > 
> > > I don't understand.  drmModeGetPlaneResources should give you all the
> > > information you need.  Chrome already queries from it (see
> > > hardware_display_plane_manager.cc).  Can you access that info somehow?
> > > 
> > > The current method is Chrome tries drmModeAddFb with a NV12 framebuffer
and it
> > > fails on start up?  Where is this code?  IMO that's much more hacky than
> > > drmModeGetPlaneResources (since drmModeAddFb could fail for a number of
> > > reasons).
> > 
> > Oh, that's good to know. It works. I'll make
> > ClientNativePixmapFactoryDmabuf::IsConfigurationSupported() reflect the
result
> > of drmModeGetPlaneResources in runtime in another CL.
> > 
> > Note: drmModeGetPlaneResources say the following format list is overlayable
in
> > Apollo Lake.
> > DRM_FORMAT_C8
> > DRM_FORMAT_RGB565
> > DRM_FORMAT_XRGB8888
> > DRM_FORMAT_XBGR8888
> > DRM_FORMAT_ARGB8888
> > DRM_FORMAT_ABGR8888
> > DRM_FORMAT_XRGB2101010
> > DRM_FORMAT_XBGR2101010
> > DRM_FORMAT_YUYV
> > DRM_FORMAT_YVYU
> > DRM_FORMAT_UYVY
> > DRM_FORMAT_VYUY
> > 
> 
> If NV12 is not overlay-able yet, that's fine.  Let's take what the kernel
gives us and not hard-code.  Where is IsConfigurationSupported called in this
part of the code-base?  Maybe we should think about how we want to wire up the
plane/overlay resource sharing first ... dcastagna@ any opinions?

I'm not sure we want to dynamically pick the decoded format based on what is
supported for HW overlays at this point. A format that can be used as HW
overlays might not be the optimal for compositing/decoding, and we don't know
what we're doing with the decoded buffers at this point.

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:146: // TODO(dshwang): make it true when cros
kernel supports NV12 overlay.
On 2017/06/24 at 01:50:48, gurchetansingh wrote:
> Should be a runtime decision based on plane resources.

This is really just some metadata that ends up in VideoFrame and should be ok to
always leave it to true.
the DrmOverlayValidator in Ozone will make the final call if we can use it as
overlay or if we need compositing.

DS, why did you change this?

[dshwang, 2017-06-28 02:08:34]

new patch set chooses usage in runtime, which resolves all concerns. It moves
the decison logic about scanout for nv12 to ozone gbm.
Daniele, Gurchetan, Pawel, could you review again?

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

https://codereview.chromium.org/2678343011/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:69: return is_dual_gmb ?
VideoFrameExternalResources::YUV_RESOURCE
On 2017/06/26 18:06:43, Daniele Castagna wrote:
> What if a VideoFrame is YUV with 3 textures? Shouldn't we still return a
> YUV_RESOURCE here?

yes, YUV_RESOURCE can handle both y & uv planes and y & u & v planes.
PIXEL_FORMAT_NV12 never has 3 textures tho.

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
File media/gpu/vaapi_drm_picture.cc (right):

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:111: // crbug.com/683347
On 2017/06/26 18:06:43, Daniele Castagna wrote:
> On 2017/06/24 at 01:50:48, gurchetansingh wrote:
> > On 2017/06/23 23:25:14, dshwang wrote:
> > > On 2017/06/23 03:18:33, gurchetansingh wrote:
> > > > On 2017/06/23 01:07:05, dshwang wrote:
> > > > > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > > > > Shouldn't you be able to query drm plane resources whether or not
this
> > > > > > particular kernel supports scanout?  We'll have certain  kernels
> (v3.18)
> > > > that
> > > > > > will not support NV12 and other kernels (v4.4) that will
> > > > > 
> > > > > there is good news and bad news.
> > > > > bad news: There is a bit messy way. drmModeRmFB() failed with NV12
> request.
> > > > > chromium can try to create NV12 framebuffer in startup time to make it
> > > > > queryable.
> > > > > 
> > > > > good news: We never use NV12 framebuffer for v3.18. We will enable
> hardware
> > > > > overlay only if the kernel version is >=4.4.
> > > > 
> > > > I don't understand.  drmModeGetPlaneResources should give you all the
> > > > information you need.  Chrome already queries from it (see
> > > > hardware_display_plane_manager.cc).  Can you access that info somehow?
> > > > 
> > > > The current method is Chrome tries drmModeAddFb with a NV12 framebuffer
> and it
> > > > fails on start up?  Where is this code?  IMO that's much more hacky than
> > > > drmModeGetPlaneResources (since drmModeAddFb could fail for a number of
> > > > reasons).
> > > 
> > > Oh, that's good to know. It works. I'll make
> > > ClientNativePixmapFactoryDmabuf::IsConfigurationSupported() reflect the
> result
> > > of drmModeGetPlaneResources in runtime in another CL.
> > > 
> > > Note: drmModeGetPlaneResources say the following format list is
overlayable
> in
> > > Apollo Lake.
> > > DRM_FORMAT_C8
> > > DRM_FORMAT_RGB565
> > > DRM_FORMAT_XRGB8888
> > > DRM_FORMAT_XBGR8888
> > > DRM_FORMAT_ARGB8888
> > > DRM_FORMAT_ABGR8888
> > > DRM_FORMAT_XRGB2101010
> > > DRM_FORMAT_XBGR2101010
> > > DRM_FORMAT_YUYV
> > > DRM_FORMAT_YVYU
> > > DRM_FORMAT_UYVY
> > > DRM_FORMAT_VYUY
> > > 
> > 
> > If NV12 is not overlay-able yet, that's fine.  Let's take what the kernel
> gives us and not hard-code.  Where is IsConfigurationSupported called in this
> part of the code-base?  Maybe we should think about how we want to wire up the
> plane/overlay resource sharing first ... dcastagna@ any opinions?
> 
> I'm not sure we want to dynamically pick the decoded format based on what is
> supported for HW overlays at this point. A format that can be used as HW
> overlays might not be the optimal for compositing/decoding, and we don't know
> what we're doing with the decoded buffers at this point.

In my opinion, we should choose NV12 always because it's the most efficient
format for vaapi decoders. cc will try to make it overlay later. If the hardware
(i.e. display controller) can overlay it, cc will do it.

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:114: gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
On 2017/06/24 01:50:48, gurchetansingh wrote:
> On 2017/06/23 23:25:14, dshwang wrote:
> > On 2017/06/23 03:18:33, gurchetansingh wrote:
> > > On 2017/06/23 01:07:05, dshwang wrote:
> > > > On 2017/06/22 23:31:59, gurchetansingh wrote:
> > > > > What GBM use flags is the NV12 buffer allocated with?
> > > > 
> > > > gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to
> GBM_BO_USE_LINEAR.
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_thread.cc?...
> > > 
> > > That's incorrect.  NV12 is Y-tiled in the upstream kernel, and we
composite
> > > (texture) from this buffer. 
> > 
> > Does "NV12 is Y-tiled in the upstream kernel" mean that scanout framebuffer
> need
> > y-tiling? That's true, but it requires on-going patches about NV12 in
upstream
> > kernel.
> > https://patchwork.freedesktop.org/series/25377/
> > 
> > > Are you only allocating LINEAR b/c of the
> > > BO_USE_TEXTURE issue in minigbm? 
> > 
> > It's because vaapi requires linear buffer currently.
> > gfx::BufferUsage::GPU_READ_CPU_READ_WRITE is converted to GBM_BO_USE_LINEAR
to
> > BO_USE_LINEAR.
> > If I use tiling buffer, some glitch happen because vaapi lib cannot handle
it.
> > I'll raise this issue to intel media team, but it will take long time. If
this
> > CL is landed, it will help me a lot to provide reproducible test to the
team.
> > 
> > > Can you try the minigbm fix I suggested?
> > 
> > Yes, I submitted https://chromium-review.googlesource.com/c/546884/
> 
> So the buffer is 
> 
> 1) imported into VAAPI, 
> 2) a video stream is decoded into the NV12 buffer 
> 3) A glitch happens -- what kind of a glitch?
> 
> I noticed on the Android side tiled YV12 buffers on i915 cause artifacts.
Since
> one of the goals of overlays is end to end lossless tiled compression, IMO we
> should really get to the bottom why this not working.  Can you file a bug
> against the VAAPI team if they are Chromium committers?
> 
> In regards to test ability, we could just wire up one the drm-tests to use the
> video API as well.  I don't want to commit things just so we can test.  

I could not reproduce glitch anymore after repo sync. next patch uses tiling
texture, which is gfx::BufferUsage::GPU_READ. When I find the reason, I'll
report.

next patch set changes here to choose usage in runtime.

https://codereview.chromium.org/2678343011/diff/100001/media/gpu/vaapi_drm_pi...
media/gpu/vaapi_drm_picture.cc:146: // TODO(dshwang): make it true when cros
kernel supports NV12 overlay.
On 2017/06/26 18:06:43, Daniele Castagna wrote:
> On 2017/06/24 at 01:50:48, gurchetansingh wrote:
> > Should be a runtime decision based on plane resources.
> 
> This is really just some metadata that ends up in VideoFrame and should be ok
to
> always leave it to true.
> the DrmOverlayValidator in Ozone will make the final call if we can use it as
> overlay or if we need compositing.
> 
> DS, why did you change this?

new patch set remove this change. as usage is chosen in runtime, it returns
either true or false in runtime.

[dshwang, 2017-06-28 02:13:16]

Patchset #6 (id:120001) has been deleted

[gurchetansingh, 2017-06-28 20:59:18]

https://codereview.chromium.org/2678343011/diff/140001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/common/drm_util.cc (right):

https://codereview.chromium.org/2678343011/diff/140001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/common/drm_util.cc:538: case GBM_BO_USE_TEXTURING:
After the gbm.h change lands, you still have to roll minigbm in
~/src/third_party/minigbm/ in the Chrome tree before this can land.

[dshwang, 2017-07-13 00:11:16]

relocate to gerrit https://chromium-review.googlesource.com/c/569144/