Skip to content

WIP: G-API: Internal IE flag check in public headers issue#15419

Merged
alalek merged 2 commits intoopencv:masterfrom
OrestChura:gapi_headers_internal_flag_issue
Sep 3, 2019
Merged

WIP: G-API: Internal IE flag check in public headers issue#15419
alalek merged 2 commits intoopencv:masterfrom
OrestChura:gapi_headers_internal_flag_issue

Conversation

@OrestChura
Copy link
Copy Markdown
Contributor

This pullrequest changes

  • HAVE_INF_ENGINE flag check in headers "infer/ie.hpp" and "infer/ie/util.hpp" is deleted
  • headers in "infer/" and "infer/ie/" folders are included into gapi_ext_hdrs;
  • because of that a few #includes are required in the headers

OpenCV public API should not have features-based conditional compilation in general (because these internal flags are not available in user apps). Instead of using such condoitions, user should add checks into their app.

Also the headers for G-API inference should be put into gapi_ext_hdrs CMake varaible

…ext_hdrs;

+ because of that a few #includes are required in the headers
- HAVE_INF_ENGINE flag check in headers "infer/ie.hpp" and "infer/ie/util.hpp" is deleted
@OrestChura
Copy link
Copy Markdown
Contributor Author

@AsyaPronina @dmatveev @alalek please review

@AsyaPronina AsyaPronina requested review from alalek and dmatveev August 30, 2019 07:21
@OrestChura OrestChura changed the title G-API: Internal IE flag check in public headers issue WIP: G-API: Internal IE flag check in public headers issue Aug 30, 2019
@alalek
Copy link
Copy Markdown
Member

alalek commented Aug 30, 2019

GAPI_EXPORTS std::vector to_ocv(const InferenceEngine::SizeVector &dims);
InferenceEngine::SizeVector

3rdparty types are not allowed in OpenCV regular public API.
Move this code into separate header under "details"/"private" sub-directory.

…ests; it's been moved to the scr directory to the place next to the implementation file "ie/giebackend.cpp"

- the path to this header in files "ie/giebackend.cpp" and "test/infer/gapi_infer_ie_test.cpp" is updated
- As it's private header now and explicitly depends on IE, the "HAVE_INF_ENGINE" flag check is returned
@OrestChura
Copy link
Copy Markdown
Contributor Author

GAPI_EXPORTS std::vector to_ocv(const InferenceEngine::SizeVector &dims);
InferenceEngine::SizeVector

3rdparty types are not allowed in OpenCV regular public API.
Move this code into separate header under "details"/"private" sub-directory.

@dmatveev

I have added changes you mentioned to make the header private, but mb it's not the right decision as the features could be helpful for a user.

Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@@ -27,5 +30,6 @@ GAPI_EXPORTS InferenceEngine::Blob::Ptr to_ie(cv::Mat &blob);

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.

Since this file is internal now, please put a note here why these functions are EXPORTed (to make them accessible by the test suite)

@alalek alalek merged commit 55c1720 into opencv:master Sep 3, 2019
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
…_flag_issue

* - headers in "infer/" and "infer/ie/" folders are included into gapi_ext_hdrs;
+ because of that a few #includes are required in the headers
- HAVE_INF_ENGINE flag check in headers "infer/ie.hpp" and "infer/ie/util.hpp" is deleted

* - the "ie/util.hpp" header is a private header now as it's used for tests; it's been moved to the scr directory to the place next to the implementation file "ie/giebackend.cpp"
- the path to this header in files "ie/giebackend.cpp" and "test/infer/gapi_infer_ie_test.cpp" is updated
- As it's private header now and explicitly depends on IE, the "HAVE_INF_ENGINE" flag check is returned
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.

4 participants