New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 811912 link

Starred by 7 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 794608



Sign in to add a comment

VP9 video encoding is not available in CrOs Intel devices that should have it

Project Member Reported by mcasas@chromium.org, Feb 13 2018

Issue description

chrome://gpu doesn't show VP9 as available video encoder on Intel
Chromebook devices where it should (i.e. on Kaby Lake devices [1,2,3]).
We should enable, add it to the stainless test waterfall and use it 
for e.g. MediaRecorder and PeerConnections.

[1] http://blog.hacktohell.org/vp9-hardware-decoding-arch-linux/
[2] https://en.wikichip.org/wiki/intel/microarchitectures/kaby_lake#Hardware_Accelerated_Video
[3] https://en.wikichip.org/wiki/intel/microarchitectures/skylake_(client)#Hardware_Accelerated_Video
 
V4L2 already had VP9 and I enabled WebRTC use earlier, see  issue 687650 . Intel VAAPI VP9 discussion was going on  issue 794608  as well, maybe worth merging?

Comment 2 by mcasas@chromium.org, Feb 13 2018

Blocking: 794608

Comment 3 by mcasas@chromium.org, Feb 13 2018

emircan@: Made this guy blocked on  Issue 794608  since this one is wider
in scope.

Comment 4 by mcasas@chromium.org, Feb 13 2018

Owner: mcasas@chromium.org
Status: Assigned (was: Unconfirmed)
#3: s/blocked/blocking/
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ecc37b2952d0915bfb74b11fec4e5a2475cfff3e

commit ecc37b2952d0915bfb74b11fec4e5a2475cfff3e
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue Feb 13 22:02:49 2018

Vaapi decode: correct attributes used for enumeration

ToT enumerates encode capabilities using a number of VAConfigAttrib,
in particular VAConfigAttribEncPackedHeaders, but those are
H.264 specific, which prevents the enumeration of e.g. VP9.
This CL corrects that by only using those attributes for H264 (AVC1)
enumeration.

The said atributes are VA_ENC_PACKED_HEADER* [1,2]
[1] https://github.com/01org/libva/blob/libva-1.8.3/va/va.h#L560
[2] https://github.com/01org/libva/blob/libva-1.8.3/va/va.h#L562

Note that enumerating is not tantamount to using :-) -- that'll
happen in subsequent CLs; but at this point we should enumerate
what the hardware supports. (next is landing autotests)

TEST: tested on soraka w/ simplechrome, correct enumeration.
BUG: 811912
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Id10e7e3f36ae67baceade0ba3d871a249c7be1cd
Reviewed-on: https://chromium-review.googlesource.com/823722
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536498}
[modify] https://crrev.com/ecc37b2952d0915bfb74b11fec4e5a2475cfff3e/media/gpu/vaapi/vaapi_wrapper.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b15526fb53df7b2118d558a3d511ad165cb8898d

commit b15526fb53df7b2118d558a3d511ad165cb8898d
Author: Miguel Casas <mcasas@chromium.org>
Date: Wed Feb 14 15:33:06 2018

Vaapi: baseline trace names and add a video_encode

This CL changes the TRACE_ for accelerated video decode from the
current "Video Decoder" to "media.gpu", since trace categories
are never more than one word, and there seems to be plenty of
categories composed with dot (e.g. cc.debug.quads etc). This
category doesn't seem to be named in Chromium OS tests, so
this change should not affect anyone.

This CL also adds a new entry for Vaapi video encoding.

Bug: 811912
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I170252752ab32c49bb4e479469700be396f6421b
Reviewed-on: https://chromium-review.googlesource.com/916887
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536718}
[modify] https://crrev.com/b15526fb53df7b2118d558a3d511ad165cb8898d/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/b15526fb53df7b2118d558a3d511ad165cb8898d/media/gpu/vaapi/vaapi_video_decode_accelerator.cc
[modify] https://crrev.com/b15526fb53df7b2118d558a3d511ad165cb8898d/media/gpu/vaapi/vaapi_video_encode_accelerator.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f6074258ae47aa33baa3afbacf1eb66e2f789e60

commit f6074258ae47aa33baa3afbacf1eb66e2f789e60
Author: Miguel Casas <mcasas@chromium.org>
Date: Wed Feb 14 21:37:09 2018

vaapi: extract a new BUILD.gn

This CL adds a new //media/gpu/vaapi/BUILD.gn and extracts the necesary
parts of //media/gpu/BUILD.gn into it.  The files in ...vaapi/ include
a few of //media/gpu, so I extracted those into a //media/gpu:common
source_set.

