[libcamera-devel] [PATCH] libcamera: pipeline: simple: Add support for controls

Benjamin Schaaf ben.schaaf at gmail.com
Thu Dec 9 12:57:43 CET 2021


On Thu, Dec 9, 2021 at 10:33 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi Benjamin
>
> On Thu, Dec 09, 2021 at 09:55:33PM +1100, Benjamin Schaaf wrote:
> > Thanks for the review, I'll put the updated patch at the end.
>
> Please don't :)
>
> Always send a new patch for new versions!

No worries, I'll do that.

> >
> > 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.
>
> So you use void to indicate that we don't care about the value but the
> control presence signify that, in example, the autofocus routine
> should be started.
>
> We don't have 'one shot' control so far (the assumption is that once a
> control is set to a value, the value stays the same until it's not
> updated), and this could be a valid usage of type: void indeed
>
> >
> > > > +      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.
> >
>
> In the long term, draft control will be replaced by standard controls.
> What I'm afraid of is that if applications start relying on draft
> controls it will be harder to remove them, as it will likely require a
> different type of mapping. But I have no better suggestions to provide
> atm
>
> > > >    - 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?
> >
>
> Yes but we usually prefer anonymous namespaces... Not a big deal
> 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?
>
> I mean the SoC.
>
> Simple is used (afaik) on imx6, imx8mq, allwinner, mediatek and
> possibly others.
>
> The mapping between v4l2 controls and libcamera controls does
> generically apply to all of them ? Surely not the values the controls
> transport, but if we assume the application knows what platform it
> operates on that's fine.

I don't see how they wouldn't. The V4L2 controls are standardized in
the same way the android ones are. FWIW the controls aren't SoC
specific, they're sensor + SoC +  device-tree specific.

> >
> > 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.
>
> This is a bit of a stretch.
>
> What's happening here is that you defined controls equal to the V4L2
> ones, and have the app set the 'right' values for the sensor/platform
> in use (a 'gain' value does not have the same meaning between
> different sensors, in example).
>
> In 'regular' platforms with an ISP, an IPA etc the pipeline receives
> libcamera::controls and with the help of IPA and statistics computes
> the right v4l2 controls for the platform (the pipeline handler knows
> what platform it runs on, except for simple) and for the sensor
> through a set of CameraSensorHelpers that aid with the translation.
>
> Now we change the landscape a bit, and we assume the app knows what
> platforms it runs on, something that defeats the purpose of libcamera
> usage, but I understand there aren't may way around that to support
> your use case.
>
> As a pipeline handler is charge of:
>
> 1) Registering what control it supports to expose that to application
> 2) Translate libcamera control to v4l2 controls (not in your case)
> 3) Apply controls to the right device/subdevice at the right time (the
>   time when to set a control is not trivially calculated as most
>   controls have a delay and should be applied in advance)
>
> Now assuming point 2) is moved to the app in your case, point 1 and 3
> do not apply generically to all platforms using the simple pipeline
> handler. Some might support control the other does not support. Some
> might want to set a control to the video devices while others will
> apply the same control on the sensor subdev. It all very depends on
> the driver architecture of the SoC.
>
> Now, even for simple boolean/button controls like the ones you're dealing with,
> for which not much reasoning and translations are required, a platform
> specific component seems to be needed to address 1) and 3).
>
> Does it make sense to you ?

In terms of registering what controls are exposed that's done
generically in SimpleCameraData::init right? As for when controls are
applied the assumption being made in libcamera is that they can be
applied before a frame no?

> >
> > > 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?
> >
>
> Not since
> https://patchwork.libcamera.org/project/libcamera/list/?series=2749&state=*
>
> > > > +
> > > > +#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