Skip to content

[G-API] Introduce cv.gin/cv.descr_of for python#19322

Merged
alalek merged 16 commits intoopencv:masterfrom
TolyaTalamanov:at/python-callbacks
Mar 1, 2021
Merged

[G-API] Introduce cv.gin/cv.descr_of for python#19322
alalek merged 16 commits intoopencv:masterfrom
TolyaTalamanov:at/python-callbacks

Conversation

@TolyaTalamanov
Copy link
Copy Markdown
Contributor

@TolyaTalamanov TolyaTalamanov commented Jan 14, 2021

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

Overview

This PR is a part of #18900. To make review process easier decided to split this on several PR's.

Problem

To pass input arguments into GComputation::apply G-API has function cv::gin() which takes variadic number of arguments and make cv::GRunArgs from them.

Let's to consider this call:

img   = np.random.random((3,3))
array = np.random.random(5)
comp.apply(cv.gin(img, array))

Both of img and array are numpy arrays, but the first one should be represented as cv::Mat and the second as std::vector based on graph meta of course.
Another example: Both of cv::Scalar and cv::Rect are represented in python as tuple with 4 values.
The same problem for cv::descr_of.

How to implement cv::gin for python, if we don't know actual type of object and have only python representation like numpy array or tuple.

Solution

We can't unpack input PyObject* to cv::GRunArgs at the time we received it in cv::gin. But we can unpack data based on graph metadata. Let's create some functor which stores this PyObject* and implement method which will take input metadata and based on this metadata perform converting.

  1. Collect input meta info during graph compilation
struct GTypeInfo
{
    cv::GShape shape;
    cv::detail::OpaqueKind kind;
    cv::detail::HostCtor ctor;
};
  1. In cv.gin make the functor and return back to the python.
  2. Take this functor in apply method and unpack based on collected input meta.

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.1.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
Copy link
Copy Markdown
Contributor Author

@dmatveev @smirnov-alexey Could you take a look ?

@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@dmatveev @smirnov-alexey Could you take a look ?

@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@dmatveev Should we ignore this warning ?
https://pullrequest.opencv.org/buildbot/builders/precommit_windows64/builds/27875/steps/compile%20release/logs/warnings%20%282%29

Does it make sense to shorten struct name to fix that warning ?

@smirnov-alexey
Copy link
Copy Markdown
Contributor

@dmatveev Should we ignore this warning ?
https://pullrequest.opencv.org/buildbot/builders/precommit_windows64/builds/27875/steps/compile%20release/logs/warnings%20%282%29

Does it make sense to shorten struct name to fix that warning ?

I guess it breaks the build so you need to fix it anyway

Copy link
Copy Markdown
Contributor

@smirnov-alexey smirnov-alexey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@dmatveev Could you take a look, please ?

Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach is still questionable.

Please setup a follow-up call to discuss.

@TolyaTalamanov TolyaTalamanov force-pushed the at/python-callbacks branch 2 times, most recently from 854e47e to eb0a4af Compare February 10, 2021 13:36
@TolyaTalamanov TolyaTalamanov force-pushed the at/python-callbacks branch 4 times, most recently from f07cf3e to db14b51 Compare February 11, 2021 10:17
@TolyaTalamanov TolyaTalamanov force-pushed the at/python-callbacks branch 2 times, most recently from db14b51 to 27a8694 Compare February 11, 2021 10:50
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh seems I failed to submit a review at the time - sorry for that

@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@alalek

@TolyaTalamanov TolyaTalamanov force-pushed the at/python-callbacks branch 2 times, most recently from 9cf3a03 to 76fec86 Compare February 15, 2021 14:44
@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@alalek

@TolyaTalamanov TolyaTalamanov force-pushed the at/python-callbacks branch 3 times, most recently from 30e220e to 76c177a Compare February 16, 2021 12:08
@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@alalek I hope all are passed!

* Avoid using default in switches
@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

@alalek all checks are passed, can it be merged ?

@alalek alalek merged commit eb82ba3 into opencv:master Mar 1, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
[G-API] Introduce cv.gin/cv.descr_of for python

* Implement cv.gin/cv.descr_of

* Fix macos build

* Fix gcomputation tests

* Add test

* Add using to a void exceeded length for windows build

* Add using to a void exceeded length for windows build

* Fix comments to review

* Fix comments to review

* Update from latest master

* Avoid graph compilation to obtain in/out info

* Fix indentation

* Fix comments to review

* Avoid using default in switches

* Post output meta for giebackend
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants