GAPI: Add OAK backend by smirnov-alexey · Pull Request #20785 · opencv/opencv

smirnov-alexey · 2021-10-01T05:09:58Z

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 other license that is incompatible with OpenCV
The PR is proposed to proper branch
There is reference to 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

force_builders=Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

build_image:Custom=ubuntu-openvino-2021.4.2:20.04
build_image:Custom Win=openvino-2021.4.2
build_image:Custom Mac=openvino-2021.4.2

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

buildworker:Custom=linux-1
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

smirnov-alexey · 2021-10-01T05:10:58Z

@dmatveev @TolyaTalamanov @rgarnov fyi. It's still WIP though

smirnov-alexey · 2021-10-11T13:32:24Z

@dmatveev @TolyaTalamanov @AsyaPronina @rgarnov please, take a look. This is rather a draft version and still need some polishing even for a hard-coded pipeline. Still, I would appreciate any feedback. Am I moving to the right direction here?

smirnov-alexey · 2021-10-11T13:48:58Z

The issue with corrupted frames still persists though

modules/gapi/samples/oak_rgb_encoding.cpp

modules/gapi/src/backends/oak/goak.cpp

modules/gapi/src/backends/oak/goak_media_adapter.cpp

modules/gapi/include/opencv2/gapi/oak/oak_media_adapter.hpp

modules/gapi/src/backends/oak/goakbackend.cpp

modules/gapi/test/oak/gapi_tests_oak.cpp

modules/gapi/src/backends/oak/goakbackend.cpp

dmatveev

Good to know it's alive, but it is far even from an experimental backend yet.

What I'd consider as a bare minimum for merge:

Generic handling of DAI operations via our kernel interface (let it be NOT public for now)
Generic handling of camera/non-camera case + tests for these two configurations

Inference, CV ops, etc. can be added atop of this baseline and once the fundamental issues (1) and (2) are solved.

modules/gapi/include/opencv2/gapi/oak/oak.hpp

dmatveev · 2021-10-26T22:17:48Z

modules/gapi/include/opencv2/gapi/oak/oak.hpp

