Open Bug 2021722 Opened 2 months ago Updated 3 days ago

Add Vulkan Video path to FFmpegVideoDecoder in Firefox

Categories

(Core :: Audio/Video: Playback, enhancement)

Product:
Core
Component:
Audio/Video: Playback
Version:
Firefox 150
Type:
enhancement

Tracking

()

Status:
ASSIGNED

People

(Reporter: tboiko, Assigned: tboiko)

References

(Regressed 1 open bug)

Details

(Keywords: leave-open)

Attachments

(8 files, 1 obsolete file)

Bug 2021722 - Added Vulkan Video path to FFmpegVideoDecoder. r?stransky
48 bytes, text/x-phabricator-request
Details | Review
Bug 2021722 - Added hwcontext_vulkan.h and updated ve rsion.h header. r?stransky
48 bytes, text/x-phabricator-request
Details | Review
Bug 2021722 - Added (vendored) Khronos Vulkan-Headers (vulkan-sdk-1.4.341.0) under third_party. r?stransky
48 bytes, text/x-phabricator-request
Details | Review
Bug 2021722 - MOZ/GTK - Prefs, packaging, configure, and GTK platform hooks for Vulkan video. r?stransky
48 bytes, text/x-phabricator-request
Details | Review
Bug 2021722 - GFX - GL/EGL, layers, and compositor IPC for Vulkan video. r?stransky
48 bytes, text/x-phabricator-request
Details | Review
Bug 2021722 - DOM Media - FFmpeg Vulkan Video decode in RDD. r?stransky
48 bytes, text/x-phabricator-request
Details | Review
Bug 2021722 - Sandbox - RDD sandbox policy for Vulkan video. r?stransky
48 bytes, text/x-phabricator-request
Details | Review
Bug 2021722 - MOZ/GTK - Fix DRM_FORMAT_MOD_INVALID redefinition in DMABufFormats.cpp. r?stransky
48 bytes, text/x-phabricator-request
Details | Review

Steps to reproduce:

Play any video in the Firefox browser on (Aarch64 on Linux)

Actual results:

No hardware acceleration for video decoding on Aarch64 (on Linux)

Expected results:

Browser should utilize GPU HW decoder when video decoding if any available on Aarch64 (on Linux)

Added Vulkan Video path to FFmpegVideoDecoder in Firefox:

  • Vulkan video decode: Added Vulkan video decode with DRM modifier handling using FFmpeg Vulkan API to FFmpegVideoDecoder class;
  • Fixes Aarch64 HW detection and decoding (MOZ_FFVPX_AUDIOONLY so it didn't not disable HW decode on Aarch64);
  • Fixed EGL texture creation on DMA-BUF import on UV plane (renderer device may not support RG88 or GR88) (by querying supported plame 1 DRM format with queryDmaBufModifierExt);
  • Compositor–decoder DRM formats: Add IPC negotiation of supported DRM formats, opaque fd semaphore (lists intersection).
  • Synchronization: decode–copy image sync was done via opaque or sync fd binary vk semaphore;
  • Added DRM modifier decoder output: use direct DMA-BUF export when FFmpeg supports VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT, otherwise use copy path; pass DRM modifier list via hw_device_ctx->user_opaque.
  • Added distinction between direct/copy on signle/cross gpu scenarios;
  • Added poll fallback if android_native_fence_fd is not supported;
  • Enabled HW decoding tests for Linux.
Assignee: nobody → tboiko
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

The Bugbug bot thinks this bug should belong to the 'Core::Graphics' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Graphics
Product: Firefox → Core
Component: Graphics → Audio/Video: Playback

Hello Tymur, thanks for the patch!

It looks great but we can't land it 'as is' as it may break existing decoding workflow. I suggest to split it to series of smaller patches and land them independently to integrate the whole solution. I'll go through the patch and suggest which parts should be separated to patches. There are also some parts I'd like to clarify how it's supposed to work and what's the intention.

Also, is that solution supposed to work with other hardware too? I don't have any NVIDIA hardware available, only Intel/AMD (and I can't make it work there - looks like dmabuf frames are not copied properly from vkImage - but that can be investigated later).
Thanks.

Flags: needinfo?(tboiko)

Hi Martin,
Thank you for the comments - I will reply on them.
Yes, this change should work with other hardware too If you have Vulkan SDK and FFmpeg 6.1.1 installed. However, I have only checked it on NVIDIA and Intel GPUs.
What is the exact error log from you configuration?

Flags: needinfo?(tboiko)

