Skip to content

Fix issues with VA_INCLUDE_HEADERS when building with CUDA support#22712

Merged
opencv-pushbot merged 1 commit intoopencv:4.xfrom
dmatveev:dm/fix_va_headers
Oct 28, 2022
Merged

Fix issues with VA_INCLUDE_HEADERS when building with CUDA support#22712
opencv-pushbot merged 1 commit intoopencv:4.xfrom
dmatveev:dm/fix_va_headers

Conversation

@dmatveev
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev commented Oct 27, 2022

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

@dmatveev dmatveev mentioned this pull request Oct 27, 2022
Closed
SergeyIvanov87
SergeyIvanov87 approved these changes Oct 27, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@SergeyIvanov87 SergeyIvanov87 left a comment •
edited
Loading

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

alalek
alalek reviewed Oct 27, 2022
View reviewed changes
modules/gapi/CMakeLists.txt Outdated
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
Copy Markdown
Member

@alalek alalek Oct 27, 2022

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
Copy Markdown
Contributor Author

@dmatveev dmatveev Oct 28, 2022

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
Copy Markdown
Contributor Author

dmatveev commented Oct 28, 2022

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
Copy Markdown
Contributor

mshabunin commented Oct 28, 2022

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
Copy Markdown
Contributor Author

dmatveev commented Oct 29, 2022

@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
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek left review comments

@SergeyIvanov87 SergeyIvanov87 SergeyIvanov87 approved these changes

Assignees

@SergeyIvanov87 SergeyIvanov87

Labels

category: build/install category: g-api / gapi

Projects

None yet

Milestone

4.7.0

Development

Successfully merging this pull request may close these issues.

5 participants

@dmatveev @mshabunin @alalek @SergeyIvanov87 @opencv-pushbot