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

Implement vulkan video extensions #916

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ By disabling the default `std` feature, this crate compiles in a [`no_std` envir
- Added `VK_KHR_calibrated_timestamps` device extension (#890)
- Added `VK_KHR_maintenance6` device extension (#891)
- Added `VK_NV_copy_memory_indirect` device extension (#892)
- Added support for vulkan video extensions (#916)
Copy link
Collaborator

@MarijnS95 MarijnS95 May 13, 2024

Choose a reason for hiding this comment

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

This can't retroactively be part of the 0.30.0 release that was already published. Please move this line under ## [Unreleased] - ReleaseDate above, and list out the extension names explicitly.


### Changed

Expand Down
3 changes: 3 additions & 0 deletions ash/src/extensions/khr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ pub mod surface;
pub mod swapchain;
pub mod synchronization2;
pub mod timeline_semaphore;
pub mod video_decode_queue;
pub mod video_encode_queue;
pub mod video_queue;
pub mod wayland_surface;
pub mod win32_surface;
pub mod xcb_surface;
Expand Down
15 changes: 15 additions & 0 deletions ash/src/extensions/khr/video_decode_queue.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//! <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_video_decode_queue.html>

use crate::vk;

impl crate::khr::video_decode_queue::Device {
/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCmdDecodeVideoKHR.html>
#[inline]
pub unsafe fn cmd_decode_video(
&self,
command_buffer: vk::CommandBuffer,
decode_info: &vk::VideoDecodeInfoKHR<'_>,
) {
(self.fp.cmd_decode_video_khr)(command_buffer, decode_info)
}
}
49 changes: 49 additions & 0 deletions ash/src/extensions/khr/video_encode_queue.rs
Copy link
Collaborator

Choose a reason for hiding this comment

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

VK_KHR_video_encode_queue also has one instance-level function that hasn't been wrapped here?

Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
//! <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_video_encode_queue.html>

use crate::prelude::VkResult;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The idea of a prelude is to always import *.

use crate::vk;
use core::mem;

impl crate::khr::video_encode_queue::Device {
/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetEncodedVideoSessionParametersKHR.html>
pub unsafe fn get_encoded_video_session_parameters(
&self,
video_session_parameters_info: &vk::VideoEncodeSessionParametersGetInfoKHR<'_>,
) -> VkResult<(vk::VideoEncodeSessionParametersFeedbackInfoKHR<'_>, Vec<u8>)> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

vk::VideoEncodeSessionParametersFeedbackInfoKHR<'_> is a struct with a p_next chain that allows the user to request additional info. It should be a &mut function argument instead of being output-only.

loop {
let mut count: usize = 0;
let _ = (self.fp.get_encoded_video_session_parameters_khr)(
self.handle,
video_session_parameters_info,
core::ptr::null_mut(),
&mut count,
core::ptr::null_mut(),
);

let mut data: Vec<u8> = Vec::with_capacity(count);
let mut feedback_info = mem::MaybeUninit::uninit();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This has an sType and pNext field that cannot be uninitalized when calling the underlying Vulkan function, as it will read those to understand what structure(s) (if any) to initialize.

It is also Optional.

let result = (self.fp.get_encoded_video_session_parameters_khr)(
self.handle,
video_session_parameters_info,
feedback_info.as_mut_ptr(),
&mut count,
data.as_mut_ptr() as _,
);
if result != vk::Result::INCOMPLETE {
break result
.assume_init_on_success(feedback_info)
.map(|value| (value, data));
}
}
Comment on lines +13 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason for this to be a copy-paste of read_into_uninitialized_vector(), instead of calling the helper directly?

}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCmdEncodeVideoKHR.html>
#[inline]
pub unsafe fn cmd_encode_video(
&self,
command_buffer: vk::CommandBuffer,
encode_info: &vk::VideoEncodeInfoKHR<'_>,
) {
(self.fp.cmd_encode_video_khr)(command_buffer, encode_info)
}
}
220 changes: 220 additions & 0 deletions ash/src/extensions/khr/video_queue.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
//! <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_video_encode_queue.html>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops:

Suggested change
//! <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_video_encode_queue.html>
//! <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_video_queue.html>


use crate::prelude::VkResult;
use crate::{vk, RawPtr};
Copy link
Collaborator

Choose a reason for hiding this comment

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

The convention has been to separate these imports per line.

use core::mem;
use std::ptr;

impl crate::khr::video_queue::Instance {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: we've typically put impl Instance last.

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetPhysicalDeviceVideoCapabilitiesKHR.html>
#[inline]
pub unsafe fn get_physical_device_video_capabilities(
&self,
physical_device: vk::PhysicalDevice,
video_profile: &vk::VideoProfileInfoKHR<'_>,
capabilities: &mut vk::VideoCapabilitiesKHR<'_>
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is quite surprising that there is no trailing , here, but then it looks like rustfmt has not been ran.

) -> VkResult<()> {
(self.fp.get_physical_device_video_capabilities_khr)(
physical_device,
video_profile,
capabilities,
).result()
}

// Retrieve the number of elements to pass to [`get_physical_device_video_format_properties`][Self::get_physical_device_video_format_properties]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doc-comment please.

If this was copy-pasted from another extension, it is strangely also missing ()?

#[inline]
pub unsafe fn get_physical_device_video_format_properties_len(
&self,
physical_device: vk::PhysicalDevice,
video_format_info: &vk::PhysicalDeviceVideoFormatInfoKHR<'_>,
) -> usize {
let mut memory_requirements_count = mem::MaybeUninit::uninit();
let _ = (self.fp.get_physical_device_video_format_properties_khr)(
Copy link
Collaborator

Choose a reason for hiding this comment

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

We've never ignored Vulkan errors, and you have no reason to do so either? A low-level wrapper should pass all the info that it gets right back to the caller and not hide anything.

physical_device,
video_format_info,
memory_requirements_count.as_mut_ptr(),
ptr::null_mut(),
);
memory_requirements_count.assume_init() as _
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetPhysicalDeviceVideoFormatPropertiesKHR.html>
///
/// Call [`get_physical_device_video_format_properties_len()`][Self::get_physical_device_video_format_properties_len()] to query the number of elements to pass to `out`.
/// Be sure to [`Default::default()`]-initialize these elements and optionally set their `p_next` pointer.
#[inline]
pub unsafe fn get_physical_device_video_format_properties(
&self,
physical_device: vk::PhysicalDevice,
video_format_info: &vk::PhysicalDeviceVideoFormatInfoKHR<'_>,
out: &mut [vk::VideoFormatPropertiesKHR<'_>],
) -> VkResult<()> {
let mut count = out.len() as u32;
(self.fp.get_physical_device_video_format_properties_khr)(
physical_device,
video_format_info,
&mut count,
out.as_mut_ptr(),
).result()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing an assert that count wasn't updated to value != out.len().

}

impl crate::khr::video_queue::Device {
/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCreateVideoSessionKHR.html>
#[inline]
pub unsafe fn create_video_session(
&self,
create_info: &vk::VideoSessionCreateInfoKHR<'_>,
allocation_callbacks: Option<&vk::AllocationCallbacks<'_>>,
) -> VkResult<vk::VideoSessionKHR> {
let mut video_session = mem::MaybeUninit::uninit();
(self.fp.create_video_session_khr)(
self.handle,
create_info,
allocation_callbacks.as_raw_ptr(),
video_session.as_mut_ptr(),
)
.assume_init_on_success(video_session)
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkDestroyVideoSessionKHR.html>
#[inline]
pub unsafe fn destroy_video_session(
&self,
video_session: vk::VideoSessionKHR,
allocation_callbacks: Option<&vk::AllocationCallbacks<'_>>,
) {
(self.fp.destroy_video_session_khr)(
self.handle,
video_session,
allocation_callbacks.as_raw_ptr(),
)
}

// Retrieve the number of elements to pass to [`get_video_session_memory_requirements`][Self::get_video_session_memory_requirements]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

#[inline]
pub unsafe fn get_video_session_memory_requirements_len(
&self,
video_session: vk::VideoSessionKHR,
) -> usize {
let mut memory_requirements_count = mem::MaybeUninit::uninit();
let _ = (self.fp.get_video_session_memory_requirements_khr)(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the first time in ash that a Result gets ignored. With zero (comment) explanations why.

self.handle,
video_session,
memory_requirements_count.as_mut_ptr(),
ptr::null_mut(),
);
memory_requirements_count.assume_init() as _
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetVideoSessionMemoryRequirementsKHR.html>
///
/// Call [`get_video_session_memory_requirements_len()`][Self::get_video_session_memory_requirements_len()] to query the number of elements to pass to `out`.
/// Be sure to [`Default::default()`]-initialize these elements and optionally set their `p_next` pointer.
#[inline]
pub unsafe fn get_video_session_memory_requirements(
&self,
video_session: vk::VideoSessionKHR,
out: &mut [vk::VideoSessionMemoryRequirementsKHR<'_>],
) -> VkResult<()> {
let mut count = out.len() as u32;
(self.fp.get_video_session_memory_requirements_khr)(
self.handle,
video_session,
&mut count,
out.as_mut_ptr(),
).result()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same assert is missing here.


/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkBindVideoSessionMemoryKHR.html>
#[inline]
pub unsafe fn bind_video_session_memory(
&self,
video_session: vk::VideoSessionKHR,
bind_session_memory_infos: &[vk::BindVideoSessionMemoryInfoKHR<'_>],
) -> VkResult<()> {
(self.fp.bind_video_session_memory_khr)(
self.handle,
video_session,
bind_session_memory_infos.len() as _,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only khr/acceleration_structures.rs is erroneously using typeless casts. Please specify u32 here explicitly.

bind_session_memory_infos.as_ptr(),
)
.result()
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCreateVideoSessionParametersKHR.html>
#[inline]
pub unsafe fn create_video_session_parameters(
&self,
create_info: &vk::VideoSessionParametersCreateInfoKHR<'_>,
allocation_callbacks: Option<&vk::AllocationCallbacks<'_>>,
) -> VkResult<vk::VideoSessionParametersKHR> {
let mut parameters = mem::MaybeUninit::uninit();
(self.fp.create_video_session_parameters_khr)(
self.handle,
create_info,
allocation_callbacks.as_raw_ptr(),
parameters.as_mut_ptr(),
)
.assume_init_on_success(parameters)
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkUpdateVideoSessionParametersKHR.html>
#[inline]
pub unsafe fn update_video_session_parameters(
&self,
video_session_parameters: vk::VideoSessionParametersKHR,
update_info: &vk::VideoSessionParametersUpdateInfoKHR<'_>,
) -> VkResult<()> {
(self.fp.update_video_session_parameters_khr)(
self.handle,
video_session_parameters,
update_info,
)
.result()
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkDestroyVideoSessionParametersKHR.html>
#[inline]
pub unsafe fn destroy_video_session_parameters(
&self,
video_session_parameters: vk::VideoSessionParametersKHR,
allocation_callbacks: Option<&vk::AllocationCallbacks<'_>>,
) {
(self.fp.destroy_video_session_parameters_khr)(
self.handle,
video_session_parameters,
allocation_callbacks.as_raw_ptr(),
)
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCmdBeginVideoCodingKHR.html>
#[inline]
pub unsafe fn cmd_begin_video_coding(
&self,
command_buffer: vk::CommandBuffer,
begin_info: &vk::VideoBeginCodingInfoKHR<'_>,
) {
(self.fp.cmd_begin_video_coding_khr)(command_buffer, begin_info)
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCmdEndVideoCodingKHR.html>
#[inline]
pub unsafe fn cmd_end_video_coding(
&self,
command_buffer: vk::CommandBuffer,
end_coding_info: &vk::VideoEndCodingInfoKHR<'_>,
) {
(self.fp.cmd_end_video_coding_khr)(command_buffer, end_coding_info)
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCmdControlVideoCodingKHR.html>
#[inline]
pub unsafe fn cmd_control_video_coding(
&self,
command_buffer: vk::CommandBuffer,
coding_control_info: &vk::VideoCodingControlInfoKHR<'_>,
) {
(self.fp.cmd_control_video_coding_khr)(command_buffer, coding_control_info)
}
}