Lifetimes don't make sense - Lifetimes have to go

[TheQuantumPhysicist]

Hi. First I'd like to thank you for creating this library. Great work. I just have a little complaint, if you will. Maybe I'm misunderstanding something here.

I can see you've created a lifetime for something like CameraList<'a>. I don't think that's necessary. I don't think the lifetime of Camera is necessary either. All lifetimes are covariant, yet they're not related to each other at all... why do they even exist? What are we trying to protect against, exactly?

The problem with this is that if anyone tries to put these objects in a struct, it'll create a self-referencial structs and/or it won't be possible to return any of these objects in methods, because their lifetimes cannot be related to the struct owning them. For example, it's not possible to create a Camera object from a local CameraList... why?

Even worse, because these structs are not send/sync, we can't have a singleton to solve that lifetime problem.

The only trick I have under my sleeve left is to run a dedicated thread with a dedicated event loop and pull frames from that with the main thread being for command and control... a complicated design, if you ask me.

Has anyone succeeded in controlling these structs in one unifying struct under a trait for generic cameras? I don't see how that's possible.

The big question is: Is there a reason I'm missing why these lifetimes exist?

I'm not the ultimate expert in covariance and subtyping... but this design doesn't make sense to me. All these lifetimes have to go! The Drop trait is probably more than enough.

Apologies for the long post.

[SoZ0]

I have the same opinion, I'm not sure if there is some underlying reason to why there are these life times they only seem to hinder the ability to use the lib and provide unnecessary roadblocks. I've been working on a project for the past few months with the lifetimes remove and or altered with no issues so far so I'm not sure why they exist, but maybe there is some edge case I have yet to run into to.

[fishrockz]

@SoZ0 could you share your changes?

So I have messed with some life times and I have had seg faults when I have not been very careful, especially when writing multi threaded code.

The life times mostly reflect the properties of the cpp lib, libcamera. Writing ffi rust is hard and doing so in a way that creates a safe wrapper around the cpp is even harder and afaict most of the added life times are there to make sure that the rust structs don't live longer than the cpp lib will garenttee then to live.

That said there may well be places we're things can change.

I'm not the original author and generally I have been really pleased with this code based even if the original authors are a little quiet ATM.

As for a thread and a queue that is how I have written quite a bit of the code I have worked on but pipelining graphics stacks is fairly common afaict..

[chemicstry]

@fishrockz is right, most lifetimes are there due to C++ code requirements. Some lifetimes and Send/Sync requirements could likely be relaxed, but this needs a careful investigation of the ffi code on a case by case basis.

[fishrockz]

I think the way to progress this is to add some runtime tests. This might need vmic kernel modules being active like the libcamera repo have.

This allows us to write the sorts of tests that would flush out any issues with these sort of changes.

I think then also using miri would further help us understand the rules.

An alternative approach would be for @TheQuantumPhysicist to suggest the clones (by sharing a branch) they would find most helpful and the rest of us can review and test?

[chemicstry]

Runtime tests would not catch issues with incorrect lifetime annotations, Send/Sync issues, etc at the library level. You would need to test all possible library usage permutations to find if some lifetime bound is missing, which causes use after free. Do update me if there is now a magic tool that can do this.

I believe that current approach closely follows C++ libcamera and thus offers the most flexibility (not ergonomics), while preventing unsafe usage. I understand that there is a desire for simpler API in end-user applications, but this could be implemented on top of current API where thread management, buffer allocations, request handling, etc is handled under the hood and you get a simple opencv-like API camera.read_frame().

[AThilenius]

I'm really suck because of this. I have two cameras with different framerates I need to read from. I can read from one camera, having everything on a dedicated thread and sending the frames into an MPSC, but I see no way to handle multiple cameras. CameraManager cannot be stored in a Mutex as it needs to be pinned to a thread, and the transitive list of objects spawned from it are all tied to the lifetime of the CameraManager. Meaning I have no way to get an ActiveCamera on anther thread. Since you also cannot send the ActiveCamera (which is needed in the handler to enqueue the next request) I see no way to read two cameras with different framerates. The only thing I could imagine is reading from one then another in a loop, effectively linking their framerates (I have one 30hz camera and a 120hz camera for VSLAM).

Anyone have a workaround for this? The lifetimes are immensely challenging to deal with. As a last resort I might spawn multiple processes and use an IPC for frame data, but I'm guessing that will be brutal on performance 😬

[AThilenius]

I guess I can answer my own question, but it makes me want to pour bleach in my eyes.
https://gist.github.com/AThilenius/aa156c3bc7ce2103ae05868b86e18de4

I had to duplicate every single init variable with a unique name (they cannot be stored in any type of container, as Rust can't guarantee lifetimes, and nothing is Send+Sync) and then duplicate handler code with a bool if_camera_one type flag on the mpsc.

I see no other way to do this given the over-restrictive lifetimes though, and I see no way to add cameras at runtime because everything has to be statically defined as variables in the function itself.

[AThilenius]

Also (triple post, sorry) I hope that doesn't come across as negative! I really appreciate your crate, and thank you for your work on it! I'm just grumpy from fighting with rustc 😂

[chemicstry]

Yeah I have not considered multi-camera use case and this definitely makes it a pain. I guess we could have CameraManager in an Arc and have cameras hold a copy of it so it is not dropped before cameras are. It's a bit hard to trace lifetime requirements of libcamera since C++ does not have convenient annotations like rust has, but rummaging through docs I found the following contracts:

• CameraManager must not be dropped while there are references to cameras. This can be solved by holding Arc<CameraManager> in cameras.
• Creating second instance of CameraManager is UB. So we have globally track when it is created/destroyed and panic when second instance is created.
• Cameras are only valid between start() and stop() calls on the manager. I think this is already handled by Drop implementations.

I will be happy to accept a PR if someone raises a hand to implement this.

[AThilenius]

I'll try to take a peak in the next few days if I get some free time. Might be worth implementing a singleton API for CameraManager to make it clear in the type system that only one can ever live. It's safe to send that between threads though, right?

Unrelated, and a total shot in the dark: on a Pi5 libcamera (even build in --release) is really rough on performance. Which is really weird because ffmpeg can H.264 encode the feed using maybe 15% CPU usage, but just polling frames off the camera is coming close to pegging a whole core. I haven't profiled yet to figure out why, but figured I would see if you had any better-guesses than I. It must be some difference in how the buffers are being allocated/filled/copied.