TEST= simplechrome on soraka; CQ should cover this changes nonetheless.

Bug: 811912
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I585e56c1168d8b9438a74ef4ec0514348339108d
Reviewed-on: https://chromium-review.googlesource.com/919301
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536825}
[modify] https://crrev.com/f6074258ae47aa33baa3afbacf1eb66e2f789e60/media/gpu/BUILD.gn
[add] https://crrev.com/f6074258ae47aa33baa3afbacf1eb66e2f789e60/media/gpu/vaapi/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ab1820fd8546c5b9b183ae9d337f8dc8c1f18db

commit 5ab1820fd8546c5b9b183ae9d337f8dc8c1f18db
Author: Miguel Casas <mcasas@chromium.org>
Date: Wed Feb 14 22:21:17 2018

Vaapi: correct trace names

This CL corrects my mistake in r536718: I made the Trace names "media.gpu"
when in reality it should be "media,gpu" (with a comma to indicate both
categories).

sed -i -e's/"media.gpu"/"media,gpu"/g' media/gpu/vaapi/vaapi_video_decode_accelerator.cc media/gpu/v4l2/v4l2_video_decode_accelerator.cc media/gpu/vaapi/vaapi_video_encode_accelerator.cc

TBR=dalecurtis@chromium.org (this CL is acting upon a comment)

Bug: 811912
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I19fb95bfdd45f9fbfda47db477d534895f8243a1
Reviewed-on: https://chromium-review.googlesource.com/919864
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536848}
[modify] https://crrev.com/5ab1820fd8546c5b9b183ae9d337f8dc8c1f18db/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/5ab1820fd8546c5b9b183ae9d337f8dc8c1f18db/media/gpu/vaapi/vaapi_video_decode_accelerator.cc
[modify] https://crrev.com/5ab1820fd8546c5b9b183ae9d337f8dc8c1f18db/media/gpu/vaapi/vaapi_video_encode_accelerator.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a3eeac3a74c9c2056e1b5cce7aeab4758e58953

commit 6a3eeac3a74c9c2056e1b5cce7aeab4758e58953
Author: Chrome Cunningham <chcunningham@chromium.org>
Date: Wed Feb 14 22:44:42 2018

Revert "vaapi: extract a new BUILD.gn"

This reverts commit f6074258ae47aa33baa3afbacf1eb66e2f789e60.

Reason for revert: Broke the build, closed the tree

https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/4265

Original change's description:
> vaapi: extract a new BUILD.gn
> 
> This CL adds a new //media/gpu/vaapi/BUILD.gn and extracts the necesary
> parts of //media/gpu/BUILD.gn into it.  The files in ...vaapi/ include
> a few of //media/gpu, so I extracted those into a //media/gpu:common
> source_set.
> 
> TEST= simplechrome on soraka; CQ should cover this changes nonetheless.
> 
> Bug: 811912
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: I585e56c1168d8b9438a74ef4ec0514348339108d
> Reviewed-on: https://chromium-review.googlesource.com/919301
> Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
> Commit-Queue: Miguel Casas <mcasas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#536825}

TBR=dalecurtis@chromium.org,mcasas@chromium.org,dcastagna@chromium.org

Change-Id: I2b5c7df025e5ff29000d8bfd3f2719cd15506668
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 811912
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/920181
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536853}
[modify] https://crrev.com/6a3eeac3a74c9c2056e1b5cce7aeab4758e58953/media/gpu/BUILD.gn
[delete] https://crrev.com/fd6806b8dae4614967be60035c47303b5ec36d76/media/gpu/vaapi/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b23e641340ea1ebfbe7dda2545deb5c2c1f7e7a2

commit b23e641340ea1ebfbe7dda2545deb5c2c1f7e7a2
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Feb 15 14:04:14 2018

RELAND: Vaapi: extract a new BUILD.gn

Original CL got reverted after breaking linux-chromeos-dbg:
FAILED: viz.service
...
error: undefined reference to 'media::VaapiWrapper::PreSandboxInitialization()'

because the original CL was missing a
  defines = [ "MEDIA_GPU_IMPLEMENTATION" ]
inside the new BUILD.gn

Also this CL removes an extra level of vaapi/ folder in the auto
generated stub files.

Cq-Include-Trybots: master.tryserver.chromium.chromiumos:linux-chromeos-dbg
TBR=dalecurtis@chromium.org (minor changes)

Original CL description -----------------------------------------------

This CL adds a new //media/gpu/vaapi/BUILD.gn and extracts the necesary
parts of //media/gpu/BUILD.gn into it.  The files in ...vaapi/ include
a few of //media/gpu, so I extracted those into a //media/gpu:common
source_set.

TEST= simplechrome on soraka; CQ should cover this changes nonetheless.

Bug: 811912
Change-Id: I76a24913c48cd407131712ff6d573c5ee94fe14e
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/919301
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#536825}
Reviewed-on: https://chromium-review.googlesource.com/919971
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537007}
[modify] https://crrev.com/b23e641340ea1ebfbe7dda2545deb5c2c1f7e7a2/media/gpu/BUILD.gn
[add] https://crrev.com/b23e641340ea1ebfbe7dda2545deb5c2c1f7e7a2/media/gpu/vaapi/BUILD.gn

Just found this bug report(thank you mcasas)

I have create another one recently. https://bugs.chromium.org/p/chromium/issues/detail?id=908658

Shall we close one of this?


Cc: mcasas@chromium.org sreerenj...@intel.corp-partner.google.com
Reassigning to sreerenj.balachandran@ but keeping myself as owner since
sreerenj.balachandran@intel.corp-partner.google.com "is not a project
member" :-?
 Issue 908658  has been merged into this issue.
Thank you Miguel.
I would really appreciate if someone can add my name(sreerenj balachandran <sreerenj.balachandran@intel.com> or <sreerenj.balachandran@intel.corp-partner.google.com > to project member list.

It would be handy to have permission to edit CC list since I wanna add a few names to the bugs I'm opening (a few are going to be opened in the next few days too)
Owner: sreerenj...@intel.com
I have finished the coding for the basic implementation of vp9 encoder. Hopefully by next week I will upload the patches.
Cc: hiroh@chromium.org
@Miguel, @Hiro,
I have defined a new flag in about_flagss.cc similar to VP8
+    {"enable-webrtc-hw-vp9-encoding",
+     flag_descriptions::kWebrtcHwVP9EncodingName,
+     flag_descriptions::kWebrtcHwVP9EncodingDescription, kOsAndroid | kOsCrOS,
+     FEATURE_VALUE_TYPE(features::kWebRtcHWVP9Encoding)},

and updated the enum.xml with  <int value="399039205" label="enable-webrtc-hw-vp9-encoding"/>


For VP8, there seems to be 3 entries in enums.xml,
1: WebRtcHWVP8Encoding:disabled
2: WebRtcHWVP8Encoding:enabled
3: disable-webrtc-hw-vp8-encoding

What is the logic behind this? Do I need to write 3 enums for vp9 ? why is the enum name is "disable-webrtc-hw-vp8-encoding" even though the flag name is "enable-webrtc-hw-vp8-encoding" in about_flags.cc?

Cc: sprang@chromium.org posciak@chromium.org
+posciak@, sprang@,

I don't know the historical reason about the question #19.
#19, re enums.xml, FeatureEntry are automatically recorded via UMA [1],
so each value (disabled, enabled), needs to be added to enums.xml. 
Speculating now, some FeatureEntry were surfaced after command-line
flags, and that's where disable-webrtc-hw-vp8-encoding comes from [2].

Also you don't need to support kOsAndroid for VP9 encoding.

[1] https://cs.chromium.org/chromium/src/chrome/browser/about_flags.cc?type=cs&q=able-webrtc-hw-vp8-encoding&sq=package:chromium&g=0&l=1191
[2] https://cs.chromium.org/chromium/src/content/public/common/content_switches.cc?type=cs&q=disable-webrtc-hw-encoding&sq=package:chromium&g=0&l=871


To give more context, Issue 599650 tracked an early effort at cleaning
up the webrtc/mediarecording hw deco/enco flags: at the time of writing
they were all mixed together, whereas we thought better to have a 
hierarchical approach and separate RTC and MR, e.g. where 
kDisableWebRtcHWEncoding would encompass e.g. kWebRtcHWH264Encoding 
and kWebRtcHWVP8Encoding etc, but would leave untouched the 
MediaRecorder use of the same, for example.  I'm not sure if all this 
has been cleaned out, but I doubt it; moreover the mentioned Issue was
primarily concerned with separating VideoDecode and HwEncoding (the 
former overruled the latter).
@mcasas Thank you for the details.

Sign in to add a comment