ash-rs / ash

Vulkan bindings for Rust
Apache License 2.0
1.86k stars 190 forks source link

Safe wrapper for the video extension #722

Open pingpongcat opened 1 year ago

pingpongcat commented 1 year ago

Hello everyone, as a member of vulkan video working group under Khronos I would like contribute to this project by providing safe wrapper for the video extension. Spec for the encode is still in flux but I volunteer to ensure compatibility in future releases.

For I while Im have ash fork with added extensions: https://github.com/neurotok/ash.git And a basic encode example: https://github.com/neurotok/ash-video.git

But the problem which I cannot overcome is a panic when calling even basic functions from the encode: https://docs.rs/ash/latest/ash/vk/struct.KhrVideoQueueFn.html# (using my ash-video example)

I had some success when I wrote this wrapper as an instance extension, but I'm running out of ideas on how the wrapper should be designed as device one.

@MarijnS95 , @MaikKlein ? Could you please take a look at my ash fork and provided example ?

Ralith commented 1 year ago

What is the panic message?

pingpongcat commented 1 year ago

Hi @Ralith Im getting "thread 'main' panicked at 'Unable to load get_physical_device_video_capabilities_khr'"

This is caused by calling get_physical_device_video_capabilities in ash-video\src\main.rs:224

This function is implemented here ash/src/extensions/khr/video_encode_queue.rs:25

To run the examples you'll need nvidia beta drivers with video support

Ralith commented 1 year ago

Your code is using vkGetDeviceProcAddr to load that function. vkGetDeviceProcAddr is specified to return null for physical-device-level commands such as vkGetPhysicalDeviceVideoCapabilities.

MarijnS95 commented 1 year ago

@neurotok Do you want to use this as tracking issue or can we close it now that the initial problem has been resolved (nothing to do with Ash it seems, just general Vulkan usage)?

Also keep in mind that the Video bindings are currently generated from the headers because they landed in an era where video.xml with bitfield struct members did not yet exist. We might change that in the future, though most likely with the generator rewrite and not in the current setup.

pingpongcat commented 1 year ago

Hi @MarijnS95, of course you were right, I already update m fork but let's keep this as a issue tracker for now please. At least until I have a working sample project with decoding.

Do you want some help with the generator to pull video extension from xml?

MarijnS95 commented 1 year ago

We already generate the full video extension bindings, don't need help for that but it'll likely change with the generator rewrite. In the event that you do need to make changes, use this tracking issue to discuss so first please.

MarijnS95 commented 6 months ago

@pingpongcat another user seems to have beat you to submitting the Video bindings in #916, though in a less mergeable state than your fork would have been (a year ago when it was proposed; we've made significant breaking/simplification changes to hand-written extension wrappers now).

Would you be able to help reviewing and testing the proposed implementation, once all basic review is carried out?

pingpongcat commented 6 months ago

Sure, I'll look at it @MarijnS95. The main reason the further progress for this extension was delayed was the lack of crates that could handle video file demuxing and the extraction of the SPS/PPS parameters from the video bytestream. A year ago, there were no crates that could handle this, which meant I wasn't 100% sure that the bindings would work in real life. The Vulkan video extension itself has no ability to do this as a part of some API call, and it expects these parameters to be provided by the developer. Hey @Cach30verfl0w, did you have a chance to test these extension bindings in "action"? If so, how did you solve the SPS/PPS parameters issue?

Cach30verfl0w commented 6 months ago

Sure I'll look at it @MarijnS95 the main reason the further progress for this extension to be developed was the lack of crates that could handle video file demuxing and the extraction of the SPS/PPS parameters from the video bytestrem , year ago there was no crates that could handle this which meant I wasn't 100% sure that the bindings would work in real life. Vulkan video extension itself have no ability to do this as a part some API call and expecting these parameters to be provided by the developer . Hey @Cach30verfl0w did you have a chance to test these extension binding in "action" ? and if so how you solved the SPS/PPS parameters issue ?

I am currently working on a project where these extension bindings will definitely be used. But I haven't had a chance to use the bindings in action yet.

MarijnS95 commented 6 months ago

I haven't yet played with this either, but please let me know (and maybe we could list it in the readme somewhere) about good crates that fulfill this purpose!

colinmarc commented 1 day ago

I use the encode extensions in my project. I could upstream the wrappers I use, if that's what's meant by "safe wrappers"?

Edit: #965