Skip to content
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

Demo code: in-memory image encoding and npz data encoding #1263

Merged
merged 18 commits into from
Jan 29, 2024

Conversation

s-trinh
Copy link
Contributor

@s-trinh s-trinh commented Oct 26, 2023

Why

  • on another project, I have discovered the .npy or .npz file formats used in NumPy
  • for a ROS project I have used cnpy for my needs: for a sequence of data, save some basic datatypes (int, bool, ...) and save some vector of double data
  • it should allow exchanging data easily between C++ and Python/NumPy
  • maybe this file format can also be useful here

But

  • the ViSP library is already quite big and only if it is beneficial to other people this code can be added
  • it requires C++11 and lots of works are still needed to clean the code
  • I have used cnpy but I have not searched if better options exist or if there are already in ViSP the code to achieve the same thing

What

  • the demo code is:
    • in tutorial-mb-generic-tracker-save.cpp, save the images + some data to be displayed into a npz file
    • in tutorial-mb-generic-tracker-read.cpp, open the npz file, display the images + the model of the teabox

How

  • code from the cnpy library are directly included into the vpIoTools class for facility
  • readPNGfromMem and writePNGtoMem functions are added to be able to save compressed image data using code from the stb library, the same can be done for jpg encoding
  • warning: Java binding must be disable to be able to build the library correctly

Copy link

codecov bot commented Jan 14, 2024

Codecov Report

Attention: 164 lines in your changes are missing coverage. Please review.

Comparison is base (d70f9cd) 53.58% compared to head (4cfa003) 52.47%.
Report is 18 commits behind head on master.

Files Patch % Lines
modules/core/src/tools/file/vpIoTools.cpp 0.00% 147 Missing ⚠️
modules/core/include/visp3/core/vpIoTools.h 0.00% 9 Missing ⚠️
modules/io/src/image/vpImageIo.cpp 80.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1263      +/-   ##
==========================================
- Coverage   53.58%   52.47%   -1.12%     
==========================================
  Files         399      401       +2     
  Lines       50509    51826    +1317     
==========================================
+ Hits        27067    27196     +129     
- Misses      23442    24630    +1188     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fspindle
Copy link
Contributor

@s-trinh Nice job. Can I merge or do you want to add other functionalities?

@s-trinh
Copy link
Contributor Author

s-trinh commented Jan 15, 2024

@fspindle Thanks!

To be done:

  • read/write with OpenCV backend for color images
  • handling of alpha channel with OpenCV backend
  • check if RGBa to RGB conversion is needed with stb lib wrt. to this stride_bytes param
  • check endianness handling [POSTPONED]
  • should we export only the following functions instead of the whole lib? And potentially export more functions if needed?
    • npz_load
    • npy_save
  • add documentation for npz_load and npy_save functions
  • show how to check if a data variable is present in a npz file with C++ code?
  • add some basic unit tests?
  • add some demo code with Python?

I will ping you back when the PR will be ready.

…lld’ expects argument of type ‘long long int’, but argument 5 has type ‘size_t’ {aka ‘long unsigned int’} [-Wformat=]" and "tutorial-mb-generic-tracker-save.cpp:171:56: warning: format ‘%lld’ expects argument of type ‘long long int’, but argument 5 has type ‘size_t’ {aka ‘long unsigned int’} [-Wformat=]" warnings on Ubuntu and x86-64 platform.
…c-dealloc-mismatch (malloc vs operator delete [])"
…z file.

Update "script/PlotCameraTrajectory.py" to read poses from npz file.
Add documentation to "visp::cnpy::npz_load()", "npz_save()", [...] functions.
@s-trinh
Copy link
Contributor Author

s-trinh commented Jan 29, 2024

@fspindle PR is ready from my side.

To quickly test:

  • ./tutorial-mb-generic-tracker-save
  • ./tutorial-mb-generic-tracker-read
  • ./tutorial-mb-generic-tracker-full --video [...]/visp-images/mbt/cube/image%04d.png --model [...]/visp-images/mbt/cube_and_cylinder.cao --save test/tracking_%06d.png --save-results test/tracking_results.npz
  • python3 PlotCameraTrajectory.py -p [...]/test/tracking_results.npz --npz -m [...]/visp-images/mbt/cube_and_cylinder.cao

Note:

  • data are not compressed when using visp::cnpy::npz_save() (it is not equivalent to numpy.savez_compressed)
  • you can open a npz file on Linux with an archive manager software
  • this PR provides basic functions for loading/saving arrays of data into binary file, with easy compatibility with NumPy, as an alternative to simple file text. For more advanced needs, see Comparison of data-serialization formats

PR #1311 reintroduced some C++98 compatibility code.
Does it mean ViSP is no more a >= C++11 library (#1267)?

Should we add the minimum C++ version required for the ViSP library here? https://github.com/lagadic/visp/blob/master/README.md


TODO one day:

  • looks like there is some bug when plotting cylinder object with PlotCameraTrajectory.py script (I think the error is how to draw a cylinder and coordinates)

@s-trinh s-trinh marked this pull request as ready for review January 29, 2024 01:21
@fspindle
Copy link
Contributor

@s-trinh Thanks.

As far as c++98 is concerned, we need to reintroduce this functionality for a project we have with a partner. The idea is to make an effort to make it possible to build ViSP with c++98, but certainly not to port all the classes to c++98. In ViSP you can continue to develop and use c++11 and later features, just remember to disable them if c++98.

@fspindle fspindle merged commit 1d7a809 into lagadic:master Jan 29, 2024
51 of 52 checks passed
@s-trinh
Copy link
Contributor Author

s-trinh commented Jan 29, 2024

Is there any public news on this project?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants