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

Fix issues with VA_INCLUDE_HEADERS when building with CUDA support #22712

Merged
merged 1 commit into from Oct 28, 2022

Conversation

dmatveev
Copy link
Contributor

Issue description

  • For some reason, WITH_ is used instead of HAVE_ when checking for VA libs in G-API CMakeLists
  • WITH_VA is TRUE by default on x86 (see opencv/CMakeLists.txt:414)
  • When configuring samples, the test for VA haven't been made (but WITH_VA is still true and it is ok)
  • Removed redundant call to OpenCVFindVA.cmake as it is done anyway (see opencv/CMakeLists.txt:743 and opencv/cmake/OpenCVFindLibsVideo.cmake)
  • Removed FATAL ERROR case from G-API's CMakeLists.txt if VA isn't available. Have no idea why it is there

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

Copy link
Contributor

@SergeyIvanov87 SergeyIvanov87 left a comment

Choose a reason for hiding this comment

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

LGTM

But I would recommend to add warning because vpl on Linux Is useless without va_api

endif(WITH_VA)
if(UNIX AND HAVE_VA)
ocv_target_include_directories(${the_module} SYSTEM PRIVATE ${VA_INCLUDE_DIR})
ocv_target_include_directories(opencv_test_gapi SYSTEM PRIVATE ${VA_INCLUDE_DIR})
Copy link
Member

Choose a reason for hiding this comment

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

opencv_test_gapi

Test target should be guarded, see code below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, updated. Thanks

@dmatveev
Copy link
Contributor Author

Also tested the build with VA being available:

root@7973346b3aad:~/opencv/build# ldd bin/example_gapi_onevpl_infer_single_roi | grep va
        libva.so.2 => /lib/x86_64-linux-gnu/libva.so.2 (0x00007fc3af609000)
        libva-drm.so.2 => /lib/x86_64-linux-gnu/libva-drm.so.2 (0x00007fc3af604000)

At least it builds now

@mshabunin
Copy link
Contributor

For some reason, WITH_ is used instead of HAVE_ when checking for VA libs in G-API CMakeLists
Removed redundant call to OpenCVFindVA.cmake as it is done anyway

Perhaps it was designed like this for standalone G-API build. Do we still support this configuration?

@opencv-pushbot opencv-pushbot merged commit 1293e99 into opencv:4.x Oct 28, 2022
@dmatveev
Copy link
Contributor Author

@mshabunin yes we do, but VPL & VAAPI were never a part of it. Good question if it is still tested on the CI though..

@alalek alalek mentioned this pull request Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants