[libcamera-devel] [PATCH v3 1/5] libcamera: controls: Create ControlInfoMap with ControlIdMap
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Aug 13 02:30:37 CEST 2021
Hi Jacopo,
I know this has been merged already, but there's one coment below.
On Tue, Aug 10, 2021 at 09:44:57AM +0200, Jacopo Mondi wrote:
> On Tue, Aug 10, 2021 at 03:45:46AM +0300, Laurent Pinchart wrote:
> > On Mon, Aug 09, 2021 at 05:23:04PM +0200, Jacopo Mondi wrote:
> > > ControlInfoMap does not have a ControlId map associated, but rather
> > > creates one with the generateIdMap() function at creation time.
> > >
> > > As a consequence, when in the need to de-serialize a ControlInfoMap all
> > > the ControlId it contains are created by the deserializer instance, not
> > > being able to discern if the controls the ControlIdMap refers to are the
> > > global libcamera controls (and proeprties) or instances local to the
> > > V4L2 device that has first initialized the controls.
> > >
> > > As a consequence the ControlId stored in a de-serialized map will always
> > > be newly created entities, preventing lookup by ControlId reference on a
> > > de-serialized ControlInfoMap.
> > >
> > > In order to make it possible to use globally available ControlId
> > > instances whenever possible, create ControlInfoMap with a reference to
> > > an externally allocated ControlIdMap instead of generating one
> > > internally.
> > >
> > > Has a consequence the class constructors take and additional argument,
> >
> > s/Has/As/
> >
> > > which might be not pleaseant to type in, but enforces the concepts that
> >
> > s/pleaseant/pleasant/
> >
> > > ControlInfoMap should be created with controls part of the same id map.
> > >
> > > As the ControlIdMap the ControlInfoMap refers to needs to be allocated
> > > externally:
> > > - Use the globally available controls::controls (or
> > > properties::properties) id map when referring to libcamera controls
> > > - The V4L2 device that creates ControlInfoMap by parsing the device's
> > > controls has to allocate a ControlIdMap
> > > - The ControlSerializer that de-serializes a ControlInfoMap has to
> > > create and store the ControlIdMap the de-serialized info map refers to
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> > > ---
> > > include/libcamera/controls.h | 13 ++-
> > > .../libcamera/internal/control_serializer.h | 1 +
> > > include/libcamera/internal/v4l2_device.h | 1 +
> > > include/libcamera/ipa/raspberrypi.h | 40 ++++----
> > > src/libcamera/control_serializer.cpp | 11 ++-
> > > src/libcamera/controls.cpp | 94 +++++--------------
> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-
> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 +-
> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +-
> > > src/libcamera/pipeline/vimc/vimc.cpp | 2 +-
> > > src/libcamera/v4l2_device.cpp | 3 +-
> > > .../ipa_data_serializer_test.cpp | 14 +--
> > > 12 files changed, 77 insertions(+), 110 deletions(-)
> > >
> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > > index de733bd868a6..e9e09c200da2 100644
> > > --- a/include/libcamera/controls.h
> > > +++ b/include/libcamera/controls.h
> > > @@ -309,12 +309,11 @@ public:
> > >
> > > ControlInfoMap() = default;
> > > ControlInfoMap(const ControlInfoMap &other) = default;
> > > - ControlInfoMap(std::initializer_list<Map::value_type> init);
> > > - ControlInfoMap(Map &&info);
> > > + ControlInfoMap(std::initializer_list<Map::value_type> init,
> > > + const ControlIdMap &idmap);
> > > + ControlInfoMap(Map &&info, const ControlIdMap &idmap);
> > >
> > > ControlInfoMap &operator=(const ControlInfoMap &other) = default;
> > > - ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
> > > - ControlInfoMap &operator=(Map &&info);
> > >
> > > using Map::key_type;
> > > using Map::mapped_type;
> > > @@ -339,12 +338,12 @@ public:
> > > iterator find(unsigned int key);
> > > const_iterator find(unsigned int key) const;
> > >
> > > - const ControlIdMap &idmap() const { return idmap_; }
> > > + const ControlIdMap &idmap() const { return *idmap_; }
> > >
> > > private:
> > > - void generateIdmap();
> > > + const ControlIdMap *idmap_;
> > >
> > > - ControlIdMap idmap_;
> > > + void validate();
> >
> > We usually put functions before variables.
> >
>
> Ups, trivial
>
> > > };
> > >
> > > class ControlList
> > > diff --git a/include/libcamera/internal/control_serializer.h b/include/libcamera/internal/control_serializer.h
> > > index 7d4426c95d12..8a66be324138 100644
> > > --- a/include/libcamera/internal/control_serializer.h
> > > +++ b/include/libcamera/internal/control_serializer.h
> > > @@ -48,6 +48,7 @@ private:
> > >
> > > unsigned int serial_;
> > > std::vector<std::unique_ptr<ControlId>> controlIds_;
> > > + std::vector<std::unique_ptr<ControlIdMap>> controlIdMaps_;
> > > std::map<unsigned int, ControlInfoMap> infoMaps_;
> > > std::map<const ControlInfoMap *, unsigned int> infoMapHandles_;
> > > };
> > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> > > index 77b835b3cb80..423c8fb11845 100644
> > > --- a/include/libcamera/internal/v4l2_device.h
> > > +++ b/include/libcamera/internal/v4l2_device.h
> > > @@ -69,6 +69,7 @@ private:
> > >
> > > std::map<unsigned int, struct v4l2_query_ext_ctrl> controlInfo_;
> > > std::vector<std::unique_ptr<ControlId>> controlIds_;
> > > + ControlIdMap controlIdMap_;
> > > ControlInfoMap controls_;
> > > std::string deviceNode_;
> > > int fd_;
> > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> > > index a8790000e2d9..521eaecd19b2 100644
> > > --- a/include/libcamera/ipa/raspberrypi.h
> > > +++ b/include/libcamera/ipa/raspberrypi.h
> > > @@ -27,26 +27,26 @@ namespace RPi {
> > > * and the pipeline handler may be reverted so that it aborts when an
> > > * unsupported control is encountered.
> > > */
> > > -static const ControlInfoMap Controls = {
> > > - { &controls::AeEnable, ControlInfo(false, true) },
> > > - { &controls::ExposureTime, ControlInfo(0, 999999) },
> > > - { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > > - { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },
> > > - { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
> > > - { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },
> > > - { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> > > - { &controls::AwbEnable, ControlInfo(false, true) },
> > > - { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > > - { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
> > > - { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > > - { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> > > - { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> > > - { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> > > - { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> > > - { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
> > > - { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },
> > > - { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
> > > -};
> > > +static const ControlInfoMap Controls({
> > > + { &controls::AeEnable, ControlInfo(false, true) },
> > > + { &controls::ExposureTime, ControlInfo(0, 999999) },
> > > + { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > > + { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },
> > > + { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
> > > + { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },
> > > + { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> > > + { &controls::AwbEnable, ControlInfo(false, true) },
> > > + { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > > + { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
> > > + { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > > + { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> > > + { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> > > + { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> > > + { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> > > + { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
> > > + { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },
> > > + { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) }
> > > + }, controls::controls);
> > >
> > > } /* namespace RPi */
> > >
> > > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> > > index 300466285a57..df6ed93f477e 100644
> > > --- a/src/libcamera/control_serializer.cpp
> > > +++ b/src/libcamera/control_serializer.cpp
> > > @@ -93,6 +93,7 @@ void ControlSerializer::reset()
> > > infoMapHandles_.clear();
> > > infoMaps_.clear();
> > > controlIds_.clear();
> > > + controlIdMaps_.clear();
> > > }
> > >
> > > size_t ControlSerializer::binarySize(const ControlValue &value)
> > > @@ -376,6 +377,8 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
> > > }
> > >
> > > ControlInfoMap::Map ctrls;
> > > + controlIdMaps_.emplace_back(std::make_unique<ControlIdMap>());
> > > + ControlIdMap *localIdMap = controlIdMaps_.back().get();
> >
> > My suggestion to use ->at() was wrong, but how about the other one,
> > using
> >
> > ControlIdMap &localIdMap = *controlIdMaps_.back().get();
> >
> > here ?
> >
>
> As I replied to the previous version, I need a pointer in the next
> patch, so having a & here to turn it into a pointer immediately after
> doesn't bring much value, doesn't it ?
>
> > >
> > > for (unsigned int i = 0; i < hdr->entries; ++i) {
> > > const struct ipa_control_info_entry *entry =
> > > @@ -392,6 +395,8 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
> > > * purpose.
> > > */
> > > controlIds_.emplace_back(std::make_unique<ControlId>(entry->id, "", type));
> > > + ControlId *controlId = controlIds_.back().get();
> > > + (*localIdMap)[entry->id] = controlId;
> >
> > You will be able to write
> >
> > localIdMap[entry->id] = controlId;
> >
> > >
> > > if (entry->offset != values.offset()) {
> > > LOG(Serializer, Error)
> > > @@ -401,15 +406,15 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
> > > }
> > >
> > > /* Create and store the ControlInfo. */
> > > - ctrls.emplace(controlIds_.back().get(),
> > > - loadControlInfo(type, values));
> > > + ctrls.emplace(controlId, loadControlInfo(type, values));
> > > }
> > >
> > > /*
> > > * Create the ControlInfoMap in the cache, and store the map to handle
> > > * association.
> > > */
> > > - ControlInfoMap &map = infoMaps_[hdr->handle] = std::move(ctrls);
> > > + infoMaps_[hdr->handle] = ControlInfoMap(std::move(ctrls), *localIdMap);
> > > + ControlInfoMap &map = infoMaps_[hdr->handle];
> > > infoMapHandles_[&map] = hdr->handle;
> > >
> > > return map;
> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > > index 283472c5ab4d..f891692da669 100644
> > > --- a/src/libcamera/controls.cpp
> > > +++ b/src/libcamera/controls.cpp
> > > @@ -625,10 +625,10 @@ std::string ControlInfo::toString() const
> > > * constructed, and thus only exposes the read accessors of the
> > > * std::unsorted_map<> base class.
> > > *
> > > - * In addition to the features of the standard unsorted map, this class also
> > > - * provides access to the mapped elements using numerical ID keys. It maintains
> > > - * an internal map of numerical ID to ControlId for this purpose, and exposes it
> > > - * through the idmap() method to help construction of ControlList instances.
> > > + * The class needs to be constructed with a reference to the ControlIdMap all
> > > + * the ControlId belongs to. The ControlIdMap is used to provide, in addition to
> >
> > s/belongs/belong/
> >
> > > + * the features of the standard unsorted map, access to the mapped elements
> > > + * using numerical ID keys.
> >
> > I'd like to put a stronger emphasis on the fact that all the entries
> > have to belong to the same map:
> >
> > * The class is constructed with a reference to a ControlIdMap. This allows
> > * providing access to the mapped elements using numerical ID keys, in addition
> > * to the features of the standard unsorted map. All ControlId keys in the map
> > * must appear in the ControlIdMap.
> >
> > (Not sure I'm 100% happy with the result)
>
> I would s/must appear/must belong/ but I got the point
>
> >
> > > */
> > >
> > > /**
> > > @@ -645,24 +645,27 @@ std::string ControlInfo::toString() const
> > > /**
> > > * \brief Construct a ControlInfoMap from an initializer list
> > > * \param[in] init The initializer list
> > > + * \param[in] idmap The idmap used by the ControlInfoMap
> > > */
> > > -ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)
> > > - : Map(init)
> > > +ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init,
> > > + const ControlIdMap &idmap)
> > > + : Map(init), idmap_(&idmap)
> > > {
> > > - generateIdmap();
> > > + validate();
> > > }
> > >
> > > /**
> > > * \brief Construct a ControlInfoMap from a plain map
> > > * \param[in] info The control info plain map
> > > + * \param[in] idmap The idmap used by the ControlInfoMap
> > > *
> > > * Construct a new ControlInfoMap and populate its contents with those of
> > > * \a info using move semantics. Upon return the \a info map will be empty.
> > > */
> > > -ControlInfoMap::ControlInfoMap(Map &&info)
> > > - : Map(std::move(info))
> > > +ControlInfoMap::ControlInfoMap(Map &&info, const ControlIdMap &idmap)
> > > + : Map(std::move(info)), idmap_(&idmap)
> > > {
> > > - generateIdmap();
> > > + validate();
> > > }
> > >
> > > /**
> > > @@ -672,32 +675,14 @@ ControlInfoMap::ControlInfoMap(Map &&info)
> > > * \return A reference to the ControlInfoMap
> > > */
> > >
> > > -/**
> > > - * \brief Replace the contents with those from the initializer list
> > > - * \param[in] init The initializer list
> > > - * \return A reference to the ControlInfoMap
> > > - */
> > > -ControlInfoMap &ControlInfoMap::operator=(std::initializer_list<Map::value_type> init)
> > > +void ControlInfoMap::validate()
> > > {
> > > - Map::operator=(init);
> > > - generateIdmap();
> > > - return *this;
> > > -}
> > > + for (const auto &controlInfo : *this) {
> > > + const ControlId *id = controlInfo.first;
> > > + auto it = idmap_->find(id->id());
> > >
> > > -/**
> > > - * \brief Move assignment operator from a plain map
> > > - * \param[in] info The control info plain map
> > > - *
> > > - * Populate the map by replacing its contents with those of \a info using move
> > > - * semantics. Upon return the \a info map will be empty.
> > > - *
> > > - * \return A reference to the populated ControlInfoMap
> > > - */
> > > -ControlInfoMap &ControlInfoMap::operator=(Map &&info)
> > > -{
> > > - Map::operator=(std::move(info));
> > > - generateIdmap();
> > > - return *this;
> > > + ASSERT(it != idmap_->end());
> > > + }
> >
> > You also need to check that it->second == id, as the numerical IDs could
> > be the same, but the ControlId different.
>
> I'll test and see what happens when the IPA is isolated, as the
> deserializer creates new ControlIds for v4l2 controls so maybe the
> check would fail...
>
> >
> > To compile the code out in non-debug builds, we could move the assert
> > out of the loop:
> >
> > ASSERT(!std::count_if(begin(), end(),
> > [&](const Map::value_type &item) {
> > const ControlId *id = item.first;
> > const auto it = idmap_->find(id->id());
> > return it == idmap_->end() || it->second != id;
> > }));
> >
> > Or maybe
> >
> > ASSERT(validate());
> >
> > in the callers would be cleaner :-)
>
> Yes, that's probably better, I started with that but then changed it
> but it was probably better to have a bool validate() and assert in the
> caller.
>
> >
> > > }
> > >
> > > /**
> > > @@ -707,7 +692,7 @@ ControlInfoMap &ControlInfoMap::operator=(Map &&info)
> > > */
> > > ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
> > > {
> > > - return at(idmap_.at(id));
> > > + return at(idmap_->at(id));
> > > }
> > >
> > > /**
> > > @@ -717,7 +702,7 @@ ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
> > > */
> > > const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
> > > {
> > > - return at(idmap_.at(id));
> > > + return at(idmap_->at(id));
> > > }
> > >
> > > /**
> > > @@ -732,7 +717,7 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
> > > * entries, we can thus just count the matching entries in idmap to
> > > * avoid an additional lookup.
> > > */
> > > - return idmap_.count(id);
> > > + return idmap_->count(id);
> > > }
> > >
> > > /**
> > > @@ -743,8 +728,8 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
> > > */
> > > ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
> > > {
> > > - auto iter = idmap_.find(id);
> > > - if (iter == idmap_.end())
> > > + auto iter = idmap_->find(id);
> > > + if (iter == idmap_->end())
> > > return end();
> > >
> > > return find(iter->second);
> > > @@ -758,8 +743,8 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
> > > */
> > > ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
> > > {
> > > - auto iter = idmap_.find(id);
> > > - if (iter == idmap_.end())
> > > + auto iter = idmap_->find(id);
> > > + if (iter == idmap_->end())
> > > return end();
> > >
> > > return find(iter->second);
> > > @@ -776,33 +761,6 @@ ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
> > > * \return The ControlId map
> > > */
> > >
> > > -void ControlInfoMap::generateIdmap()
> > > -{
> > > - idmap_.clear();
> > > -
> > > - for (const auto &ctrl : *this) {
> > > - /*
> > > - * For string controls, min and max define the valid
> > > - * range for the string size, not for the individual
> > > - * values.
> > > - */
> > > - ControlType rangeType = ctrl.first->type() == ControlTypeString
> > > - ? ControlTypeInteger32 : ctrl.first->type();
> > > - const ControlInfo &info = ctrl.second;
> > > -
> > > - if (info.min().type() != rangeType) {
> > > - LOG(Controls, Error)
> > > - << "Control " << utils::hex(ctrl.first->id())
> > > - << " type and info type mismatch";
> > > - idmap_.clear();
> > > - clear();
> > > - return;
> > > - }
> >
> > Thanks for adding the validate function, but I think you forgot to
> > include this check in there :-)
>
> Do we still need to validate the type in your opinion ? when using
> globally defined control id maps (controls and properties) the control
> info are created by the pipeline handler, when using v4l2 controls the
> control info type is retrieved from the v4l2 controls.. it's hard to
> get it wrong, but checking doesn't hurt I guess...
Good point. If you think it's pointless, we can drop it.
> > > -
> > > - idmap_[ctrl.first->id()] = ctrl.first;
> > > - }
> > > -}
> > > -
> > > /**
> > > * \class ControlList
> > > * \brief Associate a list of ControlId with their values for an object
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 19cb5c4ec9c3..9c23788e5231 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -1071,7 +1071,8 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
> > >
> > > controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);
> > >
> > > - data->controlInfo_ = std::move(controls);
> > > + data->controlInfo_ = ControlInfoMap(std::move(controls),
> > > + controls::controls);
> > >
> > > return 0;
> > > }
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > index 42911a8fdfdb..710b9309448e 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > @@ -935,7 +935,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> > > std::forward_as_tuple(&controls::AeEnable),
> > > std::forward_as_tuple(false, true));
> > >
> > > - data->controlInfo_ = std::move(ctrls);
> > > + data->controlInfo_ = ControlInfoMap(std::move(ctrls),
> > > + controls::controls);
> > >
> > > data->sensor_ = std::make_unique<CameraSensor>(sensor);
> > > ret = data->sensor_->init();
> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > index 0f634b8da609..573d8042c18c 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -535,7 +535,7 @@ int UVCCameraData::init(MediaDevice *media)
> > > addControl(cid, info, &ctrls);
> > > }
> > >
> > > - controlInfo_ = std::move(ctrls);
> > > + controlInfo_ = ControlInfoMap(std::move(ctrls), controls::controls);
> > >
> > > return 0;
> > > }
> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> > > index 12f7517fd0ae..d4b041d33eb4 100644
> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > > @@ -512,7 +512,7 @@ int VimcCameraData::init()
> > > ctrls.emplace(id, info);
> > > }
> > >
> > > - controlInfo_ = std::move(ctrls);
> > > + controlInfo_ = ControlInfoMap(std::move(ctrls), controls::controls);
> > >
> > > /* Initialize the camera properties. */
> > > properties_ = sensor_->properties();
> > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > > index 98d93a12a7be..6ccd8eb86016 100644
> > > --- a/src/libcamera/v4l2_device.cpp
> > > +++ b/src/libcamera/v4l2_device.cpp
> > > @@ -611,12 +611,13 @@ void V4L2Device::listControls()
> > > << " (" << utils::hex(ctrl.id) << ")";
> > >
> > > controlIds_.emplace_back(v4l2ControlId(ctrl));
> > > + controlIdMap_[ctrl.id] = controlIds_.back().get();
> > > controlInfo_.emplace(ctrl.id, ctrl);
> > >
> > > ctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl));
> > > }
> > >
> > > - controls_ = std::move(ctrls);
> > > + controls_ = ControlInfoMap(std::move(ctrls), controlIdMap_);
> > > }
> > >
> > > /**
> > > diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp
> > > index 880bcd02c6be..bf7e34e28af3 100644
> > > --- a/test/serialization/ipa_data_serializer_test.cpp
> > > +++ b/test/serialization/ipa_data_serializer_test.cpp
> > > @@ -32,13 +32,13 @@
> > > using namespace std;
> > > using namespace libcamera;
> > >
> > > -static const ControlInfoMap Controls = {
> > > - { &controls::AeEnable, ControlInfo(false, true) },
> > > - { &controls::ExposureTime, ControlInfo(0, 999999) },
> > > - { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > > - { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > > - { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > > -};
> > > +static const ControlInfoMap Controls = ControlInfoMap({
> > > + { &controls::AeEnable, ControlInfo(false, true) },
> > > + { &controls::ExposureTime, ControlInfo(0, 999999) },
> > > + { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > > + { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > > + { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > > + }, controls::controls);
> > >
> > > namespace libcamera {
> > >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list