CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
[G-API] Python ROI generic inference #19318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@dmatveev @smirnov-alexey Could you take a look ? |
@TolyaTalamanov seems like it comes atop of a bunch of other PRs. Can't you separate the changes here? |
bed239e
to
a5031b9
Compare
86e057f
to
48bb099
Compare
48bb099
to
244eea6
Compare
@smirnov-alexey Could you have a look, please ? |
@AsyaPronina @smirnov-alexey Could you have a look, please ? |
|
||
|
||
def make_roi(self, img, roi): | ||
return img[roi[1]:roi[1] + roi[3], roi[0]:roi[0] + roi[2], ...] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
roi - is a tuple (x,y,h,w)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(x,y,h,w)
Probably it is (x,y,w,h)
Clear code (to avoid unclear error messages):
x, y, w, h = roi
return img[y:y+h, x:x+w, ...]
if not cv.dnn.DNN_TARGET_CPU in cv.dnn.getAvailableTargets(cv.dnn.DNN_BACKEND_INFERENCE_ENGINE): | ||
return | ||
|
||
root_path = '/omz_intel_models/intel/age-gender-recognition-retail-0013/FP32/age-gender-recognition-retail-0013' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we have some policy regarding our tests, that if we don't find models/data, we don't run the tests? Won't those tests start failing later with new paths?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we don't care about it in these tests.
@@ -318,12 +320,9 @@ static cv::GRunArg extract_run_arg(const cv::GTypeInfo& info, PyObject* item) | |||
reinterpret_cast<pyopencv_gapi_wip_IStreamSource_t*>(item)->v; | |||
return source; | |||
} | |||
else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep the else
here, but it's up to you
This bunch of PRs already in master, there is only actual separate changes |
public: | ||
GAPI_WRAP GInferListInputs(); | ||
GAPI_WRAP void setInput(const std::string& name, const cv::GArray<cv::GMat>& value); | ||
GAPI_WRAP void setInput(const std::string& name, const cv::GArray<cv::Rect>& value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where's the implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only see .hpp files in this MR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no implementation, this file isn't compiled at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a trigger for bindings generator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why do we need it then? How it works? Can you please comment it in the code so then strangers like me wouldn't ask those questions again and again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put the comment to the python wrapper thing somewhere later.
@alalek Can it be merged ? |
if not cv.dnn.DNN_TARGET_CPU in cv.dnn.getAvailableTargets(cv.dnn.DNN_BACKEND_INFERENCE_ENGINE): | ||
return | ||
|
||
root_path = '/omz_intel_models/intel/age-gender-recognition-retail-0013/FP32/age-gender-recognition-retail-0013' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove leading slash /
|
||
|
||
def make_roi(self, img, roi): | ||
return img[roi[1]:roi[1] + roi[3], roi[0]:roi[0] + roi[2], ...] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(x,y,h,w)
Probably it is (x,y,w,h)
Clear code (to avoid unclear error messages):
x, y, w, h = roi
return img[y:y+h, x:x+w, ...]
@@ -13,7 +13,7 @@ | |||
('cpu' , cv.gapi.core.cpu.kernels()), | |||
('fluid' , cv.gapi.core.fluid.kernels()) | |||
# ('plaidml', cv.gapi.core.plaidml.kernels()) | |||
] | |||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps whole block should be reformatted
…-infer [G-API] Python ROI generic inference * Python generic infer overloads * Move wrappers to appropriate file
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.
Build configuration