| Message ID | 20200426002530.24747-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Sun, Apr 26, 2020 at 03:25:30AM +0300, Laurent Pinchart wrote: > From: Naushir Patuck <naush@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@raspberrypi.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Jacopo Mondi <jacopo@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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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;