search

nvpro-samples / vk_video_samples

Vulkan video samples
Apache License 2.0
252 stars 41 forks source link

Build instructs still refer to needing beta drivers -- no longer the case w/ 531.18! #33

Closed BattleAxeVR closed 1 year ago

BattleAxeVR commented 1 year ago

Hey, just wanted to mention, on a whim, I just installed the mainline Nvidia drivers (non-beta) and the decoder app works fine, as well as my own integration based on the latest code here.

This is a great milestone, thanks for all the hard work. It took a while to get there but better late than never.

I'd love to see more and more features added over time, new formats (alpha channel), multiple streams in a single MP4/MKV, the decoder app actually being a useful playback tool (don't decode at fullspeed but the actual framerate of the video, or at least toggle that or full tilt decoding as a command line toggle), audio support.

One thing that is a bit redundant in this repo, is the use of a VK interface that makes it harder to integrate. the Beta define to vulkan.h no longer needs to be defined, so I don't see the point in doing vk::FuntionCall instead of vkFunctionCall everywhere. I would personally much rather a more c-like clean basic Vulkan calls since we no longer need to patch in those function pointers since Vulkan Video 1.0 is officially out.

Thanks for listening, these are all just suggestions, it's fine if you ignore them. Carry on with the great work and look forward to more updates.

zlatinski commented 1 year ago

Thank you, BattleAxeVR for the feedback. I agree with the above points. Those are good suggestions! Thank you for being a patient user of the video extensions and the sample :) !

BattleAxeVR commented 1 year ago

The latest code refactors you did made it really simple to use now with only a couple contact points and inheriting once from your shell base class, so I don't even need to do a search replace of vk::Function to vkFunction anymore. So we can close this. I would just suggest you update the README here to reflect that you no longer need to install the beta drivers to use this sample or functionality in general.

thanks again! I look forward to seeing this develop further.