Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safe wrapper for the video extension #722

Open
pingpongcat opened this issue Mar 24, 2023 · 10 comments · May be fixed by #916
Open

Safe wrapper for the video extension #722

pingpongcat opened this issue Mar 24, 2023 · 10 comments · May be fixed by #916

Comments

@pingpongcat
Copy link
Contributor

pingpongcat commented Mar 24, 2023

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
Copy link
Collaborator

Ralith commented Mar 24, 2023

What is the panic message?

@pingpongcat
Copy link
Contributor Author

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
Copy link
Collaborator

Ralith commented Mar 24, 2023

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

@MarijnS95
Copy link
Collaborator

@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
Copy link
Contributor Author

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
Copy link
Collaborator

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 MarijnS95 linked a pull request May 13, 2024 that will close this issue
@MarijnS95
Copy link
Collaborator

@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
Copy link
Contributor Author

pingpongcat commented May 14, 2024

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
Copy link

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
Copy link
Collaborator

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!

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 a pull request may close this issue.

4 participants