lit-robotics / libcamera-rs

Experimental Rust bindings for libcamera
Apache License 2.0
51 stars 16 forks source link

Unable to change FrameDurationLimits #31

Closed DreamerChaserHAH closed 1 week ago

DreamerChaserHAH commented 1 year ago

/include/libcamera/controls.h:152: T libcamera::ControlValue::get() const [with T = libcamera::Rectangle; typename std::enable_if<((! libcamera::details::is_span<U>::value) && (! std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1> >::type>::value)), std::nullptr_t>::type <anonymous> = nullptr]: Assertiontype_ == details::control_type<std::remove_cv_t>::value' failed.`

I am trying to change camera's control value (FrameDurationLimit)(to change FPS)

req.controls_mut().set(FrameDurationLimits([16700, 16700]));

What could be the issue here? And, how should I change the config?

chemicstry commented 1 year ago

I don't have a camera, which would support FrameDurationLimits so I can't reproduce it at the moment.

It is weird that it crashes in get() instead of a set() method. Maybe it sets the value and then crashes later once internal methods try to access this control.

Could you try and see what is the output of:

req.controls_mut().set(FrameDurationLimits([16700, 16700]));
println!("Controls: {:#?}", req.controls());
DreamerChaserHAH commented 1 year ago

Sorry, for the delay. I will get back to you shortly

DreamerChaserHAH commented 1 year ago

I don't know why, but now I am able to set the value, but it doesn't have noticeable impact on the framerate. I am using PI Camera Module 3. FPS is still stucked at around 7/8 FPS for some reason.

println!("Controls: {:#?}", req.controls());

This returns me

OK(FrameDurationLimits(16700,16700))

But, if I try to set the controls on the camera itself, like this.

   controls_to_be_set.set(FrameDuration(50000)).expect("Control Limits not working");
   // TODO: Set `Control::FrameDuration()` here. Blocked on https://github.com/lit-robotics/libcamera-rs/issues/2
   cam.start(Some(
                &controls_to_be_set
            )).unwrap();

It returns this instead.

[0:59:29.864135043] [2188] ERROR IPAProxy raspberrypi_ipa_proxy.cpp:467 Failed to call unmapBuffers
[0:59:29.877567179] [2195] ERROR IPCUnixSocket ipc_unixsocket.cpp:191 Failed to send: Transport endpoint is not connected
[0:59:29.877715408] [2195] ERROR IPCPipe ipc_pipe_unixsocket.cpp:66 Failed to call sync
[0:59:29.877769418] [2195] ERROR IPAProxy raspberrypi_ipa_proxy.cpp:467 Failed to call unmapBuffers
[0:59:29.879360929] [2195] ERROR IPCUnixSocket ipc_unixsocket.cpp:191 Failed to send: Transport endpoint is not connected
[0:59:29.879521033] [2195] ERROR IPCPipe ipc_pipe_unixsocket.cpp:80 Failed to call async

I did notice that there's a comment that is linked to an existing issue. Any updates?

Additional Information My version of raspbian comes with 0.0.4 libcamera originally. In order to use libcamera 0.1.0 with this crate, I rebuild and reinstall it on pi without uninstalling the original 0.0.4. Can that be causing any issues?

qwerty19106 commented 11 months ago

@chemicstry I have the same problem.

The code

let mut control_list = ControlList::new();
control_list
    .set(FrameDurationLimits([
        1000000 / config.fps as i64,
        1000000 / config.fps as i64,
    ]))
    .unwrap();
control_list.set(NoiseReductionMode::Fast).unwrap();
info!("{:?}", control_list);
cam.start(Some(&control_list)).unwrap();

returns