(In reply to Tymur Boiko from comment #5)

What is the exact error log from you configuration?

I think I have all Vulkan bits installed - tested mpv vulkan video playback and WebGPU in Firefox. The bug may not be related to Vulkan but it's GL related - it fails to create EGLImage over the exported dmabuf. But tested on AMD only, will check Intel too. But that may not matter for now - it's fine to land it for NVIDIA first and and adjust for other setup.

I need to test this again on other hardware - after that I'll confirm if it works (or I will fix it).

(In reply to Tymur Boiko from comment #7)

I need to test this again on other hardware - after that I'll confirm if it works (or I will fix it).

Well, I think it's okay to just make it work on NVIDIA first as far is doesn't break other arches. I added comments about the EGLImage creation which should be adjusted to make sure the recent VA-API setup works.

Added hwcontext_vulkan.h from FFmpeg tag n6.1.1, n7.1
.1, n8.1.0, version.h from tag n8.1.0

Accidentally created 2 revisions of the vulkan-headers patch, prefer the latter

(D290317)

Attachment #9559010 - Attachment is obsolete: true

Thanks, landing the latter and obsoleted the former.

Pushed by stransky@redhat.com: https://github.com/mozilla-firefox/firefox/commit/e13534a525fb https://hg.mozilla.org/integration/autoland/rev/4f01fc22a0d4 Added (vendored) Khronos Vulkan-Headers (vulkan-sdk-1.4.341.0) under third_party. r=stransky
Pushed by asilaghi@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/340b752d07ea https://hg.mozilla.org/integration/autoland/rev/ccfc3c544041 Revert "Bug 2021722 - Added (vendored) Khronos Vulkan-Headers (vulkan-sdk-1.4.341.0) under third_party. r=stransky" for causing bugzilla lint failures at khronos/vulkan-headers

Backout for causing bugzilla lint failures at khronos/vulkan-headers
Backout link
Push with failure
Failure log
Failure line FileToBugzillaMappingError: Missing Bugzilla component: third_party/khronos/vulkan-headers/LICENSE.md - Set the BUG_COMPONENT in the moz.build file to fix the issue.

Flags: needinfo?(tboiko)

(In reply to Silaghi Andreea from comment #19)

Backout for causing bugzilla lint failures at khronos/vulkan-headers
Backout link
Push with failure
Failure log
Failure line FileToBugzillaMappingError: Missing Bugzilla component: third_party/khronos/vulkan-headers/LICENSE.md - Set the BUG_COMPONENT in the moz.build file to fix the issue.

will look at it.

Flags: needinfo?(tboiko) → needinfo?(stransky)

Add Mozilla-side enablement for Vulkan video on Linux/GTK:

  • Static prefs and package-manifest / build wiring needed for the feature.
  • toolkit/moz.configure and related knobs (including AArch64 hardware decode
    vs MOZ_FFVPX_AUDIOONLY so HW decode is not incorrectly disabled on aarch64).
  • Widget/GTK: GfxInfo / DMA-BUF paths and local vulkantest where applicable
    for capability reporting and validation.

Graphics stack support for Vulkan decode to compositor:

  • EGL / DMA-BUF import fixes for the UV plane when the renderer device may not
    support RG88/GR88 (e.g. query supported plane-1 DRM format via
    queryDmaBufModifierExt).
  • Compositor-decoder DRM format negotiation over IPC (supported DRM formats,
    opaque / sync-fd semaphore; intersection of capability lists).
  • Synchronization between decode and copy paths using opaque or sync-fd
    Vulkan binary semaphores where applicable (CompositorTypes / layers / WR
    bridge).

Decoder and tests:

  • Vulkan video decode in FFmpegVideoDecoder using FFmpeg Vulkan API, with
    DRM modifier handling.
  • DRM modifier output: direct DMA-BUF export when FFmpeg supports
    VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT, otherwise copy path; pass modifier
    lists via hw_device_ctx->user_opaque as needed.
  • Direct vs copy paths for same-GPU vs cross-GPU scenarios.
  • Poll fallback when android_native_fence_fd sync is not available.
  • Enable / extend hardware decoding tests on Linux (mochitest updates).

Sandbox:

  • Allow required syscalls and broker paths for Vulkan video in the RDD process
    (seccomp and filesystem broker policy updates for drivers, sockets, and
    related resources).

Although the revisions are stacked across 4 patches for review and CI convenience, but each commit is independent, applies on its own, and targets a distinct area: RDD sandbox policy, DOM/Media FFmpeg Vulkan decode, GFX compositor/EGL/IPC, and MOZ/GTK enablement (prefs, build, GfxInfo/vulkantest).

D290521 is actually needed for the dmabuf patch as we do EGLimage copy and modifier extraction from GL.

Pushed by stransky@redhat.com: https://github.com/mozilla-firefox/firefox/commit/2c8a2018a02f https://hg.mozilla.org/integration/autoland/rev/b62491de1ada Added (vendored) Khronos Vulkan-Headers (vulkan-sdk-1.4.341.0) under third_party. r=stransky
Flags: needinfo?(stransky)

Isn't this a duplicate of bug#1753129 ?

Duplicate of this bug: 1753129

Tymur, if you update the changeset to latest trunk we can land the patches up to D290521 which means we'll have the support in dmabuf. (I tried to apply it but it fails - better if you do the rebase).

Hi Martin,
Done, rebased all patches onto latest mozilla-central and updated the Phabricator revisions.

These errors are false positives from the bot building each differential in isolation without the prerequisite patches in the stack. The patches are ordered by review domain (prefs/GTK, GFX, DOM media, sandbox) rather than strict dependency order — for example, D290520 (MOZ/GTK) uses GL bindings and IPC fields that are added by D290521 (GFX), so none of the patches can build standalone. FFmpegVulkanVideoDecoder.cpp is additionally a body-include file intentionally not compiled standalone. All errors disappear when the full stack is applied.

The separation into 4 patches was done for review clarity, but the cross-domain dependencies mean independent builds are not possible by design.

However, D290520+D290521 together might build.

Pushed by stransky@redhat.com: https://github.com/mozilla-firefox/firefox/commit/50eed584edf8 https://hg.mozilla.org/integration/autoland/rev/cdd49c4d1c81 MOZ/GTK - Prefs, packaging, configure, and GTK platform hooks for Vulkan video. r=stransky https://github.com/mozilla-firefox/firefox/commit/e930cad7fa2a https://hg.mozilla.org/integration/autoland/rev/2e325646cf3f GFX - GL/EGL, layers, and compositor IPC for Vulkan video. r=stransky,lsalzman
Pushed by rperta@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/c72fae6f46d3 https://hg.mozilla.org/integration/autoland/rev/1743ac0be9bb Revert "Bug 2021722 - GFX - GL/EGL, layers, and compositor IPC for Vulkan video. r=stransky,lsalzman" for causing build bustages

Added #undef DRM_FORMAT_MOD_INVALID before #include <libdrm/drm_fourcc.h> in
DMABufFormats.cpp to avoid a macro redefinition error in unified builds.
Matches the existing pattern in DMABufSurface.cpp.

A potential fix has been submitted https://phabricator.services.mozilla.com/D296451

Tymur, please merge D296451 (the build fix) to D290520 so we can land it (right now there's a fork).

You can use 'hg rebase' tool to rebase D290521 on top of D296451 which makes one patch chain and then use 'hg histedit' to merge D296451 and D290520 and then abandon the orginal 'potential fix'.

If you wish I can also do that.
Thanks.

Flags: needinfo?(stransky) → needinfo?(tboiko)

Hi Martin,
Done — merged D296451 into D290520 and rebased D290521 on top. Updated both revisions in Phabricator. Please double check it

Flags: needinfo?(tboiko)

Thanks, looks good, landing again.

Tymur, looks like the D290520 revision needs an update. Applying to latest trunk I'm getting:

$moz-phab patch D290521 --apply-to bf6944807b55
Revision D290521 has child commits.  Would you like to patch the full stack?. (YES/No/Always)? No
Patching revisions: D290520 D290521
Checked out bf6944807b55
Bookmark set to phab-D290521
patching file widget/gtk/DMABufFormats.cpp
Hunk #1 FAILED at 17
1 out of 3 hunks FAILED -- saving rejects to file widget/gtk/DMABufFormats.cpp.rej
patching file widget/GfxInfoFeatureDefs.inc
Hunk #1 FAILED at 31
1 out of 1 hunks FAILED -- saving rejects to file widget/GfxInfoFeatureDefs.inc.rej
abort: patch failed to apply
CommandError: command 'import' failed to complete successfully
Run moz-phab again with '--trace' to show debugging output

Do you mind to check?
Thanks.

Flags: needinfo?(tboiko)

Sorry, looks like D290520 is outdated.

Sorry, I have updated the branches, please double check again

Flags: needinfo?(tboiko)

D290520 works, D290521 needs update:

$moz-phab patch D290521 --apply-to 081c265a6023
Local VCS (hg) is different from the one defined in the repository (git).
Revision D290521 has child commits.  Would you like to patch the full stack?. (YES/No/Always)? No
Patching revisions: D290520 D290521
Checked out 081c265a6023
Bookmark set to phab-D290521_1
file widget/gtk/vulkantest/vulkantest.cpp already exists
1 out of 1 hunks FAILED -- saving rejects to file widget/gtk/vulkantest/vulkantest.cpp.rej
file widget/gtk/vulkantest/moz.build already exists
1 out of 1 hunks FAILED -- saving rejects to file widget/gtk/vulkantest/moz.build.rej
abort: patch failed to apply
CommandError: command 'import' failed to complete successfully
Run moz-phab again with '--trace' to show debugging output
Flags: needinfo?(tboiko)

Hi Martin, isn't D290520 already landed? If so, we only need D290521 (I submitted 2 of them today, changing that).
Updated D290521 on top of the main Fabricator.

Flags: needinfo?(tboiko)

Sorry, I see they were reverted. I'll reapply both again on the rebased main branch.

I have updated/rebase the patches again, please try again

Note that I'm using the official Mozilla git mirror, not hg

Pushed by stransky@redhat.com: https://github.com/mozilla-firefox/firefox/commit/adeda526b1d5 https://hg.mozilla.org/integration/autoland/rev/f55edc75fd9c MOZ/GTK - Prefs, packaging, configure, and GTK platform hooks for Vulkan video. r=stransky https://github.com/mozilla-firefox/firefox/commit/157217280755 https://hg.mozilla.org/integration/autoland/rev/54abb3e4fbd0 GFX - GL/EGL, layers, and compositor IPC for Vulkan video. r=stransky,lsalzman
Pushed by csabou@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/b1d82a575362 https://hg.mozilla.org/integration/autoland/rev/0deb117694a7 Revert "Bug 2021722 - GFX - GL/EGL, layers, and compositor IPC for Vulkan video. r=stransky,lsalzman" for causing build bustages on TestDMABufSurface

Backed out for causing build bustages on TestDMABufSurface

Push with failures

Failure log

Backout link

Flags: needinfo?(tboiko)

origin/main has 22 fields (including nullable FileHandleWrapper semaphoreFd) -> TestDMABufSurface.cpp calls with 22 args -> works
D290521 adds bool semaphoreFdIsSyncFd as a 23rd field -> test still calls with 22 args -> build fails
Updated (resubmitted) D290520, D290521

Flags: needinfo?(tboiko)

Hi Martin, can we try submitting again. Sorry for the build bustages - I didn't catch the error on my side during common firefox build/run

Pushed by stransky@redhat.com: https://github.com/mozilla-firefox/firefox/commit/c1f5aae79695 https://hg.mozilla.org/integration/autoland/rev/009ece72f8eb MOZ/GTK - Prefs, packaging, configure, and GTK platform hooks for Vulkan video. r=stransky https://github.com/mozilla-firefox/firefox/commit/fc65b037d367 https://hg.mozilla.org/integration/autoland/rev/f6affd94efbc GFX - GL/EGL, layers, and compositor IPC for Vulkan video. r=stransky,lsalzman
Regressions: 2035749

I duplicate a message I already posted in the above regression:

"Looks like Build A has vulkantest (line 54), Build B (baseline) doesn't. That's it — no binary content difference, just a new file.
This is comparing the patched build against the base commit which doesn't have vulkantest. The "failure" is simply that we added a new binary — it's expected and not a real reproducibility issue.

Is that correct?"

Regressions: 2036839

Thanks Tymur and Martin.

I had a chance to look into why this doesn't work on AMD. radeonsi (EGL) does not accept the DMABUF frames because the pitch that EGLCreateImage is called with is not correct.

The pitch comes from FFmpegDescToVA, which sets it to uncrop_width for both planes. AMD surfaces have alignment requirements -- it should pull the pitch from aDesc.layers[0].planes[i].pitch. This path seems to not be used by VAAPI before, so that's why it was never hit. However, I'm not sure if my suggestion may regress V4L2?

Hi Benjamin,
Thanks for the investigation. We can make this fix Vulkan-specific only - uvPitch is now taken from planes[1].pitch only when the HW device type is AV_HWDEVICE_TYPE_VULKAN, leaving VA-API and V4L2 paths unchanged with the prior behavior. Please see the updated commit.

Yes, it works, thanks for the quick fix.

I'm not a Firefox maintainer nor am I familiar with V4L2, but I feel making the fix Vulkan only is a bit hacky. IMO the AVDRMFrameDescriptor coming from V4L2 should be outputting the correct pitch also. I tried confirming this by reading the ffmpeg code, but it seems like there's some ffmpeg patches required for the V4L2 path in Firefox to work, because I don't see how upstream ffmpeg can output a AVDRMFrameDescriptor for V4L2.

Regressions: 2037864
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: