CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
G-API: add documentation on serialization functionality #20163
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: add documentation on serialization functionality #20163
Conversation
@dmatveev @TolyaTalamanov @mpashchenkov please, review. Not sure if I need to add more examples (for instance, on |
* Check different overloads for more examples. | ||
* @param p serialized vector of bytes. | ||
* @return deserialized GComputation object. | ||
*/ | ||
template<> inline | ||
cv::GComputation deserialize(const std::vector<char> &p) { |
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.
@alalek could you help a little? Do you know how to correctly document template specialization (or whatever the issue causes docs corruption)? Currently it looks like this: https://pullrequest.opencv.org/buildbot/export/pr/20163/docs/db/d06/group__gapi__serialization.html
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 it looks ok in your example - isn't it?
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 see only two derializes there though)
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.
Let's ignore this for now. I will create a separate task to fix it later
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.
Good start
@TolyaTalamanov @mpashchenkov please, take a look once the doc build is ready |
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.
Overall looks good.
|
||
int main(int argc, char *argv[]) | ||
{ | ||
(void) argc; |
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 don't use int main()
instead ?
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.
Fixed
148f47d
to
78282bb
Compare
@alalek can we merge this? |
#include <opencv2/gapi/garg.hpp> | ||
|
||
int main() | ||
{ |
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.
modules/gapi/samples
It makes sense to put these documentation-only non-functional samples somewhere 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.
@alalek I believe gapi/samples
is the only place for such things - it provides examples for docs (via @snippet
) and also checks that the code compiles
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.
OpenCV 'core' snippets are stored into <opencv>/samples/cpp/tutorial_code/core/...
Users expecting in gapi/samples
full-functional samples which can be run and show some meanungful result.
examples for docs
Check generated Doxyfile - there are many paths for storing that.
@snippet custom_type_serialization.cpp
This filename is not enough to be unique across whole project (other cases have similar problem).
Use path with gapi
entry (append path prefix) to avoid ambiguous problem in the future.
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.
@alalek I fixed the prefixes where applicable. Speaking of an appropriate place for these snippets: I've found 2 incomplete samples in g-api: dynamic_graph.cpp
and kernel_api_snippets.cpp
. I've copied std::cout << "This sample is non-complete. It is used as code snippents in documentation." << std::endl;
to my samples. Is that enough? If you think that's still an issue can it be fixed separately?
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.
Also, can we merge this without a second Dmitry's review?
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 makes sense to put these documentation-only non-functional samples somewhere else.
@alalek , Our documentation-related code (so it is "snippets) was there from the early beginning but if there's a better place, we can move it there for sure for all the snippets.
#include <opencv2/gapi/garg.hpp> | ||
|
||
int main() | ||
{ |
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.
OpenCV 'core' snippets are stored into <opencv>/samples/cpp/tutorial_code/core/...
Users expecting in gapi/samples
full-functional samples which can be run and show some meanungful result.
examples for docs
Check generated Doxyfile - there are many paths for storing that.
@snippet custom_type_serialization.cpp
This filename is not enough to be unique across whole project (other cases have similar problem).
Use path with gapi
entry (append path prefix) to avoid ambiguous problem in the future.
@dmatveev please, take a look once again |
* in GCompileArgs deserializable. | ||
* | ||
* @param bytes vector of bytes to deserialize GCompileArgs object from. | ||
* @return GCompileArgs object. | ||
* @see GCompileArgs S11N | ||
* @see GCompileArgs cv::gapi::s11n::detail::S11N |
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.
hmm it is strange to see detail::
thing in the documentation -- is our overloadable S11N
structure still there?
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.
Yes, it's still there
@alalek can we merge this, please? |
…tion_docs G-API: add documentation on serialization functionality * Add documentation on serialization/deserialization * Add docs on bind() methods * Fix typo * Docs refactoring * Fix s11n docs * Fix deserialize() docs * Change deserialize docs * Fix warning * Address review comments * Fix sample * Fix warnings and errors * Fix docs warnings * Fix warnings * Address review comments * Add prefixes to snippets and fix indentation * Address review comments and move snippets to a single file
Docs: https://pullrequest.opencv.org/buildbot/export/pr/20163/docs/
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.