G-API: Support GFrame for ONNX infer by mpashchenkov · Pull Request #19070 · opencv/opencv

mpashchenkov · 2020-12-10T10:57:06Z

Support cv::MediaFrame for infer input in ONNX backend.

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

Custom build:

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-onnx:20.04
build_image:Custom Win=ubuntu-onnx:20.04
build_image:Custom Mac=ubuntu-onnx:20.04

test_modules:Custom=gapi
test_modules:Custom Win=gapi
test_modules:Custom Mac=gapi

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

mpashchenkov · 2020-12-10T12:14:24Z

@smirnov-alexey, please review.

smirnov-alexey · 2020-12-10T13:57:09Z

@mpashchenkov please extend builds section (can take from here #19009 I believe)

modules/gapi/src/backends/onnx/gonnxbackend.cpp

smirnov-alexey · 2020-12-10T14:12:57Z

modules/gapi/src/backends/onnx/gonnxbackend.cpp

+                break;
+            }
+            case cv::GMetaArg::index_of<cv::GFrameDesc>(): {
+                // FIXME: Is there any validation for GFrame ?


Maybe you need to check color formats here? If there are similar cases with no checks for GFrameDesc lets just left this as is

Is it done?

Added check for GFrameDesc parameters. I think it should be a function (analogue of isND()) because IE backend should do similar checks. Create a ticket for this?

modules/gapi/src/backends/onnx/gonnxbackend.cpp

smirnov-alexey · 2020-12-10T14:14:33Z

Overall looks good, thanks!

mpashchenkov · 2020-12-16T10:10:18Z

@TolyaTalamanov, @dmatveev, it is.

TolyaTalamanov · 2020-12-16T10:59:55Z

modules/gapi/src/backends/onnx/gonnxbackend.cpp

    }
 }

