Remaining ControlId Methods

[SoZ0]

NOTE

• This PR is an extension of Expanded support for Libcamera Controls (ControlInfoMap and ControlInfo) #46

Reasoning

At the moment there is no way to get information about a control's read-write permissions pragmatically or the enumerator data. This allows developers to get this information without needing to know what controls are and are not read/writable.

Disruptions to existing code

There should be no disruptions to existing code as these changes only extend the capabilities of ControlId and do not edit, move or remove functionality

Changes

• generate_from_git.rs: to generate the missing ControlId methods. PropertyId's generation is slightly modified to use *self as u32 instead but functionality is unaffected
• controls.cpp: add the methods to interface with the missing ControlId methods, more details below
• controls.h: add the methods to interface with the missing ControlId methods, more details below

enum libcamera_control_id_enum libcamera_control_id(libcamera_control_id_t *control);
const char *libcamera_control_name(libcamera_control_id_t *control);
enum libcamera_control_type libcamera_control_id_type(libcamera_control_id_t *control);
const char *libcamera_control_id_vendor(libcamera_control_id_t *control);
enum libcamera_control_direction libcamera_control_id_direction(libcamera_control_id_t *control);
bool libcamera_control_id_is_input(libcamera_control_id_t *control);
bool libcamera_control_id_is_output(libcamera_control_id_t *control);
bool libcamera_control_id_is_array(libcamera_control_id_t *control);
size_t libcamera_control_id_size(libcamera_control_id_t *control);
size_t libcamera_control_id_enumerator_count(libcamera_control_id_t *control);
const char *libcamera_control_id_enumerator_name(libcamera_control_id_t *control,int32_t value);
int32_t libcamera_control_id_enumerator_value(libcamera_control_id_t *control, const char *name);
size_t libcamera_control_id_enumerators_len(libcamera_control_id_t *control);
int32_t libcamera_control_id_enumerators_key(libcamera_control_id_t *control, size_t index);
const char *libcamera_control_id_enumerators_name_by_index(libcamera_control_id_t *control, size_t index);

Examples

• camera_control_id_info.rs: displays what these changes introduce

Next Steps

• control_type(): currently returns the index of the ControlType value and not the ControlType itself, it does this because ControlType has not been properly implemented within the library yet.

[SoZ0]

Im not sure if implementing ControlType would have larger implications on the library as a whole.

I could just make an enum that has no linking to ControlValue to fill the gap but that doesn't feel like a proper solution to me. But maybe I am overthinking this decision

[SoZ0]

I realized after reading the docs and looking at the source code for libcamera, that they dont actually line up in some cases. For example controltype in the docs does not list every value in the enum. So that helped clear up some confusion I had.

For now I have added in a ControlType enum for the time being, I'm not the biggest fan of this solution but it gets the job done. If anyone has any suggestions I'm open to them

[Swarkin]

Once you're done make sure to mark the PR as ready, Id love to check whether this resolves my current issues with the crate

[SoZ0]

Once you're done make sure to mark the PR as ready, Id love to check whether this resolves my current issues with the crate

@Swarkin I've been using this branch for a week now with no issues so far. Let me know if you have issues or if something is missing

[fishrockz]

Sorry i had not got to this PR, I will have to give it a proper review as its quite big.

Thank you for all the work.

I also appreciate the example!

[chemicstry]

Could you rebase this on main and implement ControlId methods outside the generated code, like in #69? I think it is better to keep unsafe away from generated code for easier validation, and also reduces code duplication.

[SoZ0]

@chemicstry changes have been made, that is definitely a better way of doing it and would have saved me a lot of headache a few weeks ago. I will investigate further in to camera manager in a future PR as I need that stop function for some of my projects.

[chemicstry]

Thanks for updating this quickly, I just left a few more comments

[chemicstry]

No need to check for null here, c_str() should never return a null pointer

[chemicstry]

libcamera_control_id_enumerator_name and libcamera_control_id_enumerator_value are never used

[chemicstry]

I'm not sure if we need null check in C shim when we can ensure this by rust type system without any performance cost. For example it should not be possible to have an invalid control id, because ControlId is enum with fixed values, so ControlId::as_ptr() is never null. Maybe add an assert from the rust side instead?

[chemicstry]

I think this would work better as an iterator, see ControlListRefIterator implementation to iterate a C++ map. Current implementation will cost O(2n log n) to collect everything into hashmap, because std::advance(it, index) is O(log n) for C++ map, which is called for each entry twice. I know that this is not a performance critical path, but I would like it to be consistent inside the repo. A separate convenience function can just .collect() rust iterator into a HashMap<i32, String>.

[chemicstry]

Thanks!