G-API: IE. Adding support for INT32 type. by mpashchenkov · Pull Request #19792 · opencv/opencv

mpashchenkov · 2021-03-26T12:43:26Z

Summary:

This PR should solve problem with semantic-segmentation-adas-0001 network.
Added support for int32 input and output;
Added sample for semantic-segmentation-adas-0001.

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

Magic:

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.1.0

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=*

modules/gapi/test/infer/gapi_infer_ie_test.cpp

modules/gapi/samples/semantic_segmentation.cpp

OrestChura · 2021-03-26T23:25:07Z

modules/gapi/samples/semantic_segmentation.cpp

+    { 230, 0,   0 },
+    { 32,  11,  119 },
+    { 0,   74,  111 },
+};


These two things above can be stored into config namespace not to leave it without namespace..

What do you want to use namespace for here?

Hm, actually the goal is just to use namespace. To have all the supporting stuff in one place and be sure you're referring to the them right

tend do agree, config::colors or config::keys would be easier to read when you see it in the code below

OrestChura · 2021-03-26T23:26:54Z

modules/gapi/samples/semantic_segmentation.cpp

+    CV_Assert(ext == ".xml");
+    return model_path.substr(0u, sz - EXT_LEN) + ".bin";
+}
+} // anonymous namespace


I think it's not really needed to store it in an anonymous namespace.
Maybe, to one of around - below or above?

Unfortunately, need.

So, at your discretion whether to move it into, for example, custom namespace below

Don't we have this feature already? I believe someone in the team has proposed that a while ago -- only passing .xml so that .bin is identified automatically

TolyaTalamanov · 2021-03-29T08:48:15Z

modules/gapi/test/infer/gapi_infer_ie_test.cpp

    validate();
 }
+
+#if INF_ENGINE_RELEASE >= 2020010000


Why such #if needed in there, if it's handled inside giewrapper

It is for findDataFile(). For age gender we can find path for old IE. I don't know old path for semantic segmentation. And old IE isn't tested now, old path for network is deprecated.

old path - look at std::string SUBDIR *

Should I delete it?

Removed test.

OrestChura

Everything's great, thanks. Some cavils are not important

OrestChura · 2021-03-29T11:50:38Z

modules/gapi/samples/semantic_segmentation.cpp

+    { 230, 0,   0 },
+    { 32,  11,  119 },
+    { 0,   74,  111 },
+};


Hm, actually the goal is just to use namespace. To have all the supporting stuff in one place and be sure you're referring to the them right

OrestChura · 2021-03-29T12:10:29Z

modules/gapi/samples/semantic_segmentation.cpp

+    CV_Assert(ext == ".xml");
+    return model_path.substr(0u, sz - EXT_LEN) + ".bin";
+}
+} // anonymous namespace


So, at your discretion whether to move it into, for example, custom namespace below

dmatveev

Please prioritize merge

dmatveev · 2021-03-29T12:56:08Z

modules/gapi/samples/semantic_segmentation.cpp

+    { 230, 0,   0 },
+    { 32,  11,  119 },
+    { 0,   74,  111 },
+};


tend do agree, config::colors or config::keys would be easier to read when you see it in the code below

dmatveev · 2021-03-29T12:57:25Z

modules/gapi/samples/semantic_segmentation.cpp

+    CV_Assert(ext == ".xml");
+    return model_path.substr(0u, sz - EXT_LEN) + ".bin";
+}
+} // anonymous namespace


Don't we have this feature already? I believe someone in the team has proposed that a while ago -- only passing .xml so that .bin is identified automatically

dmatveev · 2021-03-29T12:59:20Z

modules/gapi/samples/semantic_segmentation.cpp

+    }
+};
+
+GAPI_OCV_KERNEL(OCVPostProcessing, PostProcessing) {


I'd call it "colorize" or smt like this

dmatveev · 2021-03-29T13:00:55Z

modules/gapi/samples/semantic_segmentation.cpp

+        cv::resize(maskImg, out, in.size());
+        const float blending = 0.3f;
+        out = in * blending + out * (1 - blending);


I'd move it out of the kernel, if possible, and make it part of the graph.

Custom kernel could only produce a mask, and the rest done with our standard ops.

There are performance reasons to do that (e.g. involving a fluid backend for *, +, 1- and so on)

Again, it can be done after we merge this fix itself, so just take all sample-related comments as a to-do

TolyaTalamanov

LGTM 👍

@alalek I believe it can be merged

G-API: IE. Adding support for INT32 type. * Added support for int32 * Added sample for semantic-segmentation-adas-0001 * Alignment * Alignment 2 * Rstrt build * Removed test for sem seg

mpashchenkov added category: g-api / gapi test labels Mar 26, 2021

Added support for int32

77dd154

mpashchenkov force-pushed the mp/ie-add-int32 branch from 04e831a to 77dd154 Compare March 26, 2021 12:48

mpashchenkov requested a review from TolyaTalamanov March 26, 2021 12:48

mpashchenkov assigned TolyaTalamanov Mar 26, 2021

mpashchenkov requested a review from OrestChura March 26, 2021 12:53

Added sample for semantic-segmentation-adas-0001

57bae61

OrestChura reviewed Mar 26, 2021

View reviewed changes

Alignment

ecc7903

TolyaTalamanov reviewed Mar 29, 2021

View reviewed changes

Maxim Pashchenkov added 2 commits March 29, 2021 11:56

Alignment 2

5137bfc

Rstrt build

459b628

mpashchenkov added this to the 4.5.2 milestone Mar 29, 2021

Removed test for sem seg

fe7abe1

OrestChura approved these changes Mar 29, 2021

View reviewed changes

dmatveev approved these changes Mar 29, 2021

View reviewed changes

TolyaTalamanov approved these changes Mar 29, 2021

View reviewed changes

alalek merged commit e700766 into opencv:master Mar 29, 2021

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #19885

Merged

Uh oh!

Conversation

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

Uh oh!

Summary:

Pull Request Readiness Checklist

Magic:

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!

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!

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

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

Choose a reason for hiding this comment

Uh oh!

TolyaTalamanov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mpashchenkov commented Mar 26, 2021 •

edited

Loading