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

cmake build rework #93

Merged
merged 6 commits into from
Nov 12, 2024
Merged

Conversation

dabrain34
Copy link
Contributor

Factorize the build of vk_video_samples to one CMakefile containing the definitions for both decoder and encoder
project.
Can now enable disable the build of encoder/decoder by a CMake option.

@dabrain34 dabrain34 force-pushed the dab_cmake_build_rework branch from 8a763a2 to 251c363 Compare October 10, 2024 19:27
@dabrain34
Copy link
Contributor Author

dabrain34 commented Oct 18, 2024

@zlatinski ^

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

LGTM - Thanks for the improvements, Stephan!

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

I need some time to review these cmakefile changes. Thank you, Stephan!

@dabrain34 dabrain34 force-pushed the dab_cmake_build_rework branch from 251c363 to ecb922b Compare October 24, 2024 08:39
Copy link
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.

I think this one looks good, too. Before I merge this change, have you tested this on Windows, Stephan?

@dabrain34
Copy link
Contributor Author

I just tested today on Windows and unfortunately the build is failing. Investigating..
Have you been able to discuss to enable a CI on this repository ? It would help to track this kind of regression. :)

@dabrain34 dabrain34 force-pushed the dab_cmake_build_rework branch from ecb922b to 8c529d3 Compare November 12, 2024 10:04
Add a common Cmake file to build encoder/decoder
projects.

Signed-off-by: Stéphane Cerveau <[email protected]>
Signed-off-by: Stéphane Cerveau <[email protected]>
@dabrain34 dabrain34 force-pushed the dab_cmake_build_rework branch from 8c529d3 to 78c02a6 Compare November 12, 2024 10:36
@dabrain34
Copy link
Contributor Author

This is now building on both Windows and Linux. I moved the generate_dispatch_table to a common place to be generated only once.

@dabrain34 dabrain34 force-pushed the dab_cmake_build_rework branch from 78c02a6 to e9d3856 Compare November 12, 2024 11:08
Copy link
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.

LGTM, thanks for the change!

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

LGTM, thanks for the change!

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

LGTM, thanks for the change!

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

LGTM, thanks for the change!

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

LGTM, thanks for the change!

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

LGTM, thanks for the change!

@zlatinski zlatinski merged commit a72c1aa into nvpro-samples:main Nov 12, 2024
1 check failed
@dabrain34
Copy link
Contributor Author

dabrain34 commented Nov 12, 2024

I'm wondering if it would make sense to have a vkCodecUtils static library with:

  • VkCodecUtils/VulkanDeviceContext.cpp
  • VulkanDeviceMemoryImpl.cpp
    ...

What do you think @zlatinski ?

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.

2 participants