dnn: add plugin support for OpenVINO by alalek · Pull Request #21745 · opencv/opencv

alalek · 2022-03-18T07:04:06Z

TODO:

Fix Windows build scripts for plugin: disable PCH
Fix Windows build scripts for plugin: .simd.hpp support
(postpone) BUILD_PLUGIN -> OPENCV_BUILD_PLUGIN (backlog, to be done as separate PR - need to fix other modules too)
Myriad detection (NCS/NCS2) - 2021.4+ supports only NCS2

force_builders=Custom,Custom Win
Xforce_builders=linux,docs,Custom,Custom Win

Xbuild_image:Custom=ubuntu-openvino-2022.1.0:20.04
build_image:Custom=ubuntu-openvino-2021.4.2:20.04
Xbuild_image:Custom=ubuntu-openvino-2022.1.0.dev20220131:20.04
build_image:Custom Win=openvino-2021.4.2
build_image:Custom Mac=openvino-2021.4.2

test_modules:Custom=dnn,gapi,python2,python3,java
test_modules:Custom Win=dnn,gapi,python2,python3,java
test_modules:Custom Mac=dnn,gapi,python2,python3,java

buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=ON
test_bigdata:Custom=1
test_filter:Custom=*

buildworker:Custom Win=windows-1
test_bigdata:Custom Win=1
test_filter:Custom Win=*
test_opencl:Custom Win=ON
build_contrib:Custom Win=OFF

allow_multiple_commits=1

alalek · 2022-09-13T11:32:06Z

@rogday It makes sense to start review from "backend API" in this file: modules/dnn/src/backend.hpp

mshabunin

Overall looks good to me, though I haven't reviewed some parts in details.

Some notes:

perhaps we need to extract common registry/factory/backend/cmake part from all plugin-compatible modules
we don't have world builder in CI, will we support this configuration?

mshabunin · 2022-09-22T10:31:24Z

modules/dnn/CMakeLists.txt

  endif()
-  list(APPEND dnn_runtime_libs ocv.3rdparty.openvino)
+  if("openvino" IN_LIST DNN_PLUGIN_LIST OR DNN_PLUGIN_LIST STREQUAL "all")
+    add_subdirectory(misc/plugin/openvino)  # plugin doesn't support PCH, separate directory scope is necessary


It does not work with world build:

$ cmake -GNinja \ -DWITH_OPENVINO=ON \ -DBUILD_opencv_world=ON \ ../opencv

CMake Error at modules/dnn/CMakeLists.txt:207 (add_subdirectory): add_subdirectory given source "misc/plugin/openvino" which is not an existing directory. Call Stack (most recent call first): modules/world/CMakeLists.txt:13 (include) modules/world/CMakeLists.txt:32 (include_one_module)

Fixed.

To properly use OV plugin need to disable GAPI module or OV-in-GAPI:

-DBUILD_opencv_gapi=OFF

or -DOPENCV_GAPI_WITH_OPENVINO=OFF

Otherwise gapi links OpenVINO to opencv_world directly, which affects DNN code compilation flags too.

mshabunin · 2022-09-22T20:16:51Z

modules/dnn/src/net_openvino.cpp

-//}  // Net::Impl
+
+#if 0
+#define printf_(args) printf args


Maybe it would be better to use logging facilities? Are they available in the plugin?

It makes sense in general.
Currently it is out of scope of this PR as mentioned code is a copy-paste.

rogday

Thank you for contribution! Design looks good.
A few minor comments.

modules/dnn/src/net_openvino.cpp

modules/dnn/src/net_impl.hpp

rogday · 2022-10-04T12:30:31Z

modules/dnn/src/plugin_wrapper.impl.hpp

+            {
+                CV_Assert(instancePtr);
+                // TODO C++20 "aliasing constructor"
+                return std::shared_ptr<cv::dnn_backend::NetworkBackend>(instancePtr, [](cv::dnn_backend::NetworkBackend*){});  // empty deleter


Maybe we could capture shared_from_this() inside a deleter for now?
Seems to achieve the same goal.

This requires changes in class inheritance (use std::enable_shared_from_this<T>). I prefer to keep interfaces clear as much as possible.

I don't want to transfer object lifetime control from the plugin (it could not prevent unloading of plugin code anyway).

Line 22: class PluginDNNBackend CV_FINAL: public std::enable_shared_from_this<PluginDNNBackend>

Then why shared_ptr is used in the first place?

Does TODO makes sense then?

PluginDNNBackend != NetworkBackend.

To avoid uncontrolled usage of RAW pointers.

AFAIK, to alias on plugins instance (the same is for highgui/parallel plugins - code borrowed from there)

We are in a class that is derived from enable_shared_from_this. The idea behind capturing shared_from_this()(PluginDNNBackend) inside a deleter of NetworkBackend was that the former wouldn't run its destructor before the latter.

In its current state they are equivalent.

The idea seems to be similar to p.1 above.

I believe we need a separate proposal which covers similar cases from other plugins.

rogday · 2022-10-04T12:34:55Z

modules/dnn/src/plugin_wrapper.impl.hpp

+        // TODO: implemented accurate versions-based comparator
+        std::sort(candidates.begin(), candidates.end(), std::greater<std::string>());
+        CV_LOG_DEBUG(NULL, "    - " << path << ": " << candidates.size());
+        copy(candidates.begin(), candidates.end(), back_inserter(results));


Not needed, as ADL should work here well.
(also there are other places with the same code which works well)

It's not immediately obvious why sort is used with std, but copy - without. Just consistency nitpick.

alalek · 2022-10-07T17:03:06Z

Rebased and removed test commit.

alalek · 2022-10-08T22:31:41Z

👍

fengyuentau · 2022-10-09T02:38:50Z

Great refactoring! I think I can have the same design as openvino (NetImplOpenVINO) for the CANN backend to have higher freedom to utilize CANN features.

dnn: use inheritance for OpenVINO net impl

ca7f964

asmorkalov requested a review from rogday September 9, 2022 12:09

mshabunin reviewed Sep 23, 2022

View reviewed changes

rogday reviewed Oct 4, 2022

View reviewed changes

asmorkalov assigned rogday Oct 4, 2022

dnn: plugin support for OpenVINO

43b2bb2

alalek unassigned rogday Oct 8, 2022

opencv-pushbot merged commit 3472469 into opencv:4.x Oct 8, 2022

alalek mentioned this pull request Jan 8, 2023

(5.x) Merge 4.x #23113

Merged

dkurt mentioned this pull request May 25, 2023

Plugin Inference Engine G-API Backend #23578

Open

Uh oh!

Conversation

alalek commented Mar 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalek commented Sep 13, 2022

Uh oh!

mshabunin left a comment

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!

rogday left a comment

Choose a reason for hiding this comment

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!

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!

alalek commented Oct 7, 2022

Uh oh!

alalek commented Oct 8, 2022

Uh oh!

fengyuentau commented Oct 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alalek commented Mar 18, 2022 •

edited

Loading