[libcamera-devel] [PATCH/RFC 2/2] libcamera: Use V4L2 Control instances
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Feb 8 23:54:29 CET 2021
Use the V4L2 Control instances instead of the numerical V4L2_CID_*
identifiers where possible. This simplify usage of the ControlList API
in several places, and allows printing control names in the CameraSensor
class and the RaspberryPi IPA.
There are still locations where the numerical identifiers are used:
- In the IPU3 and RaspberryPi code, for custom controls not defined in
linux/v4l2-controls.h, which can be fixed by adding additional
device-specific Control instances.
- In the DelayedControls and V4L2Device controls APIs, which we could
move to ControlId if desired.
- In the uvcvideo and vimc pipeline handlers, when translating between
V4L2 and libcamera controls, for switch/case or for code that operate
on different controls in a generic way. This would be more difficult
to replace.
Further improvements to the controls API may be possible on top of this,
such as dropping the ID-based ControlList::get() and ControlList::set()
functions (at possibly replacing them with a version that takes a
ControlId pointer). Another possibly interesting improvement would be to
add a lookup API to the ControlInfoMap class that would take a
Control<T> reference, and return a type-aware wrapper around ControlInfo
to avoid having to deal manually with ControlValue.
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
src/ipa/ipu3/ipu3.cpp | 10 +++---
src/ipa/raspberrypi/raspberrypi.cpp | 25 ++++++-------
src/ipa/rkisp1/rkisp1.cpp | 10 +++---
src/libcamera/camera_sensor.cpp | 36 +++++++++----------
src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
.../pipeline/raspberrypi/raspberrypi.cpp | 16 ++++-----
src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 29 +++++++--------
src/libcamera/pipeline/vimc/vimc.cpp | 10 +++---
8 files changed, 68 insertions(+), 70 deletions(-)
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index b11b03efa6ce..5ee3fd302e01 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -11,7 +11,6 @@
#include <sys/mman.h>
#include <linux/intel-ipu3.h>
-#include <linux/v4l2-controls.h>
#include <libcamera/buffer.h>
#include <libcamera/control_ids.h>
@@ -23,6 +22,7 @@
#include "libcamera/internal/buffer.h"
#include "libcamera/internal/log.h"
+#include "libcamera/internal/v4l2_controls.h"
namespace libcamera {
@@ -80,13 +80,13 @@ void IPAIPU3::configure([[maybe_unused]] const CameraSensorInfo &info,
ctrls_ = entityControls.at(0);
- const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
+ const auto itExp = ctrls_.find(&v4l2::EXPOSURE);
if (itExp == ctrls_.end()) {
LOG(IPAIPU3, Error) << "Can't find exposure control";
return;
}
- const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);
+ const auto itGain = ctrls_.find(&v4l2::ANALOGUE_GAIN);
if (itGain == ctrls_.end()) {
LOG(IPAIPU3, Error) << "Can't find gain control";
return;
@@ -214,8 +214,8 @@ void IPAIPU3::setControls(unsigned int frame)
op.operation = IPU3_IPA_ACTION_SET_SENSOR_CONTROLS;
ControlList ctrls(ctrls_);
- ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
- ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
+ ctrls.set(v4l2::EXPOSURE, exposure_);
+ ctrls.set(v4l2::ANALOGUE_GAIN, gain_);
op.controls.push_back(ctrls);
queueFrameAction.emit(frame, op);
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index ff14cfc4b706..897f3f0b91f1 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -542,16 +542,16 @@ void IPARPi::reportMetadata()
bool IPARPi::validateSensorControls()
{
- static const uint32_t ctrls[] = {
- V4L2_CID_ANALOGUE_GAIN,
- V4L2_CID_EXPOSURE,
- V4L2_CID_VBLANK,
+ static const ControlId *const ctrls[] = {
+ &v4l2::ANALOGUE_GAIN,
+ &v4l2::EXPOSURE,
+ &v4l2::VBLANK,
};
for (auto c : ctrls) {
if (sensorCtrls_.find(c) == sensorCtrls_.end()) {
LOG(IPARPI, Error) << "Unable to find sensor control "
- << utils::hex(c);
+ << c->name();
return false;
}
}
@@ -1038,10 +1038,8 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
LOG(IPARPI, Debug) << "Applying WB R: " << awbStatus->gain_r << " B: "
<< awbStatus->gain_b;
- ctrls.set(V4L2_CID_RED_BALANCE,
- static_cast<int32_t>(awbStatus->gain_r * 1000));
- ctrls.set(V4L2_CID_BLUE_BALANCE,
- static_cast<int32_t>(awbStatus->gain_b * 1000));
+ ctrls.set(v4l2::RED_BALANCE, awbStatus->gain_r * 1000);
+ ctrls.set(v4l2::BLUE_BALANCE, awbStatus->gain_b * 1000);
}
void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuration)
@@ -1100,15 +1098,14 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
* exposure time without us knowing. The next time though this function should
* clip exposure correctly.
*/
- ctrls.set(V4L2_CID_VBLANK, vblanking);
- ctrls.set(V4L2_CID_EXPOSURE, exposureLines);
- ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);
+ ctrls.set(v4l2::VBLANK, vblanking);
+ ctrls.set(v4l2::EXPOSURE, exposureLines);
+ ctrls.set(v4l2::ANALOGUE_GAIN, gainCode);
}
void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)
{
- ctrls.set(V4L2_CID_DIGITAL_GAIN,
- static_cast<int32_t>(dgStatus->digital_gain * 1000));
+ ctrls.set(v4l2::DIGITAL_GAIN, dgStatus->digital_gain * 1000);
}
void IPARPi::applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls)
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 39783abd731b..ce47ba8cb38a 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -13,7 +13,6 @@
#include <sys/mman.h>
#include <linux/rkisp1-config.h>
-#include <linux/v4l2-controls.h>
#include <libcamera/buffer.h>
#include <libcamera/control_ids.h>
@@ -25,6 +24,7 @@
#include <libipa/ipa_interface_wrapper.h>
#include "libcamera/internal/log.h"
+#include "libcamera/internal/v4l2_controls.h"
namespace libcamera {
@@ -91,13 +91,13 @@ void IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
ctrls_ = entityControls.at(0);
- const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
+ const auto itExp = ctrls_.find(&v4l2::EXPOSURE);
if (itExp == ctrls_.end()) {
LOG(IPARkISP1, Error) << "Can't find exposure control";
return;
}
- const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);
+ const auto itGain = ctrls_.find(&v4l2::ANALOGUE_GAIN);
if (itGain == ctrls_.end()) {
LOG(IPARkISP1, Error) << "Can't find gain control";
return;
@@ -261,8 +261,8 @@ void IPARkISP1::setControls(unsigned int frame)
op.operation = RKISP1_IPA_ACTION_V4L2_SET;
ControlList ctrls(ctrls_);
- ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
- ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
+ ctrls.set(v4l2::EXPOSURE, exposure_);
+ ctrls.set(v4l2::ANALOGUE_GAIN, gain_);
op.controls.push_back(ctrls);
queueFrameAction.emit(frame, op);
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index c9e8d49b7935..eb96c4841708 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -280,16 +280,16 @@ int CameraSensor::validateSensorDriver()
* Optional controls are used to register optional sensor properties. If
* not present, some values will be defaulted.
*/
- static constexpr uint32_t optionalControls[] = {
- V4L2_CID_CAMERA_ORIENTATION,
- V4L2_CID_CAMERA_SENSOR_ROTATION,
+ static const ControlId *const optionalControls[] = {
+ &v4l2::CAMERA_ORIENTATION,
+ &v4l2::CAMERA_SENSOR_ROTATION,
};
- const ControlIdMap &controls = subdev_->controls().idmap();
- for (uint32_t ctrl : optionalControls) {
+ const ControlInfoMap &controls = subdev_->controls();
+ for (const ControlId *ctrl : optionalControls) {
if (!controls.count(ctrl))
LOG(CameraSensor, Debug)
- << "Optional V4L2 control " << utils::hex(ctrl)
+ << "Optional V4L2 control " << ctrl->name()
<< " not supported";
}
@@ -346,18 +346,18 @@ int CameraSensor::validateSensorDriver()
* For raw sensors, make sure the sensor driver supports the controls
* required by the CameraSensor class.
*/
- static constexpr uint32_t mandatoryControls[] = {
- V4L2_CID_EXPOSURE,
- V4L2_CID_HBLANK,
- V4L2_CID_PIXEL_RATE,
- V4L2_CID_VBLANK,
+ static const ControlId *const mandatoryControls[] = {
+ &v4l2::EXPOSURE,
+ &v4l2::HBLANK,
+ &v4l2::PIXEL_RATE,
+ &v4l2::VBLANK,
};
err = 0;
- for (uint32_t ctrl : mandatoryControls) {
+ for (const ControlId *ctrl : mandatoryControls) {
if (!controls.count(ctrl)) {
LOG(CameraSensor, Error)
- << "Mandatory V4L2 control " << utils::hex(ctrl)
+ << "Mandatory V4L2 control " << ctrl->name()
<< " not available";
err = -EINVAL;
}
@@ -425,7 +425,7 @@ int CameraSensor::initProperties()
const ControlInfoMap &controls = subdev_->controls();
int32_t propertyValue;
- const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION);
+ const auto &orientation = controls.find(&v4l2::CAMERA_ORIENTATION);
if (orientation != controls.end()) {
int32_t v4l2Orientation = orientation->second.def().get<int32_t>();
@@ -450,7 +450,7 @@ int CameraSensor::initProperties()
}
properties_.set(properties::Location, propertyValue);
- const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);
+ const auto &rotationControl = controls.find(&v4l2::CAMERA_SENSOR_ROTATION);
if (rotationControl != controls.end()) {
propertyValue = rotationControl->second.def().get<int32_t>();
properties_.set(properties::Rotation, propertyValue);
@@ -775,11 +775,11 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const
return -EINVAL;
}
- int32_t hblank = ctrls.get(V4L2_CID_HBLANK).get<int32_t>();
+ int32_t hblank = ctrls.get(v4l2::HBLANK);
info->lineLength = info->outputSize.width + hblank;
- info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();
+ info->pixelRate = ctrls.get(v4l2::PIXEL_RATE);
- const ControlInfo vblank = ctrls.infoMap()->at(V4L2_CID_VBLANK);
+ const ControlInfo vblank = ctrls.infoMap()->at(&v4l2::VBLANK);
info->minFrameLength = info->outputSize.height + vblank.min().get<int32_t>();
info->maxFrameLength = info->outputSize.height + vblank.max().get<int32_t>();
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 9bc3df3352fd..d3fc51518598 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -845,7 +845,7 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
double lineDuration = sensorInfo.lineLength
/ (sensorInfo.pixelRate / 1e6);
const ControlInfoMap &sensorControls = sensor->controls();
- const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
+ const ControlInfo &v4l2Exposure = sensorControls.find(&v4l2::EXPOSURE)->second;
int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;
int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;
int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 0804a4393c19..90f846530346 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -992,8 +992,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT;
ControlList ctrls(dev->controls());
- ctrls.set(V4L2_CID_HFLIP, 0);
- ctrls.set(V4L2_CID_VFLIP, 0);
+ ctrls.set(v4l2::HFLIP, 0);
+ ctrls.set(v4l2::VFLIP, 0);
dev->setControls(&ctrls);
}
@@ -1246,10 +1246,10 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
*/
if (supportsFlips_) {
ControlList ctrls(unicam_[Unicam::Image].dev()->controls());
- ctrls.set(V4L2_CID_HFLIP,
- static_cast<int32_t>(!!(rpiConfig->combinedTransform_ & Transform::HFlip)));
- ctrls.set(V4L2_CID_VFLIP,
- static_cast<int32_t>(!!(rpiConfig->combinedTransform_ & Transform::VFlip)));
+ ctrls.set(v4l2::HFLIP,
+ !!(rpiConfig->combinedTransform_ & Transform::HFlip));
+ ctrls.set(v4l2::VFLIP,
+ !!(rpiConfig->combinedTransform_ & Transform::VFlip));
unicam_[Unicam::Image].dev()->setControls(&ctrls);
}
@@ -1383,8 +1383,8 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
auto it = mappedEmbeddedBuffers_.find(bufferId);
if (it != mappedEmbeddedBuffers_.end()) {
uint32_t *mem = reinterpret_cast<uint32_t *>(it->second.maps()[0].data());
- mem[0] = ctrl.get(V4L2_CID_EXPOSURE).get<int32_t>();
- mem[1] = ctrl.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();
+ mem[0] = ctrl.get(v4l2::EXPOSURE);
+ mem[1] = ctrl.get(v4l2::ANALOGUE_GAIN);
} else {
LOG(RPI, Warning) << "Failed to find embedded buffer "
<< bufferId;
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index 08a594175091..a80b51f174a9 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -260,20 +260,20 @@ void PipelineHandlerUVC::stop(Camera *camera)
int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
const ControlValue &value)
{
- uint32_t cid;
+ const ControlId *cid;
if (id == controls::Brightness)
- cid = V4L2_CID_BRIGHTNESS;
+ cid = &v4l2::BRIGHTNESS;
else if (id == controls::Contrast)
- cid = V4L2_CID_CONTRAST;
+ cid = &v4l2::CONTRAST;
else if (id == controls::Saturation)
- cid = V4L2_CID_SATURATION;
+ cid = &v4l2::SATURATION;
else if (id == controls::AeEnable)
- cid = V4L2_CID_EXPOSURE_AUTO;
+ cid = &v4l2::EXPOSURE_AUTO;
else if (id == controls::ExposureTime)
- cid = V4L2_CID_EXPOSURE_ABSOLUTE;
+ cid = &v4l2::EXPOSURE_ABSOLUTE;
else if (id == controls::AnalogueGain)
- cid = V4L2_CID_GAIN;
+ cid = &v4l2::GAIN;
else
return -EINVAL;
@@ -286,18 +286,18 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
* See UVCCameraData::addControl() for explanations of the different
* value mappings.
*/
- switch (cid) {
+ switch (cid->id()) {
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)));
+ controls->set(v4l2::BRIGHTNESS, 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)));
+ controls->set(v4l2::SATURATION, lroundf(fvalue));
break;
}
@@ -305,12 +305,13 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
int32_t ivalue = value.get<bool>()
? V4L2_EXPOSURE_APERTURE_PRIORITY
: V4L2_EXPOSURE_MANUAL;
- controls->set(V4L2_CID_EXPOSURE_AUTO, ivalue);
+ controls->set(v4l2::EXPOSURE_AUTO, ivalue);
break;
}
case V4L2_CID_EXPOSURE_ABSOLUTE:
- controls->set(cid, value.get<int32_t>() / 100);
+ controls->set(v4l2::EXPOSURE_ABSOLUTE,
+ value.get<int32_t>() / 100);
break;
case V4L2_CID_CONTRAST:
@@ -324,13 +325,13 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
}
float fvalue = (value.get<float>() - p) / m;
- controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
+ controls->set(cid->id(), static_cast<int32_t>(lroundf(fvalue)));
break;
}
default: {
int32_t ivalue = value.get<int32_t>();
- controls->set(cid, ivalue);
+ controls->set(cid->id(), ivalue);
break;
}
}
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index 36325ffbbd7d..24b39b7dd879 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -342,24 +342,24 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
for (auto it : request->controls()) {
unsigned int id = it.first;
+ const ControlId *cid;
unsigned int offset;
- uint32_t cid;
if (id == controls::Brightness) {
- cid = V4L2_CID_BRIGHTNESS;
+ cid = &v4l2::BRIGHTNESS;
offset = 128;
} else if (id == controls::Contrast) {
- cid = V4L2_CID_CONTRAST;
+ cid = &v4l2::CONTRAST;
offset = 0;
} else if (id == controls::Saturation) {
- cid = V4L2_CID_SATURATION;
+ cid = &v4l2::SATURATION;
offset = 0;
} else {
continue;
}
int32_t value = lroundf(it.second.get<float>() * 128 + offset);
- controls.set(cid, std::clamp(value, 0, 255));
+ controls.set(cid->id(), std::clamp(value, 0, 255));
}
for (const auto &ctrl : controls)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list