[{"id":891,"web_url":"https://patchwork.libcamera.org/comment/891/","msgid":"<20190226171852.p5bm5jyg5aa4o7lg@uno.localdomain>","date":"2019-02-26T17:18:52","subject":"Re: [libcamera-devel] [PATCH 6/8] libcamera: store stream pointers\n\tin sets instead of a vectors","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Feb 26, 2019 at 03:18:55AM +0100, Niklas Söderlund wrote:\n> The arrays that store Stream pointers shall always contain unique\n> values. Storing them in vectors opens up for the same stream pointer\n> appearing twice. Remove this possibility by storing them in a set which\n> grantees each element is unique.\n>\n\nHave you considered unordered_set of the ordering of the strems is\nsomehow relevant?\n\nThanks\n j\n\n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/camera.h | 11 ++++++-----\n> src/cam/main.cpp | 10 +++++-----\n> src/libcamera/camera.cpp | 10 +++++-----\n> src/libcamera/include/pipeline_handler.h | 2 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---\n> src/libcamera/pipeline/uvcvideo.cpp | 6 +++---\n> src/libcamera/pipeline/vimc.cpp | 6 +++---\n> 7 files changed, 26 insertions(+), 25 deletions(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 8c8545b074e8ae13..1e75a6dafa4e7c5f 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -9,6 +9,7 @@\n>\n> #include \n> #include \n> +#include \n> #include \n>\n> #include \n> @@ -27,7 +28,7 @@ class Camera final\n> public:\n> \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> \t\t\t\t\t const std::string &name,\n> -\t\t\t\t\t const std::vector &streams);\n> +\t\t\t\t\t const std::set &streams);\n>\n> \tCamera(const Camera &) = delete;\n> \tCamera &operator=(const Camera &) = delete;\n> @@ -40,9 +41,9 @@ public:\n> \tint acquire();\n> \tvoid release();\n>\n> -\tconst std::vector &streams() const;\n> +\tconst std::set &streams() const;\n> \tstd::map\n> -\tstreamConfiguration(std::vector &streams);\n> +\tstreamConfiguration(std::set &streams);\n> \tint configureStreams(std::map &config);\n>\n> \tint allocateBuffers();\n> @@ -73,8 +74,8 @@ private:\n>\n> \tstd::shared_ptr pipe_;\n> \tstd::string name_;\n> -\tstd::vector streams_;\n> -\tstd::vector activeStreams_;\n> +\tstd::set streams_;\n> +\tstd::set activeStreams_;\n>\n> \tState state_;\n> };\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 8df8844c33a2d052..13aa35968f38964d 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -75,10 +75,10 @@ static int parseOptions(int argc, char *argv[])\n> \treturn 0;\n> }\n>\n> -static int configureStreams(Camera *camera, std::vector &streams)\n> +static int configureStreams(Camera *camera, std::set &streams)\n> {\n> \tKeyValueParser::Options format = options[OptFormat];\n> -\tStream *id = streams.front();\n> +\tStream *id = *streams.begin();\n>\n> \tstd::map config =\n> \t\tcamera->streamConfiguration(streams);\n> @@ -132,7 +132,7 @@ static int capture()\n> {\n> \tint ret;\n>\n> -\tstd::vector streams = camera->streams();\n> +\tstd::set streams = camera->streams();\n> \tstd::vector requests;\n>\n> \tret = configureStreams(camera.get(), streams);\n> @@ -141,7 +141,7 @@ static int capture()\n> \t\treturn ret;\n> \t}\n>\n> -\tStream *stream = streams.front();\n> +\tStream *stream = *streams.begin();\n>\n> \tret = camera->allocateBuffers();\n> \tif (ret) {\n> @@ -237,7 +237,7 @@ int main(int argc, char **argv)\n> \t\t\tgoto out;\n> \t\t}\n>\n> -\t\tconst std::vector &streams = camera->streams();\n> +\t\tconst std::set &streams = camera->streams();\n> \t\tif (streams.size() != 1) {\n> \t\t\tstd::cout << \"Camera has \" << streams.size()\n> \t\t\t\t << \" streams, only 1 is supported\"\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index c50b14bbd904fc1c..938f21fe80ef8ceb 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -66,7 +66,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> */\n> std::shared_ptr Camera::create(PipelineHandler *pipe,\n> \t\t\t\t const std::string &name,\n> -\t\t\t\t const std::vector &streams)\n> +\t\t\t\t const std::set &streams)\n> {\n> \tstruct Allocator : std::allocator {\n> \t\tvoid construct(void *p, PipelineHandler *pipe,\n> @@ -223,10 +223,10 @@ void Camera::release()\n> *\n> * \\return An array of all the camera's streams.\n> */\n> -const std::vector &Camera::streams() const\n> +const std::set &Camera::streams() const\n> {\n> \tif (!stateIsAtleast(Free))\n> -\t\tstd::vector{};\n> +\t\tstd::set{};\n>\n> \treturn streams_;\n> }\n> @@ -248,7 +248,7 @@ const std::vector &Camera::streams() const\n> * empty list on error.\n> */\n> std::map\n> -Camera::streamConfiguration(std::vector &streams)\n> +Camera::streamConfiguration(std::set &streams)\n> {\n> \tif (!stateIsAtleast(Free) || !streams.size())\n> \t\treturn std::map{};\n> @@ -300,7 +300,7 @@ int Camera::configureStreams(std::map &config)\n> \t\tconst StreamConfiguration &cfg = iter.second;\n>\n> \t\tstream->configuration_ = cfg;\n> -\t\tactiveStreams_.push_back(stream);\n> +\t\tactiveStreams_.insert(stream);\n>\n> \t\t/*\n> \t\t * Allocate buffer objects in the pool.\n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 4363dcd8ed8ead97..4cd9b90c609c067e 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -45,7 +45,7 @@ public:\n> \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n>\n> \tvirtual std::map\n> -\tstreamConfiguration(Camera *camera, std::vector &streams) = 0;\n> +\tstreamConfiguration(Camera *camera, std::set &streams) = 0;\n> \tvirtual int configureStreams(Camera *camera,\n> \t\t\t\t std::map &config) = 0;\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 9694d0ce51abda41..1066fbe350d934a0 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -32,7 +32,7 @@ public:\n>\n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::vector &streams) override;\n> +\t\t\t std::set &streams) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n>\n> @@ -95,7 +95,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()\n>\n> std::map\n> PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n> -\t\t\t\t\t std::vector &streams)\n> +\t\t\t\t\t std::set &streams)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> \tstd::map configs;\n> @@ -374,7 +374,7 @@ void PipelineHandlerIPU3::registerCameras()\n> \t\tstd::unique_ptr data = utils::make_unique();\n>\n> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> -\t\tstd::vector streams{ &data->stream_ };\n> +\t\tstd::set streams{ &data->stream_ };\n> \t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n>\n> \t\t/*\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index b7b8ff109109e9c5..7b697c0685630d92 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -27,7 +27,7 @@ public:\n>\n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::vector &streams) override;\n> +\t\t\t std::set &streams) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n>\n> @@ -63,7 +63,7 @@ PipelineHandlerUVC::~PipelineHandlerUVC()\n>\n> std::map\n> PipelineHandlerUVC::streamConfiguration(Camera *camera,\n> -\t\t\t\t\tstd::vector &streams)\n> +\t\t\t\t\tstd::set &streams)\n> {\n> \tstd::map configs;\n>\n> @@ -171,7 +171,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \t}\n>\n> -\tstd::vector streams{ &stream_ };\n> +\tstd::set streams{ &stream_ };\n> \tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> \tregisterCamera(std::move(camera));\n> \thotplugMediaDevice(media_.get());\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 812777cffbb3e618..9ba96323d335e010 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -27,7 +27,7 @@ public:\n>\n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::vector &streams) override;\n> +\t\t\t std::set &streams) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n>\n> @@ -62,7 +62,7 @@ PipelineHandlerVimc::~PipelineHandlerVimc()\n>\n> std::map\n> PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> -\t\t\t\t std::vector &streams)\n> +\t\t\t\t\t std::set &streams)\n> {\n> \tstd::map configs;\n>\n> @@ -171,7 +171,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \t}\n>\n> -\tstd::vector streams{ &stream_ };\n> +\tstd::set streams{ &stream_ };\n> \tstd::shared_ptr camera = Camera::create(this, \"VIMC Sensor B\",\n> \t\t\t\t\t\t\tstreams);\n> \tregisterCamera(std::move(camera));\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B3714610B2\n\tfor ;\n\tTue, 26 Feb 2019 18:18:23 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 2DB90E0009;\n\tTue, 26 Feb 2019 17:18:22 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 26 Feb 2019 18:18:52 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190226171852.p5bm5jyg5aa4o7lg@uno.localdomain>","References":"<20190226021857.28255-1-niklas.soderlund@ragnatech.se>\n\t<20190226021857.28255-7-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"ql3umohe6zarpxdu\"","Content-Disposition":"inline","In-Reply-To":"<20190226021857.28255-7-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 6/8] libcamera: store stream pointers\n\tin sets instead of a vectors","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 26 Feb 2019 17:18:23 -0000"}},{"id":896,"web_url":"https://patchwork.libcamera.org/comment/896/","msgid":"<20190226190609.GM899@bigcity.dyn.berto.se>","date":"2019-02-26T19:06:09","subject":"Re: [libcamera-devel] [PATCH 6/8] libcamera: store stream pointers\n\tin sets instead of a vectors","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your feedback.\n\nOn 2019-02-26 18:18:52 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Tue, Feb 26, 2019 at 03:18:55AM +0100, Niklas Söderlund wrote:\n> > The arrays that store Stream pointers shall always contain unique\n> > values. Storing them in vectors opens up for the same stream pointer\n> > appearing twice. Remove this possibility by storing them in a set which\n> > grantees each element is unique.\n> >\n> \n> Have you considered unordered_set of the ordering of the strems is\n> somehow relevant?\n\nI did consider it and judge that moving to is probably what we want. \nHowever we have not discussed if the order we add Streams to a array \nmatters or not so I went for std::set instead as it behaves similar to a \nstd::vector in this regard while enforcing the problem I wish to solve, \nthat a Stream pointer shall not be present more then once in the array.\n\nIf/Once we judge that injection order is not important it's trivial to \nchange this to std::unorderd_set. The only drawback of std::set is that \nit is a tad slower.\n\n> \n> Thanks\n> j\n> \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/camera.h | 11 ++++++-----\n> > src/cam/main.cpp | 10 +++++-----\n> > src/libcamera/camera.cpp | 10 +++++-----\n> > src/libcamera/include/pipeline_handler.h | 2 +-\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---\n> > src/libcamera/pipeline/uvcvideo.cpp | 6 +++---\n> > src/libcamera/pipeline/vimc.cpp | 6 +++---\n> > 7 files changed, 26 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 8c8545b074e8ae13..1e75a6dafa4e7c5f 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -9,6 +9,7 @@\n> >\n> > #include \n> > #include \n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -27,7 +28,7 @@ class Camera final\n> > public:\n> > \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > \t\t\t\t\t const std::string &name,\n> > -\t\t\t\t\t const std::vector &streams);\n> > +\t\t\t\t\t const std::set &streams);\n> >\n> > \tCamera(const Camera &) = delete;\n> > \tCamera &operator=(const Camera &) = delete;\n> > @@ -40,9 +41,9 @@ public:\n> > \tint acquire();\n> > \tvoid release();\n> >\n> > -\tconst std::vector &streams() const;\n> > +\tconst std::set &streams() const;\n> > \tstd::map\n> > -\tstreamConfiguration(std::vector &streams);\n> > +\tstreamConfiguration(std::set &streams);\n> > \tint configureStreams(std::map &config);\n> >\n> > \tint allocateBuffers();\n> > @@ -73,8 +74,8 @@ private:\n> >\n> > \tstd::shared_ptr pipe_;\n> > \tstd::string name_;\n> > -\tstd::vector streams_;\n> > -\tstd::vector activeStreams_;\n> > +\tstd::set streams_;\n> > +\tstd::set activeStreams_;\n> >\n> > \tState state_;\n> > };\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 8df8844c33a2d052..13aa35968f38964d 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -75,10 +75,10 @@ static int parseOptions(int argc, char *argv[])\n> > \treturn 0;\n> > }\n> >\n> > -static int configureStreams(Camera *camera, std::vector &streams)\n> > +static int configureStreams(Camera *camera, std::set &streams)\n> > {\n> > \tKeyValueParser::Options format = options[OptFormat];\n> > -\tStream *id = streams.front();\n> > +\tStream *id = *streams.begin();\n> >\n> > \tstd::map config =\n> > \t\tcamera->streamConfiguration(streams);\n> > @@ -132,7 +132,7 @@ static int capture()\n> > {\n> > \tint ret;\n> >\n> > -\tstd::vector streams = camera->streams();\n> > +\tstd::set streams = camera->streams();\n> > \tstd::vector requests;\n> >\n> > \tret = configureStreams(camera.get(), streams);\n> > @@ -141,7 +141,7 @@ static int capture()\n> > \t\treturn ret;\n> > \t}\n> >\n> > -\tStream *stream = streams.front();\n> > +\tStream *stream = *streams.begin();\n> >\n> > \tret = camera->allocateBuffers();\n> > \tif (ret) {\n> > @@ -237,7 +237,7 @@ int main(int argc, char **argv)\n> > \t\t\tgoto out;\n> > \t\t}\n> >\n> > -\t\tconst std::vector &streams = camera->streams();\n> > +\t\tconst std::set &streams = camera->streams();\n> > \t\tif (streams.size() != 1) {\n> > \t\t\tstd::cout << \"Camera has \" << streams.size()\n> > \t\t\t\t << \" streams, only 1 is supported\"\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index c50b14bbd904fc1c..938f21fe80ef8ceb 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -66,7 +66,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> > */\n> > std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > \t\t\t\t const std::string &name,\n> > -\t\t\t\t const std::vector &streams)\n> > +\t\t\t\t const std::set &streams)\n> > {\n> > \tstruct Allocator : std::allocator {\n> > \t\tvoid construct(void *p, PipelineHandler *pipe,\n> > @@ -223,10 +223,10 @@ void Camera::release()\n> > *\n> > * \\return An array of all the camera's streams.\n> > */\n> > -const std::vector &Camera::streams() const\n> > +const std::set &Camera::streams() const\n> > {\n> > \tif (!stateIsAtleast(Free))\n> > -\t\tstd::vector{};\n> > +\t\tstd::set{};\n> >\n> > \treturn streams_;\n> > }\n> > @@ -248,7 +248,7 @@ const std::vector &Camera::streams() const\n> > * empty list on error.\n> > */\n> > std::map\n> > -Camera::streamConfiguration(std::vector &streams)\n> > +Camera::streamConfiguration(std::set &streams)\n> > {\n> > \tif (!stateIsAtleast(Free) || !streams.size())\n> > \t\treturn std::map{};\n> > @@ -300,7 +300,7 @@ int Camera::configureStreams(std::map &config)\n> > \t\tconst StreamConfiguration &cfg = iter.second;\n> >\n> > \t\tstream->configuration_ = cfg;\n> > -\t\tactiveStreams_.push_back(stream);\n> > +\t\tactiveStreams_.insert(stream);\n> >\n> > \t\t/*\n> > \t\t * Allocate buffer objects in the pool.\n> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> > index 4363dcd8ed8ead97..4cd9b90c609c067e 100644\n> > --- a/src/libcamera/include/pipeline_handler.h\n> > +++ b/src/libcamera/include/pipeline_handler.h\n> > @@ -45,7 +45,7 @@ public:\n> > \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n> >\n> > \tvirtual std::map\n> > -\tstreamConfiguration(Camera *camera, std::vector &streams) = 0;\n> > +\tstreamConfiguration(Camera *camera, std::set &streams) = 0;\n> > \tvirtual int configureStreams(Camera *camera,\n> > \t\t\t\t std::map &config) = 0;\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 9694d0ce51abda41..1066fbe350d934a0 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -32,7 +32,7 @@ public:\n> >\n> > \tstd::map\n> > \tstreamConfiguration(Camera *camera,\n> > -\t\t\t std::vector &streams) override;\n> > +\t\t\t std::set &streams) override;\n> > \tint configureStreams(Camera *camera,\n> > \t\t\t std::map &config) override;\n> >\n> > @@ -95,7 +95,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()\n> >\n> > std::map\n> > PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n> > -\t\t\t\t\t std::vector &streams)\n> > +\t\t\t\t\t std::set &streams)\n> > {\n> > \tIPU3CameraData *data = cameraData(camera);\n> > \tstd::map configs;\n> > @@ -374,7 +374,7 @@ void PipelineHandlerIPU3::registerCameras()\n> > \t\tstd::unique_ptr data = utils::make_unique();\n> >\n> > \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > -\t\tstd::vector streams{ &data->stream_ };\n> > +\t\tstd::set streams{ &data->stream_ };\n> > \t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n> >\n> > \t\t/*\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index b7b8ff109109e9c5..7b697c0685630d92 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -27,7 +27,7 @@ public:\n> >\n> > \tstd::map\n> > \tstreamConfiguration(Camera *camera,\n> > -\t\t\t std::vector &streams) override;\n> > +\t\t\t std::set &streams) override;\n> > \tint configureStreams(Camera *camera,\n> > \t\t\t std::map &config) override;\n> >\n> > @@ -63,7 +63,7 @@ PipelineHandlerUVC::~PipelineHandlerUVC()\n> >\n> > std::map\n> > PipelineHandlerUVC::streamConfiguration(Camera *camera,\n> > -\t\t\t\t\tstd::vector &streams)\n> > +\t\t\t\t\tstd::set &streams)\n> > {\n> > \tstd::map configs;\n> >\n> > @@ -171,7 +171,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> > \t\treturn false;\n> > \t}\n> >\n> > -\tstd::vector streams{ &stream_ };\n> > +\tstd::set streams{ &stream_ };\n> > \tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> > \tregisterCamera(std::move(camera));\n> > \thotplugMediaDevice(media_.get());\n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index 812777cffbb3e618..9ba96323d335e010 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -27,7 +27,7 @@ public:\n> >\n> > \tstd::map\n> > \tstreamConfiguration(Camera *camera,\n> > -\t\t\t std::vector &streams) override;\n> > +\t\t\t std::set &streams) override;\n> > \tint configureStreams(Camera *camera,\n> > \t\t\t std::map &config) override;\n> >\n> > @@ -62,7 +62,7 @@ PipelineHandlerVimc::~PipelineHandlerVimc()\n> >\n> > std::map\n> > PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> > -\t\t\t\t std::vector &streams)\n> > +\t\t\t\t\t std::set &streams)\n> > {\n> > \tstd::map configs;\n> >\n> > @@ -171,7 +171,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> > \t\treturn false;\n> > \t}\n> >\n> > -\tstd::vector streams{ &stream_ };\n> > +\tstd::set streams{ &stream_ };\n> > \tstd::shared_ptr camera = Camera::create(this, \"VIMC Sensor B\",\n> > \t\t\t\t\t\t\tstreams);\n> > \tregisterCamera(std::move(camera));\n> > --\n> > 2.20.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F892610B2\n\tfor ;\n\tTue, 26 Feb 2019 20:06:11 +0100 (CET)","by mail-lj1-x235.google.com with SMTP id j19so11761459ljg.5\n\tfor ;\n\tTue, 26 Feb 2019 11:06:11 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\th5sm3313121lfm.29.2019.02.26.11.06.09\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 26 Feb 2019 11:06:09 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=7wkIG39pbdKfSsDQaeGz1JXgoIyFAURcwxKTLdMYEU0=;\n\tb=X0M81kwILhrhLO6oxqmAGVLcxPBKk7mmY/64ocL3pG2V2uMD+hrUb/SwzcUm6M4Gbj\n\tWgvRcFKZTo0mS5Fmvv7/bovRSVSJXrEiustdBQx65RLDuNeiwVJdSILK2uH5OPnkZ7n7\n\tJ9wJwdfKIYA9ZHSy9ztER6oNevDm/v1vIM+wBveU6yG4f/WY4z30AiXZdmFjnttOBbpk\n\taIxj+8XN013IDZS/rYqs4feIaku+RJEL9snC2xnxNJsyB/6O/bhZsLi1k8gDZuTYsRD+\n\tgVicwKwzOGmgyqzAWRPQP8gXOLl2FMwgi+yDEXlF50OstfcAYTRWjs83NGAv3wW92dDB\n\tWCMQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=7wkIG39pbdKfSsDQaeGz1JXgoIyFAURcwxKTLdMYEU0=;\n\tb=oLIpcyA0iZfkSSgxmh6pKtZ7fsazgWYjo4oATCdg/eG3lBhjQbW3slzbr1sKL3rV31\n\thpl+Cyxn+zr7pOwTDuCKCr/jKfgWRy78eqWfx1kP+cJ9VrP+LtlVPU1LMSA9g0c3odCH\n\tUb+Udboiz8+mi5+5Q3fH+JDuwNvLO69FQNVyaJtrp4lWr46UpD/TonFpt4K5J4rGNUEU\n\tnIptEX2OF+/9v1da6pHd6HQSaT7ayGrH1MH04DBnxP64TsvAoyO32vVKG3fwP7eoyf6V\n\tXSVi32NEsFdfBiXSy9LI4qzw0F454zl4QdEeX3I4waplJ883tuJnPommrJaKlpNAYsQT\n\tke4Q==","X-Gm-Message-State":"AHQUAuZpMB6FoLGI8EJJbmADo8b14xJuRLJifXxf3mzyNjEL99vu3IFa\n\tSwo59l0I0QtshNdh3c6bD8wVjw==","X-Google-Smtp-Source":"AHgI3Ia00Gfo32LYfHnq2aSBUcO4L136S3xKMGf0N+z0lFrJvURnRIvIpFGMrWvfl5SufIgWgWwebg==","X-Received":"by 2002:a2e:7a08:: with SMTP id\n\tv8mr14288078ljc.161.1551207970194; \n\tTue, 26 Feb 2019 11:06:10 -0800 (PST)","Date":"Tue, 26 Feb 2019 20:06:09 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190226190609.GM899@bigcity.dyn.berto.se>","References":"<20190226021857.28255-1-niklas.soderlund@ragnatech.se>\n\t<20190226021857.28255-7-niklas.soderlund@ragnatech.se>\n\t<20190226171852.p5bm5jyg5aa4o7lg@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190226171852.p5bm5jyg5aa4o7lg@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 6/8] libcamera: store stream pointers\n\tin sets instead of a vectors","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 26 Feb 2019 19:06:11 -0000"}},{"id":914,"web_url":"https://patchwork.libcamera.org/comment/914/","msgid":"<20190227095047.2ga64hfimpg6mcx3@uno.localdomain>","date":"2019-02-27T09:50:47","subject":"Re: [libcamera-devel] [PATCH 6/8] libcamera: store stream pointers\n\tin sets instead of a vectors","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Feb 26, 2019 at 08:06:09PM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your feedback.\n>\n> On 2019-02-26 18:18:52 +0100, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >\n> > On Tue, Feb 26, 2019 at 03:18:55AM +0100, Niklas Söderlund wrote:\n> > > The arrays that store Stream pointers shall always contain unique\n> > > values. Storing them in vectors opens up for the same stream pointer\n> > > appearing twice. Remove this possibility by storing them in a set which\n> > > grantees each element is unique.\n> > >\n> >\n> > Have you considered unordered_set of the ordering of the strems is\n> > somehow relevant?\n>\n> I did consider it and judge that moving to is probably what we want.\n> However we have not discussed if the order we add Streams to a array\n> matters or not so I went for std::set instead as it behaves similar to a\n> std::vector in this regard while enforcing the problem I wish to solve,\n> that a Stream pointer shall not be present more then once in the array.\n>\n> If/Once we judge that injection order is not important it's trivial to\n> change this to std::unorderd_set. The only drawback of std::set is that\n> it is a tad slower.\n>\nYes, the advantage is surely limited, this is not performance critical\ncode.\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> >\n> > Thanks\n> > j\n> >\n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > include/libcamera/camera.h | 11 ++++++-----\n> > > src/cam/main.cpp | 10 +++++-----\n> > > src/libcamera/camera.cpp | 10 +++++-----\n> > > src/libcamera/include/pipeline_handler.h | 2 +-\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---\n> > > src/libcamera/pipeline/uvcvideo.cpp | 6 +++---\n> > > src/libcamera/pipeline/vimc.cpp | 6 +++---\n> > > 7 files changed, 26 insertions(+), 25 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > index 8c8545b074e8ae13..1e75a6dafa4e7c5f 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -9,6 +9,7 @@\n> > >\n> > > #include \n> > > #include \n> > > +#include \n> > > #include \n> > >\n> > > #include \n> > > @@ -27,7 +28,7 @@ class Camera final\n> > > public:\n> > > \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > > \t\t\t\t\t const std::string &name,\n> > > -\t\t\t\t\t const std::vector &streams);\n> > > +\t\t\t\t\t const std::set &streams);\n> > >\n> > > \tCamera(const Camera &) = delete;\n> > > \tCamera &operator=(const Camera &) = delete;\n> > > @@ -40,9 +41,9 @@ public:\n> > > \tint acquire();\n> > > \tvoid release();\n> > >\n> > > -\tconst std::vector &streams() const;\n> > > +\tconst std::set &streams() const;\n> > > \tstd::map\n> > > -\tstreamConfiguration(std::vector &streams);\n> > > +\tstreamConfiguration(std::set &streams);\n> > > \tint configureStreams(std::map &config);\n> > >\n> > > \tint allocateBuffers();\n> > > @@ -73,8 +74,8 @@ private:\n> > >\n> > > \tstd::shared_ptr pipe_;\n> > > \tstd::string name_;\n> > > -\tstd::vector streams_;\n> > > -\tstd::vector activeStreams_;\n> > > +\tstd::set streams_;\n> > > +\tstd::set activeStreams_;\n> > >\n> > > \tState state_;\n> > > };\n> > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > > index 8df8844c33a2d052..13aa35968f38964d 100644\n> > > --- a/src/cam/main.cpp\n> > > +++ b/src/cam/main.cpp\n> > > @@ -75,10 +75,10 @@ static int parseOptions(int argc, char *argv[])\n> > > \treturn 0;\n> > > }\n> > >\n> > > -static int configureStreams(Camera *camera, std::vector &streams)\n> > > +static int configureStreams(Camera *camera, std::set &streams)\n> > > {\n> > > \tKeyValueParser::Options format = options[OptFormat];\n> > > -\tStream *id = streams.front();\n> > > +\tStream *id = *streams.begin();\n> > >\n> > > \tstd::map config =\n> > > \t\tcamera->streamConfiguration(streams);\n> > > @@ -132,7 +132,7 @@ static int capture()\n> > > {\n> > > \tint ret;\n> > >\n> > > -\tstd::vector streams = camera->streams();\n> > > +\tstd::set streams = camera->streams();\n> > > \tstd::vector requests;\n> > >\n> > > \tret = configureStreams(camera.get(), streams);\n> > > @@ -141,7 +141,7 @@ static int capture()\n> > > \t\treturn ret;\n> > > \t}\n> > >\n> > > -\tStream *stream = streams.front();\n> > > +\tStream *stream = *streams.begin();\n> > >\n> > > \tret = camera->allocateBuffers();\n> > > \tif (ret) {\n> > > @@ -237,7 +237,7 @@ int main(int argc, char **argv)\n> > > \t\t\tgoto out;\n> > > \t\t}\n> > >\n> > > -\t\tconst std::vector &streams = camera->streams();\n> > > +\t\tconst std::set &streams = camera->streams();\n> > > \t\tif (streams.size() != 1) {\n> > > \t\t\tstd::cout << \"Camera has \" << streams.size()\n> > > \t\t\t\t << \" streams, only 1 is supported\"\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index c50b14bbd904fc1c..938f21fe80ef8ceb 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -66,7 +66,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> > > */\n> > > std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > > \t\t\t\t const std::string &name,\n> > > -\t\t\t\t const std::vector &streams)\n> > > +\t\t\t\t const std::set &streams)\n> > > {\n> > > \tstruct Allocator : std::allocator {\n> > > \t\tvoid construct(void *p, PipelineHandler *pipe,\n> > > @@ -223,10 +223,10 @@ void Camera::release()\n> > > *\n> > > * \\return An array of all the camera's streams.\n> > > */\n> > > -const std::vector &Camera::streams() const\n> > > +const std::set &Camera::streams() const\n> > > {\n> > > \tif (!stateIsAtleast(Free))\n> > > -\t\tstd::vector{};\n> > > +\t\tstd::set{};\n> > >\n> > > \treturn streams_;\n> > > }\n> > > @@ -248,7 +248,7 @@ const std::vector &Camera::streams() const\n> > > * empty list on error.\n> > > */\n> > > std::map\n> > > -Camera::streamConfiguration(std::vector &streams)\n> > > +Camera::streamConfiguration(std::set &streams)\n> > > {\n> > > \tif (!stateIsAtleast(Free) || !streams.size())\n> > > \t\treturn std::map{};\n> > > @@ -300,7 +300,7 @@ int Camera::configureStreams(std::map &config)\n> > > \t\tconst StreamConfiguration &cfg = iter.second;\n> > >\n> > > \t\tstream->configuration_ = cfg;\n> > > -\t\tactiveStreams_.push_back(stream);\n> > > +\t\tactiveStreams_.insert(stream);\n> > >\n> > > \t\t/*\n> > > \t\t * Allocate buffer objects in the pool.\n> > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> > > index 4363dcd8ed8ead97..4cd9b90c609c067e 100644\n> > > --- a/src/libcamera/include/pipeline_handler.h\n> > > +++ b/src/libcamera/include/pipeline_handler.h\n> > > @@ -45,7 +45,7 @@ public:\n> > > \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n> > >\n> > > \tvirtual std::map\n> > > -\tstreamConfiguration(Camera *camera, std::vector &streams) = 0;\n> > > +\tstreamConfiguration(Camera *camera, std::set &streams) = 0;\n> > > \tvirtual int configureStreams(Camera *camera,\n> > > \t\t\t\t std::map &config) = 0;\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 9694d0ce51abda41..1066fbe350d934a0 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -32,7 +32,7 @@ public:\n> > >\n> > > \tstd::map\n> > > \tstreamConfiguration(Camera *camera,\n> > > -\t\t\t std::vector &streams) override;\n> > > +\t\t\t std::set &streams) override;\n> > > \tint configureStreams(Camera *camera,\n> > > \t\t\t std::map &config) override;\n> > >\n> > > @@ -95,7 +95,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()\n> > >\n> > > std::map\n> > > PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n> > > -\t\t\t\t\t std::vector &streams)\n> > > +\t\t\t\t\t std::set &streams)\n> > > {\n> > > \tIPU3CameraData *data = cameraData(camera);\n> > > \tstd::map configs;\n> > > @@ -374,7 +374,7 @@ void PipelineHandlerIPU3::registerCameras()\n> > > \t\tstd::unique_ptr data = utils::make_unique();\n> > >\n> > > \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > > -\t\tstd::vector streams{ &data->stream_ };\n> > > +\t\tstd::set streams{ &data->stream_ };\n> > > \t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n> > >\n> > > \t\t/*\n> > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > > index b7b8ff109109e9c5..7b697c0685630d92 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > > @@ -27,7 +27,7 @@ public:\n> > >\n> > > \tstd::map\n> > > \tstreamConfiguration(Camera *camera,\n> > > -\t\t\t std::vector &streams) override;\n> > > +\t\t\t std::set &streams) override;\n> > > \tint configureStreams(Camera *camera,\n> > > \t\t\t std::map &config) override;\n> > >\n> > > @@ -63,7 +63,7 @@ PipelineHandlerUVC::~PipelineHandlerUVC()\n> > >\n> > > std::map\n> > > PipelineHandlerUVC::streamConfiguration(Camera *camera,\n> > > -\t\t\t\t\tstd::vector &streams)\n> > > +\t\t\t\t\tstd::set &streams)\n> > > {\n> > > \tstd::map configs;\n> > >\n> > > @@ -171,7 +171,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> > > \t\treturn false;\n> > > \t}\n> > >\n> > > -\tstd::vector streams{ &stream_ };\n> > > +\tstd::set streams{ &stream_ };\n> > > \tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> > > \tregisterCamera(std::move(camera));\n> > > \thotplugMediaDevice(media_.get());\n> > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > > index 812777cffbb3e618..9ba96323d335e010 100644\n> > > --- a/src/libcamera/pipeline/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc.cpp\n> > > @@ -27,7 +27,7 @@ public:\n> > >\n> > > \tstd::map\n> > > \tstreamConfiguration(Camera *camera,\n> > > -\t\t\t std::vector &streams) override;\n> > > +\t\t\t std::set &streams) override;\n> > > \tint configureStreams(Camera *camera,\n> > > \t\t\t std::map &config) override;\n> > >\n> > > @@ -62,7 +62,7 @@ PipelineHandlerVimc::~PipelineHandlerVimc()\n> > >\n> > > std::map\n> > > PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> > > -\t\t\t\t std::vector &streams)\n> > > +\t\t\t\t\t std::set &streams)\n> > > {\n> > > \tstd::map configs;\n> > >\n> > > @@ -171,7 +171,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> > > \t\treturn false;\n> > > \t}\n> > >\n> > > -\tstd::vector streams{ &stream_ };\n> > > +\tstd::set streams{ &stream_ };\n> > > \tstd::shared_ptr camera = Camera::create(this, \"VIMC Sensor B\",\n> > > \t\t\t\t\t\t\tstreams);\n> > > \tregisterCamera(std::move(camera));\n> > > --\n> > > 2.20.1\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n>\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"","Received":["from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B4B97601E2\n\tfor ;\n\tWed, 27 Feb 2019 10:50:18 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 3D0CBC0002;\n\tWed, 27 Feb 2019 09:50:17 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 27 Feb 2019 10:50:47 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190227095047.2ga64hfimpg6mcx3@uno.localdomain>","References":"<20190226021857.28255-1-niklas.soderlund@ragnatech.se>\n\t<20190226021857.28255-7-niklas.soderlund@ragnatech.se>\n\t<20190226171852.p5bm5jyg5aa4o7lg@uno.localdomain>\n\t<20190226190609.GM899@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"a465s7krbo6jg2sb\"","Content-Disposition":"inline","In-Reply-To":"<20190226190609.GM899@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 6/8] libcamera: store stream pointers\n\tin sets instead of a vectors","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 27 Feb 2019 09:50:18 -0000"}},{"id":924,"web_url":"https://patchwork.libcamera.org/comment/924/","msgid":"<20190227164504.GG4813@pendragon.ideasonboard.com>","date":"2019-02-27T16:45:04","subject":"Re: [libcamera-devel] [PATCH 6/8] libcamera: store stream pointers\n\tin sets instead of a vectors","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Tue, Feb 26, 2019 at 03:18:55AM +0100, Niklas Söderlund wrote:\n> The arrays that store Stream pointers shall always contain unique\n> values. Storing them in vectors opens up for the same stream pointer\n> appearing twice. Remove this possibility by storing them in a set which\n> grantees each element is unique.\n\ns/grantees/guarantees/\n\n> \n> Signed-off-by: Niklas Söderlund \n\nWith the changes needed to rebase on top of v2 of 5/8,\n\nReviewed-by: Laurent Pinchart \n\n> ---\n> include/libcamera/camera.h | 11 ++++++-----\n> src/cam/main.cpp | 10 +++++-----\n> src/libcamera/camera.cpp | 10 +++++-----\n> src/libcamera/include/pipeline_handler.h | 2 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---\n> src/libcamera/pipeline/uvcvideo.cpp | 6 +++---\n> src/libcamera/pipeline/vimc.cpp | 6 +++---\n> 7 files changed, 26 insertions(+), 25 deletions(-)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 8c8545b074e8ae13..1e75a6dafa4e7c5f 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -9,6 +9,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -27,7 +28,7 @@ class Camera final\n> public:\n> \tstatic std::shared_ptr create(PipelineHandler *pipe,\n> \t\t\t\t\t const std::string &name,\n> -\t\t\t\t\t const std::vector &streams);\n> +\t\t\t\t\t const std::set &streams);\n> \n> \tCamera(const Camera &) = delete;\n> \tCamera &operator=(const Camera &) = delete;\n> @@ -40,9 +41,9 @@ public:\n> \tint acquire();\n> \tvoid release();\n> \n> -\tconst std::vector &streams() const;\n> +\tconst std::set &streams() const;\n> \tstd::map\n> -\tstreamConfiguration(std::vector &streams);\n> +\tstreamConfiguration(std::set &streams);\n> \tint configureStreams(std::map &config);\n> \n> \tint allocateBuffers();\n> @@ -73,8 +74,8 @@ private:\n> \n> \tstd::shared_ptr pipe_;\n> \tstd::string name_;\n> -\tstd::vector streams_;\n> -\tstd::vector activeStreams_;\n> +\tstd::set streams_;\n> +\tstd::set activeStreams_;\n> \n> \tState state_;\n> };\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 8df8844c33a2d052..13aa35968f38964d 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -75,10 +75,10 @@ static int parseOptions(int argc, char *argv[])\n> \treturn 0;\n> }\n> \n> -static int configureStreams(Camera *camera, std::vector &streams)\n> +static int configureStreams(Camera *camera, std::set &streams)\n> {\n> \tKeyValueParser::Options format = options[OptFormat];\n> -\tStream *id = streams.front();\n> +\tStream *id = *streams.begin();\n> \n> \tstd::map config =\n> \t\tcamera->streamConfiguration(streams);\n> @@ -132,7 +132,7 @@ static int capture()\n> {\n> \tint ret;\n> \n> -\tstd::vector streams = camera->streams();\n> +\tstd::set streams = camera->streams();\n> \tstd::vector requests;\n> \n> \tret = configureStreams(camera.get(), streams);\n> @@ -141,7 +141,7 @@ static int capture()\n> \t\treturn ret;\n> \t}\n> \n> -\tStream *stream = streams.front();\n> +\tStream *stream = *streams.begin();\n> \n> \tret = camera->allocateBuffers();\n> \tif (ret) {\n> @@ -237,7 +237,7 @@ int main(int argc, char **argv)\n> \t\t\tgoto out;\n> \t\t}\n> \n> -\t\tconst std::vector &streams = camera->streams();\n> +\t\tconst std::set &streams = camera->streams();\n> \t\tif (streams.size() != 1) {\n> \t\t\tstd::cout << \"Camera has \" << streams.size()\n> \t\t\t\t << \" streams, only 1 is supported\"\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index c50b14bbd904fc1c..938f21fe80ef8ceb 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -66,7 +66,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> */\n> std::shared_ptr Camera::create(PipelineHandler *pipe,\n> \t\t\t\t const std::string &name,\n> -\t\t\t\t const std::vector &streams)\n> +\t\t\t\t const std::set &streams)\n> {\n> \tstruct Allocator : std::allocator {\n> \t\tvoid construct(void *p, PipelineHandler *pipe,\n> @@ -223,10 +223,10 @@ void Camera::release()\n> *\n> * \\return An array of all the camera's streams.\n> */\n> -const std::vector &Camera::streams() const\n> +const std::set &Camera::streams() const\n> {\n> \tif (!stateIsAtleast(Free))\n> -\t\tstd::vector{};\n> +\t\tstd::set{};\n> \n> \treturn streams_;\n> }\n> @@ -248,7 +248,7 @@ const std::vector &Camera::streams() const\n> * empty list on error.\n> */\n> std::map\n> -Camera::streamConfiguration(std::vector &streams)\n> +Camera::streamConfiguration(std::set &streams)\n> {\n> \tif (!stateIsAtleast(Free) || !streams.size())\n> \t\treturn std::map{};\n> @@ -300,7 +300,7 @@ int Camera::configureStreams(std::map &config)\n> \t\tconst StreamConfiguration &cfg = iter.second;\n> \n> \t\tstream->configuration_ = cfg;\n> -\t\tactiveStreams_.push_back(stream);\n> +\t\tactiveStreams_.insert(stream);\n> \n> \t\t/*\n> \t\t * Allocate buffer objects in the pool.\n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 4363dcd8ed8ead97..4cd9b90c609c067e 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -45,7 +45,7 @@ public:\n> \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n> \n> \tvirtual std::map\n> -\tstreamConfiguration(Camera *camera, std::vector &streams) = 0;\n> +\tstreamConfiguration(Camera *camera, std::set &streams) = 0;\n> \tvirtual int configureStreams(Camera *camera,\n> \t\t\t\t std::map &config) = 0;\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 9694d0ce51abda41..1066fbe350d934a0 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -32,7 +32,7 @@ public:\n> \n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::vector &streams) override;\n> +\t\t\t std::set &streams) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n> \n> @@ -95,7 +95,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()\n> \n> std::map\n> PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n> -\t\t\t\t\t std::vector &streams)\n> +\t\t\t\t\t std::set &streams)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> \tstd::map configs;\n> @@ -374,7 +374,7 @@ void PipelineHandlerIPU3::registerCameras()\n> \t\tstd::unique_ptr data = utils::make_unique();\n> \n> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> -\t\tstd::vector streams{ &data->stream_ };\n> +\t\tstd::set streams{ &data->stream_ };\n> \t\tstd::shared_ptr camera = Camera::create(this, cameraName, streams);\n> \n> \t\t/*\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index b7b8ff109109e9c5..7b697c0685630d92 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -27,7 +27,7 @@ public:\n> \n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::vector &streams) override;\n> +\t\t\t std::set &streams) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n> \n> @@ -63,7 +63,7 @@ PipelineHandlerUVC::~PipelineHandlerUVC()\n> \n> std::map\n> PipelineHandlerUVC::streamConfiguration(Camera *camera,\n> -\t\t\t\t\tstd::vector &streams)\n> +\t\t\t\t\tstd::set &streams)\n> {\n> \tstd::map configs;\n> \n> @@ -171,7 +171,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \t}\n> \n> -\tstd::vector streams{ &stream_ };\n> +\tstd::set streams{ &stream_ };\n> \tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n> \tregisterCamera(std::move(camera));\n> \thotplugMediaDevice(media_.get());\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 812777cffbb3e618..9ba96323d335e010 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -27,7 +27,7 @@ public:\n> \n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::vector &streams) override;\n> +\t\t\t std::set &streams) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n> \n> @@ -62,7 +62,7 @@ PipelineHandlerVimc::~PipelineHandlerVimc()\n> \n> std::map\n> PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> -\t\t\t\t std::vector &streams)\n> +\t\t\t\t\t std::set &streams)\n> {\n> \tstd::map configs;\n> \n> @@ -171,7 +171,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \t}\n> \n> -\tstd::vector streams{ &stream_ };\n> +\tstd::set streams{ &stream_ };\n> \tstd::shared_ptr camera = Camera::create(this, \"VIMC Sensor B\",\n> \t\t\t\t\t\t\tstreams);\n> \tregisterCamera(std::move(camera));\n> -- \n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B51EF610B3\n\tfor ;\n\tWed, 27 Feb 2019 17:45:14 +0100 (CET)","from pendragon.ideasonboard.com (unknown [83.145.195.18])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2559049;\n\tWed, 27 Feb 2019 17:45:12 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1551285914;\n\tbh=95hLmzNPppWdG2GDssDqBcArG3f4awqAp5PmQGwIH5I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=K0fkua3YmBu/j/BnENdRG32QVd5f4HzUZN8Or8001QTmKUiOkvPpvIYY9hfMDwupn\n\tWd9WAjSuipCqaYZp5T0yyvkEBuvYIdVMSbKY6dwD5MlBjSxUPpuHUqI7WP9Mll3Kng\n\tThk71JeTVE95IyWsCZuPA7b5kIP/kR8ogWV7Nc2A=","Date":"Wed, 27 Feb 2019 18:45:04 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190227164504.GG4813@pendragon.ideasonboard.com>","References":"<20190226021857.28255-1-niklas.soderlund@ragnatech.se>\n\t<20190226021857.28255-7-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190226021857.28255-7-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 6/8] libcamera: store stream pointers\n\tin sets instead of a vectors","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 27 Feb 2019 16:45:14 -0000"}}]