[2023-11-28T10:19:41.567Z INFO  logic::camera] ControlList{NoiseReductionMode: Fast, FrameDurationLimits: FrameDurationLimits([33333, 33333])}
logic: ../include/libcamera/controls.h:152: T libcamera::ControlValue::get() const [with T = libcamera::Rectangle; typename std::enable_if<((! libcamera::details::is_span<U>::value) && (! std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1> >::type>::value)), std::nullptr_t>::type <anonymous> = nullptr]: Assertion `type_ == details::control_type<std::remove_cv_t<T>>::value' failed.

This code req.controls_mut().set(FrameDurationLimits([16700, 16700])); cause the same error in libcamera thread.

Debian GNU/Linux 12 (bookworm) libcamera-dev = 0.1.0+rpt20231020-1 libcamera.rs = "0.2.3" Camera module: ov5647

dbartussek commented 8 months ago

The issue is caused by this library still using the libcamera 0.1 control id definitions. libcamera 0.2 added more controls, which caused the id of FrameDurationLimits to shift from 25 as currently defined in Rust to 27 (libcamera control_ids.h FRAME_DURATION = 27).

I have updated the definition yamls in a fork see commit https://github.com/lit-robotics/libcamera-rs/commit/5aa0322777361422982a0f2bc07c5bbc8a1ac9d1

However, if this library wants to support both libcamera 0.1 and 0.2, it would probably be best to no longer parse the yaml and generate Rust directly, but run bindgen on control_ids.h and property_ids.h to get the matching definitions for the users system. My fork will cause the exact same issues reported in this thread for anyone using libcamera 0.1.

chemicstry commented 8 months ago

@dbartussek thank you for investigating and figuring this out. I never thought that controls ids would change.

I agree that parsing C headers would probably be the best solution here, to not be tied to a particular libcamera version. One complication though is that generated Rust code is dependent on libcamera crate and can't be put in libcamera-sys crate. I would also like to avoid direct bindgen dependency in libcamera, and leave that solely in libcamera-sys. So there's some experimentation needed to see how to implement this. Would you be interested in attempting this?

dbartussek commented 7 months ago

Apologies ahead of my response for being away so long, I was a bit busy lately.

I've been thinking about how best to approach this for some time now and I believe that the best solution would be to generate multiple versions of the bindings (one each for v0.1.0 and v0.2.0) and select the correct one at build time using the libcamera/version.h file.

The big disadvantage of this approach is that it will necessitate a slight manual edit every time a new version of libcamera is released, but I don't think this will happen too often and I would probably set up a little monitoring program to run the most current utils/gen-version.sh of libcamera and check for any new tags every day to be notified when a new version is released so we can quickly have support for it without someone having to inform us because their build failed.

As a step further, this process could probably be automated by using the git version tags of libcamera, but for now, I'd like to just get a proof of concept to work. I've never scripted over a git repository before, so this might be a fun weekend project.

dbartussek commented 7 months ago

Ok, so I've rewritten how the code generation works: https://github.com/dbartussek/libcamera-rs/commit/437fb8d2d8b1458dcf8269683005e0920f0062b7. It should still look the exact same from the users perspective, but will now take different versions of libcamera into account under the hood.

Going over the changes in detail:

Something I am not happy with and would love to hear feedback on is the version selection mechanism. Right now, I simply link libcamera-sys in the build script for libcamera, read the LIBCAMERA_VERSION_MAJOR/MINOR/PATCH constants and copy over the corresponding generated files to the build directory. My thoughs on this are:

Lastly, I'd still like to improve the documentation in two ways:

  1. Add documentation which features must be enabled for certain controls. I've found an explanation on StackOverflow how to do this: https://stackoverflow.com/questions/61417452/how-to-get-a-feature-requirement-tag-in-the-documentation-generated-by-cargo-do
  2. Make the documentation not depend on libcamera being present on the system. I presume this is what leads to the documentation not rendering on docs.rs: https://docs.rs/crate/libcamera/latest I'd presumably add some pregenerated binding files and fall back to those in doc builds should linking to the system libcamera fail.

All in all, I'd love to hear your feedback. In the meantime, I will now test this code with our project at work to make sure it does not contain bugs and is backwards compatible!

ItzSiL3Nce commented 1 month ago

27

Now it's 28

jkneer commented 1 month ago

@dbartussek I agree that your solution is not there yet. Nonetheless, it is a tremendous improvement over the broken state of the current official main branch and I would suggest to merge it in.

chemicstry commented 3 weeks ago

@dbartussek Sorry for not replying earlier. I think your solution is the best we can do right now and I would like to merge it. Could you open a PR?

dbartussek commented 2 weeks ago

Hey, nice to hear from you! I'm also sorry that I haven't engaged much with the discussion anymore, my health kept me from programming for a lot of this year.

Sure, I'll create a new branch with the clean changes for a pull request over the weekend!

chemicstry commented 1 week ago

This is now fixed with #38 and released as v0.3.0.

Thanks @dbartussek!