[libcamera-devel] [PATCH v5.1 5/7] libcamera: controls: Reorder and update description of existing controls

Jacopo Mondi jacopo at jmondi.org
Mon Apr 27 16:43:34 CEST 2020


Hi Laurent,

On Sun, Apr 26, 2020 at 03:25:30AM +0300, Laurent Pinchart wrote:
> From: Naushir Patuck <naush at raspberrypi.com>
>
> Group AE, AWB, etc. controls together for accessibility.
>
> Update descriptions for Contrast, Brightness, and Saturation controls.
>
> Update the uvcvideo and vimc pipeline handlers to use the new
> brightness, contrast and saturation units. UVC has no explicit units for
> those controls, so map them with a best effort heuristic. For VIMC,
> hardcode the control range based on knowledge of the driver
> implementation for simplicity.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> ---
> Changes since v5:
>
> - Include math.h in vimc.cpp
> ---
>  src/libcamera/control_ids.yaml               | 42 +++++++-----
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 67 +++++++++++++++++---
>  src/libcamera/pipeline/vimc/vimc.cpp         | 30 ++++++---
>  3 files changed, 104 insertions(+), 35 deletions(-)
>
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index d8bdb3829be4..f2ac052b3d3e 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -25,23 +25,6 @@ controls:
>
>          \sa AeEnable
>
> -  - AwbEnable:
> -      type: bool
> -      description: |
> -        Enable or disable the AWB.
> -
> -  - Brightness:
> -      type: int32_t
> -      description: Specify a fixed brightness parameter
> -
> -  - Contrast:
> -      type: int32_t
> -      description: Specify a fixed contrast parameter
> -
> -  - Saturation:
> -      type: int32_t
> -      description: Specify a fixed saturation parameter
> -
>    - ExposureTime:
>        type: int32_t
>        description: |
> @@ -58,4 +41,29 @@ controls:
>          colour channels. This value cannot be lower than 1.0.
>
>          \sa ExposureTime AeEnable
> +
> +  - Brightness:
> +      type: float
> +      description: |
> +        Specify a fixed brightness parameter. Positive values (up to 1.0)
> +        produce brighter images; negative values (up to -1.0) produce darker
> +        images and 0.0 leaves pixels unchanged.
> +
> +  - Contrast:
> +      type: float
> +      description:  |
> +        Specify a fixed contrast parameter. Normal contrast is given by the
> +        value 1.0; larger values produce images with more contrast.
> +
> +  - AwbEnable:
> +      type: bool
> +      description: |
> +        Enable or disable the AWB.
> +
> +  - Saturation:
> +      type: float
> +      description:  |
> +        Specify a fixed saturation parameter. Normal saturation is given by
> +        the value 1.0; larger values produce more saturated colours; 0.0
> +        produces a greyscale image.
>  ...
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 08462542a79b..030ac6864752 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -263,12 +263,29 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>  		return -EINVAL;
>
>  	const ControlInfo &v4l2Info = controls->infoMap()->at(cid);
> +	int32_t min = v4l2Info.min().get<int32_t>();
> +	int32_t def = v4l2Info.def().get<int32_t>();
> +	int32_t max = v4l2Info.max().get<int32_t>();
>
>  	/*
>  	 * See UVCCameraData::addControl() for explanations of the different
>  	 * value mappings.
>  	 */
>  	switch (cid) {
> +	case V4L2_CID_BRIGHTNESS: {
> +		float scale = std::max(max - def, def - min);
> +		float fvalue = value.get<float>() * scale + def;
> +		controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> +		break;
> +	}
> +
> +	case V4L2_CID_SATURATION: {
> +		float scale = def - min;
> +		float fvalue = value.get<float>() * scale + min;
> +		controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> +		break;
> +	}
> +
>  	case V4L2_CID_EXPOSURE_AUTO: {
>  		int32_t ivalue = value.get<bool>()
>  			       ? V4L2_EXPOSURE_APERTURE_PRIORITY
> @@ -281,11 +298,8 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>  		controls->set(cid, value.get<int32_t>() / 100);
>  		break;
>
> +	case V4L2_CID_CONTRAST:
>  	case V4L2_CID_GAIN: {
> -		int32_t min = v4l2Info.min().get<int32_t>();
> -		int32_t max = v4l2Info.max().get<int32_t>();
> -		int32_t def = v4l2Info.def().get<int32_t>();
> -
>  		float m = (4.0f - 1.0f) / (max - def);
>  		float p = 1.0f - m * def;
>
> @@ -457,6 +471,38 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
>  	int32_t def = v4l2Info.def().get<int32_t>();
>
>  	switch (cid) {
> +	case V4L2_CID_BRIGHTNESS: {
> +		/*
> +		 * The Brightness control is a float, with 0.0 mapped to the
> +		 * default value. The control range is [-1.0, 1.0], but the V4L2
> +		 * default may not be in the middle of the V4L2 range.
> +		 * Accommodate this by restricting the range of the libcamera
> +		 * control, but always within the maximum limits.
> +		 */
> +		float scale = std::max(max - def, def - min);
> +
> +		info = ControlInfo{
> +			{ static_cast<float>(min - def) / scale },
> +			{ static_cast<float>(max - def) / scale },
> +			{ 0.0f }
> +		};
> +		break;
> +	}
> +
> +	case V4L2_CID_SATURATION:
> +		/*
> +		 * The Saturation control is a float, with 0.0 mapped to the
> +		 * minimum value (corresponding to a fully desaturated image)
> +		 * and 1.0 mapped to the default value. Calculate the maximum
> +		 * value accordingly.
> +		 */
> +		info = ControlInfo{
> +			{ 0.0f },
> +			{ static_cast<float>(max - min) / (def - min) },
> +			{ 1.0f }
> +		};
> +		break;
> +
>  	case V4L2_CID_EXPOSURE_AUTO:
>  		info = ControlInfo{ false, true, true };
>  		break;
> @@ -473,14 +519,15 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
>  		};
>  		break;
>
> +	case V4L2_CID_CONTRAST:
>  	case V4L2_CID_GAIN: {
>  		/*
> -		 * The AnalogueGain control is a float, with 1.0 mapped to the
> -		 * default value. UVC doesn't specify units, and cameras have
> -		 * been seen to expose very different ranges for the gain
> -		 * control. Arbitrarily assume that the minimum and maximum
> -		 * values are respectively no lower than 0.5 and no higher than
> -		 * 4.0.
> +		 * The Contrast and AnalogueGain controls are floats, with 1.0
> +		 * mapped to the default value. UVC doesn't specify units, and
> +		 * cameras have been seen to expose very different ranges for
> +		 * the controls. Arbitrarily assume that the minimum and
> +		 * maximum values are respectively no lower than 0.5 and no
> +		 * higher than 4.0.
>  		 */
>  		float m = (4.0f - 1.0f) / (max - def);
>  		float p = 1.0f - m * def;
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index c5eea3a01b0e..77de4c38bf01 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -8,6 +8,7 @@
>  #include <algorithm>
>  #include <array>
>  #include <iomanip>
> +#include <math.h>
>  #include <tuple>
>
>  #include <linux/media-bus-format.h>
> @@ -304,14 +305,24 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
>
>  	for (auto it : request->controls()) {
>  		unsigned int id = it.first;
> -		ControlValue &value = it.second;
> +		unsigned int offset;
> +		uint32_t cid;
>
> -		if (id == controls::Brightness)
> -			controls.set(V4L2_CID_BRIGHTNESS, value);
> -		else if (id == controls::Contrast)
> -			controls.set(V4L2_CID_CONTRAST, value);
> -		else if (id == controls::Saturation)
> -			controls.set(V4L2_CID_SATURATION, value);
> +		if (id == controls::Brightness) {
> +			cid = V4L2_CID_BRIGHTNESS;
> +			offset = 128;
> +		} else if (id == controls::Contrast) {
> +			cid = V4L2_CID_CONTRAST;
> +			offset = 0;
> +		} else if (id == controls::Saturation) {
> +			cid = V4L2_CID_SATURATION;
> +			offset = 0;
> +		} else {
> +			continue;
> +		}
> +
> +		int32_t value = lroundf(it.second.get<float>() * 128 + offset);
> +		controls.set(cid, utils::clamp(value, 0, 255));
>  	}
>
>  	for (const auto &ctrl : controls)
> @@ -434,18 +445,21 @@ int VimcCameraData::init(MediaDevice *media)
>  	ControlInfoMap::Map ctrls;
>
>  	for (const auto &ctrl : controls) {
> -		const ControlInfo &info = ctrl.second;
>  		const ControlId *id;
> +		ControlInfo info;
>
>  		switch (ctrl.first->id()) {
>  		case V4L2_CID_BRIGHTNESS:
>  			id = &controls::Brightness;
> +			info = ControlInfo{ { -1.0f }, { 1.0f }, { 0.0f } };
>  			break;
>  		case V4L2_CID_CONTRAST:
>  			id = &controls::Contrast;
> +			info = ControlInfo{ { 0.0f }, { 2.0f }, { 1.0f } };
>  			break;
>  		case V4L2_CID_SATURATION:
>  			id = &controls::Saturation;
> +			info = ControlInfo{ { 0.0f }, { 2.0f }, { 1.0f } };
>  			break;
>  		default:
>  			continue;
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel


More information about the libcamera-devel mailing list