[libcamera-devel] [PATCH v2 2/7] libcamera: Controls: Add ControlTypeMenu

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Wed, Apr 21, 2021 at 01:23:41PM +0900, Hirokazu Honda wrote:
> This adds control type for v4l2 menu.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  include/libcamera/controls.h | 28 ++++++++++++++++++++++++++++
>  src/libcamera/controls.cpp   |  7 +++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 1a5690a5..357819e7 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -9,6 +9,7 @@
>  #define __LIBCAMERA_CONTROLS_H__
>  
>  #include <assert.h>
> +#include <sstream>
>  #include <stdint.h>
>  #include <string>
>  #include <unordered_map>
> @@ -32,6 +33,28 @@ enum ControlType {
>  	ControlTypeString,
>  	ControlTypeRectangle,
>  	ControlTypeSize,
> +	ControlTypeMenu,
> +};
> +
> +struct ControlMenu {
> +	uint32_t index;
> +	bool isName = false;
> +
> +	union {
> +		char name[28];
> +		int64_t value;
> +	};
> +
> +	std::string toString() const
> +	{
> +		std::stringstream ss;
> +		ss << "index: " << index;
> +		if (isName)
> +			ss << "name: " << name;
> +		else
> +			ss << "value: " << value;
> +		return ss.str();
> +	}
>  };

As this is used for V4L2 controls only, I'm really not keen on adding
support for named menu entries :-( Especially given that 28 is a
completely arbitrary limit. I'd like to instead store the value as an
integer, and, if we really need support for named menu entries, to add
them to the ControlInfo class instead ? The mapping between numerical
values and names should be constant, it shouldn't be a property of the
ControlValue.

>  
>  namespace details {
> @@ -85,6 +108,11 @@ struct control_type<Size> {
>  	static constexpr ControlType value = ControlTypeSize;
>  };
>  
> +template<>
> +struct control_type<ControlMenu> {
> +	static constexpr ControlType value = ControlTypeMenu;
> +};
> +
>  template<typename T, std::size_t N>
>  struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> {
>  };
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index c58ed394..8ab5ac92 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -60,6 +60,7 @@ static constexpr size_t ControlValueSize[] = {
>  	[ControlTypeString]		= sizeof(char),
>  	[ControlTypeRectangle]		= sizeof(Rectangle),
>  	[ControlTypeSize]		= sizeof(Size),
> +	[ControlTypeMenu]		= sizeof(ControlMenu),
>  };
>  
>  } /* namespace */
> @@ -254,6 +255,12 @@ std::string ControlValue::toString() const
>  			str += value->toString();
>  			break;
>  		}
> +		case ControlTypeMenu: {
> +			const ControlMenu *value =
> +				reinterpret_cast<const ControlMenu *>(data);
> +			str += value->toString();
> +			break;
> +		}
>  		case ControlTypeNone:
>  		case ControlTypeString:
>  			break;

-- 
Regards,

Laurent Pinchart