-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
Improve Python API including enabled AsynchronousOut mode #1593
Improve Python API including enabled AsynchronousOut mode #1593
Conversation
Updated to only set up producer when we need synchronous input |
a09c5bb
to
777fe9b
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Please review! |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Please review! |
Sorry for the HUGE delay! I have finally reviewed it, there is a big question I have: The new Python example you added (examples/tutorial_api_python/12_asynchronous_custom_output.py), how is it an asynchronous example? Where in that example you set it to be asynchronous? |
… AsynchronousOut mode
aca530e
to
6d83b9a
Compare
You're right -- that seems to have been an oversight. Fixed. |
Is there anything I can do to make this easier to review and merge? |
Pushed, this is an amazing PR, thanks a lot! (It just took me forever to review it given its complexity, sorry about that, but amazing PR!) |
Currently AsynchronousOut mode won't work with the Python API since it is not possible to get ahold of a Datum created inside OpenPose from the Python side. The reason for this is at the moment, lists are copied when going over the Python => C++ boundary. See https://pybind11.readthedocs.io/en/stable/advanced/cast/stl.html#making-opaque-types . To get the semantics of passing a mutable reference, we can wrap an STL type.
Unfortunately this is not quite sufficient. We need to pass a shared_pointer<vector<shared_pointer>> to
openpose::Wrapper
. The outer shared pointer is eventually pointed somewhere else inqueue.hpp
:openpose/include/openpose/thread/queue.hpp
Line 73 in 3915b92
So we just swap that vector with ours, which should avoid copying the datum I think. I'm a bit confused about why there are so many levels of nesting though. A more idiomatic approach would be to just return a value in Python, but since the Python/C++ APIs are currently very similar, I thought I'd keep it this way and just adjust it to work in this case also.
This PR also contains the following related changes:
And a couple of unrelated bug fixes to the Python API:
type_caster<op::Array<long long>>
so datum.poseIds can be accessed from Python