Skip to content

Make the code more generic for different compiler settings#140

Closed
vasn1k wants to merge 10 commits intonvpro-samples:mainfrom
vasn1k:GenericComp
Closed

Make the code more generic for different compiler settings#140
vasn1k wants to merge 10 commits intonvpro-samples:mainfrom
vasn1k:GenericComp

Conversation

@vasn1k
Copy link
Copy Markdown
Contributor

@vasn1k vasn1k commented May 17, 2025

This fixes some common compiler issues seen with the sample application when used outside of the cmake build environment with more strict settings.

@dabrain34
Copy link
Copy Markdown
Contributor

can it be related and merged with KhronosGroup/Vulkan-Video-Samples#7 ?

@vasn1k
Copy link
Copy Markdown
Contributor Author

vasn1k commented May 22, 2025

can it be related and merged with KhronosGroup/Vulkan-Video-Samples#7 ?

I didn't see too much overlap in the changes, so I think they should get merged in parallel. I don't expect merge conflicts.

@vasn1k vasn1k force-pushed the GenericComp branch 5 times, most recently from fbdb564 to 91f0b03 Compare May 29, 2025 04:21
Comment thread common/libs/VkCodecUtils/DecoderConfig.h Outdated
Copy link
Copy Markdown
Contributor

@zlatinski zlatinski left a comment

Choose a reason for hiding this comment

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

Thank you, Vassili, for the changes! I would like you to make some changes to your patch, please.

Comment thread common/libs/VkCodecUtils/VulkanVideoProcessor.cpp Outdated
Comment thread common/libs/VkCodecUtils/VulkanVideoProcessor.cpp Outdated
Comment thread common/libs/VkShell/ShellWayland.cpp Outdated
Comment thread vk_video_decoder/include/vulkan_video_decoder.h Outdated
Comment thread vk_video_decoder/include/vulkan_video_decoder.h Outdated
Comment thread vk_video_decoder/libs/NvVideoParser/src/VulkanVideoDecoder.cpp Outdated
Comment thread vk_video_decoder/src/vulkan_video_decoder.cpp
Comment thread vk_video_decoder/src/vulkan_video_decoder.cpp
Comment thread vk_video_encoder/libs/VkVideoEncoder/VkEncoderDpbH264.cpp Outdated
Comment thread vk_video_encoder/src/vulkan_video_encoder.cpp
Copy link
Copy Markdown
Contributor

@zlatinski zlatinski left a comment

Choose a reason for hiding this comment

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

Thank you, Vassili! Please look at my below comments. I still need to review your Vulkan chain changes.

Comment thread common/libs/VkCodecUtils/VulkanShaderCompiler.cpp
Comment thread common/libs/VkCodecUtils/VulkanVideoProcessor.h Outdated
Comment thread vk_video_decoder/libs/VkDecoderUtils/VideoStreamDemuxer.cpp
Comment thread common/libs/VkCodecUtils/VkVideoFrameToFile.cpp
Comment thread common/libs/VkCodecUtils/VkVideoFrameToFile.cpp
@vasn1k vasn1k requested a review from zlatinski May 30, 2025 02:59
@vasn1k vasn1k force-pushed the GenericComp branch 2 times, most recently from 99f7625 to c709593 Compare May 30, 2025 03:05
Comment thread vk_video_decoder/libs/NvVideoParser/src/VulkanAV1Decoder.cpp Outdated
Comment thread vk_video_decoder/libs/NvVideoParser/src/VulkanAV1GlobalMotionDec.cpp Outdated
Comment thread vk_video_encoder/libs/VkVideoEncoder/VkVideoEncoder.cpp Outdated
Comment thread vk_video_encoder/libs/VkVideoEncoder/VkEncoderDpbH264.h
Comment thread common/libs/VkCodecUtils/Helpers.h
@vasn1k vasn1k force-pushed the GenericComp branch 3 times, most recently from 3a0d1a0 to ab86c2b Compare June 4, 2025 00:13
vasn1k added 7 commits June 3, 2025 17:13
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>
@vasn1k vasn1k force-pushed the GenericComp branch 2 times, most recently from beb362d to b97229f Compare June 4, 2025 15:53
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>
vasn1k added 2 commits June 4, 2025 17:16
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>
Signed-off-by: Vassili Nikolaev (NVIDIA) <vnikolaev@nvidia.com>

virtual ~ElementaryStream() {
#ifdef USE_SIMPLE_MALLOC
if (m_videoCodecType == VK_VIDEO_CODEC_OPERATION_DECODE_AV1_BIT_KHR) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

if (fileName != nullptr) {
m_outputFile = fopen(fileName, "wb");
if (m_outputFile) {
#if !defined(REDUCE_VK_VIDEO_STDOUT_SPEW)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please just commend this line out. Now we also have a logger in the Khronos repository, and I would rather not have so many ways to disable logging.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to comment that line out because it would affect how the sample app works currently. My goal is to make these changes not impact the apps logic or output at all.

const bool gfxRendererIsEnabled = (m_videoRenderer != nullptr);
m_frameCount++;

#if !defined(REDUCE_VK_VIDEO_STDOUT_SPEW)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For all these locations, please add a local runtime config like m_printInfo

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If changes are incoming that remove the need for this change, then lets not impact the runtime.

[argv](const char **, const ProgramArgs &a) {
int rtn = showHelp(argv, a);
exit(EXIT_SUCCESS);
safe_exit(EXIT_SUCCESS);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I still see the original code here. Where is the change to a return value? Am I looking at the fixes here or the original change?

[argv](const char **, const ProgramArgs &a) {
int rtn = showHelp(argv, a);
safe_exit(EXIT_SUCCESS);
SAFE_EXIT(EXIT_SUCCESS, "Help requested - exiting successfully");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oho, this is a separate change with the fixes. I thought you'd fixed the original commit. Let me look again, please.

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 this pull request may close these issues.

4 participants