[RFC,v1,4/7] libcamera: controls: ControlList: Add `erase()`

24450 diff mbox series

Add two `erase()` functions that can be used to remove elements
from a `ControlList` instance.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/controls.h |  3 +++
 src/libcamera/controls.cpp   | 17 +++++++++++++++++
 2 files changed, 20 insertions(+)

Hi Barnabás,

Thank you for the patch.

Quoting Barnabás Pőcze (2025-09-24 14:47:09)
> Add two `erase()` functions that can be used to remove elements
> from a `ControlList` instance.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

This looks all reasonable to me. I wonder however why that was never
implemented before. Was there a reason not to remove elements from a
ControlList? Does that have any implications on the C-wrapper?

Otherwise
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

Regards,
Stefan

> ---
>  include/libcamera/controls.h |  3 +++
>  src/libcamera/controls.cpp   | 17 +++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 5d4a53c46..9045caff7 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -470,6 +470,9 @@ public:
>         const ControlValue &get(unsigned int id) const;
>         void set(unsigned int id, const ControlValue &value);
>  
> +       bool erase(unsigned int id);
> +       bool erase(const ControlId &ctrl) { return erase(ctrl.id()); }
> +
>         const ControlInfoMap *infoMap() const { return infoMap_; }
>         const ControlIdMap *idMap() const { return idmap_; }
>  
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 54cd3b703..83f8bf056 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -1156,6 +1156,23 @@ void ControlList::set(unsigned int id, const ControlValue &value)
>         *val = value;
>  }
>  
> +/**
> + * \brief Remove the value of control \a id
> + * \param[in] id The control ID
> + * \return \a true if \a id was present, \a false otherwise
> + */
> +bool ControlList::erase(unsigned int id)
> +{
> +       return controls_.erase(id);
> +}
> +
> +/**
> + * \fn ControlList::erase(const ControlId &ctrl)
> + * \brief Remove the value of control \a ctrl
> + * \param[in] ctrl The control
> + * \return \a true if \a ctrl was present, \a false otherwise
> + */
> +
>  /**
>   * \fn ControlList::infoMap()
>   * \brief Retrieve the ControlInfoMap used to construct the ControlList
> -- 
> 2.51.0
>

Hi

2025. 10. 06. 15:10 keltezéssel, Stefan Klug írta:
> Hi Barnabás,
> 
> Thank you for the patch.
> 
> Quoting Barnabás Pőcze (2025-09-24 14:47:09)
>> Add two `erase()` functions that can be used to remove elements
>> from a `ControlList` instance.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> This looks all reasonable to me. I wonder however why that was never
> implemented before. Was there a reason not to remove elements from a
> ControlList? Does that have any implications on the C-wrapper?

I can only recall https://lists.libcamera.org/pipermail/libcamera-devel/2025-June/050761.html
regarding this topic. I don't think this change has problematic implications.


Regards,
Barnabás Pőcze


> 
> Otherwise
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> Regards,
> Stefan
> 
>> ---
>>   include/libcamera/controls.h |  3 +++
>>   src/libcamera/controls.cpp   | 17 +++++++++++++++++
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
>> index 5d4a53c46..9045caff7 100644
>> --- a/include/libcamera/controls.h
>> +++ b/include/libcamera/controls.h
>> @@ -470,6 +470,9 @@ public:
>>          const ControlValue &get(unsigned int id) const;
>>          void set(unsigned int id, const ControlValue &value);
>>
>> +       bool erase(unsigned int id);
>> +       bool erase(const ControlId &ctrl) { return erase(ctrl.id()); }
>> +
>>          const ControlInfoMap *infoMap() const { return infoMap_; }
>>          const ControlIdMap *idMap() const { return idmap_; }
>>
>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>> index 54cd3b703..83f8bf056 100644
>> --- a/src/libcamera/controls.cpp
>> +++ b/src/libcamera/controls.cpp
>> @@ -1156,6 +1156,23 @@ void ControlList::set(unsigned int id, const ControlValue &value)
>>          *val = value;
>>   }
>>
>> +/**
>> + * \brief Remove the value of control \a id
>> + * \param[in] id The control ID
>> + * \return \a true if \a id was present, \a false otherwise
>> + */
>> +bool ControlList::erase(unsigned int id)
>> +{
>> +       return controls_.erase(id);
>> +}
>> +
>> +/**
>> + * \fn ControlList::erase(const ControlId &ctrl)
>> + * \brief Remove the value of control \a ctrl
>> + * \param[in] ctrl The control
>> + * \return \a true if \a ctrl was present, \a false otherwise
>> + */
>> +
>>   /**
>>    * \fn ControlList::infoMap()
>>    * \brief Retrieve the ControlInfoMap used to construct the ControlList
>> --
>> 2.51.0
>>