vaapi: Skip 'nvidia-drm' to block `nvidia_drv_video` on NVIDIA GPUs [chromium/src : main]
Jianhui J Dai (Gerrit)
Jianhui J Dai has uploaded this change for review.
vaapi: Skip 'nvidia-drm' to block `nvidia_drv_video` on NVIDIA GPUs
This CL skips 'nvidia-drm' to block `nvidia_drv_video` on NVIDIA GPUs
[1], because it does not support Chromium [2] and can crash in
`vaInitialize()`.
[1] https://github.com/intel/libva/blob/b4870fdfe2d41b579036dae280dfc7a5e732127f/va/drm/va_drm_utils.c#L67
[2] https://github.com/elFarto/nvidia-vaapi-driver/tree/v0.0.10#chrome
Bug: 1478925
Change-Id: Id64db9bc6cf4642e8e3466eb8e44c3a1564f49d9
---
M media/gpu/vaapi/vaapi_wrapper.cc
1 file changed, 8 insertions(+), 54 deletions(-)
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.
Andres Calderon Jaramillo (Gerrit)
Attention is currently required from: Jianhui J Dai.
1 comment:
Patchset:
Q: is this still needed after https://bugs.chromium.org/p/chromium/issues/detail?id=1478925#c31?
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.
Miguel Casas (Gerrit)
File media/gpu/vaapi/vaapi_wrapper.cc:
// Skip 'nvidia-drm' to block `nvidia_drv_video.so` on NVIDIA GPUs [1],
// because it does not support Chromium [2] and can crash in
// `vaInitialize()`, see crbug.com/1478925.
//
// [1]
// https://github.com/intel/libva/blob/b4870fdfe2d41b579036dae280dfc7a5e732127f/va/drm/va_drm_utils.c#L67
// [2] https://github.com/elFarto/nvidia-vaapi-driver/tree/v0.0.10#chrome
if (base::EqualsCaseInsensitiveASCII(version_name, "nvidia-drm")) {I like this solution better than ToT because it's simpler,
but just to confirm, with this we would never ever be able
to use VA-API on any nVidia card, correct?If yes then
a) This CL should update the markdown file associated with
vaapi on linux [1] (it's very outdated though).
b) A better solution might be based on gpu/config [2];
this route would make it evident to users that accelerated
video is disabled on their platform (via chrome:gpu) and
would allow developers to circumvent it (via command line
--disable-gpu-driver-bug-workarounds).WDYT?
[1] https://chromium.googlesource.com/chromium/src/+/main/docs/gpu/vaapi.md#vaapi-on-linux[2] https://source.chromium.org/chromium/chromium/src/+/main:gpu/config/gpu_driver_bug_list.json
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Miguel Casas, Quang Minh Phan.
2 comments:
Patchset:
Q: is this still needed after https://bugs.chromium. […]
Thanks for reviewing.
This CL is less prioritized now that the PartitionAlloc CL has fixed the crash.
However, I will continue working on it to clean up the hard code, and use --disable-gpu-driver-bug-workarounds to block VA on NVIDIA GPUs.
File media/gpu/vaapi/vaapi_wrapper.cc:
// Skip 'nvidia-drm' to block `nvidia_drv_video.so` on NVIDIA GPUs [1],
// because it does not support Chromium [2] and can crash in
// `vaInitialize()`, see crbug.com/1478925.
//
// [1]
// https://github.com/intel/libva/blob/b4870fdfe2d41b579036dae280dfc7a5e732127f/va/drm/va_drm_utils.c#L67
// [2] https://github.com/elFarto/nvidia-vaapi-driver/tree/v0.0.10#chrome
if (base::EqualsCaseInsensitiveASCII(version_name, "nvidia-drm")) {
I like this solution better than ToT because it's simpler, […]
Right.
We should use --disable-gpu-driver-bug-workarounds to disable certain drivers instead of hard-coding the workaround.
I will submit a new patchset for this change.
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Miguel Casas, Quang Minh Phan.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
// Skip 'nvidia-drm' to block `nvidia_drv_video.so` on NVIDIA GPUs [1],
// because it does not support Chromium [2] and can crash in
// `vaInitialize()`, see crbug.com/1478925.
//
// [1]
// https://github.com/intel/libva/blob/b4870fdfe2d41b579036dae280dfc7a5e732127f/va/drm/va_drm_utils.c#L67
// [2] https://github.com/elFarto/nvidia-vaapi-driver/tree/v0.0.10#chrome
if (base::EqualsCaseInsensitiveASCII(version_name, "nvidia-drm")) {
Right. […]
Just realized that `gpu::GpuDriverBugWorkarounds` is not available in `VaapiWrapper`.
It's not trivial to pass `gpu::GpuDriverBugWorkarounds`, especially for Utility Process, see the POC CL [1].
Should we just use media_switches instead, or continue refine and implement the CL [1]?
[1] [POC] Pass `gpu::GpuDriverBugWorkarounds` to `VaapiWrapper` (4945770) · Gerrit Code Review
https://chromium-review.googlesource.com/c/chromium/src/+/4945770
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.
Miguel Casas-Sanchez (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Jianhui J Dai, Miguel Casas, Quang Minh Phan.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
// Skip 'nvidia-drm' to block `nvidia_drv_video.so` on NVIDIA GPUs [1],
// because it does not support Chromium [2] and can crash in
// `vaInitialize()`, see crbug.com/1478925.
//
// [1]
// https://github.com/intel/libva/blob/b4870fdfe2d41b579036dae280dfc7a5e732127f/va/drm/va_drm_utils.c#L67
// [2] https://github.com/elFarto/nvidia-vaapi-driver/tree/v0.0.10#chrome
if (base::EqualsCaseInsensitiveASCII(version_name, "nvidia-drm")) {
Just realized that `gpu::GpuDriverBugWorkarounds` is not available in `VaapiWrapper`. […]
Reading "This CL is less prioritized now that the PartitionAlloc
CL has fixed the crash." - I understand that now we don't need
to avoid touching the nVidia backend, only avoid using it
for de/encoding, correct? IOW ToT has to use the function
IsBrokenNvidiaVaapiDriverPresent() to prevent trying to enumerate
supported profiles because that crashes, correct? But after the
PartitionAlloc fix, it shouldn't crash anymore, correct?If yes and the crash is fixed, then we can just remove all the
IsBrokenNvidiaVaapiDriverPresent() and associated code, let
Chromium use the nVidia backend, and enumerate supported profiles,
and use the gpu-driver-bug-workaround to disable all encode and
decode profiles (something similar to [1]). Again, this will
prevent the normal user from using nVidia backends, will make
such situation evident in chrome:gpu, and will allow developers
to keep developing by running any Chrome build with
--disable-gpu-driver-bug-workarounds.WDYT?
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.
Quang Minh Phan (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Jianhui J Dai, Miguel Casas, Miguel Casas-Sanchez.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
// Skip 'nvidia-drm' to block `nvidia_drv_video.so` on NVIDIA GPUs [1],
// because it does not support Chromium [2] and can crash in
// `vaInitialize()`, see crbug.com/1478925.
//
// [1]
// https://github.com/intel/libva/blob/b4870fdfe2d41b579036dae280dfc7a5e732127f/va/drm/va_drm_utils.c#L67
// [2] https://github.com/elFarto/nvidia-vaapi-driver/tree/v0.0.10#chrome
if (base::EqualsCaseInsensitiveASCII(version_name, "nvidia-drm")) {
IOW ToT has to use the function
IsBrokenNvidiaVaapiDriverPresent() to prevent trying to enumerate
supported profiles because that crashes, correct? But after the
PartitionAlloc fix, it shouldn't crash anymore, correct?
If yes and the crash is fixed, then we can just remove all the
IsBrokenNvidiaVaapiDriverPresent() and associated code, let
Chromium use the nVidia backend, and enumerate supported profiles,
and use the gpu-driver-bug-workaround to disable all encode and
decode profiles (something similar to [1]).
There are actually two known different NVIDIA VA-API drivers in existence. The first one is `vdpau-va-driver` which has been unmaintained for over 10 years now and will always crash with or without the PartitionAlloc fix (and is what's being detected by `IsBrokenNvidiaVaapiDriverPresent`). The second one, `nvidia-vaapi-driver` does not work with Chromium right now but it does not have any crashing bugs either - it just so happens to trigger a PartitionAlloc bug in component builds.
CL that added `IsBrokenNvidiaVaapiDriverPresent`: CL:4790147
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Miguel Casas, Miguel Casas-Sanchez, Quang Minh Phan.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
// Skip 'nvidia-drm' to block `nvidia_drv_video.so` on NVIDIA GPUs [1],
// because it does not support Chromium [2] and can crash in
// `vaInitialize()`, see crbug.com/1478925.
//
// [1]
// https://github.com/intel/libva/blob/b4870fdfe2d41b579036dae280dfc7a5e732127f/va/drm/va_drm_utils.c#L67
// [2] https://github.com/elFarto/nvidia-vaapi-driver/tree/v0.0.10#chrome
if (base::EqualsCaseInsensitiveASCII(version_name, "nvidia-drm")) {
> IOW ToT has to use the function […]
As @phanquangminh217 said, there are two VA-API drivers on NVIDIA GPUs:
- nvidia-vaapi-driver: This driver does not work with Chromium right now, but it does not have any crashing bugs either. No actions are required after CL:4930940.
- vdpau-va-driver: This driver crashes in vaInitialize at the PreSandbox stage. IsBrokenNvidiaVaapiDriverPresent is used to block it (CL:4790147). And Chromium does not have a change to know its supported profiles.
I agree that Chromium should not block any VA-API drivers by hard code. It should leave some flags for developers to continue improving the VA-API driver, even if that driver is unmaintained for years.
I am thinking of using a new feature flag `disable_vaapi_on_nvidia_gpus` in gpu-driver-bug-workaround to implement the same logic as in CL:4790147.
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.
Miguel Casas-Sanchez (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Jianhui J Dai, Miguel Casas, Quang Minh Phan.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
// Skip 'nvidia-drm' to block `nvidia_drv_video.so` on NVIDIA GPUs [1],
// because it does not support Chromium [2] and can crash in
// `vaInitialize()`, see crbug.com/1478925.
//
// [1]
// https://github.com/intel/libva/blob/b4870fdfe2d41b579036dae280dfc7a5e732127f/va/drm/va_drm_utils.c#L67
// [2] https://github.com/elFarto/nvidia-vaapi-driver/tree/v0.0.10#chrome
if (base::EqualsCaseInsensitiveASCII(version_name, "nvidia-drm")) {
As @phanquangminh217 said, there are two VA-API drivers on NVIDIA GPUs: […]
Thanks for the explanation folks, it wasn't fully clear to me that
both drivers failed in such different ways. Some ideas:- For nvidia-vaapi-driver, since it's OK to use for enumeration
- but it fails to actually work, I'd vote for the route of disabling
- via gpu/config/gpu_driver_bug_list.json because it's very evident
- why it's disabled and is straightforward for developers to bypass
- it (via --disable-gpu-driver-bug-workarounds, again).
- For vdpau-va-driver (which is crashy and we don't want to
- touch), could we avoid calling VaapiWrapper::PreSandboxInitialization()
- entirely, by using gpu_prefs.disable_accelerated_video_decode [1] ?
- I'm not sure if that route would be possible, we might need to wire
- gpu_info_.active_gpu() into those lines. Basically I'd rather rely
- on existing gpu/ ways to work, because developers are familiar with
- those. WDYT?
Could I ask for either of you to add a summary of the two kinds
of drivers and their respective state in the doc
https://chromium.googlesource.com/chromium/src/+/main/docs/gpu/vaapi.md#vaapi-on-linux ? (In another CL would be fine) Thanks heaps.
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.
Jianhui J Dai (Gerrit)
Attention is currently required from: Andres Calderon Jaramillo, Miguel Casas, Miguel Casas-Sanchez, Quang Minh Phan.
1 comment:
File media/gpu/vaapi/vaapi_wrapper.cc:
// Skip 'nvidia-drm' to block `nvidia_drv_video.so` on NVIDIA GPUs [1],
// because it does not support Chromium [2] and can crash in
// `vaInitialize()`, see crbug.com/1478925.
//
// [1]
// https://github.com/intel/libva/blob/b4870fdfe2d41b579036dae280dfc7a5e732127f/va/drm/va_drm_utils.c#L67
// [2] https://github.com/elFarto/nvidia-vaapi-driver/tree/v0.0.10#chrome
if (base::EqualsCaseInsensitiveASCII(version_name, "nvidia-drm")) {
Thanks for the explanation folks, it wasn't fully clear to me that […]
Sure.
We will use gpu_prefs/gpu_driver_bug_list to block the drivers, and then update the vaapi.md#vaapi-on-linux.
To view, visit change 4933644. To unsubscribe, or for help writing mail filters, visit settings.