[Ozone/Linux] Support VA-API on Linux Ozone/Wayland [chromium/src : main]
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Kramer Ge, Maksim Sisov, Robert Kroeger.
2 comments:
Patchset:
Update new patch set to use ozone property for VA-API, also the description in commit message.
File media/gpu/vaapi/vaapi_wrapper.cc:
Patch Set #13, Line 758:
if (ui::GetOzonePlatformId() == ui::kPlatformX11)Thanks. I will look into the ozone property solution for it.
Done
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Robert Kroeger (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Jianhui J Dai, Kramer Ge, Maksim Sisov.
Patch set 14:Code-Review +1
2 comments:
Patchset:
Great. Thanks.
File media/gpu/vaapi/vaapi_picture_factory.cc:
Patch Set #14, Line 53:
#if BUILDFLAG(USE_VAAPI_X11)I note in passing that (afaik) the vaapi code is only on Linux-like platforms and they're all ozone users so the #ifs are probably unnecessary.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Kramer Ge, Maksim Sisov, Robert Kroeger.
1 comment:
File media/gpu/vaapi/vaapi_picture_factory.cc:
Patch Set #14, Line 53:
#if BUILDFLAG(USE_VAAPI_X11)
I note in passing that (afaik) the vaapi code is only on Linux-like platforms and they're all ozone […]
I think ChromeOS don't need the X11 code, for less binary size.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Robert Kroeger (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Jianhui J Dai, Kramer Ge, Maksim Sisov.
1 comment:
File media/gpu/vaapi/vaapi_picture_factory.cc:
Patch Set #14, Line 53:
#if BUILDFLAG(USE_VAAPI_X11)
I think ChromeOS don't need the X11 code, for less binary size.
My understanding is that CrOS no longer defines LINUX. But you should feel free to leave this as is.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Kramer Ge, Maksim Sisov.
1 comment:
Patchset:
@andres friendly ping if review comment from VA-API perspective.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Andres Calderon Jaramillo (Gerrit)
Attention is currently required from: Jianhui J Dai, Kramer Ge, Maksim Sisov.
3 comments:
Patchset:
I'm mostly fine with this CL :)
File media/gpu/vaapi/vaapi_wrapper.cc:
Patch Set #14, Line 746:
#if BUILDFLAG(USE_VAAPI_X11)nit: with this CL, `USE_VAAPI_X11` may no longer be a good name because setting that to true doesn't mean we'll use VA-API/X11 (or even try to). Please change [this](https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/args.gni;l=9-12;drc=cf0720f7ee1734aa91405756c9782b64187a2c02) to:
```
# Build Chrome support for using VA-API over X11. Note that setting this to true is
# not a guarantee that Chrome will use (or even try to use) VA-API over X11. In
# particular, it is possible to build Chrome with support for VA-API over X11 but
# pick Wayland as the Ozone backend at runtime. In this case, Chrome will try to
# use VA-API over DRM.
support_vaapi_over_x11 = is_linux && ozone_platform_x11 &&
(target_cpu == "x86" || target_cpu == "x64") && !is_castos
```Patch Set #14, Line 813:
case gl::kGLImplementationEGLANGLE:Question: did you test with ANGLE on Ozone/Wayland?
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Maksim Sisov (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Jianhui J Dai, Kramer Ge.
Patch set 14:Code-Review +1
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
Patch Set #14, Line 813:
case gl::kGLImplementationEGLANGLE:
Question: did you test with ANGLE on Ozone/Wayland?
It's not supported yet. Landed, reverted, landed, reverted. There are some issues we are addressing now.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Kramer Ge, Maksim Sisov.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
Patch Set #14, Line 813:
case gl::kGLImplementationEGLANGLE:
It's not supported yet. Landed, reverted, landed, reverted. […]
Yes. ANGLE is not supported by Wayland yet.
I could test after it is enabled. VA-API uses drm for Wayland/ANGLE in current code.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Kramer Ge, Maksim Sisov, Robert Kroeger.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
Patch Set #14, Line 746:
#if BUILDFLAG(USE_VAAPI_X11)nit: with this CL, `USE_VAAPI_X11` may no longer be a good name because setting that to true doesn't […]
Done
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Robert Kroeger (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Jianhui J Dai, Kramer Ge.
1 comment:
Patchset:
The CQ failure looks relevant.
Also, note the +1's are no longer sticky.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Kramer Ge, Robert Kroeger.
1 comment:
Patchset:
The CQ failure looks relevant. […]
Pass the CQ dry run. And friendly ping reviewers.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Robert Kroeger (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Jianhui J Dai, Kramer Ge.
Patch set 17:Code-Review +1
1 comment:
Patchset:
ui/ozone lgtm
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Kramer Ge, Maksim Sisov.
4 comments:
File media/gpu/vaapi/vaapi_picture_factory.cc:
Patch Set #14, Line 53:
#if BUILDFLAG(USE_VAAPI_X11)
My understanding is that CrOS no longer defines LINUX. But you should feel free to leave this as is.
Done
File media/gpu/vaapi/vaapi_wrapper.cc:
Patch Set #14, Line 746:
#if BUILDFLAG(USE_VAAPI_X11)
Done
@andres, fixed the naming.
Still need your review on VA-API change to move forward. : )
Patch Set #14, Line 813:
case gl::kGLImplementationEGLANGLE:
Yes. ANGLE is not supported by Wayland yet. […]
Done
File ui/ozone/BUILD.gn:
Patch Set #13, Line 196:
"platform_selection.h",You're opening the door to making a leakier abstraction and I would prefer that we not do that.
Done
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Kramer Ge.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
Patch Set #14, Line 746:
#if BUILDFLAG(USE_VAAPI_X11)
@andres, fixed the naming. […]
@andres, a friendly ping :)
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Maksim Sisov (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Jianhui J Dai, Kramer Ge.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
Patch Set #14, Line 746:
#if BUILDFLAG(USE_VAAPI_X11)
@andres, a friendly ping :)
@andres, do you think we can already land this?
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Andres Calderon Jaramillo (Gerrit)
Attention is currently required from: Jianhui J Dai, Kramer Ge.
Patch set 17:Code-Review +1
15 comments:
Patchset:
Sorry for the delay -- LGTM mostly, but the main thing is applying the changes in CL:3894976 (see my comments in vaapi_wrapper.cc first). After you make those changes, I can also test it on my setup and send it to the CQ.
File media/gpu/vaapi/BUILD.gn:
if (support_vaapi_over_x11) {
deps += [ "//ui/ozone" ]
}nit: You can remove this assuming you make the changes in my other comments.
File media/gpu/vaapi/vaapi_picture_factory.cc:
Patch Set #17, Line 18:
#include "ui/ozone/public/ozone_platform.h"nit: You can remove this assuming you make the changes in my other comments.
ui::OzonePlatform::IsInitialized() && ui::OzonePlatform::GetInstance()
->GetPlatformProperties()
.supports_vaapi_x11Assuming that you apply the changes in CL:3894976, change this to:
```
absl::optional<bool> may_use_vaapi_over_x11 = VaapiWrapper::MayUseVaapiOverX11();
CHECK(may_use_vaapi_over_x11.has_value());
if (may_use_vaapi_over_x11.value()) {
...
}
```gl::kGLImplementationEGLANGLE,
vaapi_implement_for_egl_angleHave you had a chance to test this on Wayland?
if (ui::OzonePlatform::IsInitialized() && ui::OzonePlatform::GetInstance()
->GetPlatformProperties()
.supports_vaapi_x11) {
return GL_TEXTURE_2D;
}Here too:
```
absl::optional<bool> may_use_vaapi_over_x11 = VaapiWrapper::MayUseVaapiOverX11();
CHECK(may_use_vaapi_over_x11.has_value());
if (may_use_vaapi_over_x11.value())
return GL_TEXTURE_2D;
```#if BUILDFLAG(SUPPORT_VAAPI_OVER_X11)
if (ui::OzonePlatform::IsInitialized() && ui::OzonePlatform::GetInstance()
->GetPlatformProperties()
.supports_vaapi_x11) {
return gfx::BufferFormat::RGBX_8888;
}
#endif // BUILDFLAG(SUPPORT_VAAPI_OVER_X11)Here too:
```
absl::optional<bool> may_use_vaapi_over_x11 = VaapiWrapper::MayUseVaapiOverX11();
CHECK(may_use_vaapi_over_x11.has_value());
if (may_use_vaapi_over_x11.value())
return gfx::BufferFormat::RGBX_8888;
```DCHECK(ui::OzonePlatform::GetInstance()
->GetPlatformProperties()
.supports_vaapi_x11);`DCHECK(VaapiWrapper::MayUseVaapiOverX11().value_or(false));`
DCHECK(ui::OzonePlatform::GetInstance()
->GetPlatformProperties()
.supports_vaapi_x11);`DCHECK(VaapiWrapper::MayUseVaapiOverX11().value_or(false));`
File media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc:
#if BUILDFLAG(SUPPORT_VAAPI_OVER_X11)
// GN doesn't understand conditional includes, so we need nogncheck here.
// See crbug.com/1125897.
#include "ui/ozone/public/ozone_platform.h" // nogncheck
#endif // BUILDFLAG(SUPPORT_VAAPI_OVER_X11)nit: You can remove this assuming you make the changes in my comments.
ui::OzonePlatform::IsInitialized() && ui::OzonePlatform::GetInstance()
->GetPlatformProperties()
.supports_vaapi_x11Assuming that you apply the changes in CL:3894976, change this to:
```
absl::optional<bool> may_use_vaapi_over_x11 = VaapiWrapper::MayUseVaapiOverX11();
CHECK(may_use_vaapi_over_x11.has_value());
if (may_use_vaapi_over_x11.value()) {
...
}
```
File media/gpu/vaapi/vaapi_wrapper.cc:
//
// Use DRM display backend if Ozone platform is not initialized. It only
// happens in VA-API unit tests on Linux.nit: Delete this since you've landed CL:3724915.
ui::OzonePlatform::IsInitialized() && ui::OzonePlatform::GetInstance()
->GetPlatformProperties()
.supports_vaapi_x11I'm not too comfortable with this being here and in a bunch of other places -- I'd much rather you compute this very early once (among other reasons, I want to get rid of this Ozone check in the medium term and always use the VA-API DRM backend -- this will make it easier for me to find the places where we rely on this check). There are a few small changes to do this, but instead of putting them in this comment, I've uploaded them as CL:3894976. Can you apply those changes here?
Patch Set #17, Line 814:
switch (gl::GetGLImplementation()) {If you have only tested one GL implementation, can we reject the other ones until we know they work?
File ui/ozone/public/ozone_platform.h:
// TODO(crbug.com/1116701, crbug.com/1326754): add VA-API support for other
// Ozone platforms on Linux. VA-API supports DRM display backend for all
// Ozone platforms, and supports X11 display backend only for Linux
// Ozone/X11. As a temporary solution, `supports_vaapi_x11` is added to
// implicitly indicate the runtime platform is Ozone/X11. It means VA-API is
// supported on all Linux Ozone platforms through DRM display
// backend by default; if `supports_vaapi_x11` is set, VA-API is supported
// through X11 display backend.I'm not sure I understand this TODO. After this CL, the statements `add VA-API support for other Ozone platforms on Linux` and `It means VA-API is supported on all Linux Ozone platforms through DRM display backend by default` are somewhat contradictory.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Kramer Ge.
1 comment:
Patchset:
Sorry for the delay -- LGTM mostly, but the main thing is applying the changes in CL:3894976 (see my […]
Thank you Andres. Let me apply that patch and solve comments.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Maksim Sisov (Gerrit)
Attention is currently required from: Jianhui J Dai, Kramer Ge.
1 comment:
Patchset:
it seems like it doesn't work for me -
[30926:15:0915/130158.713529:ERROR:decoder_selector.cc(334)] GetAndInitializeNextDecoder: initializing Unknown Video Decoder
[30926:15:0915/130158.713739:ERROR:decoder_selector.cc(347)] OnDecoderInitializeDone: VaapiVideoDecoder success=202
[30926:15:0915/130158.713812:ERROR:decoder_selector.cc(354)] Failed to initialize VaapiVideoDecoderDebugging the code, it seems like it stops at [1]
Here is my vainfo
libva info: VA-API version 1.14.0
libva info: Trying to open /usr/lib/x86_64-linux-gnu/dri/radeonsi_drv_video.so
libva info: Found init function __vaDriverInit_1_14
libva info: va_openDriver() returns 0
vainfo: VA-API version: 1.14 (libva 2.12.0)
vainfo: Driver version: Mesa Gallium driver 22.0.5 for AMD Radeon RX 460 Graphics (polaris11, LLVM 13.0.1, DRM 3.42, 5.15.0-47-generic)
vainfo: Supported profile and entrypoints
VAProfileMPEG2Simple : VAEntrypointVLD
VAProfileMPEG2Main : VAEntrypointVLD
VAProfileVC1Simple : VAEntrypointVLD
VAProfileVC1Main : VAEntrypointVLD
VAProfileVC1Advanced : VAEntrypointVLD
VAProfileH264ConstrainedBaseline: VAEntrypointVLD
VAProfileH264ConstrainedBaseline: VAEntrypointEncSlice
VAProfileH264Main : VAEntrypointVLD
VAProfileH264Main : VAEntrypointEncSlice
VAProfileH264High : VAEntrypointVLD
VAProfileH264High : VAEntrypointEncSlice
VAProfileHEVCMain : VAEntrypointVLD
VAProfileHEVCMain : VAEntrypointEncSlice
VAProfileHEVCMain10 : VAEntrypointVLD
VAProfileJPEGBaseline : VAEntrypointVLD
VAProfileNone : VAEntrypointVideoProc
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Kramer Ge, Maksim Sisov.
1 comment:
Patchset:
it seems like it doesn't work for me - […]
Just realized current patchset17 is built with chromium minigbm, with VDA only.
Build args:
```
use_system_minigbm = false
use_intel_minigbm = true
use_amdgpu_minigbm = true
```Running flags:
```
--use-gl=egl \
--ignore-gpu-blocklist \
--disable-gpu-driver-bug-workaround \
--enable-features=VaapiVideoDecoder \
--disable-features=UseChromeOSDirectVideoDecoder
--enable-hardware-overlays="" \
--ozone-platform=wayland
```In order to run on mesa minigbm, we have to use RGBX_8888, will fix in next patchset.
```
uint32_t VaapiPictureFactory::GetGLTextureTarget() {
#if BUILDFLAG(SUPPORT_VAAPI_OVER_X11)
return GL_TEXTURE_2D;
#else
return GL_TEXTURE_EXTERNAL_OES;
#endif // BUILDFLAG(SUPPORT_VAAPI_OVER_X11)
}gfx::BufferFormat VaapiPictureFactory::GetBufferFormat() {
#if BUILDFLAG(SUPPORT_VAAPI_OVER_X11)
return gfx::BufferFormat::RGBX_8888;
#else
return gfx::BufferFormat::YUV_420_BIPLANAR;
#endif // BUILDFLAG(SUPPORT_VAAPI_OVER_X11)
}
```
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Patchset:
Just realized current patchset17 is built with chromium minigbm, with VDA only. […]
To enable VaapiVideoDecoder, there are other future works:
- NV12 buffer issue in system minigbm
- Fix DCHECK in ozone/wayland overlay.
To view, visit change 3646633. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Kramer Ge, Maksim Sisov.
15 comments:
Patchset:
To enable VaapiVideoDecoder, there are other future works: […]
Update patchset-19 to make VDA work on system minigbm.
File media/gpu/vaapi/BUILD.gn:
if (support_vaapi_over_x11) {
deps += [ "//ui/ozone" ]
}nit: You can remove this assuming you make the changes in my other comments.
Done
File media/gpu/vaapi/vaapi_picture_factory.cc:
Patch Set #17, Line 18:
#include "ui/ozone/public/ozone_platform.h"nit: You can remove this assuming you make the changes in my other comments.
Done
ui::OzonePlatform::IsInitialized() && ui::OzonePlatform::GetInstance()
->GetPlatformProperties()
.supports_vaapi_x11
Assuming that you apply the changes in CL: