[G-API] Fix blocking desc usage by TolyaTalamanov · Pull Request #19738 · opencv/opencv

TolyaTalamanov · 2021-03-17T09:46:07Z

resolves #19719

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

Build configuration

Xforce_builders_only=linux,docs
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.1.0:20.04
build_image:Custom Win=openvino-2021.1.0
build_image:Custom Mac=openvino-2021.2.0

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
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

TolyaTalamanov · 2021-03-17T09:47:52Z

This going to fix: #19719

TolyaTalamanov · 2021-03-17T09:48:52Z

@AsyaPronina Could you have a look ?

modules/gapi/src/backends/ie/giebackend.cpp

alalek

It make sense to add C++ regression test for that case.

alalek · 2021-03-18T08:26:47Z

modules/gapi/src/backends/ie/giebackend.cpp

+                               {0, 2, 3, 1} /* order for NHWC   */,
+                               0            /* offset           */,
                               {0, 0, 0, 0} /* offsets for dims */,
                               {strideH * height, strideH, channels, 1} /* strides for dims */);


{1, channels, height, width} /* dims */
...
{strideH * height, strideH, channels, 1} /* strides for dims */

Does strides correspond to dims after the change?

Usually dims follow in logical order - NCHW, but strides describe actual layout.
The problem was in step we don't need to consider size of precision in bytes there.
For instance: CV_FLOAT32C3 - 3 elements in strides, but we used 3 * sizeof(float)

@ilya-lavrenov Could you please take a look how to properly pass cv::Mat to InferenceEngine here? (or point to some example in IE / OMZ)

https://docs.openvinotoolkit.org/latest/openvino_docs_IE_DG_Memory_primitives.html

Code after the fix looks good to me

CC @ilyachur

modules/gapi/src/backends/ie/giebackend.cpp

TolyaTalamanov · 2021-03-18T12:54:01Z

@AsyaPronina The fix is OK ?

AsyaPronina · 2021-03-18T14:03:09Z

@AsyaPronina The fix is OK ?

Yes, thanks!

TolyaTalamanov · 2021-03-18T20:18:30Z

@alalek Could you help with CI, please ?
https://pullrequest.opencv.org/buildbot/builders/precommit_custom_mac/builds/1791/steps/test_python3/logs/stdio

alalek · 2021-03-18T20:27:59Z

There is email notification about NCS2 stick in public CI (~1 week ago). The latest IE version should be used.

ilya-lavrenov · 2021-03-21T19:54:32Z

modules/gapi/src/backends/ie/giebackend.cpp


-        const size_t strideH  = mat.step[0];
-
-        IE::BlockingDesc bdesc({1, height, width, channels} /* dims */,


looks like the original code actually is correct in terms of order for "blocked_dims", they should come in actual order NHWC..

I agree. BlockedDims already describes the physical dimensions.

TolyaTalamanov requested a review from AsyaPronina March 17, 2021 09:48

TolyaTalamanov commented Mar 17, 2021

View reviewed changes

modules/gapi/src/backends/ie/giebackend.cpp Show resolved Hide resolved

alalek reviewed Mar 18, 2021

View reviewed changes

TolyaTalamanov commented Mar 18, 2021

View reviewed changes

modules/gapi/src/backends/ie/giebackend.cpp Show resolved Hide resolved

AsyaPronina approved these changes Mar 18, 2021

View reviewed changes

Fix blocking desc usage

bad08d2

TolyaTalamanov force-pushed the at/giebackend-blockingdesc-fix branch from bdc301c to bad08d2 Compare March 19, 2021 09:48

alalek assigned AsyaPronina Mar 19, 2021

opencv-pushbot merged commit 3f52d0e into opencv:master Mar 19, 2021

ilya-lavrenov reviewed Mar 21, 2021

View reviewed changes

TolyaTalamanov mentioned this pull request Mar 22, 2021

[G-API] Blocking dims order fix #19763

Merged

6 tasks

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #19885

Merged


		const size_t strideH = mat.step[0];

		IE::BlockingDesc bdesc({1, height, width, channels} /* dims */,

Uh oh!

Conversation

TolyaTalamanov commented Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Build configuration

Uh oh!

TolyaTalamanov commented Mar 17, 2021

Uh oh!

TolyaTalamanov commented Mar 17, 2021

Uh oh!

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

alalek Mar 18, 2021

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov Mar 18, 2021

Choose a reason for hiding this comment

Uh oh!

alalek Mar 18, 2021

Choose a reason for hiding this comment

Uh oh!

ilya-lavrenov Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

ilya-lavrenov Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TolyaTalamanov commented Mar 18, 2021

Uh oh!

AsyaPronina commented Mar 18, 2021

Uh oh!

TolyaTalamanov commented Mar 18, 2021

Uh oh!

alalek commented Mar 18, 2021

Uh oh!

ilya-lavrenov Mar 21, 2021

Choose a reason for hiding this comment

Uh oh!

ilyachur Mar 22, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

TolyaTalamanov commented Mar 17, 2021 •

edited

Loading