Skip to content

vo/vulkan: sync requested extensions and features from ffmpeg - #16657

Closed
philipl wants to merge 0 commit into
mpv-player:masterfrom
philipl:new-vulkan-features
Closed

vo/vulkan: sync requested extensions and features from ffmpeg#16657
philipl wants to merge 0 commit into
mpv-player:masterfrom
philipl:new-vulkan-features

Conversation

@philipl

@philipl philipl commented Aug 10, 2025

Copy link
Copy Markdown
Member

Over time, the list of requested extensions and features has diverged from
ffmpeg (libhwcontext_vulkan), and while this isn't guaranteed to cause
problems, as it depends on driver behaviour, it definitely makes it hard to
compare validation errors between the two implementations.

There is a large list of feature activations that have to be handled by an
update to libplacebo, which I'm making separately.

@philipl
philipl force-pushed the new-vulkan-features branch from 98b6339 to 4aa360e Compare August 10, 2025 22:02
@philipl philipl changed the title New vulkan features vo/vulkan: sync requested extensions and features from ffmpeg Aug 10, 2025
@github-actions

github-actions Bot commented Aug 10, 2025

Copy link
Copy Markdown

Comment thread video/out/vulkan/context.c Outdated
Comment thread video/out/vulkan/context.c Outdated
@kasper93

Copy link
Copy Markdown
Member

it definitely makes it hard to compare validation errors between the two implementations.

Remember that since those are optional extensions, they might not exists, and validation errors or the implementation should work fine both with them not loaded.

@philipl

philipl commented Aug 10, 2025

Copy link
Copy Markdown
Member Author

it definitely makes it hard to compare validation errors between the two implementations.

Remember that since those are optional extensions, they might not exists, and validation errors or the implementation should work fine both with them not loaded.

For sure. But I mean when doing a side-by-side comparison between ffmpeg and mpv, in which case the Vulkan environment will be the same.

@philipl
philipl force-pushed the new-vulkan-features branch from 4aa360e to 09e660e Compare August 11, 2025 02:36
@philipl
philipl requested a review from kasper93 August 11, 2025 02:36
@philipl
philipl force-pushed the new-vulkan-features branch from 09e660e to 5190061 Compare August 11, 2025 02:37
Comment thread video/out/vulkan/context.c
Comment thread video/out/vulkan/context.c Outdated
@kasper93

kasper93 commented Aug 11, 2025

Copy link
Copy Markdown
Member

For the record about Vulkan headers version supported. Current Android NDK (r28c) have 275 headers. Debian stable (trixie) (released two days ago) has 309, while oldstable (bookworm) has 239. Ubuntu 24.04 (LTS) has 275.

So, if we ignore oldstable, we could bump requirement to 275, but this covers only 2/7 of those extensions. Probably not worth it.

Of course those are only headers, but we aimed to not require side loading them, at least for latest Debian and NDK, which are generally what's lagging behind. Unless there is a need to do so.

@philipl
philipl force-pushed the new-vulkan-features branch from 5190061 to 766cf74 Compare August 11, 2025 03:21
@philipl

philipl commented Aug 11, 2025

Copy link
Copy Markdown
Member Author

For the record about Vulkan headers version supported. Current Android NDK (r28c) have 275 headers. Debian stable (trixie) (released two days ago) has 309, while oldstable (bookworm) has 239. Ubuntu 24.04 (LTS) has 275.

So, if we ignore oldstable, we could bump requirement to 275, but this covers only 2/7 of those extensions. Probably not worth it.

Of course those are only headers, but we aimed to not require side loading them, at least for latest Debian and NDK, which are generally what's lagging behind. Unless there is a need to do so.

Still seems worth bumping as a matter of principle. If we let debian oldstable hold us back, we'll be here forever. There's a bunch of other clean up we can do now that trixie is out, right?

@philipl
philipl force-pushed the new-vulkan-features branch from 766cf74 to 09c3062 Compare August 11, 2025 15:32

@kasper93 kasper93 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I didn't check exact versions, but I trust you on that.

Comment thread video/out/vulkan/context.c

@sfan5 sfan5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't test but it compiles fine for Android.

@philipl

philipl commented Aug 13, 2025

Copy link
Copy Markdown
Member Author

I'm going to take another pass at this later this week to add all the extensions/features I initial proposed to add directly into libplacebo. Based on the discussion around that, they should all be here too.

Comment thread video/out/vulkan/context.c
@philipl
philipl force-pushed the new-vulkan-features branch from 09c3062 to 311aa7a Compare August 16, 2025 04:28
@philipl

philipl commented Aug 16, 2025

Copy link
Copy Markdown
Member Author

I have updated to add the additional extensions/features that I had previously added directly in libplacebo. As discussed, I'm putting all of them here so that no libplacebo changes are made.

@philipl philipl closed this Aug 18, 2025
@philipl
philipl force-pushed the new-vulkan-features branch from 311aa7a to 12a7825 Compare August 18, 2025 15:46
@philipl
philipl deleted the new-vulkan-features branch August 18, 2025 15:47
@philipl
philipl restored the new-vulkan-features branch August 18, 2025 15:47
@philipl
philipl deleted the new-vulkan-features branch August 18, 2025 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants