[libcamera-devel] [PATCH 6/8] libcamera: store stream pointers in sets instead of a vectors
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Feb 27 17:45:04 CET 2019
Hi Niklas,
Thank you for the patch.
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.
s/grantees/guarantees/
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
With the changes needed to rebase on top of v2 of 5/8,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> 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,
Laurent Pinchart
More information about the libcamera-devel
mailing list