CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
G-API: Extend python bindings #20271
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
G-API: Extend python bindings #20271
Conversation
* Wrap timestamp, seqNo, seq_id * Wrap copy * Wrap parseSSD, parseYolo
@mpashchenkov Have a look, please |
modules/python/src2/cv2.cpp
Outdated
{ | ||
if (!obj || obj == Py_None) | ||
{ | ||
return true; |
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.
Why does the if
return true
here?
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 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.
this is used for handling "default values" of arguments
…end-python-bindings
@alalek @mpashchenkov Could you have a look, please ? |
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.
what is about adding of test code for new functionality?
modules/python/src2/cv2.cpp
Outdated
@@ -1170,6 +1170,41 @@ PyObject* pyopencv_from(const int64& value) | |||
return PyLong_FromLongLong(value); | |||
} | |||
|
|||
template<> | |||
bool pyopencv_to(PyObject* obj, int64& value, const ArgInfo& info) |
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.
Bindings generator doesn't support int64
type in functions signatures. No plans to add that due to ambiguous using and compatibility issues with other languages. Consider using int
/ size_t
instead.
It makes sense to add note here.
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.
Can I implement pyopencv_to(int64_t)
with static_cast to size_t
?
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.
Main problem with size_t
is that it is unsigned.
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.
Removed this implementation. Looks like it isn't necessary to read int64
data from python so far.
modules/python/src2/cv2.cpp
Outdated
{ | ||
if (!obj || obj == Py_None) | ||
{ | ||
return true; |
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.
this is used for handling "default values" of arguments
75993c7
to
a998612
Compare
@@ -386,6 +402,7 @@ static cv::detail::OpaqueRef extract_opaque_ref(PyObject* from, cv::detail::Opaq | |||
HANDLE_CASE(RECT, cv::Rect); | |||
HANDLE_CASE(UNKNOWN, cv::GArg); | |||
UNSUPPORTED(UINT64); | |||
UNSUPPORTED(INT64); |
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.
Unsupported for pyopencv_to
@@ -85,6 +85,19 @@ class GGraphMetaBackendImpl final: public cv::gapi::GBackend::Priv { | |||
const std::vector<cv::gimpl::Data>&) const override { | |||
return EPtr{new GraphMetaExecutable(graph, nodes)}; | |||
} | |||
|
|||
virtual bool controlsMerge() const override |
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.
@dmatveev FYI
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.
what is about adding of test code for new functionality?
Added test: seq_id
, seqNo
, timestamp
.
Already testing: parseSSD
, networks
Didn't add test for parseYolo
since there is no suitable model on CI
@alalek Could you have a look, please ? |
0f5fae4
to
dcfddce
Compare
@@ -346,22 +344,11 @@ static PyObject* extract_proto_args(PyObject* py_args, PyObject* kw) | |||
} | |||
else | |||
{ | |||
PyErr_SetString(PyExc_TypeError, "Unsupported type for cv.GIn()/cv.GOut()"); | |||
return NULL; | |||
util::throw_error(std::logic_error("Unsupported type for GProtoArgs")); |
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.
Does the message appear in error log? Why is not PyErr_SetString
?
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.
Because caller function handles this exception and setup PyErr_SetString
.
https://github.com/TolyaTalamanov/opencv/blob/at/extend-python-bindings/modules/gapi/misc/python/pyopencv_gapi.hpp#L929
has_frame, (ts, seqno, seqid) = ccomp.pull() | ||
|
||
if not has_frame: | ||
break |
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.
Should the if
contain an error message?
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 don't think so, in fact I just put it in case video less than 10 frames
@alalek Can it be merged ? |
…bindings G-API: Extend python bindings * Extend G-API bindings * Wrap timestamp, seqNo, seq_id * Wrap copy * Wrap parseSSD, parseYolo * Rewrap cv.gapi.networks * Add test for metabackend in pytnon * Remove int64 pyopencv_to
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