[libcamera-devel] [PATCH] libcamera: pipeline: simple: Add support for controls
David Plowman
david.plowman at raspberrypi.com
Thu Dec 9 13:12:02 CET 2021
Hi Benjamin, everyone
Thanks for posting this patch. I think it's important to get autofocus
working, and indeed started a discussion on how it should look to
applications and users some time ago (email to this mailing list on 21
October).
Back then I was concerned about creating an interface that would make
sense to user applications and also Android, working typically at a
slightly higher level than V4L2. I certainly see the appeal of a
fairly direct translation between libcamera controls and V4L2 ones,
but I'm not sure it's the level at which a user, or Android, wants to
work. Having said that, maybe there could be a reason to have both?
What I proposed then was the following controls:
"AF Mode" - either "auto" or "continuous" (I didn't define an "off".
"auto" is identical to "off" if you don't send any "triggers").
"AF Trigger" - "start" or "cancel". Triggers (or cancels) an AF sweep
if in "auto" mode. Ignored during CAF (continuous AF).
"AF Range" - "macro", "full" or "normal". The part of the focus range
to sweep - "macro" (just very close distances), "full" (everything),
and "normal" (which is "full" minus "macro" - the reason being that
you spend a long time sweeping the macro end so it's often helpful to
be able to skip it).
"AF Speed" - particularly with CAF, you may want the algorithm to move
the lens more slowly (during video capture, typically) or faster (when
in preview mode before stills capture). Possibly this applies to
regular AF sweeps too, where "slower" = "more precise" and "faster" =
"take that picture *now*!".
Though I didn't mention it in October, I'd also like to have:
"AF Metering" - "centre" or "multi-spot". "centre" is pretty
self-explanatory, you focus on the centre of the image. "multi-spot"
means that you have multiple AF windows all across the image and you
monitor all of them when you sweep. At the end, you choose the one
with the closest focal distance. This has been quite common in many
camera systems.
"lens position" - move the lens to this position. I'm a bit in two
minds as to whether to use lens driver chip values directly, or more
Android-like numbers such that 0 = infinity. At some point we'll have
to support "calibrated" lenses where Android formally uses units of
dioptres. Using driver chip values here pushes that calibration into
the Android layer. Not sure.
The min/max/default values for the "lens position" control could be:
min = infinity, max = the value corresponding to the closest focal
position, and default = hyperfocal. There's still a bit of a question
as to where we get these numbers from as they depend on the module as
a whole (not just the lens driver chip).
And there will need to be some metadata coming back:
"lens position" - where the lens was for this frame
"AF state" - state of the AF algorithm, maybe "scanning" (sweep in
progress), "focused" (sweep finished and succeeded), "failed" (sweep
failed) and "reset" (when the camera has just started, or a scan was
cancelled, or the lens moved manually).
So the question in my mind is how all this co-exists with all those
rather lower-level V4L2 controls. Can we implement the above API in
terms of those V4L2 controls, do you think?
Anyway, I'd love to hear people's thoughts on this matter. I am trying
to get round to writing a slightly more formal proposal for all the
controls that I have outlined, hopefully it won't be too much longer.
Thanks and best regards
David
On Thu, 9 Dec 2021 at 10:55, Benjamin Schaaf <ben.schaaf at gmail.com> wrote:
>
> Thanks for the review, I'll put the updated patch at the end.
>
> On Thu, Dec 9, 2021 at 7:57 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
> >
> > Hello Benjamin,
> >
> > On Wed, Dec 08, 2021 at 11:35:23PM +1100, Benjamin Schaaf wrote:
> > > Controls and control info are translated between libcamera and V4L2
> > > inside the simple pipeline. Request controls are applied when a request
> > > is next to be completed.
> > >
> > > This also adds some additional draft controls needed for the PinePhone.
> >
> > Sorry to get straight to this before looking at the patch content, but
> > we do enforce a code style as reported here
> > https://libcamera.org/coding-style.html#coding-style-guidelines
> >
> > We have a tool that might help you catching style issues at
> > utils/checkstyle.py
> >
> > Care to reformat your patches to comply with the project code style ?
> >
> > >
> > > Signed-off-by: Benjamin Schaaf <ben.schaaf at gmail.com>
> > > ---
> > > src/libcamera/control_ids.yaml | 24 +++
> > > src/libcamera/controls.cpp | 6 -
> > > src/libcamera/pipeline/simple/controls.cpp | 230 +++++++++++++++++++++
> > > src/libcamera/pipeline/simple/controls.h | 26 +++
> > > src/libcamera/pipeline/simple/meson.build | 1 +
> > > src/libcamera/pipeline/simple/simple.cpp | 30 ++-
> > > 6 files changed, 310 insertions(+), 7 deletions(-)
> > > create mode 100644 src/libcamera/pipeline/simple/controls.cpp
> > > create mode 100644 src/libcamera/pipeline/simple/controls.h
> > >
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > index 9d4638ae..2af230c3 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -406,6 +406,30 @@ controls:
> > > The camera will cancel any active or completed metering sequence.
> > > The AE algorithm is reset to its initial state.
> > >
> > > + - AutoGain:
> > > + type: bool
> > > + draft: true
> > > + description: |
> > > + Control for Automatic Gain. Currently identical to V4L2_CID_AUTOGAIN.
> > > +
> > > + - AfEnabled:
> > > + type: bool
> > > + draft: true
> > > + description: |
> > > + Control for AF. Currently identical to V4L2_CID_FOCUS_AUTO.
> > > +
> > > + - AfStart:
> > > + type: void
> >
> > Why type void ? Isn't this a boolean ?
>
> V4L2_CID_AUTO_FOCUS_START has type V4L2_CTRL_TYPE_BUTTON, which simply
> performs an action when the control is set. Thus type void. Same for
> V4L2_CID_AUTO_FOCUS_END.
>
> It seems I forgot to include some required type conversion logic in
> v4l2_device, not sure how that got missed.
>
> > > + draft: true
> > > + description: |
> > > + Control for AF. Currently identical to V4L2_CID_AUTO_FOCUS_START.
> > > +
> > > + - AfStop:
> > > + type: void
> > > + draft: true
> > > + description: |
> > > + Control for AF. Currently identical to V4L2_CID_AUTO_FOCUS_END.
> > > +
> >
> > We're in the process of reworking controls related to gain and focus,
> > but for the moment, as we comply with Android by having their controls
> > defined as draft, I'm not opposed to have these here.
> >
> > I'm worried once we have applications use them, we will never move to
> > the newly defined ones though unless we forcefully remove them in
> > future...
>
> FWIW given that libcamera doesn't have versions/releases I don't
> personally expect a stable API.
>
> > > - AfTrigger:
> > > type: int32_t
> > > draft: true
> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > > index 0d8c0a5c..1f65fc73 100644
> > > --- a/src/libcamera/controls.cpp
> > > +++ b/src/libcamera/controls.cpp
> > > @@ -1052,9 +1052,6 @@ const ControlValue *ControlList::find(unsigned
> > > int id) const
> > > {
> > > const auto iter = controls_.find(id);
> > > if (iter == controls_.end()) {
> > > - LOG(Controls, Error)
> > > - << "Control " << utils::hex(id) << " not found";
> > > -
> >
> > Ouch, why remove the error message ?
>
> My bad, seems I was misinterpreting when that error could show up.
> I'll add it back in.
>
> > > return nullptr;
> > > }
> > >
> > > @@ -1064,9 +1061,6 @@ const ControlValue *ControlList::find(unsigned
> > > int id) const
> > > ControlValue *ControlList::find(unsigned int id)
> > > {
> > > if (validator_ && !validator_->validate(id)) {
> > > - LOG(Controls, Error)
> > > - << "Control " << utils::hex(id)
> > > - << " is not valid for " << validator_->name();
> > > return nullptr;
> > > }
> > >
> > > diff --git a/src/libcamera/pipeline/simple/controls.cpp
> > > b/src/libcamera/pipeline/simple/controls.cpp
> > > new file mode 100644
> > > index 00000000..32695749
> > > --- /dev/null
> > > +++ b/src/libcamera/pipeline/simple/controls.cpp
> > > @@ -0,0 +1,230 @@
> >
> > File license ? You can copy the SPDX header from other files and
> > attribute the copyright to you or any one you like
> >
> > > +#include "controls.h"
> > > +
> > > +#include <linux/v4l2-controls.h>
> > > +
> > > +#include <libcamera/base/log.h>
> > > +
> > > +#include <libcamera/control_ids.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +LOG_DECLARE_CATEGORY(SimplePipeline)
> > > +
> > > +/*
> > > + * These controls can be directly mapped between libcamera and V4L2 without
> > > + * doing any conversion to the ControlValue.
> > > + */
> > > +static std::unordered_map<unsigned int, unsigned int> controlsToV4L2 = {
> > > + { controls::AUTO_GAIN, V4L2_CID_AUTOGAIN },
> > > + { controls::AF_ENABLED, V4L2_CID_FOCUS_AUTO },
> > > + { controls::AF_START, V4L2_CID_AUTO_FOCUS_START },
> > > + { controls::AF_STOP, V4L2_CID_AUTO_FOCUS_STOP },
> > > + { controls::AE_ENABLE, V4L2_CID_EXPOSURE_AUTO },
> > > + { controls::EXPOSURE_VALUE, V4L2_CID_EXPOSURE },
> > > + { controls::DIGITAL_GAIN, V4L2_CID_GAIN },
> > > + { controls::ANALOGUE_GAIN, V4L2_CID_ANALOGUE_GAIN },
> > > + { controls::AF_STATE, V4L2_CID_AUTO_FOCUS_STATUS },
> > > +};
> >
> > If them map is meant for internal use only you can declare it in an
> > anonymous namespace
> >
> > namsepace {
> > std::unordered_map<>...
> >
> > };
> >
> > namespace libcamera {
> >
> > };
>
> That's the same as static though?
>
> > > +
> > > +/*
> > > + * Convert from a libcamera control to a V4L2 control, optionally
> > > also convert a
> > > + * set of ControlValues.
> > > + */
> >
> > We use doxygen for documenting the code. Please see other files as an
> > example.
> >
> > > +bool simpleControlToV4L2(unsigned int control,
> > > + unsigned int *v4l2_control,
> > > + const ControlValue *control_values,
> > > + ControlValue *v4l2_values,
> > > + size_t num_values)
> >
> > Before looking at the implementation, let's reason a bit on the
> > architecture.
> >
> > Is this control mapping valid for all platforms using the simple
> > pipeline handler ?
> >
> > Is the device on which controls have to be applied the same for all
> > platforms ?
> >
> > Should control handling be broken down to a platform specific
> > component to be selected at compile time ?
>
> I'm not sure what you mean by platform here? Do you mean Linux vs
> Android, x86 vs arm or PinePhone vs Librem 5?
>
> The way I see it, the simple pipeline is just a simple abstraction on
> V4L2 and this is a simple conversion between V4L2 and libcamera
> controls.
>
> > Also, I would like to see this implemented thorough a componenet with
> > a single interface towards the pipeline handler rather than a raw set
> > of helper functions. We can indeed help designing that once we have
> > the previous question clarified.
> >
> > I'm not even sure this is the direction we want to got with the simple
> > pipeline handler (platform-specific backends), and I would like to
> > hear Laurent's opinion on this, but I see a potential for doing what
> > we do with the android backend selection through the
> > 'android_platform' build option.
> >
> > option('android_platform',
> > type : 'combo',
> > choices : ['cros', 'generic'],
> > value : 'generic',
> > description : 'Select the Android platform to compile for')
> >
> > > +{
> > > + // Convert controls
> > > + if (v4l2_control) {
> > > + auto it = controlsToV4L2.find(control);
> > > + if (it == controlsToV4L2.end())
> > > + return false;
> > > +
> > > + *v4l2_control = it->second;
> > > + }
> > > +
> > > + // Convert values
> > > + if (num_values == 0)
> > > + return true;
> > > +
> > > + switch (control) {
> > > + case controls::AE_ENABLE:
> > > + for (size_t i = 0; i < num_values; ++i)
> > > + v4l2_values[i] =
> > > ControlValue((int32_t)(control_values[i].get<bool>() ?
> > > V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL));
> > > + return true;
> > > + case controls::EXPOSURE_VALUE:
> > > + case controls::DIGITAL_GAIN:
> > > + case controls::ANALOGUE_GAIN:
> > > + for (size_t i = 0; i < num_values; ++i)
> > > + v4l2_values[i] =
> > > ControlValue((int32_t)control_values[i].get<float>());
> > > + return true;
> > > + // Read only
> > > + case controls::AF_STATE:
> > > + return false;
> > > + default:
> > > + for (size_t i = 0; i < num_values; ++i)
> > > + v4l2_values[i] = control_values[i];
> > > + return true;
> > > + }
> > > +
> > > +}
> > > +
> > > +static std::unordered_map<unsigned int, unsigned int> controlsFromV4L2;
> > > +
> > > +/*
> > > + * Convert from a V4L2 control to a libcamera control, optionally
> > > also convert a
> > > + * set of ControlValues.
> > > + */
> > > +bool simpleControlFromV4L2(unsigned int v4l2_control,
> > > + unsigned int *control,
> > > + const ControlValue *v4l2_values,
> > > + ControlValue *control_values,
> > > + size_t num_values)
> > > +{
> > > + // Initialize the inverse of controlsToV4L2
> > > + if (controlsFromV4L2.empty()) {
> > > + for (const auto &v : controlsToV4L2) {
> > > + controlsFromV4L2[v.second] = v.first;
> > > + }
> > > + }
> > > +
> > > + // Convert control
> > > + if (control) {
> > > + auto it = controlsFromV4L2.find(v4l2_control);
> > > + if (it == controlsFromV4L2.end())
> > > + return false;
> > > +
> > > + *control = it->second;
> > > + }
> > > +
> > > + // Convert values
> > > + if (num_values == 0)
> > > + return true;
> > > +
> > > + switch (v4l2_control) {
> > > + case V4L2_CID_EXPOSURE_AUTO:
> > > + for (size_t i = 0; i < num_values; ++i)
> > > + control_values[i] =
> > > ControlValue(v4l2_values[i].get<int32_t>() == V4L2_EXPOSURE_AUTO);
> > > + return true;
> > > + case V4L2_CID_EXPOSURE:
> > > + case V4L2_CID_GAIN:
> > > + case V4L2_CID_ANALOGUE_GAIN:
> > > + for (size_t i = 0; i < num_values; ++i)
> > > + control_values[i] =
> > > ControlValue((float)v4l2_values[i].get<int32_t>());
> > > + return true;
> > > + case V4L2_CID_AUTO_FOCUS_STATUS:
> > > + for (size_t i = 0; i < num_values; ++i) {
> > > + switch (v4l2_values[i].get<int32_t>()) {
> > > + case V4L2_AUTO_FOCUS_STATUS_IDLE:
> > > + control_values[i] =
> > > ControlValue((int32_t)controls::draft::AfStateInactive);
> > > + break;
> > > + case V4L2_AUTO_FOCUS_STATUS_BUSY:
> > > + control_values[i] =
> > > ControlValue((int32_t)controls::draft::AfStateActiveScan);
> > > + break;
> > > + case V4L2_AUTO_FOCUS_STATUS_REACHED:
> > > + control_values[i] =
> > > ControlValue((int32_t)controls::draft::AfStatePassiveFocused);
> > > + break;
> > > + case V4L2_AUTO_FOCUS_STATUS_FAILED:
> > > + control_values[i] =
> > > ControlValue((int32_t)controls::draft::AfStatePassiveUnfocused);
> > > + break;
> > > + default:
> > > + LOG(SimplePipeline, Error)
> > > + << "AUTO_FOCUS_STATUS has invalid value: "
> > > + << utils::hex(v4l2_values[i].get<int32_t>());
> > > + /*TODO: Log Error*/
> > > + return false;
> > > + }
> > > + }
> > > + return true;
> > > + default:
> > > + for (size_t i = 0; i < num_values; ++i)
> > > + control_values[i] = v4l2_values[i];
> > > + return true;
> > > + }
> > > +}
> > > +
> > > +/*
> > > + * Convert a ControlInfoMap from V4L2 to libcamera. Converts both the control
> > > + * identifiers as well as all values.
> > > + */
> > > +ControlInfoMap simpleControlInfoFromV4L2(const ControlInfoMap &v4l2_info_map)
> > > +{
> > > + ControlInfoMap::Map info_map;
> > > +
> > > + for (const auto &pair : v4l2_info_map) {
> > > + unsigned int v4l2_control = pair.first->id();
> > > + const ControlInfo &v4l2_info = pair.second;
> > > +
> > > + unsigned int control;
> > > + ControlValue def;
> > > + if (!simpleControlFromV4L2(v4l2_control, &control,
> > > &v4l2_info.def(), &def, 1))
> > > + continue;
> > > +
> > > + const ControlId *control_id = controls::controls.at(control);
> > > +
> > > + // ControlInfo has either a list of values or a minimum and
> > > + // maximum. This includes controls that have no values or are
> > > + // booleans.
> > > + ControlInfo info;
> > > + if (v4l2_info.values().empty()) {
> > > + ControlValue min, max;
> > > + simpleControlFromV4L2(v4l2_control, nullptr,
> > > &v4l2_info.min(), &min, 1);
> > > + simpleControlFromV4L2(v4l2_control, nullptr,
> > > &v4l2_info.max(), &max, 1);
> > > + info = ControlInfo(std::move(min), std::move(max), std::move(def));
> > > + } else {
> > > + std::vector<ControlValue> values;
> > > + values.resize(v4l2_info.values().size());
> > > + simpleControlFromV4L2(v4l2_control, nullptr,
> > > v4l2_info.values().data(), values.data(), values.size());
> > > + info = ControlInfo(std::move(values), std::move(def));
> > > + }
> > > + info_map.emplace(control_id, std::move(info));
> > > + }
> > > +
> > > + return ControlInfoMap(std::move(info_map), controls::controls);
> > > +}
> > > +
> > > +/*
> > > + * Convert a control list from libcamera to V4L2.
> > > + */
> > > +ControlList simpleControlListToV4L2(const ControlList &controls)
> > > +{
> > > + ControlList v4l2_controls;
> > > + for (const auto &pair : controls) {
> > > + unsigned int control = pair.first;
> > > + const ControlValue &value = pair.second;
> > > +
> > > + unsigned int v4l2_control;
> > > + ControlValue v4l2_value;
> > > + if (!simpleControlToV4L2(control, &v4l2_control, &value,
> > > &v4l2_value, 1)) {
> > > + LOG(SimplePipeline, Warning)
> > > + << "Control " << utils::hex(control)
> > > + << " does not have a V4L2 equivalent";
> > > + continue;
> > > + }
> > > +
> > > + v4l2_controls.set(v4l2_control, v4l2_value);
> > > + }
> > > + return v4l2_controls;
> > > +}
> > > +
> > > +/*
> > > + * Convert a control list from V4L2 to libcamera.
> > > + */
> > > +ControlList simpleControlListFromV4L2(const ControlList &v4l2_controls)
> > > +{
> > > + ControlList controls;
> > > + for (const auto &pair : v4l2_controls) {
> > > + unsigned int v4l2_control = pair.first;
> > > + const ControlValue &v4l2_value = pair.second;
> > > +
> > > + unsigned int control;
> > > + ControlValue value;
> > > + if (simpleControlFromV4L2(v4l2_control, &control,
> > > &v4l2_value, &value, 1))
> > > + controls.set(control, value);
> > > + }
> > > + return controls;
> > > +}
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/libcamera/pipeline/simple/controls.h
> > > b/src/libcamera/pipeline/simple/controls.h
> > > new file mode 100644
> > > index 00000000..114c5fc2
> > > --- /dev/null
> > > +++ b/src/libcamera/pipeline/simple/controls.h
> >
> > Same comment, license and copyright
> >
> > > @@ -0,0 +1,26 @@
> > > +#ifndef __LIBCAMERA_PIPELINE_SIMPLE_CONTROLS_H__
> > > +#define __LIBCAMERA_PIPELINE_SIMPLE_CONTROLS_H__
> >
> > #pragma once
>
> All the code I've seen uses #ifndef instead of #pragma once. I'd
> prefer to use #pragma once but it seems inconsistent with the rest of
> the codebase?
>
> > > +
> > > +#include <libcamera/controls.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +bool simpleControlToV4L2(unsigned int control,
> > > + unsigned int *v4l2_control,
> > > + const ControlValue *control_values,
> > > + ControlValue *v4l2_values,
> > > + size_t num_values);
> > > +bool simpleControlFromV4L2(unsigned int v4l2_control,
> > > + unsigned int *control,
> > > + const ControlValue *v4l2_values,
> > > + ControlValue *control_values,
> > > + size_t num_values);
> > > +
> > > +ControlInfoMap simpleControlInfoFromV4L2(const ControlInfoMap &v4l2_info);
> > > +
> > > +ControlList simpleControlListToV4L2(const ControlList &controls);
> > > +ControlList simpleControlListFromV4L2(const ControlList &controls);
> > > +
> > > +} /* namespace libcamera */
> > > +
> > > +#endif /* __LIBCAMERA_PIPELINE_SIMPLE_CONTROLS_H__ */
> > > diff --git a/src/libcamera/pipeline/simple/meson.build
> > > b/src/libcamera/pipeline/simple/meson.build
> > > index 9c99b32f..0c60d65a 100644
> > > --- a/src/libcamera/pipeline/simple/meson.build
> > > +++ b/src/libcamera/pipeline/simple/meson.build
> > > @@ -1,6 +1,7 @@
> > > # SPDX-License-Identifier: CC0-1.0
> > >
> > > libcamera_sources += files([
> > > + 'controls.cpp',
> > > 'converter.cpp',
> > > 'simple.cpp',
> > > ])
> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp
> > > b/src/libcamera/pipeline/simple/simple.cpp
> > > index a597e27f..b0d4a62a 100644
> > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > @@ -37,6 +37,7 @@
> > > #include "libcamera/internal/v4l2_videodevice.h"
> > >
> > > #include "converter.h"
> > > +#include "controls.h"
> > >
> > > namespace libcamera {
> > >
> > > @@ -181,6 +182,7 @@ public:
> > > int setupLinks();
> > > int setupFormats(V4L2SubdeviceFormat *format,
> > > V4L2Subdevice::Whence whence);
> > > + int setRequestControls(Request *request);
> > > void bufferReady(FrameBuffer *buffer);
> > >
> > > unsigned int streamIndex(const Stream *stream) const
> > > @@ -519,7 +521,8 @@ int SimpleCameraData::init()
> > > formats_[fmt] = &config;
> > > }
> > >
> > > - properties_ = sensor_->properties();
> > > + properties_ = simpleControlListFromV4L2(sensor_->properties());
> > > + controlInfo_ = simpleControlInfoFromV4L2(sensor_->controls());
> > >
> > > return 0;
> > > }
> > > @@ -624,6 +627,23 @@ int
> > > SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
> > > return 0;
> > > }
> > >
> > > +int SimpleCameraData::setRequestControls(Request *request)
> > > +{
> > > + SimplePipelineHandler *pipe = SimpleCameraData::pipe();
> > > +
> > > + // Apply controls only to one entity. If there's a subdevice use that.
> > > + V4L2Device *control_device = video_;
> > > + for (const SimpleCameraData::Entity &e : entities_) {
> > > + V4L2Subdevice *subdev = pipe->subdev(e.entity);
> > > + if (subdev) {
> > > + control_device = subdev;
> > > + }
> > > + }
> > > +
> > > + ControlList controls = simpleControlListToV4L2(request->controls());
> > > + return control_device->setControls(&controls);
> > > +}
> > > +
> > > void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> > > {
> > > SimplePipelineHandler *pipe = SimpleCameraData::pipe();
> > > @@ -666,6 +686,10 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> > > return;
> > > }
> > >
> > > + // Set the controls for the next queued request
> > > + if (!queuedRequests_.empty())
> > > + setRequestControls(queuedRequests_.front());
> > > +
> > > /*
> > > * Record the sensor's timestamp in the request metadata. The request
> > > * needs to be obtained from the user-facing buffer, as internal
> > > @@ -1033,6 +1057,10 @@ int SimplePipelineHandler::start(Camera
> > > *camera, [[maybe_unused]] const ControlL
> > > return ret;
> > > }
> > >
> > > + // Apply controls from first request
> > > + if (!data->queuedRequests_.empty())
> > > + data->setRequestControls(data->queuedRequests_.front());
> > > +
> > > if (data->useConverter_) {
> > > ret = data->converter_->start();
> > > if (ret < 0) {
> > > --
> > > 2.25.1
>
> [PATCH] libcamera: pipeline: simple: Add support for controls
>
> Controls and control info are translated between libcamera and V4L2
> inside the simple pipeline. Request controls are applied when a request
> is next to be completed.
>
> This also adds some additional draft controls needed for the PinePhone.
>
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=98
> Signed-off-by: Benjamin Schaaf <ben.schaaf at gmail.com>
> ---
> src/libcamera/control_ids.yaml | 24 ++
> src/libcamera/pipeline/simple/controls.cpp | 241 +++++++++++++++++++++
> src/libcamera/pipeline/simple/controls.h | 33 +++
> src/libcamera/pipeline/simple/meson.build | 1 +
> src/libcamera/pipeline/simple/simple.cpp | 30 ++-
> src/libcamera/v4l2_device.cpp | 12 +-
> 6 files changed, 339 insertions(+), 2 deletions(-)
> create mode 100644 src/libcamera/pipeline/simple/controls.cpp
> create mode 100644 src/libcamera/pipeline/simple/controls.h
>
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index 9d4638ae..2af230c3 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -406,6 +406,30 @@ controls:
> The camera will cancel any active or completed metering sequence.
> The AE algorithm is reset to its initial state.
>
> + - AutoGain:
> + type: bool
> + draft: true
> + description: |
> + Control for Automatic Gain. Currently identical to V4L2_CID_AUTOGAIN.
> +
> + - AfEnabled:
> + type: bool
> + draft: true
> + description: |
> + Control for AF. Currently identical to V4L2_CID_FOCUS_AUTO.
> +
> + - AfStart:
> + type: void
> + draft: true
> + description: |
> + Control for AF. Currently identical to V4L2_CID_AUTO_FOCUS_START.
> +
> + - AfStop:
> + type: void
> + draft: true
> + description: |
> + Control for AF. Currently identical to V4L2_CID_AUTO_FOCUS_END.
> +
> - AfTrigger:
> type: int32_t
> draft: true
> diff --git a/src/libcamera/pipeline/simple/controls.cpp
> b/src/libcamera/pipeline/simple/controls.cpp
> new file mode 100644
> index 00000000..f3922e45
> --- /dev/null
> +++ b/src/libcamera/pipeline/simple/controls.cpp
> @@ -0,0 +1,241 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Benjamin Schaaf
> + *
> + * controls.cpp - Simple pipeline control conversion
> + */
> +
> +#include "controls.h"
> +
> +#include <linux/v4l2-controls.h>
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/control_ids.h>
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(SimplePipeline)
> +
> +/*
> + * These controls can be directly mapped between libcamera and V4L2 without
> + * doing any conversion to the ControlValue.
> + */
> +static std::unordered_map<unsigned int, unsigned int> controlsToV4L2 = {
> + { controls::AUTO_GAIN, V4L2_CID_AUTOGAIN },
> + { controls::AF_ENABLED, V4L2_CID_FOCUS_AUTO },
> + { controls::AF_START, V4L2_CID_AUTO_FOCUS_START },
> + { controls::AF_STOP, V4L2_CID_AUTO_FOCUS_STOP },
> + { controls::AE_ENABLE, V4L2_CID_EXPOSURE_AUTO },
> + { controls::EXPOSURE_VALUE, V4L2_CID_EXPOSURE },
> + { controls::DIGITAL_GAIN, V4L2_CID_GAIN },
> + { controls::ANALOGUE_GAIN, V4L2_CID_ANALOGUE_GAIN },
> + { controls::AF_STATE, V4L2_CID_AUTO_FOCUS_STATUS },
> +};
> +
> +/**
> + * \brief Convert from a libcamera control to a V4L2 control.
> + *
> + * Can optionally convert the libcamera control and/or a set of libcamera
> + * control values to their V4L2 equivalents.
> + */
> +bool simpleControlToV4L2(unsigned int control,
> + unsigned int *v4l2_control,
> + const ControlValue *control_values,
> + ControlValue *v4l2_values,
> + size_t num_values)
> +{
> + // Convert controls
> + if (v4l2_control) {
> + auto it = controlsToV4L2.find(control);
> + if (it == controlsToV4L2.end())
> + return false;
> +
> + *v4l2_control = it->second;
> + }
> +
> + // Convert values
> + if (num_values == 0)
> + return true;
> +
> + switch (control) {
> + case controls::AE_ENABLE:
> + for (size_t i = 0; i < num_values; ++i)
> + v4l2_values[i] =
> ControlValue((int32_t)(control_values[i].get<bool>() ?
> V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL));
> + return true;
> + case controls::EXPOSURE_VALUE:
> + case controls::DIGITAL_GAIN:
> + case controls::ANALOGUE_GAIN:
> + for (size_t i = 0; i < num_values; ++i)
> + v4l2_values[i] =
> ControlValue((int32_t)control_values[i].get<float>());
> + return true;
> + // Read only
> + case controls::AF_STATE:
> + return false;
> + default:
> + for (size_t i = 0; i < num_values; ++i)
> + v4l2_values[i] = control_values[i];
> + return true;
> + }
> +}
> +
> +static std::unordered_map<unsigned int, unsigned int> controlsFromV4L2;
> +
> +/**
> + * \brief Convert from a V4L2 control to a libcamera control.
> + *
> + * Can optionally convert the V4L2 control and/or a set of V4L2 control values
> + * to their libcamera equivalents.
> + */
> +bool simpleControlFromV4L2(unsigned int v4l2_control,
> + unsigned int *control,
> + const ControlValue *v4l2_values,
> + ControlValue *control_values,
> + size_t num_values)
> +{
> + // Initialize the inverse of controlsToV4L2
> + if (controlsFromV4L2.empty()) {
> + for (const auto &v : controlsToV4L2) {
> + controlsFromV4L2[v.second] = v.first;
> + }
> + }
> +
> + // Convert control
> + if (control) {
> + auto it = controlsFromV4L2.find(v4l2_control);
> + if (it == controlsFromV4L2.end())
> + return false;
> +
> + *control = it->second;
> + }
> +
> + // Convert values
> + if (num_values == 0)
> + return true;
> +
> + switch (v4l2_control) {
> + case V4L2_CID_EXPOSURE_AUTO:
> + for (size_t i = 0; i < num_values; ++i)
> + control_values[i] =
> ControlValue(v4l2_values[i].get<int32_t>() == V4L2_EXPOSURE_AUTO);
> + return true;
> + case V4L2_CID_EXPOSURE:
> + case V4L2_CID_GAIN:
> + case V4L2_CID_ANALOGUE_GAIN:
> + for (size_t i = 0; i < num_values; ++i)
> + control_values[i] =
> ControlValue((float)v4l2_values[i].get<int32_t>());
> + return true;
> + case V4L2_CID_AUTO_FOCUS_STATUS:
> + for (size_t i = 0; i < num_values; ++i) {
> + switch (v4l2_values[i].get<int32_t>()) {
> + case V4L2_AUTO_FOCUS_STATUS_IDLE:
> + control_values[i] =
> ControlValue((int32_t)controls::draft::AfStateInactive);
> + break;
> + case V4L2_AUTO_FOCUS_STATUS_BUSY:
> + control_values[i] =
> ControlValue((int32_t)controls::draft::AfStateActiveScan);
> + break;
> + case V4L2_AUTO_FOCUS_STATUS_REACHED:
> + control_values[i] =
> ControlValue((int32_t)controls::draft::AfStatePassiveFocused);
> + break;
> + case V4L2_AUTO_FOCUS_STATUS_FAILED:
> + control_values[i] =
> ControlValue((int32_t)controls::draft::AfStatePassiveUnfocused);
> + break;
> + default:
> + LOG(SimplePipeline, Error)
> + << "AUTO_FOCUS_STATUS has invalid value: "
> + << utils::hex(v4l2_values[i].get<int32_t>());
> + /*TODO: Log Error*/
> + return false;
> + }
> + }
> + return true;
> + default:
> + for (size_t i = 0; i < num_values; ++i)
> + control_values[i] = v4l2_values[i];
> + return true;
> + }
> +}
> +
> +/**
> + * \brief Convert a ControlInfoMap from V4L2 to libcamera.
> + *
> + * Converts both the control identifiers as well as all values.
> + */
> +ControlInfoMap simpleControlInfoFromV4L2(const ControlInfoMap &v4l2_info_map)
> +{
> + ControlInfoMap::Map info_map;
> +
> + for (const auto &pair : v4l2_info_map) {
> + unsigned int v4l2_control = pair.first->id();
> + const ControlInfo &v4l2_info = pair.second;
> +
> + unsigned int control;
> + ControlValue def;
> + if (!simpleControlFromV4L2(v4l2_control, &control,
> &v4l2_info.def(), &def, 1))
> + continue;
> +
> + const ControlId *control_id = controls::controls.at(control);
> +
> + // ControlInfo has either a list of values or a minimum and
> + // maximum. This includes controls that have no values or are
> + // booleans.
> + ControlInfo info;
> + if (v4l2_info.values().empty()) {
> + ControlValue min, max;
> + simpleControlFromV4L2(v4l2_control, nullptr,
> &v4l2_info.min(), &min, 1);
> + simpleControlFromV4L2(v4l2_control, nullptr,
> &v4l2_info.max(), &max, 1);
> + info = ControlInfo(std::move(min), std::move(max), std::move(def));
> + } else {
> + std::vector<ControlValue> values;
> + values.resize(v4l2_info.values().size());
> + simpleControlFromV4L2(v4l2_control, nullptr,
> v4l2_info.values().data(), values.data(), values.size());
> + info = ControlInfo(std::move(values), std::move(def));
> + }
> + info_map.emplace(control_id, std::move(info));
> + }
> +
> + return ControlInfoMap(std::move(info_map), controls::controls);
> +}
> +
> +/**
> + * \brief Convert a control list from libcamera to V4L2.
> + */
> +ControlList simpleControlListToV4L2(const ControlList &controls)
> +{
> + ControlList v4l2_controls;
> + for (const auto &pair : controls) {
> + unsigned int control = pair.first;
> + const ControlValue &value = pair.second;
> +
> + unsigned int v4l2_control;
> + ControlValue v4l2_value;
> + if (!simpleControlToV4L2(control, &v4l2_control, &value,
> &v4l2_value, 1)) {
> + LOG(SimplePipeline, Warning)
> + << "Control " << utils::hex(control)
> + << " does not have a V4L2 equivalent";
> + continue;
> + }
> +
> + v4l2_controls.set(v4l2_control, v4l2_value);
> + }
> + return v4l2_controls;
> +}
> +
> +/**
> + * \brief Convert a control list from V4L2 to libcamera.
> + */
> +ControlList simpleControlListFromV4L2(const ControlList &v4l2_controls)
> +{
> + ControlList controls;
> + for (const auto &pair : v4l2_controls) {
> + unsigned int v4l2_control = pair.first;
> + const ControlValue &v4l2_value = pair.second;
> +
> + unsigned int control;
> + ControlValue value;
> + if (simpleControlFromV4L2(v4l2_control, &control,
> &v4l2_value, &value, 1))
> + controls.set(control, value);
> + }
> + return controls;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/simple/controls.h
> b/src/libcamera/pipeline/simple/controls.h
> new file mode 100644
> index 00000000..54b4a565
> --- /dev/null
> +++ b/src/libcamera/pipeline/simple/controls.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Benjamin Schaaf
> + *
> + * controls.h - Simple pipeline control conversion
> + */
> +
> +#ifndef __LIBCAMERA_PIPELINE_SIMPLE_CONTROLS_H__
> +#define __LIBCAMERA_PIPELINE_SIMPLE_CONTROLS_H__
> +
> +#include <libcamera/controls.h>
> +
> +namespace libcamera {
> +
> +bool simpleControlToV4L2(unsigned int control,
> + unsigned int *v4l2_control,
> + const ControlValue *control_values,
> + ControlValue *v4l2_values,
> + size_t num_values);
> +bool simpleControlFromV4L2(unsigned int v4l2_control,
> + unsigned int *control,
> + const ControlValue *v4l2_values,
> + ControlValue *control_values,
> + size_t num_values);
> +
> +ControlInfoMap simpleControlInfoFromV4L2(const ControlInfoMap &v4l2_info);
> +
> +ControlList simpleControlListToV4L2(const ControlList &controls);
> +ControlList simpleControlListFromV4L2(const ControlList &controls);
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_PIPELINE_SIMPLE_CONTROLS_H__ */
> diff --git a/src/libcamera/pipeline/simple/meson.build
> b/src/libcamera/pipeline/simple/meson.build
> index 9c99b32f..0c60d65a 100644
> --- a/src/libcamera/pipeline/simple/meson.build
> +++ b/src/libcamera/pipeline/simple/meson.build
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: CC0-1.0
>
> libcamera_sources += files([
> + 'controls.cpp',
> 'converter.cpp',
> 'simple.cpp',
> ])
> diff --git a/src/libcamera/pipeline/simple/simple.cpp
> b/src/libcamera/pipeline/simple/simple.cpp
> index a597e27f..1717a1a7 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -36,6 +36,7 @@
> #include "libcamera/internal/v4l2_subdevice.h"
> #include "libcamera/internal/v4l2_videodevice.h"
>
> +#include "controls.h"
> #include "converter.h"
>
> namespace libcamera {
> @@ -181,6 +182,7 @@ public:
> int setupLinks();
> int setupFormats(V4L2SubdeviceFormat *format,
> V4L2Subdevice::Whence whence);
> + int setRequestControls(Request *request);
> void bufferReady(FrameBuffer *buffer);
>
> unsigned int streamIndex(const Stream *stream) const
> @@ -519,7 +521,8 @@ int SimpleCameraData::init()
> formats_[fmt] = &config;
> }
>
> - properties_ = sensor_->properties();
> + properties_ = simpleControlListFromV4L2(sensor_->properties());
> + controlInfo_ = simpleControlInfoFromV4L2(sensor_->controls());
>
> return 0;
> }
> @@ -624,6 +627,23 @@ int
> SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
> return 0;
> }
>
> +int SimpleCameraData::setRequestControls(Request *request)
> +{
> + SimplePipelineHandler *pipe = SimpleCameraData::pipe();
> +
> + // Apply controls only to one entity. If there's a subdevice use that.
> + V4L2Device *control_device = video_;
> + for (const SimpleCameraData::Entity &e : entities_) {
> + V4L2Subdevice *subdev = pipe->subdev(e.entity);
> + if (subdev) {
> + control_device = subdev;
> + }
> + }
> +
> + ControlList controls = simpleControlListToV4L2(request->controls());
> + return control_device->setControls(&controls);
> +}
> +
> void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> {
> SimplePipelineHandler *pipe = SimpleCameraData::pipe();
> @@ -666,6 +686,10 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> return;
> }
>
> + // Set the controls for the next queued request
> + if (!queuedRequests_.empty())
> + setRequestControls(queuedRequests_.front());
> +
> /*
> * Record the sensor's timestamp in the request metadata. The request
> * needs to be obtained from the user-facing buffer, as internal
> @@ -1033,6 +1057,10 @@ int SimplePipelineHandler::start(Camera
> *camera, [[maybe_unused]] const ControlL
> return ret;
> }
>
> + // Apply controls from first request
> + if (!data->queuedRequests_.empty())
> + data->setRequestControls(data->queuedRequests_.front());
> +
> if (data->useConverter_) {
> ret = data->converter_->start();
> if (ret < 0) {
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 9c783c9c..2a15ae09 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -296,6 +296,13 @@ int V4L2Device::setControls(ControlList *ctrls)
> /* Set the v4l2_ext_control value for the write operation. */
> ControlValue &value = ctrl->second;
> switch (iter->first->type()) {
> + case ControlTypeBool:
> + v4l2Ctrl.value64 = value.get<bool>();
> + break;
> +
> + case ControlTypeNone:
> + break;
> +
> case ControlTypeInteger64:
> v4l2Ctrl.value64 = value.get<int64_t>();
> break;
> @@ -479,7 +486,6 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)
> return ControlTypeInteger64;
>
> case V4L2_CTRL_TYPE_MENU:
> - case V4L2_CTRL_TYPE_BUTTON:
> case V4L2_CTRL_TYPE_BITMASK:
> case V4L2_CTRL_TYPE_INTEGER_MENU:
> /*
> @@ -488,6 +494,7 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)
> */
> return ControlTypeInteger32;
>
> + case V4L2_CTRL_TYPE_BUTTON:
> default:
> return ControlTypeNone;
> }
> @@ -530,6 +537,9 @@ ControlInfo V4L2Device::v4l2ControlInfo(const
> v4l2_query_ext_ctrl &ctrl)
> static_cast<int64_t>(ctrl.maximum),
> static_cast<int64_t>(ctrl.default_value));
>
> + case V4L2_CTRL_TYPE_BUTTON:
> + return ControlInfo(ControlValue(), ControlValue(), ControlValue());
> +
> case V4L2_CTRL_TYPE_INTEGER_MENU:
> case V4L2_CTRL_TYPE_MENU:
> return v4l2MenuControlInfo(ctrl);
> --
> 2.25.1
More information about the libcamera-devel
mailing list