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

Add AJANTV2 CMake package exports #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ibstewart
Copy link
Contributor

@ibstewart ibstewart commented Mar 25, 2022

This adds CMake rules to output the AJANTV2
exports such that the ajantv2 library can be
built and linked against using the following:

  find_package(AJANTV2 16.2 REQUIRED)
  target_link_libraries(<target> AJA::ajantv2)

This adds CMake rules to output the ajantv2
config such that the ajantv2 library can be
built and linked against using the following:

  find_package(ajantv2 16.2 REQUIRED)
  target_link_libraries(<target> AJA::ajantv2)
@paulh-aja
Copy link
Contributor

This does look handy. I need to review it and make sure it doesn't break the OBS CI builds. If it doesn't break anything I will merge this soon.

-DAJAMac
-DAJA_MAC
-D__STDC_CONSTANT_MACROS)
endif()

add_definitions(${AJANTV2_COMPILE_DEFINITIONS})
Copy link
Contributor

Choose a reason for hiding this comment

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

target_compile_definitions is preferred in modern cmake if you want to attach those to specific targets (and they'll be exported alongside it)

Copy link
Contributor

Choose a reason for hiding this comment

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

We use target_compile_definitions for the target-specific definitions in the repo. However I need to refactor these global defines at some point across all of our targets to apply them to targets with target_compile_definitions.

@agirault
Copy link
Contributor

agirault commented May 8, 2022

Hi @paulh-aja . Do you have an ETA to have this potentially merged?

@paulh-aja
Copy link
Contributor

I've given this PR a cursory look with another AJA engineer and our initial feeling is that we need to see how it fits with other NTV2 CMake changes that we're planning on landing in the repo soon. We've been doing a bit of work/rework on the ajantv2 CMake versionizing system, and the CMake install directives.

I am on PTO until June 1st and the rest of the AJA engineers are currently busy on other internal tasks, so it will need to wait until early June at the soonest.

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.

3 participants