+void preprocess(const std::unique_ptr<cv::MediaFrame::View>& view,


It can be just View& as for giebackend

dmatveev

I think it can be merged

modules/gapi/src/backends/onnx/gonnxbackend.cpp

dmatveev · 2020-12-18T10:49:58Z

modules/gapi/src/backends/onnx/gonnxbackend.cpp

+    switch (ctx.in_shapes[in_idx]) {
+        case cv::GShape::GFRAME: {
+            const cv::MediaFrame& frame = ctx.inFrame(in_idx);
+            views.emplace_back(new cv::MediaFrame::View(frame.access(cv::MediaFrame::Access::R)));


Make sure your views is preallocated to the right size.

Added assert.

it should be a reserve() somewhere earlier in the stack.

the idea is to avoid vector reallocations if possible, but since elements will be moved instead of copied, it should still work for unique pointers over views.

dmatveev · 2020-12-18T10:51:20Z

modules/gapi/src/backends/onnx/gonnxbackend.cpp

+                break;
+            }
+            case cv::GMetaArg::index_of<cv::GFrameDesc>(): {
+                // FIXME: Is there any validation for GFrame ?


Is it done?

… non processed input data

dmatveev · 2020-12-22T18:10:03Z

@mpashchenkov is it ready for review?

dmatveev · 2020-12-24T08:13:12Z

modules/gapi/src/backends/onnx/gonnxbackend.cpp

+    switch (ctx.in_shapes[in_idx]) {
+        case cv::GShape::GFRAME: {
+            const cv::MediaFrame& frame = ctx.inFrame(in_idx);
+            views.emplace_back(new cv::MediaFrame::View(frame.access(cv::MediaFrame::Access::R)));


it should be a reserve() somewhere earlier in the stack.

the idea is to avoid vector reallocations if possible, but since elements will be moved instead of copied, it should still work for unique pointers over views.

dmatveev · 2020-12-24T08:16:36Z

modules/gapi/src/backends/onnx/gonnxbackend.cpp

+            break;
+        }
+        case cv::GShape::GMAT: {
+            exMat = ctx.inMat(in_idx);


It also requires preprocessing, isn't it?

...resize, normalization, etc. Where does it happen now for a regular GMat input?

oh I see you do it also in setInput(). This flow is somewhat complicated, do you have two preprocess() functions now which do different things?

dmatveev

Let's merge it now as it seem to work, but file a refactoring task to our tracker.

What is confusing here now is calling void extractMat(...) first and then accessing some .exMat field. This needs to be reworked to make side effects more clear.

G-API: Support GFrame for ONNX infer * Added GFrame for ONNX * Cut test * Removed IE from assert * Review comments * Added const/bbot rstrt * View instead unique_ptr in func. sig. * Added extractMat function, ONNXCompiled contains exMat - cv::Mat with non processed input data * Added meta check for inferList2

Added GFrame for ONNX

9f32a49

mpashchenkov added test category: g-api / gapi labels Dec 10, 2020

Cut test

67b6535

Removed IE from assert

cde78d0

smirnov-alexey reviewed Dec 10, 2020

View reviewed changes

modules/gapi/src/backends/onnx/gonnxbackend.cpp Outdated Show resolved Hide resolved

smirnov-alexey reviewed Dec 10, 2020

View reviewed changes

modules/gapi/src/backends/onnx/gonnxbackend.cpp Outdated Show resolved Hide resolved

smirnov-alexey reviewed Dec 10, 2020

View reviewed changes

modules/gapi/src/backends/onnx/gonnxbackend.cpp Outdated Show resolved Hide resolved

smirnov-alexey reviewed Dec 10, 2020

View reviewed changes

modules/gapi/src/backends/onnx/gonnxbackend.cpp Outdated Show resolved Hide resolved

smirnov-alexey reviewed Dec 10, 2020

View reviewed changes

modules/gapi/src/backends/onnx/gonnxbackend.cpp Show resolved Hide resolved

smirnov-alexey reviewed Dec 10, 2020

View reviewed changes

modules/gapi/src/backends/onnx/gonnxbackend.cpp Show resolved Hide resolved

smirnov-alexey reviewed Dec 10, 2020

View reviewed changes

modules/gapi/src/backends/onnx/gonnxbackend.cpp Show resolved Hide resolved

Maxim Pashchenkov added 2 commits December 11, 2020 12:43

Review comments

f6f2e38

Added const/bbot rstrt

7a43d39

mpashchenkov requested a review from TolyaTalamanov December 16, 2020 13:39

TolyaTalamanov reviewed Dec 16, 2020

View reviewed changes

View instead unique_ptr in func. sig.

c738ee5

mpashchenkov requested a review from dmatveev December 17, 2020 10:07

dmatveev reviewed Dec 18, 2020

View reviewed changes

Maxim Pashchenkov added 2 commits December 22, 2020 11:34

Added extractMat function, ONNXCompiled contains exMat - cv::Mat with…

27732ee

… non processed input data

Added meta check for inferList2

74558f7

dmatveev reviewed Dec 24, 2020

View reviewed changes

dmatveev added this to the 4.5.2 milestone Dec 24, 2020

dmatveev self-assigned this Dec 24, 2020

dmatveev approved these changes Dec 24, 2020

View reviewed changes

alalek merged commit 656b20a into opencv:master Dec 24, 2020

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #19885

Merged

Uh oh!

Conversation

mpashchenkov commented Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Support cv::MediaFrame for infer input in ONNX backend.

Pull Request Readiness Checklist

Custom build:

Uh oh!

mpashchenkov commented Dec 10, 2020

Uh oh!

smirnov-alexey commented Dec 10, 2020

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!

Uh oh!

smirnov-alexey commented Dec 10, 2020

Uh oh!

mpashchenkov commented Dec 16, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmatveev left a comment

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!

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!

dmatveev commented Dec 22, 2020

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!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mpashchenkov commented Dec 10, 2020 •

edited

Loading