diff mbox

[FFmpeg-devel,0/14] Vulkan fixes and improvements

Message ID Mo7hzNo--3-2@lynne.ee
State Accepted
Commit 7f3878828d88c6a8287d54818659e6f70293dabb
Headers show

Commit Message

Lynne Nov. 10, 2021, 7:20 a.m. UTC 
I've extensively fixed and improved our Vulkan codebase, and it's
now up to standard. Since it's a 14-part patchset, I've uploaded
my branch for review here:
https://github.com/cyanreg/FFmpeg/tree/vulkan

The implementation now follows modern Vulkan API usage practices,
fully takes advantage of all the hardware has to offer, it's faster,
much easier to compile, static linking is possible and most importantly,
it's bug-free and extendable.

Of interest to anyone who's only interested in public API changes
are patches 2/14 and 6/14. The changes made are backwards compatible,
and will not break API users.

I've collaborated with Niklas Haas to make our APIs compatible, and
we've both improved our codebases in the process. He'll be submitting
a long-requested vf_libplacebo filter for review as soon as this gets merged.

I'm also working on a Vulkan hwaccel using the new decode API,
the branch for which can be found here:
https://github.com/cyanreg/FFmpeg/commits/vulkan_decode
Notably, libavfilter/vulkan.c will be moved into libavcodec/vulkan.h,
and will be used to handle both filtering and decoding/encoding.
No API changes will be required for it.

I've also attached the patchset here.
Subject: [PATCH 01/14] hwcontext_vulkan: bump required Vulkan loader version
 to 1.2

---
 configure                    | 6 +++---
 libavutil/hwcontext_vulkan.c | 2 +-
 libavutil/hwcontext_vulkan.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

Comments

Lynne Nov. 11, 2021, 8:12 a.m. UTC | #1 
Nov 10, 2021, 08:20 by dev@lynne.ee:

> I've extensively fixed and improved our Vulkan codebase, and it's
> now up to standard. Since it's a 14-part patchset, I've uploaded
> my branch for review here:
> https://github.com/cyanreg/FFmpeg/tree/vulkan
>
> The implementation now follows modern Vulkan API usage practices,
> fully takes advantage of all the hardware has to offer, it's faster,
> much easier to compile, static linking is possible and most importantly,
> it's bug-free and extendable.
>
> Of interest to anyone who's only interested in public API changes
> are patches 2/14 and 6/14. The changes made are backwards compatible,
> and will not break API users.
>
> I've collaborated with Niklas Haas to make our APIs compatible, and
> we've both improved our codebases in the process. He'll be submitting
> a long-requested vf_libplacebo filter for review as soon as this gets merged.
>
> I'm also working on a Vulkan hwaccel using the new decode API,
> the branch for which can be found here:
> https://github.com/cyanreg/FFmpeg/commits/vulkan_decode
> Notably, libavfilter/vulkan.c will be moved into libavcodec/vulkan.h,
> and will be used to handle both filtering and decoding/encoding.
> No API changes will be required for it.
>
> I've also attached the patchset here.
>

Ping.
libplacebo has already had the changes needed merged.
Niklas Haas Nov. 11, 2021, 11:54 a.m. UTC | #2 
On Thu, 11 Nov 2021 09:12:35 +0100 Lynne <dev@lynne.ee> wrote:
> Nov 10, 2021, 08:20 by dev@lynne.ee:
> 
> > I've extensively fixed and improved our Vulkan codebase, and it's
> > now up to standard. Since it's a 14-part patchset, I've uploaded
> > my branch for review here:
> > https://github.com/cyanreg/FFmpeg/tree/vulkan
> >
> > The implementation now follows modern Vulkan API usage practices,
> > fully takes advantage of all the hardware has to offer, it's faster,
> > much easier to compile, static linking is possible and most importantly,
> > it's bug-free and extendable.
> >
> > Of interest to anyone who's only interested in public API changes
> > are patches 2/14 and 6/14. The changes made are backwards compatible,
> > and will not break API users.
> >
> > I've collaborated with Niklas Haas to make our APIs compatible, and
> > we've both improved our codebases in the process. He'll be submitting
> > a long-requested vf_libplacebo filter for review as soon as this gets merged.
> >
> > I'm also working on a Vulkan hwaccel using the new decode API,
> > the branch for which can be found here:
> > https://github.com/cyanreg/FFmpeg/commits/vulkan_decode
> > Notably, libavfilter/vulkan.c will be moved into libavcodec/vulkan.h,
> > and will be used to handle both filtering and decoding/encoding.
> > No API changes will be required for it.
> >
> > I've also attached the patchset here.
> >
> 
> Ping.
> libplacebo has already had the changes needed merged.

