[G-API] Implement OpenVINO 2.0 backend by TolyaTalamanov · Pull Request #23595 · opencv/opencv

TolyaTalamanov · 2023-05-09T12:52:44Z

Pull Request Readiness Checklist

Implemented basic functionality for OpenVINO 2.0 G-API backend.

Overview

Implement Infer kernel with some of essential configurable parameters + IR/Blob models format support.
Implement the rest of kernels: InferList, InferROI, Infer2 + other configurable params (e.g reshape)
Asyncrhonous execution support
Remote context support

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

modules/gapi/include/opencv2/gapi/infer/ov.hpp

TolyaTalamanov · 2023-05-09T14:15:22Z

modules/gapi/test/infer/gapi_infer_ov_tests.cpp

+    EXPECT_LE(normInf, lInf) << comment;
+}
+
+ov::Core getCore() {


Is it fine to use the different ov::Core than it's in GOVBackend? (I think so)

TolyaTalamanov · 2023-05-09T14:16:16Z

modules/gapi/test/infer/gapi_infer_ov_tests.cpp

+    static constexpr const char* tag = "age-gender-generic";
+    using Params = cv::gapi::ov::Params<cv::gapi::Generic>;
+
+    static Params params(const std::string &xml,


can be variadic with by passing all parameters

cosmetic, ignore so far

modules/gapi/src/backends/ov/govbackend.cpp

TolyaTalamanov · 2023-05-09T14:18:31Z

modules/gapi/src/backends/ov/govbackend.cpp

+    explicit OVUnit(const ParamDesc &pd)
+        : params(pd) {
+
+        if (cv::util::holds_alternative<ParamDesc::IR>(params.kind)) {


Probably we need this logic encapsulated somewhere not to check it everywhere!!!

TolyaTalamanov · 2023-05-09T14:23:02Z

modules/gapi/src/backends/ov/govbackend.cpp

+        for (auto i : ade::util::iota(ctx->uu.params.num_in)) {
+            const auto& input_name = ctx->uu.params.input_names[i];
+            auto input_tensor = infer_request.get_tensor(input_name);
+            copyToOV(ctx->inMat(i), input_tensor);


in fact it might be faster for "some" plugins to copy to pre-allocated buffer instead of creating another one over the cv::Mat memory without copy. The same for output - let plugin write to its own buffer and them copy from there.

They way of how to set/get data from openvino can be the strategy... (worth to add TODO at least)

TolyaTalamanov · 2023-05-09T14:23:54Z

@dmatveev, @smirnov-alexey Folks! Could you have a look, please?

modules/gapi/include/opencv2/gapi/infer/ov.hpp

modules/gapi/misc/python/shadow_gapi.hpp

TolyaTalamanov · 2023-05-29T12:45:05Z

@asmorkalov @opencv-alalek

Could you help me to understand what exactly goes wrong with that error:

/build/precommit_docs/4.x/opencv/modules/python/src2/typing_stubs_generator.py:52: UserWarning: Typing stubs generation has failed. Reason: Failed to resolve "cv2" namespace against "None". Errors: ['Failed to resolve "cv2.gapi" namespace against "cv2". Errors: ['Failed to resolve "cv2.gapi.ov" namespace against "cv2". Errors: [\'Failed to resolve "cv2.gapi.ov.PyParams" class against "cv2". Errors: [\\\'Failed to resolve "cv2.gapi.ov.PyParams.cfgPluginConfig" function against "cv2". Errors: [0]: Failed to resolve "config" argument: Failed to resolve "map_string_and_string" exposed as "map_string_and_string"\\\', \\\'Failed to resolve "cv2.gapi.ov.PyParams.cfgInputTensorLayout" function against "cv2". Errors: [0]: Failed to resolve "layout_map" argument: Failed to resolve "map_string_and_string" exposed as "map_string_and_string"\\\', \\\'Failed to resolve "cv2.gapi.ov.PyParams.cfgInputModelLayout" function against "cv2". Errors: [0]: Failed to resolve "layout_map" argument: Failed to resolve "map_string_and_string" exposed as "map_string_and_string"\\\', \\\'Failed to resolve "cv2.gapi.ov.PyParams.cfgOutputTensorLayout" function against "cv2". Errors: [0]: Failed to resolve "layout_map" argument: Failed to resolve "map_string_and_string" exposed as "map_string_and_string"\\\', \\\'Failed to resolve "cv2.gapi.ov.PyParams.cfgOutputModelLayout" function against "cv2". Errors: [0]: Failed to resolve "layout_map" argument: Failed to resolve "map_string_and_string" exposed as "map_string_and_string"\\\', \\\'Failed to resolve "cv2.gapi.ov.PyParams.cfgOutputTensorPrecision" function against "cv2". Errors: [0]: Failed to resolve "precision_map" argument: Failed to resolve "map_string_and_int" exposed as "map_string_and_int"\\\', \\\'Failed to resolve "cv2.gapi.ov.PyParams.cfgReshape" function against "cv2". Errors: [0]: Failed to resolve "new_shape_map" argument: Failed to resolve "map_string_and_vector_size_t" exposed as "map_string_and_vector_size_t"\\\']\']']']

dkurt · 2023-05-29T13:17:17Z

@TolyaTalamanov, seems related to #20370 (probably, need to be added something here: https://github.com/opencv/opencv/pull/20370/files#diff-d7f280d2a075f044f0b9720fa39e3fff9a6346c3a8eccb0cf588f0b7e52a9f6bR132-R138)

TolyaTalamanov · 2023-05-29T19:32:58Z

modules/gapi/src/backends/ov/govbackend.cpp

+                    auto batch_layout = (matdesc.dims[1] == 3 || matdesc.dims[1] == 1)
+                                        ? ::ov::Layout("NCHW") : ::ov::Layout("NHWC");
+
+                    cv::Size spatial_size = (matdesc.dims[1] == 3 || matdesc.dims[1] == 1)


or spatial_size can be obtained from tensor_layout if such explicitly specified by user

smirnov-alexey · 2023-05-30T20:41:58Z

modules/gapi/include/opencv2/gapi/infer/ov.hpp

+    Supported algorithms: #INTER_NEAREST, #INTER_LINEAR, #INTER_CUBIC.
+    @return reference to this parameter structure.
+    */
+    Params<Net>& cfgResize(int interpolation) {


Should we add a default value here?

Well, probably but then there won't be any default value for map case?

smirnov-alexey · 2023-05-30T20:43:22Z

modules/gapi/include/opencv2/gapi/infer/ov.hpp

+    }
+
+    /** @see ov::Params::cfgPluginConfig. */
+    Params& cfgPluginConfig(const detail::ParamDesc::PluginConfigT &config) {


Wow, is there a way to avoid copy-paste for generic params?

I didn't find, template specialization or inheritance won't work there because it returns Params&

CRTP would?

But don't do it now to expedite merge, just leave a TODO.

smirnov-alexey · 2023-05-30T20:45:36Z

modules/gapi/misc/python/test/test_gapi_infer_ov.py

+            skip_if_openvino_not_available()
+
+            root_path  = '/omz_intel_models/intel/age-gender-recognition-retail-0013/FP32/age-gender-recognition-retail-0013'
+            model_path = self.find_file(root_path + '.xml',   [os.environ.get('OPENCV_DNN_TEST_DATA_PATH')])


Could we utilize other models here? E.g. .onnx?

We're using a couple of them for ONNX backend tests.

smirnov-alexey · 2023-05-30T20:48:27Z

modules/gapi/misc/python/test/test_gapi_infer_ov.py

+            # Check
+            self.assertEqual(0.0, cv.norm(ov_gender, gapi_gender, cv.NORM_INF))
+            self.assertEqual(0.0, cv.norm(ov_age, gapi_age, cv.NORM_INF))
+


What about testing invalid cases? Where we get G-API or OV throwing an exception

Well, there is not much invalid cases. The validation is only happens for Image case where the layout is being validated to be NHWC for other case there should be check that mat dims matches to tensor dims I added TODO for that

smirnov-alexey · 2023-05-30T20:49:33Z

modules/gapi/src/api/gmat.cpp

+        if (desc.planar) os << "p";
+        os << " ";
+        os << desc.size.width << "x" << desc.size.height;
+    }


Won't it break our serialization? Is there a >> operator?

No, see: https://github.com/opencv/opencv/blob/4.x/modules/gapi/src/backends/common/serialization.cpp#L616-L621

No, as S11N is not using i/ostream as LHS operand for <<.

smirnov-alexey · 2023-05-30T20:50:44Z

modules/gapi/src/backends/ov/bindings_ov.cpp

+}
+
+cv::gapi::ov::PyParams&
+cv::gapi::ov::PyParams::cfgPluginConfig(


Why don't we have both general params and generic ones?

What stands for general?
In C++ we have templated params and their specialization for generic case.
For python there is only generic params are relevant.

smirnov-alexey · 2023-05-30T20:53:41Z

modules/gapi/src/backends/ov/util.hpp

+// NB: These functions are EXPORTed to make them accessible by the
+// test suite only.
+GAPI_EXPORTS std::vector<int> to_ocv(const ::ov::Shape &shape);
+GAPI_EXPORTS int to_ocv(const ::ov::element::Type &type);


I suggest implementing those in utils.cpp or hiding if it's not being used outside

It's only for our tests. Is there any difference in terms of where implementation should be placed?
govbackend.cpp our sources that are not exposed to user.

smirnov-alexey · 2023-05-30T20:58:09Z

modules/gapi/src/backends/common/gbackend.hpp

@@ -227,6 +227,12 @@ inline void convertInt64ToInt32(const int64_t* src, int* dst, size_t size)
                   [](int64_t el) { return static_cast<int>(el); });


Suggesting int32_t here2, just in case

smirnov-alexey · 2023-05-30T21:00:45Z

modules/gapi/src/backends/ov/govbackend.cpp

+    //   (and constructed by either bindIn/Out or resetInternal)
+    case cv::GShape::GOPAQUE:  return cv::GArg(m_res.slot<cv::detail::OpaqueRef>().at(ref.id));
+
+    case cv::GShape::GFRAME:  return cv::GArg(m_res.slot<cv::MediaFrame>().at(ref.id));


I'd remove it from here

Yes, make sense

smirnov-alexey · 2023-05-30T21:01:14Z

modules/gapi/src/backends/ov/govbackend.cpp

+
+const cv::MediaFrame& OVCallContext::inFrame(std::size_t input) const {
+    return inArg<cv::MediaFrame>(input);
+}


I'd remove it for now

smirnov-alexey · 2023-05-30T21:04:27Z

modules/gapi/src/backends/ov/govbackend.cpp

+        m_request.set_callback([](std::exception_ptr){});
+        m_notify();
+        throw;
+    }


Do we notify in the end?

Usually we notify in the callback but here execution failed earlier

smirnov-alexey · 2023-05-30T21:06:31Z

modules/gapi/src/backends/ov/govbackend.cpp

+
+IInferExecutor::Ptr cv::gimpl::ov::RequestPool::getIdleRequest() {
+    size_t id = 0u;
+    m_idle_ids.pop(id);


Where do we set those to 0?

setup() fills m_idle_ids

smirnov-alexey · 2023-05-30T21:09:19Z

modules/gapi/src/backends/ov/govbackend.cpp

+    return (model_shape.size() == 4u)                      &&
+           (!desc.isND())  /* dims == 2 */                 &&
+           (desc.chan == 1 || desc.chan == 3)              &&
+           (desc.size.height != 1 && desc.size.width != 1) &&


Btw do you remember if we might have any non-1 dims in some of our cases which are still tensors? (e.g. 2, 3, other small numbers which are definitely are not images)

We can have Mat({15, 14}, CV_8UC1) but in this case model apparently will have 2D dimensions for that input so that it would be TENSOR for us

The point here is that 2D mats may work with 4D model shape only in case of IMAGE.
For TENSOR number of dims must match model shape size. (good point for comment)

smirnov-alexey · 2023-05-30T21:10:08Z

modules/gapi/src/backends/ov/govbackend.cpp

+
+static bool isImage(const cv::GMatDesc &desc,
+                    const ::ov::Shape  &model_shape) {
+    return (model_shape.size() == 4u)                      &&


Could it be 3? Let's say if users/opencv doesn't provide N somehow?

User doesn't provide anything. model_shape comes from IR / ov::Model and in all cases for real image models have 4D layout in OpenVINO.

smirnov-alexey · 2023-05-30T21:11:18Z

modules/gapi/src/backends/ov/govbackend.cpp

+                    // NB: Double check that user provided correct layout.
+                    // In fact, there is only one correct for image.
+                    if (explicit_in_tensor_layout) {
+                        if (*explicit_in_tensor_layout != "NHWC") {


You can combine those 2 ifs

smirnov-alexey · 2023-05-30T21:12:28Z

modules/gapi/src/backends/ov/govbackend.cpp

+                                    ? ::ov::Layout(*explicit_in_tensor_layout) : model_layout;
+                                const auto H_idx = ::ov::layout::height_idx(layout);
+                                const auto W_idx = ::ov::layout::width_idx(layout);
+                                // NB: It still possible if layout is "??HW".


Please document that in our doc and/or code

smirnov-alexey · 2023-05-30T21:13:39Z

modules/gapi/src/backends/ov/govbackend.cpp

+                                input_info.preprocess().resize(toOVInterp(*explicit_resize));
+                            }
+                        } else {
+                            // NB: 2D case - know exactly where H and W...


Could you please add some debug messages here other that throws? I mean what we assume in which case

Here = part with image/tensor logic. Messages = at least comments you put here

smirnov-alexey

Overall looks great! Although, please answer my comments

TolyaTalamanov · 2023-05-31T11:30:04Z

I hope I addressed everthing...
Let's wait for the CI and merge

dmatveev · 2023-06-01T10:19:31Z

@asmorkalov could you please have a look at this one?

The CI failure looks irrelevant

[  PASSED  ] 453 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] Imgcodecs_Tiff.decode_tile16384x16384

asmorkalov · 2023-06-02T09:58:26Z

Restarted the failed build. Looks like we need to tune limits. Windows host is overbooked time to time.

dmatveev · 2023-06-02T18:08:11Z

Yay! Thanks & congrats! A big step forward!

…vino-backend [G-API] Implement OpenVINO 2.0 backend opencv#23595 ### Pull Request Readiness Checklist Implemented basic functionality for `OpenVINO` 2.0 G-API backend. #### Overview - [x] Implement `Infer` kernel with some of essential configurable parameters + IR/Blob models format support. - [ ] Implement the rest of kernels: `InferList`, `InferROI`, `Infer2` + other configurable params (e.g reshape) - [x] Asyncrhonous execution support - [ ] Remote context support See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] 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 - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake

TolyaTalamanov commented May 9, 2023

View reviewed changes

asmorkalov added category: g-api / gapi feature labels May 10, 2023

dmatveev reviewed May 13, 2023

View reviewed changes

modules/gapi/include/opencv2/gapi/infer/ov.hpp Outdated Show resolved Hide resolved

dmatveev added this to the 4.8.0 milestone May 23, 2023

TolyaTalamanov commented May 28, 2023

View reviewed changes

modules/gapi/misc/python/shadow_gapi.hpp Outdated Show resolved Hide resolved

TolyaTalamanov added 15 commits May 29, 2023 06:17

Implement basic inference

638d011

Add PrePostProcessor

2b05b8a

Add more configure options

01f22f1

Clean up

eabc3b2

Fix build without openvino

ddd5631

Fix typo

c6faa9c

Support plugin config

a092f6d

remove blank line

e03bf7e

Make plugin config map instead of unordered_map

9bb072b

Fix comments to review

471c704

Add more cfg methods

6fc1bb4

Add reshape

d8dfdc5

Add python bindings

14175d4

Fix doc warnings

896eeec

Fix windows warnings

fce1678

TolyaTalamanov force-pushed the at/implement-openvino-backend branch from c992b85 to fce1678 Compare May 29, 2023 06:28

Support parallel infer requests

5f32ed6

TolyaTalamanov added 3 commits May 29, 2023 14:59

Add batched image support

62aef86

Add predefined types

799dbdd

Add scale/mean support

f25e0d4

TolyaTalamanov commented May 29, 2023

View reviewed changes

smirnov-alexey reviewed May 30, 2023

View reviewed changes

smirnov-alexey approved these changes May 30, 2023

View reviewed changes

TolyaTalamanov added 2 commits May 31, 2023 09:25

Fix comments for review

dd734f0

Clean up and fix comments

fb6528f

asmorkalov merged commit 5330112 into opencv:4.x Jun 2, 2023

asmorkalov assigned dmatveev Jun 2, 2023

TolyaTalamanov mentioned this pull request Jun 14, 2023

OpenVino 2023.0.0 and first inference speed-up #23794

Open

asmorkalov mentioned this pull request Jul 12, 2023

(5.x) Merge 4.x #23979

Merged

		@@ -227,6 +227,12 @@ inline void convertInt64ToInt32(const int64_t* src, int* dst, size_t size)
		[](int64_t el) { return static_cast<int>(el); });

Uh oh!

Conversation

TolyaTalamanov commented May 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov commented May 9, 2023

Uh oh!

Uh oh!

Uh oh!

TolyaTalamanov commented May 29, 2023

Uh oh!

dkurt commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TolyaTalamanov May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov commented May 9, 2023 •

edited

Loading

dkurt commented May 29, 2023 •

edited

Loading

TolyaTalamanov May 29, 2023 •

edited

Loading

TolyaTalamanov May 31, 2023 •

edited

Loading

TolyaTalamanov May 31, 2023 •

edited

Loading

smirnov-alexey May 30, 2023 •

edited

Loading