[libcamera-devel] [PATCH 6/8] libcamera: store stream pointers in sets instead of a vectors
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Feb 26 20:06:09 CET 2019
Hi Jacopo,
Thanks for your feedback.
On 2019-02-26 18:18:52 +0100, Jacopo Mondi wrote:
> Hi Niklas,
>
> On Tue, Feb 26, 2019 at 03:18:55AM +0100, Niklas Söderlund wrote:
> > The arrays that store Stream pointers shall always contain unique
> > values. Storing them in vectors opens up for the same stream pointer
> > appearing twice. Remove this possibility by storing them in a set which
> > grantees each element is unique.
> >
>
> Have you considered unordered_set of the ordering of the strems is
> somehow relevant?
I did consider it and judge that moving to is probably what we want.
However we have not discussed if the order we add Streams to a array
matters or not so I went for std::set instead as it behaves similar to a
std::vector in this regard while enforcing the problem I wish to solve,
that a Stream pointer shall not be present more then once in the array.
If/Once we judge that injection order is not important it's trivial to
change this to std::unorderd_set. The only drawback of std::set is that
it is a tad slower.
>
> Thanks
> j
>
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > include/libcamera/camera.h | 11 ++++++-----
> > src/cam/main.cpp | 10 +++++-----
> > src/libcamera/camera.cpp | 10 +++++-----
> > src/libcamera/include/pipeline_handler.h | 2 +-
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---
> > src/libcamera/pipeline/uvcvideo.cpp | 6 +++---
> > src/libcamera/pipeline/vimc.cpp | 6 +++---
> > 7 files changed, 26 insertions(+), 25 deletions(-)
> >
> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > index 8c8545b074e8ae13..1e75a6dafa4e7c5f 100644
> > --- a/include/libcamera/camera.h
> > +++ b/include/libcamera/camera.h
> > @@ -9,6 +9,7 @@
> >
> > #include <map>
> > #include <memory>
> > +#include <set>
> > #include <string>
> >
> > #include <libcamera/request.h>
> > @@ -27,7 +28,7 @@ class Camera final
> > public:
> > static std::shared_ptr<Camera> create(PipelineHandler *pipe,
> > const std::string &name,
> > - const std::vector<Stream *> &streams);
> > + const std::set<Stream *> &streams);
> >
> > Camera(const Camera &) = delete;
> > Camera &operator=(const Camera &) = delete;
> > @@ -40,9 +41,9 @@ public:
> > int acquire();
> > void release();
> >
> > - const std::vector<Stream *> &streams() const;
> > + const std::set<Stream *> &streams() const;
> > std::map<Stream *, StreamConfiguration>
> > - streamConfiguration(std::vector<Stream *> &streams);
> > + streamConfiguration(std::set<Stream *> &streams);
> > int configureStreams(std::map<Stream *, StreamConfiguration> &config);
> >
> > int allocateBuffers();
> > @@ -73,8 +74,8 @@ private:
> >
> > std::shared_ptr<PipelineHandler> pipe_;
> > std::string name_;
> > - std::vector<Stream *> streams_;
> > - std::vector<Stream *> activeStreams_;
> > + std::set<Stream *> streams_;
> > + std::set<Stream *> activeStreams_;
> >
> > State state_;
> > };
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 8df8844c33a2d052..13aa35968f38964d 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -75,10 +75,10 @@ static int parseOptions(int argc, char *argv[])
> > return 0;
> > }
> >
> > -static int configureStreams(Camera *camera, std::vector<Stream *> &streams)
> > +static int configureStreams(Camera *camera, std::set<Stream *> &streams)
> > {
> > KeyValueParser::Options format = options[OptFormat];
> > - Stream *id = streams.front();
> > + Stream *id = *streams.begin();
> >
> > std::map<Stream *, StreamConfiguration> config =
> > camera->streamConfiguration(streams);
> > @@ -132,7 +132,7 @@ static int capture()
> > {
> > int ret;
> >
> > - std::vector<Stream *> streams = camera->streams();
> > + std::set<Stream *> streams = camera->streams();
> > std::vector<Request *> requests;
> >
> > ret = configureStreams(camera.get(), streams);
> > @@ -141,7 +141,7 @@ static int capture()
> > return ret;
> > }
> >
> > - Stream *stream = streams.front();
> > + Stream *stream = *streams.begin();
> >
> > ret = camera->allocateBuffers();
> > if (ret) {
> > @@ -237,7 +237,7 @@ int main(int argc, char **argv)
> > goto out;
> > }
> >
> > - const std::vector<Stream *> &streams = camera->streams();
> > + const std::set<Stream *> &streams = camera->streams();
> > if (streams.size() != 1) {
> > std::cout << "Camera has " << streams.size()
> > << " streams, only 1 is supported"
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index c50b14bbd904fc1c..938f21fe80ef8ceb 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -66,7 +66,7 @@ LOG_DECLARE_CATEGORY(Camera)
> > */
> > std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> > const std::string &name,
> > - const std::vector<Stream *> &streams)
> > + const std::set<Stream *> &streams)
> > {
> > struct Allocator : std::allocator<Camera> {
> > void construct(void *p, PipelineHandler *pipe,
> > @@ -223,10 +223,10 @@ void Camera::release()
> > *
> > * \return An array of all the camera's streams.
> > */
> > -const std::vector<Stream *> &Camera::streams() const
> > +const std::set<Stream *> &Camera::streams() const
> > {
> > if (!stateIsAtleast(Free))
> > - std::vector<Stream *>{};
> > + std::set<Stream *>{};
> >
> > return streams_;
> > }
> > @@ -248,7 +248,7 @@ const std::vector<Stream *> &Camera::streams() const
> > * empty list on error.
> > */
> > std::map<Stream *, StreamConfiguration>
> > -Camera::streamConfiguration(std::vector<Stream *> &streams)
> > +Camera::streamConfiguration(std::set<Stream *> &streams)
> > {
> > if (!stateIsAtleast(Free) || !streams.size())
> > return std::map<Stream *, StreamConfiguration>{};
> > @@ -300,7 +300,7 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
> > const StreamConfiguration &cfg = iter.second;
> >
> > stream->configuration_ = cfg;
> > - activeStreams_.push_back(stream);
> > + activeStreams_.insert(stream);
> >
> > /*
> > * Allocate buffer objects in the pool.
> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> > index 4363dcd8ed8ead97..4cd9b90c609c067e 100644
> > --- a/src/libcamera/include/pipeline_handler.h
> > +++ b/src/libcamera/include/pipeline_handler.h
> > @@ -45,7 +45,7 @@ public:
> > virtual bool match(DeviceEnumerator *enumerator) = 0;
> >
> > virtual std::map<Stream *, StreamConfiguration>
> > - streamConfiguration(Camera *camera, std::vector<Stream *> &streams) = 0;
> > + streamConfiguration(Camera *camera, std::set<Stream *> &streams) = 0;
> > virtual int configureStreams(Camera *camera,
> > std::map<Stream *, StreamConfiguration> &config) = 0;
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 9694d0ce51abda41..1066fbe350d934a0 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -32,7 +32,7 @@ public:
> >
> > std::map<Stream *, StreamConfiguration>
> > streamConfiguration(Camera *camera,
> > - std::vector<Stream *> &streams) override;
> > + std::set<Stream *> &streams) override;
> > int configureStreams(Camera *camera,
> > std::map<Stream *, StreamConfiguration> &config) override;
> >
> > @@ -95,7 +95,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()
> >
> > std::map<Stream *, StreamConfiguration>
> > PipelineHandlerIPU3::streamConfiguration(Camera *camera,
> > - std::vector<Stream *> &streams)
> > + std::set<Stream *> &streams)
> > {
> > IPU3CameraData *data = cameraData(camera);
> > std::map<Stream *, StreamConfiguration> configs;
> > @@ -374,7 +374,7 @@ void PipelineHandlerIPU3::registerCameras()
> > std::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>();
> >
> > std::string cameraName = sensor->name() + " " + std::to_string(id);
> > - std::vector<Stream *> streams{ &data->stream_ };
> > + std::set<Stream *> streams{ &data->stream_ };
> > std::shared_ptr<Camera> camera = Camera::create(this, cameraName, streams);
> >
> > /*
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index b7b8ff109109e9c5..7b697c0685630d92 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -27,7 +27,7 @@ public:
> >
> > std::map<Stream *, StreamConfiguration>
> > streamConfiguration(Camera *camera,
> > - std::vector<Stream *> &streams) override;
> > + std::set<Stream *> &streams) override;
> > int configureStreams(Camera *camera,
> > std::map<Stream *, StreamConfiguration> &config) override;
> >
> > @@ -63,7 +63,7 @@ PipelineHandlerUVC::~PipelineHandlerUVC()
> >
> > std::map<Stream *, StreamConfiguration>
> > PipelineHandlerUVC::streamConfiguration(Camera *camera,
> > - std::vector<Stream *> &streams)
> > + std::set<Stream *> &streams)
> > {
> > std::map<Stream *, StreamConfiguration> configs;
> >
> > @@ -171,7 +171,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> > return false;
> > }
> >
> > - std::vector<Stream *> streams{ &stream_ };
> > + std::set<Stream *> streams{ &stream_ };
> > std::shared_ptr<Camera> camera = Camera::create(this, media_->model(), streams);
> > registerCamera(std::move(camera));
> > hotplugMediaDevice(media_.get());
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index 812777cffbb3e618..9ba96323d335e010 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -27,7 +27,7 @@ public:
> >
> > std::map<Stream *, StreamConfiguration>
> > streamConfiguration(Camera *camera,
> > - std::vector<Stream *> &streams) override;
> > + std::set<Stream *> &streams) override;
> > int configureStreams(Camera *camera,
> > std::map<Stream *, StreamConfiguration> &config) override;
> >
> > @@ -62,7 +62,7 @@ PipelineHandlerVimc::~PipelineHandlerVimc()
> >
> > std::map<Stream *, StreamConfiguration>
> > PipelineHandlerVimc::streamConfiguration(Camera *camera,
> > - std::vector<Stream *> &streams)
> > + std::set<Stream *> &streams)
> > {
> > std::map<Stream *, StreamConfiguration> configs;
> >
> > @@ -171,7 +171,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
> > return false;
> > }
> >
> > - std::vector<Stream *> streams{ &stream_ };
> > + std::set<Stream *> streams{ &stream_ };
> > std::shared_ptr<Camera> camera = Camera::create(this, "VIMC Sensor B",
> > streams);
> > registerCamera(std::move(camera));
> > --
> > 2.20.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list