+namespace detail {
+template<> struct CompileArgTag<gapi::oak::ColorCameraParams> {
+    static const char* tag() { return "gapi.oak.colorCameraParams"; }
+};


If we have camera here as a distinct object, should its setup be passed as a graph compile arg?

Actually, when the graph is compiled there's no idea whether a file or a camera will be used as input.

I'd leave it as is for now. Non-camera input will be introduced later in a separate PR

modules/gapi/include/opencv2/gapi/oak/oak_media_adapter.hpp

modules/gapi/samples/oak_rgb_encoding.cpp

modules/gapi/src/backends/oak/goak.cpp

modules/gapi/include/opencv2/gapi/oak/oak_media_adapter.hpp

dmatveev · 2021-10-26T22:28:01Z

modules/gapi/include/opencv2/gapi/oak/oak_media_adapter.hpp

+namespace oak {
+
+enum class OAKFrameFormat {
+    BGR = 0,


MediaFrame has these enums already,

There are more color formats supported by OAK, should we stick to BGR and NV12 only?

we should extend MediaFrame enums then. :)

Change to cv::MediaFormat

modules/gapi/src/backends/oak/goak_media_adapter.cpp

modules/gapi/src/backends/oak/goakbackend.cpp

…_backend

modules/gapi/src/backends/oak/goakbackend.cpp

modules/gapi/CMakeLists.txt

modules/gapi/include/opencv2/gapi/oak/oak.hpp

modules/gapi/samples/oak_rgb_camera_encoding.cpp

modules/gapi/samples/oak_small_hetero_pipeline.cpp

modules/gapi/src/backends/oak/goak_media_adapter.cpp

modules/gapi/src/backends/oak/goakbackend.cpp

dmatveev · 2021-12-24T17:44:53Z

modules/gapi/src/backends/oak/goakbackend.cpp

+        *in = &(videoEnc->input);
+        *out = &(videoEnc->bitstream);


but finally it looks here as intended :)

dmatveev · 2021-12-24T17:45:06Z

modules/gapi/src/backends/oak/goakbackend.cpp

+
+GAPI_OAK_KERNEL(GOAKSobelXY, cv::gapi::oak::GSobelXY) {
+    static void put(const std::unique_ptr<dai::Pipeline>& pipeline,
+                    const std::tuple<int, int>& camera_wh,


why do you need this in every kernel?

Some of the DAI operations need to set max resolution size

sivanov-work · 2022-01-10T17:59:09Z

modules/gapi/src/backends/oak/goakbackend.cpp

+                            cv::Size(static_cast<int>(oak_frame->getWidth()),
+                                     static_cast<int>(oak_frame->getHeight())),
+                            cv::MediaFormat::NV12,
+                            std::move(oak_frame->getData()));


but why still not to move oak_frame by itself?

What about VectorRef case below? I think we should be consistent here

could you describe what is VectorRef case (why we introduced it here) and why MediaFrame is not enough?

We use VectorRef for encoding - it's operation's output. VectorRef is basically a vector while MediaFrame is more flexible type. This logic could also be extended in the future for other types like cv::Mat, Opaque, etc

modules/gapi/src/backends/oak/goak.cpp

modules/gapi/src/backends/oak/oak_media_adapter.hpp

modules/gapi/src/backends/oak/goakbackend.cpp

rgarnov · 2022-01-11T13:54:36Z

modules/gapi/src/backends/oak/goakbackend.cpp

+        size_t out_counter = 0;
+        for (const auto& d : outs_data)
+        {
+            for (const auto& nh : nodes)


And here you can look at d->inNodes() (should be only one producer)

modules/gapi/src/backends/oak/goakbackend.cpp

smirnov-alexey · 2022-01-13T11:59:10Z

@dmatveev @sivanov-work please, take a look

sivanov-work

LGTM

sivanov-work · 2022-01-13T13:34:29Z

modules/gapi/src/backends/oak/goakbackend.cpp

+                            cv::Size(static_cast<int>(oak_frame->getWidth()),
+                                     static_cast<int>(oak_frame->getHeight())),
+                            cv::MediaFormat::NV12,
+                            std::move(oak_frame->getData()));


could you describe what is VectorRef case (why we introduced it here) and why MediaFrame is not enough?

modules/gapi/samples/oak_small_hetero_pipeline.cpp

dmatveev · 2022-01-13T18:01:47Z

modules/gapi/src/backends/oak/goak_media_adapter.cpp

+                            cv::MediaFrame::View::Strides{static_cast<long unsigned int>(m_sz.width),
+                                                          static_cast<long unsigned int>(m_sz.height)}};


Stride is a distance between every row in a plane, in bytes.
If passing width looks legit here for Y plane, how height could become a stride for UV plane?

⚠ @sivanov-work @rgarnov am I wrong here?

That worked somehow, but you're right. Changed to width and width for both planes

dmatveev · 2022-01-13T18:03:14Z

modules/gapi/src/backends/oak/goak_media_adapter.cpp

+    m_priv(new OAKMediaAdapter::Priv(sz, fmt, y_ptr, uv_ptr)) {};
+
+MediaFrame::View OAKMediaAdapter::access(MediaFrame::Access access) {
+    return m_priv->access(access);


Its debatable but so far OAKMediaAdapter itself may be private so no extra PIMPLing required.

modules/gapi/src/backends/oak/goakbackend.cpp

dmatveev · 2022-01-13T19:15:56Z

modules/gapi/src/backends/oak/goakbackend.cpp

+
+        // 2. Link output nodes to XLinkOut nodes
+        size_t out_counter = 0;
+        for (const auto& nh : out_nodes) {


there's no strict defined order in out_nodes right?

It's aligned with the backend's out_data and is being used in that aligned way

modules/gapi/src/backends/oak/goakbackend.cpp

dmatveev · 2022-01-13T19:17:58Z

modules/gapi/src/backends/oak/goakbackend.cpp

+        // 3. Link internal nodes to their parents
+        for (const auto& nh : inter_nodes) {
+            // Input nodes in OAK doesn't have parent operation - just camera (for now)
+            if (std::find(in_nodes.begin(), in_nodes.end(), nh) == in_nodes.end()) {


This search is linear too. Also why do you need this check here at all? Is your inter_nodes incorrect?

dmatveev

I believe it is ok to merge now

GAPI: Add OAK backend * Initial tests and cmake integration * Add a public header and change tests * Stub initial empty template for the OAK backend * WIP * WIP * WIP * WIP * Runtime dai hang debug * Refactoring * Fix hang and debug frame data * Fix frame size * Fix data size issue * Move test code to sample * tmp refactoring * WIP: Code refactoring except for the backend * WIP: Add non-camera sample * Fix samples * Backend refactoring wip * Backend rework wip * Backend rework wip * Remove mat encoder * Fix namespace * Minor backend fixes * Fix hetero sample and refactor backend * Change linking logic in the backend * Fix oak sample * Fix working with ins/outs in OAK island * Trying to fix nv12 problem * Make both samples work * Small refactoring * Remove meta args * WIP refactoring kernel API * Change in/out args API for kernels * Fix build * Fix cmake warning * Partially address review comments * Partially address review comments * Address remaining comments * Add memory ownership * Change pointer-to-pointer to reference-to-pointer * Remove unnecessary reference wrappers * Apply review comments * Check that graph contains only one OAK island * Minor refactoring * Address review comments

smirnov-alexey added 9 commits September 21, 2021 18:39

Initial tests and cmake integration

914f5fb

Add a public header and change tests

eefab1b

Stub initial empty template for the OAK backend

905b851

WIP

888f20b

WIP

fdc1992

WIP

c03cc5d

WIP

36fd217

Runtime dai hang debug

83bfa17

Refactoring

30e3355

smirnov-alexey added 4 commits October 7, 2021 01:01

Fix hang and debug frame data

0d2270d

Fix frame size

d2e5837

Fix data size issue

b4d7d47

Move test code to sample

eb0a839

sivanov-work reviewed Oct 13, 2021

View reviewed changes

sivanov-work reviewed Oct 19, 2021

View reviewed changes

modules/gapi/src/backends/oak/goakbackend.cpp Outdated Show resolved Hide resolved

tmp refactoring

911cee5

dmatveev self-assigned this Oct 26, 2021

dmatveev added the category: g-api / gapi label Oct 26, 2021

dmatveev reviewed Oct 26, 2021

View reviewed changes

smirnov-alexey added 3 commits December 8, 2021 14:59

WIP: Code refactoring except for the backend

5907c30

Merge branch 'master' of https://github.com/opencv/opencv into as/oak…

4784a21

…_backend

WIP: Add non-camera sample

20030b3

sivanov-work reviewed Dec 8, 2021

View reviewed changes

modules/gapi/src/backends/oak/goakbackend.cpp Outdated Show resolved Hide resolved

smirnov-alexey added 4 commits December 10, 2021 18:26

Fix samples

88cf11e

Backend refactoring wip

a4a4e5f

Backend rework wip

9a4262e

Backend rework wip

a17a668

smirnov-alexey added 2 commits December 23, 2021 14:22

Fix build

d661f73

Fix cmake warning

287d081

dmatveev modified the milestones: 5.0, 4.6.0 Dec 24, 2021

dmatveev reviewed Dec 24, 2021

View reviewed changes

smirnov-alexey added 5 commits December 27, 2021 17:19

Partially address review comments

d3f277f

Partially address review comments

6043dbb

Address remaining comments

055884a

Add memory ownership

01140fa

Change pointer-to-pointer to reference-to-pointer

482882a

sivanov-work reviewed Jan 10, 2022

View reviewed changes

rgarnov requested changes Jan 10, 2022

View reviewed changes

rgarnov requested changes Jan 11, 2022

View reviewed changes

smirnov-alexey added 3 commits January 12, 2022 11:50

Remove unnecessary reference wrappers

6d32145

Apply review comments

165f4d6

Check that graph contains only one OAK island

3d11bb7

rgarnov requested changes Jan 12, 2022

View reviewed changes

modules/gapi/src/backends/oak/goakbackend.cpp Outdated Show resolved Hide resolved

modules/gapi/src/backends/oak/goakbackend.cpp Outdated Show resolved Hide resolved

Minor refactoring

c2d6fb4

smirnov-alexey force-pushed the as/oak_backend branch from fe682dd to c2d6fb4 Compare January 12, 2022 15:09

rgarnov approved these changes Jan 12, 2022

View reviewed changes

sivanov-work approved these changes Jan 13, 2022

View reviewed changes

dmatveev reviewed Jan 13, 2022

View reviewed changes

Address review comments

bbfcd8f

smirnov-alexey changed the title ~~WIP: GAPI: Add OAK backend~~ GAPI: Add OAK backend Jan 17, 2022

dmatveev approved these changes Jan 17, 2022

View reviewed changes

alalek merged commit f2d5d6d into opencv:4.x Jan 17, 2022

alalek mentioned this pull request Feb 22, 2022

(5.x) Merge 4.x #21651

Merged

		cv::MediaFrame::View::Strides{static_cast<long unsigned int>(m_sz.width),
		static_cast<long unsigned int>(m_sz.height)}};

Uh oh!

Conversation

smirnov-alexey commented Oct 1, 2021 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

smirnov-alexey commented Oct 1, 2021

Uh oh!

smirnov-alexey commented Oct 11, 2021

Uh oh!

smirnov-alexey commented Oct 11, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmatveev 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smirnov-alexey Dec 8, 2021 • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smirnov-alexey commented Oct 1, 2021 •

edited by alalek

Loading

smirnov-alexey Dec 8, 2021 •

edited

Loading

dmatveev Jan 13, 2022 •

edited

Loading