[libcamera-devel] [PATCH 09/10] libcamera: controls: Merge ControlInfoMap and V4L2ControlInfoMap
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Oct 15 16:39:06 CEST 2019
Hi Laurent,
On 2019-10-15 16:54:55 +0300, Laurent Pinchart wrote:
> Hi Niklas,
>
> On Tue, Oct 15, 2019 at 02:40:49AM +0200, Niklas Söderlund wrote:
> > On 2019-10-14 02:27:55 +0300, Laurent Pinchart wrote:
> > > The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with
> > > the latter adding convenience accessors based on numerical IDs for the
> > > former, as well as a cached idmap. Both features can be useful for
> > > ControlInfoMap in the context of serialisation, and merging the two
> > > classes will further simplify the IPA API.
> > >
> > > Import all the features of V4L2ControlInfoMap into ControlInfoMap,
> > > turning the latter into a real class. A few new constructors and
> > > assignment operators are added for completeness.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > The change to pipeline handler implementations to shorten the line
> > length could be done in a separate patch ;-) With out without that
> > addressed,
>
> They're not there to shorten the line length, they actually change the
> behaviour.
>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >
> > > ---
> > > include/ipa/ipa_interface.h | 2 +-
> > > include/libcamera/controls.h | 45 +++++++-
> > > src/ipa/ipa_vimc.cpp | 2 +-
> > > src/ipa/rkisp1/rkisp1.cpp | 6 +-
> > > src/libcamera/camera_sensor.cpp | 2 +-
> > > src/libcamera/controls.cpp | 139 ++++++++++++++++++++++-
> > > src/libcamera/include/camera_sensor.h | 4 +-
> > > src/libcamera/include/v4l2_controls.h | 36 +-----
> > > src/libcamera/include/v4l2_device.h | 4 +-
> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 11 +-
> > > src/libcamera/pipeline/uvcvideo.cpp | 12 +-
> > > src/libcamera/pipeline/vimc.cpp | 11 +-
> > > src/libcamera/proxy/ipa_proxy_linux.cpp | 2 +-
> > > src/libcamera/v4l2_controls.cpp | 94 +--------------
> > > src/libcamera/v4l2_device.cpp | 2 +-
> > > test/v4l2_videodevice/controls.cpp | 2 +-
> > > 16 files changed, 220 insertions(+), 154 deletions(-)
>
> [snip]
>
> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > > index 5534a2edb567..80414c6f0748 100644
> > > --- a/include/libcamera/controls.h
> > > +++ b/include/libcamera/controls.h
> > > @@ -118,7 +118,50 @@ private:
> > > };
> > >
> > > using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;
> > > -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;
> > > +
> > > +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>
> > > +{
> > > +public:
> > > + using Map = std::unordered_map<const ControlId *, ControlRange>;
> > > +
> > > + ControlInfoMap() = default;
> > > + ControlInfoMap(const ControlInfoMap &other) = default;
> > > + ControlInfoMap(std::initializer_list<Map::value_type> init);
> > > +
> > > + 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;
> > > + using Map::value_type;
> > > + using Map::size_type;
> > > + using Map::iterator;
> > > + using Map::const_iterator;
> > > +
> > > + using Map::begin;
> > > + using Map::cbegin;
> > > + using Map::end;
> > > + using Map::cend;
> > > + using Map::at;
> > > + using Map::empty;
> > > + using Map::size;
> > > + using Map::count;
> > > + using Map::find;
> > > +
> > > + mapped_type &at(unsigned int key);
> > > + const mapped_type &at(unsigned int key) const;
> > > + size_type count(unsigned int key) const;
> > > + iterator find(unsigned int key);
> > > + const_iterator find(unsigned int key) const;
> > > +
> > > + const ControlIdMap &idmap() const { return idmap_; }
> > > +
> > > +private:
> > > + void generateIdmap();
> > > +
> > > + ControlIdMap idmap_;
> > > +};
>
> [snip]
>
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > index 9b19bde8a274..7a28b03b8d38 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>
> [snip]
>
> > > @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> > > std::unique_ptr<RkISP1CameraData> data =
> > > utils::make_unique<RkISP1CameraData>(this);
> > >
> > > - data->controlInfo_.emplace(std::piecewise_construct,
> > > - std::forward_as_tuple(&controls::AeEnable),
> > > - std::forward_as_tuple(false, true));
> > > + ControlInfoMap::Map ctrls;
> > > + ctrls.emplace(std::piecewise_construct,
> > > + std::forward_as_tuple(&controls::AeEnable),
> > > + std::forward_as_tuple(false, true));
> > > +
> > > + data->controlInfo_ = std::move(ctrls);
>
> ctrls is a ControlInfoMap::Map, and data->controlInfo_ is a
> ControlInfoMap. The latter is constructed from the former using the
> ControlInfoMap::operator=(Map &&info) assignment operator. This is
> needed as ControlInfoMap doesn't provide accessors that allow modifying
> its contents.
My bad I missed the ::Map part, thanks for clarifying.
>
> > >
> > > data->sensor_ = new CameraSensor(sensor);
> > > ret = data->sensor_->init();
>
> --
> Regards,
>
> Laurent Pinchart
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list