-
Notifications
You must be signed in to change notification settings - Fork 25
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
[C-Api/Service] add new API (ML-API ext) #454
Conversation
📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #454. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/. |
|
cf3eb33
to
d182328
Compare
|
d182328
to
dfa65ee
Compare
|
dfa65ee
to
6ef890d
Compare
|
6ef890d
to
a7062cb
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
a7062cb
to
84a58f6
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
84a58f6
to
ab1af65
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
bf08356
to
3382726
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
3382726
to
b107639
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
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.
Thank you for your hard work in establishing the structure of the API that will be developed in the future.
c/include/ml-api-service.h
Outdated
typedef struct { | ||
void (*new_data) (ml_service_h handle, const char *name, const ml_tensors_data_h data, void *user_data); /**< Called when new data is processed from machine learning service. */ | ||
void (*event) (ml_service_h handle, int event, void *event_data, void *user_data); /**< Called when new event is occurred from machine learning service. */ | ||
} ml_service_callbacks_s; |
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.
Tizen does not encourage structure-type event callbacks.
It seems that ml_service_callback_s
event callbacks can be combined into one like this.
void (*event_callback) (ml_service_h handle, void *event_data, ... );
In the case of void *event_data, how about to use the one with information-handle we released since tizen8.0?
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.
Agreed, thanks for your suggestion.
We are now preparing Tizen API process as you know.
Let's talk about the event callback and its params on offline discussion, to handle new-data for inference and other events from offloading service.
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 agreed with @niley7464 's suggestion.
Instead of the structure type event callback, how about this unified form?
ml_service can be passed using the user_data, so I think it's not needed.
And let's use information handle considering later scalability
void (*ml_service_event_cb) (int event_type, ml_information_h data, void *user_data);
Reference: Tizen guide
3.5.1 Callback function prototype MUST be based on the template
The template is:
typedef void (*{module_prefix}_{actual_name}_cb)(event_type event, {event details}, void *user_data);
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 updated this PR - added event callback.
Please review the changes again.
b107639
to
e139c11
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
e139c11
to
708f316
Compare
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.
LGTM!
Thank you for considering my comment.
b0d68e8
to
1f3a072
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
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.
LGTM!
c035ed4
to
562f521
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
61afe74
to
e176449
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
Add new API set for ML service, these functions support constructing new handle from json configuration. - type for ml-service: single, pipeline Signed-off-by: Jaeyun Jung <[email protected]>
e176449
to
f76fade
Compare
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.
@jaeyun-jung, 💯 All CI checkers are successfully verified. Thanks.
Now ready to prepare new API for next Tizen release. |
Add new API set for ML service, these functions support constructing new handle from json configuration.
TODO: need discussion for ml-service remote