-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add Larod #120
base: main
Are you sure you want to change the base?
Add Larod #120
Conversation
Just adding a comment that I am still relatively new to Rust, so suggestions to improve my code is very welcome. I will also be learning about functionality of the larod library during the process of creating this wrapper. So, guidance on interfacing with larod is also very welcome. |
Looking at the larodError documentation a bit more closely, I realize what I have so far won't work, and maybe larod cannot be wrapped? Notably larodError, which is passed to nearly every function, must be uninitialized. It might be impossible for rust to produce such a pointer https://doc.rust-lang.org/std/mem/union.MaybeUninit.html. |
Since you say the code is not ready I will focus on answering your questions and assist you when you ask for it for now. Consequently I haven't looked much at the code but I address your comments below.
Diving in at the deep end with unsafe. A word of warning; In my opinion unsafe combines the worst of C(++) and Rust, and writing safe wrappers is especially frustrating since many of the safety aspects that we need to guarantee are not documented - sometimes not even considered - by the library authors. To make matters worse, we often cannot even test empirically if our code is correct because UB does not mean that it will segfault or do anything weird right now, it means it could do it now or at any point in the future. So, know know what you are getting yourself into, that it is ok to quit and don't let this experience discourage you from using Rust because there's another side to the language that is carefree and fun 🌞 🌈 What is your programming background? I will try to adjust my communication accordingly.
That makes two of us.
I have found that sometimes the documentation should be taken with a grain of salt (I have found errors) and other times it is important to read it carefully. In any case the examples can be a good supporting resource. in larodError* error = NULL;
// ...
if (!larodConnect(&conn, &error)) { We clearly don't pass an uninitialized variable but a pointer to null. I think this is what the documentation means when it says "An uninitialized handle to an error". Compare this to GError* error = NULL;
// ...
if (!ax_event_handler_declare(event_handler,
// ...
&error)) { Footnotes |
Thanks for the feedback. My background is varied engineering (R&D, so jack of all trades master of none sort of thing). I've written production C++ for embedded microcontrollers, C++ for AI using OpenCV and libtorch, lots of Python, and a few web backend type services using Rust. Again, I'm no expert, but I've done a lot of dabbling in various things. I'm ok diving into the deep end. Just more to learn hopefully :). One piece from the larod docs that I worry a bit about is this quote, "error can also be NULL if one does not want any error information." I worry that if I initialize the pointer to Null, we'll just never get any error information back. |
I'm comparing to axevent again because I recently worked on those bindings. I think this is what passing a null pointer would look like: https://github.com/AxisCommunications/acap-native-sdk-examples/blob/9b00b2fdf23672f8910421653706572201c2ed8b/axevent/send_event/app/send_event.c#L167 Compare that to passing a (not-null) pointer to a null pointer: https://github.com/AxisCommunications/acap-native-sdk-examples/blob/9b00b2fdf23672f8910421653706572201c2ed8b/axevent/send_event/app/send_event.c#L176 |
… the pointer to it is non-NULL. - Added some documentation. - Mimicked the try_from macro from the axevent crate to DRY the code.
- Stubbed out functions for Session (larodConnection) - Start implementing a few functions to list hand handle larodDevices
- Add testing documentation to README.md - Add runner specification to .cargo/config.toml (new) - Set `aarch64-unknown-linux-gnu` to default build target. - Add remote-test.sh script to run tests on remote target.
@apljungquist I just set up the P1468-LE camera and set up a remote testing workflow. I've added some documentation and configuration files in this commit. It might be useful for you in your endeavors. I have confirmed it works for the larod library so far on the P1468-LE running OS 11.11.73. It might be worth considering as a cherry pick, or I could open a separate merge request for it.
running 10 tests test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s |
Neat. I didn’t know about I currently think of tests in two categories:
Were you aware of |
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 like the simplicity of this script but before I make any suggestions about how to upstream it I have a couple of questions to make sure I understand it.
- Change error handling to better check for a non-NULL larodError pointer. - Add some documentation for LarodMap functions. - Set LarodMap functions to public. - Add some additional tests. - Hide on-device tests behind a device-tests feature. - Add a basic example.
Using that may ultimately be a better approach than using the runner as discussed above. |
…sion from which it was acquired.
Add preprocessor model and inference model Add TensorContainer struct and Tensor (WIP) Start using thiserror Expand basic example
Happy to see you are still working on this 🙂 What's are you aiming for (the whole API vs some subset, a minimal safe abstraction vs highly ergonomic abstraction, integration with VDO, etc) and how far do you think you have come? I'm sorry I still haven't made time to study the APIs and your implementation. |
Yep, I'm still working on it when I find time. I was initially planning on doing the whole API, but I'm starting to realize some elements of the larod API don't make a ton of sense. I'm also thinking covering the whole API is a bit more work than I initially realized. So, I'm leaning more towards a minimal but functional library for now that strikes a balance between being a relatively thin wrapper and providing an idiomatic Rust API. I am also starting to work on the VDO library so I can get frames from the camera. That might be a bit more difficult looking at the imageprovider.c file in the vdo-larod example. Setting up background threads and interfacing the GLib will take a bit of thinking through. I'm aiming for an API that would work something like this: fn main() -> Result<()> {
let stream = Stream::builder()
.channel()
.format()
.width()
.height()
.build();
stream.start();
while let Ok(frame) = stream.next_frame() {
...
}
Ok(())
} Example liblarod ConfusionThe larodCreateTensors function exists and simply takes a These two functions have me a bit confused. the I did a quick test (I believe, I can't find it now) where I used Code ConventionsOne thing I was struggling with was the combination of supported preprocessor configurations. We know a-priori what backends support what kind of operations. So, I would really like to create a Rust Preprocessor API which encodes these supported combinations in the type system. We could then lean on the Rust type system to make it mostly impossible to code an incompatible combination of parameters. I couldn't find a nice way of doing it with Enums. The typestate pattern might be able to do it but looked like it was going to quite a bit of library code to do. For now, I'm following the C library pretty closely with a builder pattern and doing the appropriate checks in a |
Create Resolution struct to help maintain clarity.
- Set tensor buffers - Get tensor name - Get tensor pitches - Get tensor layout
- Add memmap2 dependency - Create and store memory map object when Tensor.set_buffer() is called. - Add Tensor.copy_from_slice() to copy data to memory mapped resource.
The acap-rs workspace doesn't currently provide wrappers for the larod library. When it is ready, I hope this merge request will provide that.
Immediate goals:
Since larod uses peripheral hardware, I plan to build the tests and execute them on an Axis P1468-LE.
Checklist before requesting a review