LGTM overall (except for one comment re: glslang version check).

Will you squash the commits? (If not, there are some minor bug fixes in
later commits that should probably be squashed onto the first commit)
Lynne Nov. 11, 2021, 12:43 p.m. UTC | #3 
11 Nov 2021, 12:54 by ffmpeg@haasn.xyz:

> On Thu, 11 Nov 2021 09:12:35 +0100 Lynne <dev@lynne.ee> wrote:
>
>> Nov 10, 2021, 08:20 by dev@lynne.ee:
>>
>> > I've extensively fixed and improved our Vulkan codebase, and it's
>> > now up to standard. Since it's a 14-part patchset, I've uploaded
>> > my branch for review here:
>> > https://github.com/cyanreg/FFmpeg/tree/vulkan
>> >
>> > The implementation now follows modern Vulkan API usage practices,
>> > fully takes advantage of all the hardware has to offer, it's faster,
>> > much easier to compile, static linking is possible and most importantly,
>> > it's bug-free and extendable.
>> >
>> > Of interest to anyone who's only interested in public API changes
>> > are patches 2/14 and 6/14. The changes made are backwards compatible,
>> > and will not break API users.
>> >
>> > I've collaborated with Niklas Haas to make our APIs compatible, and
>> > we've both improved our codebases in the process. He'll be submitting
>> > a long-requested vf_libplacebo filter for review as soon as this gets merged.
>> >
>> > I'm also working on a Vulkan hwaccel using the new decode API,
>> > the branch for which can be found here:
>> > https://github.com/cyanreg/FFmpeg/commits/vulkan_decode
>> > Notably, libavfilter/vulkan.c will be moved into libavcodec/vulkan.h,
>> > and will be used to handle both filtering and decoding/encoding.
>> > No API changes will be required for it.
>> >
>> > I've also attached the patchset here.
>> >
>>
>> Ping.
>> libplacebo has already had the changes needed merged.
>>
>
> LGTM overall (except for one comment re: glslang version check).
>
> Will you squash the commits? (If not, there are some minor bug fixes in
> later commits that should probably be squashed onto the first commit)
>

Thanks, fixed all that you mentioned in the github comments.
I won't be squashing the commits. I think they're neatly segmented
into the areas they fix.
I might push this sometime late tonight if there are no further comments.

Could you submit your libplacebo patch too, if you feel like it's good enough?
Some notes about it:
 - Remove the RET redefinition
 - Replace stdbools with ints
 - I think you ought to set `apply_filmgrain` to be enabled by default,
   and rename the description to "Apply film grain side-data", since you
   also implemented support for H2645's film-grain. Would make sense to
   apply film grain if you're already doing processing.
Lynne Nov. 12, 2021, 4:55 a.m. UTC | #4 
11 Nov 2021, 13:43 by dev@lynne.ee:

> 11 Nov 2021, 12:54 by ffmpeg@haasn.xyz:
>
>> On Thu, 11 Nov 2021 09:12:35 +0100 Lynne <dev@lynne.ee> wrote:
>>
>>> Nov 10, 2021, 08:20 by dev@lynne.ee:
>>>
>>> > I've extensively fixed and improved our Vulkan codebase, and it's
>>> > now up to standard. Since it's a 14-part patchset, I've uploaded
>>> > my branch for review here:
>>> > https://github.com/cyanreg/FFmpeg/tree/vulkan
>>> >
>>> > The implementation now follows modern Vulkan API usage practices,
>>> > fully takes advantage of all the hardware has to offer, it's faster,
>>> > much easier to compile, static linking is possible and most importantly,
>>> > it's bug-free and extendable.
>>> >
>>> > Of interest to anyone who's only interested in public API changes
>>> > are patches 2/14 and 6/14. The changes made are backwards compatible,
>>> > and will not break API users.
>>> >
>>> > I've collaborated with Niklas Haas to make our APIs compatible, and
>>> > we've both improved our codebases in the process. He'll be submitting
>>> > a long-requested vf_libplacebo filter for review as soon as this gets merged.
>>> >
>>> > I'm also working on a Vulkan hwaccel using the new decode API,
>>> > the branch for which can be found here:
>>> > https://github.com/cyanreg/FFmpeg/commits/vulkan_decode
>>> > Notably, libavfilter/vulkan.c will be moved into libavcodec/vulkan.h,
>>> > and will be used to handle both filtering and decoding/encoding.
>>> > No API changes will be required for it.
>>> >
>>> > I've also attached the patchset here.
>>> >
>>>
>>> Ping.
>>> libplacebo has already had the changes needed merged.
>>>
>>
>> LGTM overall (except for one comment re: glslang version check).
>>
>> Will you squash the commits? (If not, there are some minor bug fixes in
>> later commits that should probably be squashed onto the first commit)
>>
>
> Thanks, fixed all that you mentioned in the github comments.
> I won't be squashing the commits. I think they're neatly segmented
> into the areas they fix.
> I might push this sometime late tonight if there are no further comments.
>

Patchset pushed, thanks to everyone for the reviews.
Next up, the vulkan hwaccel patch.
diff mbox

Patch

diff --git a/configure b/configure
index c01aa480c7..103829e688 100755
--- a/configure
+++ b/configure
@@ -6824,10 +6824,10 @@  enabled vdpau &&
 enabled crystalhd && check_lib crystalhd "stdint.h libcrystalhd/libcrystalhd_if.h" DtsCrystalHDVersion -lcrystalhd
 
 if enabled vulkan; then
-    require_pkg_config_cpp vulkan "vulkan >= 1.1.97" "vulkan/vulkan.h" "defined VK_VERSION_1_1" ||
-        require_cpp_condition vulkan "vulkan/vulkan.h" "defined VK_VERSION_1_1"
+    require_pkg_config_cpp vulkan "vulkan >= 1.2.189" "vulkan/vulkan.h" "defined VK_VERSION_1_2" ||
+        require_cpp_condition vulkan "vulkan/vulkan.h" "defined VK_VERSION_1_2"
     # vulkan_lib should be removed once glslang filters are updated
-    check_pkg_config vulkan_lib "vulkan >= 1.1.97" "vulkan/vulkan.h" vkCreateInstance
+    check_pkg_config vulkan_lib "vulkan >= 1.2.189" "vulkan/vulkan.h" vkCreateInstance
 fi
 
 if enabled x86; then
diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
index 2c3216857a..570ebf23bb 100644
--- a/libavutil/hwcontext_vulkan.c
+++ b/libavutil/hwcontext_vulkan.c
@@ -676,7 +676,7 @@  static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts)
     VkApplicationInfo application_info = {
         .sType              = VK_STRUCTURE_TYPE_APPLICATION_INFO,
         .pEngineName        = "libavutil",
-        .apiVersion         = VK_API_VERSION_1_1,
+        .apiVersion         = VK_API_VERSION_1_2,
         .engineVersion      = VK_MAKE_VERSION(LIBAVUTIL_VERSION_MAJOR,
                                               LIBAVUTIL_VERSION_MINOR,
                                               LIBAVUTIL_VERSION_MICRO),
diff --git a/libavutil/hwcontext_vulkan.h b/libavutil/hwcontext_vulkan.h
index e4645527d7..8d1ae50e65 100644
--- a/libavutil/hwcontext_vulkan.h
+++ b/libavutil/hwcontext_vulkan.h
@@ -50,7 +50,7 @@  typedef struct AVVulkanDeviceContext {
     PFN_vkGetInstanceProcAddr get_proc_addr;
 
     /**
-     * Vulkan instance. Must be at least version 1.1.
+     * Vulkan instance. Must be at least version 1.2.
      */
     VkInstance inst;