Add Larod #120
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 |
- 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.
|
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.
|
@apljungquist I'd like your opinion on something. I'm looking at the various supported backends for larod. My initial thought was that since we know what combinations of hardware (e.g. ARTPEC 8) and model type (e.g. tflite) are, I could use Rust's type system to emit compile time errors if invalid combinations are defined. Here's an example: mod inference {
pub trait PrivateSupportedBackend {}
}
pub trait SupportedBackend: inference::PrivateSupportedBackend {
fn new() -> Self;
fn as_str(&self) -> &str;
}
// Model types
pub struct TFLite;
pub struct CVFlowNN;
pub struct Native;
// Hardware
pub struct CPU;
pub struct Artpec8DLPU;
impl inference::PrivateSupportedBackend for InferenceBackend<TFLite, CPU> {}
impl inference::PrivateSupportedBackend for InferenceBackend<TFLite, Artpec8DLPU> {}
pub struct InferenceBackend<M, H> {
mode: M,
hardware: H,
}
impl SupportedBackend for InferenceBackend<TFLite, CPU> {
fn new() -> InferenceBackend<TFLite, CPU> {
InferenceBackend {
mode: TFLite,
hardware: CPU,
}
}
fn as_str(&self) -> &str {
"cpu-tflite"
}
}
impl SupportedBackend for InferenceBackend<TFLite, Artpec8DLPU> {
fn new() -> InferenceBackend<TFLite, Artpec8DLPU> {
InferenceBackend {
mode: TFLite,
hardware: Artpec8DLPU,
}
}
fn as_str(&self) -> &str {
"axis-a8-dlpu-tflite"
}
}
fn main() {}
// let a = larod::InferenceBackend::<Native, ArmNNCPU>::new(); // Compiler error
let b = larod::InferenceBackend::<TFLite, Artpec8DLPU>::new(); // Fine, since Artpec8DLPU can support TFLite models.
}This works, but I wonder if it is the right approach. An alternative is to just define some enums and verify things at runtime: #[derive(Debug)]
pub enum Device {
CPU,
ARTPEC7GPU,
ARTPEC8DLPU,
ARTPEC9DLPU,
}
#[derive(Debug)]
pub enum Backend {
TFLite(Device),
}
impl Backend {
pub fn as_str(&self) -> &str {
match self {
Backend::TFLite(d) => match d {
Device::CPU => "cpu-tflite",
Device::ARTPEC7GPU => "axis-a7-gpu-tflite",
Device::ARTPEC8DLPU => "axis-a8-dlpu-tflite",
Device::ARTPEC9DLPU => "a9-dlpu-tflite",
},
}
}
}This approach has the benefit of more ergonomic matching and possibly deserialization from let session = Session::new();
let devices = session
.devices()
.context("Couldn't list available devices")?;[2025-01-15T16:07:11Z INFO test] Devices:
[2025-01-15T16:07:11Z INFO test] cpu-tflite (0)
[2025-01-15T16:07:11Z INFO test] axis-ace-proc (0)
[2025-01-15T16:07:11Z INFO test] cpu-proc (0)
[2025-01-15T16:07:11Z INFO test] axis-a8-dlpu-tflite (0)
[2025-01-15T16:07:11Z INFO test] axis-a8-dlpu-native (0)
[2025-01-15T16:07:11Z INFO test] axis-a8-dlpu-proc (0)
[2025-01-15T16:07:11Z INFO test] axis-a8-gpu-proc (0) |
|
I've been happy to follow along with your conversations with Daniel and to see that you are still making progress. I agree that it would be nice to leverage Rust's type system to avoid runtime errors. I've only had time to take a quick look at the documentation so far, I'll try to make more time this weekend if needed. Here are my thoughts so far:
Another line of thought is how would a user go about selecting a backend/device? Would they have a preferred model and then try to pick the most performative hardware compatible with that model on the camera where the app is currently running? Do they perhaps need to build different apps for different cameras? |
Yes, essentially. Specific "device" (CPU, GPU, DLPU, etc.) and "model" (TFLite, CVFlowNN, etc.) combinations make up a "backend". However, it seems in general a "device" is really only capable of one type of "model", so backend and device seem to be interchangeable.
That's right, we pass a string and get a raw pointer to a struct representing the device.
Yes, we could provide an enum like this. It just doesn't look nice to me. Maybe I'm being too picky. enum Backend {
TFLiteCPU,
TFLiteArtpec7GPU,
TFLiteArtpec8DLPU,
...
}I kind of wonder why the larod API developers elected to forgo an enum and use strings instead?
This would be pretty nice, but getting the Device (perhaps a wrapper for the pointer) requires a pointer to the Session object. So, it would be something like The This is the library design I'm liking the best so far mod larod{
mod inference {
pub trait PrivateSupportedBackend {
fn as_str() -> &'static str;
}
}
// Model types
pub struct TFLite;
pub struct CVFlowNN;
pub struct Native;
// Hardware
pub struct CPU;
pub struct Artpec8DLPU;
pub struct Artpec9DLPU;
impl inference::PrivateSupportedBackend for (TFLite, CPU) {
fn as_str() -> &'static str {
"cpu-tflite"
}
}
impl inference::PrivateSupportedBackend for (TFLite, Artpec7GPU) {
fn as_str() -> &'static str {
"axis-a7-gpu-tflite"
}
}
impl<'a> InferenceModel<'a> {
pub fn new<M, H, P>(
session: &'a Session,
model_file: P,
access: LarodAccess,
name: &str,
params: Option<LarodMap>,
) -> Result<InferenceModel<'a>>
where
(M, H): inference::PrivateSupportedBackend,
P: AsRef<Path>,
{
let f = File::open(model_file).map_err(Error::IOError)?;
let Ok(device_name) = CString::new(<(M, H)>::as_str()) else {
return Err(Error::CStringAllocation);
};
...
}
}
}I think it allows for a fairly ergonomic use. I'm not a fan of having to ask the compiler to infer the fn main() {
let mut model = larod::InferenceModel::new::<TFLite, Artpec8DLPU, _>(
session,
"converted_model.tflite",
larod::LarodAccess::LAROD_ACCESS_PRIVATE,
"Example",
None,
)?;
}The logical alternative is just an enum listing the supported combinations. |
- Get Tensor as slice. - Rename JobRequest.run_job to JobRequest.run
|
I think the generic implementation could be nice, especially if there are differences between the devices that we want to help users navigate. For instance it looks like there may be room to make device specific parameters (the chip concept is, as you point out, deprecated but the function is not so I guess this is still relevant):
|
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