[{"id":1270,"web_url":"https://patchwork.libcamera.org/comment/1270/","msgid":"<20190405094746.s4da4bayokhvnodb@uno.localdomain>","date":"2019-04-05T09:47:46","subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n thanks for the patch.\n\nOn Fri, Apr 05, 2019 at 04:02:55AM +0200, Niklas Söderlund wrote:\n> To properly support both multiple streams and usages the library must\n> provide a method to map the stream usages to the returned stream\n> configuration. Add a camera configuration object to handle this mapping.\n>\n> Applications can iterate over the returned camera configuration to\n> retrieve the streams selected by the library in the same order as the\n> usages it provided to the library. Further more the application can use\n\n\"in the same order as it provided usages to the library\"\n\nfurthermore\n\n> the streams to retrieve there configuration using operator[] of the\n\n\"a stream pointer to retrieve its configuration\" or\n\"the streams to retrieve their configuration\"\n\n> camera configuration.\n>\n\nThis is a nice and clean way to replace the cumbersome map used by both streamConfiguration() and\nconfigureStream() and makes it easy to access a configuration from a\nStream *, so it's nice. Though, I don't see how this would help in\nhandling the \"what stream is what\" problem at requestComplete() time.\nWhat's missing is the association with a name/id to a configuration,\nis this something planned for later or am I missing how this should be\nused?\n\n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/camera.h | 26 ++++++++\n> src/libcamera/camera.cpp | 121 +++++++++++++++++++++++++++++++++++++\n> 2 files changed, 147 insertions(+)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 0386671c902e55e8..311a51e4070d135c 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -14,6 +14,7 @@\n>\n> #include \n> #include \n> +#include \n>\n> namespace libcamera {\n>\n> @@ -24,6 +25,31 @@ class Stream;\n> class StreamConfiguration;\n> class StreamUsage;\n>\n> +class CameraConfiguration\n> +{\n> +public:\n> +\tusing iterator = std::vector::iterator;\n> +\tusing const_iterator = std::vector::const_iterator;\n\nHave you considered giving this class iterator traits and implement\nmethods required for at least, say, ForwardIterator?\nhttp://www.cplusplus.com/reference/iterator/ForwardIterator/\n\nThat would make possible for applications to use STL library functions\nthat operates on iterators on the class. In example, can you use\nrange-based for loops on this class?\n\n> +\n> +\tCameraConfiguration();\n> +\n> +\titerator begin();\n> +\titerator end();\n> +\tconst_iterator begin() const;\n> +\tconst_iterator end() const;\n> +\n> +\tbool empty() const;\n> +\tstd::size_t size() const;\n> +\n> +\tStream *front();\n> +\n> +\tStreamConfiguration &operator[](Stream *stream);\n> +\n> +private:\n> +\tstd::vector order_;\n> +\tstd::map config_;\n\nI wonder if keeping the Stream * in two separate places might lead to\nissues in keeping the two in sync. But these are privates, so it\nshould be fine...\n\n> +};\n> +\n> class Camera final\n> {\n> public:\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 63fde0ffc3d02d6c..16162c524297012f 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -39,6 +39,127 @@ namespace libcamera {\n>\n> LOG_DECLARE_CATEGORY(Camera)\n>\n> +/**\n> + * \\class CameraConfiguration\n> + * \\brief Hold configuration for streams of the camera that applications\nI would drop what's after the \"of the camera\".\n> + * wish to modify and apply.\n> + *\n> + * The CameraConfiguration is filled with information by the application either\n> + * manually or with the streamConfiguration() helper. The helper takes a list\n> + * list of usages describing how the application intents to use the camera, the\ns/list list/list/\ns/intents/intends/\ns/use the camera/use the stream/\n\n> + * application in returns is provided with a default camera configuration it\ns/in returns/in return/\n> + * can tweak.\n> + *\n> + * Applications iterates over the CameraConfiguration to discover which streams\ns/iterates/iterate/\n> + * the camera have selected for its usages and can inspect the configuration\ns/the camera have selected for its usage/the camera has associated to a usage/\ns/can inspect/can access/\n> + * using the operator[].\n> + */\n> +\n> +/**\n> + * \\typedef CameraConfiguration::iterator\n> + * \\brief Iterator for the streams in the configuration\n> + */\n> +\n> +/**\n> + * \\typedef CameraConfiguration::const_iterator\n> + * \\brief Const iterator for the streams in the configuration\n> + */\n> +\n> +/**\n> + * \\brief Create an empty camera configuration\n> + */\n> +CameraConfiguration::CameraConfiguration()\n> +\t: order_({}), config_({})\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Retrieve an iterator to the first element of the streams\n> + *\n\nThe standard iterator documentation for begin reports:\n\"Returns an iterator pointing to the first element in the sequence\"\n\nCould we match this?\n\n> + * \\return An iterator to the first stream\n> + */\n> +std::vector::iterator CameraConfiguration::begin()\n> +{\n> +\treturn order_.begin();\n> +}\n> +\n> +/**\n> + * \\brief Retrieve an iterator to the end of the streams\n\nSame as per 'begin()':\n\n\"Returns an iterator pointing to the past-the-end element in the sequence\"\n\n> + *\n> + * \\return An iterator to the element following the last stream\n> + */\n> +std::vector::iterator CameraConfiguration::end()\n> +{\n> +\treturn order_.end();\n> +}\n> +\n> +/**\n> + * \\brief Retrieve an iterator to the first element of the streams\n> + *\n> + * \\return An iterator to the first stream\n> + */\n> +std::vector::const_iterator CameraConfiguration::begin() const\n> +{\n> +\treturn order_.begin();\n> +}\n> +\n> +/**\n> + * \\brief Retrieve an iterator to the end of the streams\n> + *\n> + * \\return An iterator to the element following the last stream\n> + */\n> +std::vector::const_iterator CameraConfiguration::end() const\n> +{\n> +\treturn order_.end();\n> +}\n> +\n> +/**\n> + * \\brief Checks whether the camera configuration is empty\n> + *\n> + * \\return True if the configuration is empty\n> + */\n> +bool CameraConfiguration::empty() const\n> +{\n> +\treturn order_.empty();\n> +}\n> +\n> +/**\n> + * \\brief Check the number of stream configurations\n> + *\n> + * \\return Number of stream configurations\n> + */\n> +std::size_t CameraConfiguration::size() const\n> +{\n> +\treturn order_.size();\n> +}\n> +\n> +/**\n> + * \\brief Access the first stream in the configuration\n> + *\n> + * \\return The first stream in the configuration\n> + */\n> +Stream *CameraConfiguration::front()\n> +{\n> +\treturn order_.front();\n> +}\n> +\n> +/**\n> + * \\brief Retrieve a reference to a stream configuration\n> + * \\param[in] stream Stream to retrieve configuration for\n> + *\n> + * If the camera configuration do not yet contain configuration for the\n\ncontain a configuration\n\n> + * requested stream an empty stream configuration is created and returned.\n\nempty one\n\n> + *\n> + * \\return Configuration for the stream\n\nThe configuration\n\n> + */\n> +StreamConfiguration &CameraConfiguration::operator[](Stream *stream)\n> +{\n> +\tif (config_.find(stream) == config_.end())\n> +\t\torder_.push_back(stream);\n> +\n> +\treturn config_[stream];\n> +}\n> +\n\nI don't know how welcome would the idea of making of this class a\nfull-fledged iterator, but I think increment, decrement and access by\ninteger index should be implemented to make it more useful. Have you\nconsidered that?\n\nThanks\n j\n\n> /**\n> * \\class Camera\n> * \\brief Camera device\n> --\n> 2.21.0\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 relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E5E160DB2\n\tfor ;\n\tFri, 5 Apr 2019 11:47:01 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 1B9636001D;\n\tFri, 5 Apr 2019 09:46:59 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 5 Apr 2019 11:47:46 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190405094746.s4da4bayokhvnodb@uno.localdomain>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-8-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"ezim3tc2hunh7hir\"","Content-Disposition":"inline","In-Reply-To":"<20190405020256.22520-8-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","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":"Fri, 05 Apr 2019 09:47:01 -0000"}},{"id":1272,"web_url":"https://patchwork.libcamera.org/comment/1272/","msgid":"<20190405102106.GU23466@bigcity.dyn.berto.se>","date":"2019-04-05T10:21:07","subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","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. I will just address conceptual thing in this \nreply and circle back to the nitty-gritty details in an other reply :-)\n\nOn 2019-04-05 11:47:46 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> thanks for the patch.\n> \n> On Fri, Apr 05, 2019 at 04:02:55AM +0200, Niklas Söderlund wrote:\n> > To properly support both multiple streams and usages the library must\n> > provide a method to map the stream usages to the returned stream\n> > configuration. Add a camera configuration object to handle this mapping.\n> >\n> > Applications can iterate over the returned camera configuration to\n> > retrieve the streams selected by the library in the same order as the\n> > usages it provided to the library. Further more the application can use\n> \n> \"in the same order as it provided usages to the library\"\n> \n> furthermore\n> \n> > the streams to retrieve there configuration using operator[] of the\n> \n> \"a stream pointer to retrieve its configuration\" or\n> \"the streams to retrieve their configuration\"\n> \n> > camera configuration.\n> >\n> \n> This is a nice and clean way to replace the cumbersome map StreamConfiguration> used by both streamConfiguration() and\n> configureStream() and makes it easy to access a configuration from a\n> Stream *, so it's nice. Though, I don't see how this would help in\n> handling the \"what stream is what\" problem at requestComplete() time.\n> What's missing is the association with a name/id to a configuration,\n> is this something planned for later or am I missing how this should be\n> used?\n\nThis works today, a dumb way for applications to implement this could \nlook like:\n\n /* Setup streams */\n CameraConfiguration config =\n camera->streamConfiguration({ Stream::Foo(), Stream::Bar() });\n\n camera->configureStreams(config);\n\n ...\n\n camera->requestCompleted.connect(requestComplete);\n\n .. star camera etc...\n\n\n void requestComplete(Request *request, const std::map &buffers)\n {\n int i = 0;\n for (stream : config) {\n if (buffers.find(stream) != buffer.end())\n printf(\"Request contains buffer for role position %d\\n\", i);\n\n i++\n }\n }\n\nMore likely the application will create its own\nstd::map data structure and populate it \nonce using the loop above.\n\n> \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/camera.h | 26 ++++++++\n> > src/libcamera/camera.cpp | 121 +++++++++++++++++++++++++++++++++++++\n> > 2 files changed, 147 insertions(+)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 0386671c902e55e8..311a51e4070d135c 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -14,6 +14,7 @@\n> >\n> > #include \n> > #include \n> > +#include \n> >\n> > namespace libcamera {\n> >\n> > @@ -24,6 +25,31 @@ class Stream;\n> > class StreamConfiguration;\n> > class StreamUsage;\n> >\n> > +class CameraConfiguration\n> > +{\n> > +public:\n> > +\tusing iterator = std::vector::iterator;\n> > +\tusing const_iterator = std::vector::const_iterator;\n> \n> Have you considered giving this class iterator traits and implement\n> methods required for at least, say, ForwardIterator?\n> http://www.cplusplus.com/reference/iterator/ForwardIterator/\n> \n> That would make possible for applications to use STL library functions\n> that operates on iterators on the class. In example, can you use\n> range-based for loops on this class?\n\nThis is a full fledge iterator already that support \nrandom_access_iterator_tag. It provides the proper begin() and end() \nfunctions which proxies the iterators for order_ which is a std::vector \n:-)\n\nThe STL library functions such as ranged based loops works and are \nalready used in the next patch of this series on this data type.\n\n> \n> > +\n> > +\tCameraConfiguration();\n> > +\n> > +\titerator begin();\n> > +\titerator end();\n> > +\tconst_iterator begin() const;\n> > +\tconst_iterator end() const;\n> > +\n> > +\tbool empty() const;\n> > +\tstd::size_t size() const;\n> > +\n> > +\tStream *front();\n> > +\n> > +\tStreamConfiguration &operator[](Stream *stream);\n> > +\n> > +private:\n> > +\tstd::vector order_;\n> > +\tstd::map config_;\n> \n> I wonder if keeping the Stream * in two separate places might lead to\n> issues in keeping the two in sync. But these are privates, so it\n> should be fine...\n> \n> > +};\n> > +\n> > class Camera final\n> > {\n> > public:\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 63fde0ffc3d02d6c..16162c524297012f 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -39,6 +39,127 @@ namespace libcamera {\n> >\n> > LOG_DECLARE_CATEGORY(Camera)\n> >\n> > +/**\n> > + * \\class CameraConfiguration\n> > + * \\brief Hold configuration for streams of the camera that applications\n> I would drop what's after the \"of the camera\".\n> > + * wish to modify and apply.\n> > + *\n> > + * The CameraConfiguration is filled with information by the application either\n> > + * manually or with the streamConfiguration() helper. The helper takes a list\n> > + * list of usages describing how the application intents to use the camera, the\n> s/list list/list/\n> s/intents/intends/\n> s/use the camera/use the stream/\n> \n> > + * application in returns is provided with a default camera configuration it\n> s/in returns/in return/\n> > + * can tweak.\n> > + *\n> > + * Applications iterates over the CameraConfiguration to discover which streams\n> s/iterates/iterate/\n> > + * the camera have selected for its usages and can inspect the configuration\n> s/the camera have selected for its usage/the camera has associated to a usage/\n> s/can inspect/can access/\n> > + * using the operator[].\n> > + */\n> > +\n> > +/**\n> > + * \\typedef CameraConfiguration::iterator\n> > + * \\brief Iterator for the streams in the configuration\n> > + */\n> > +\n> > +/**\n> > + * \\typedef CameraConfiguration::const_iterator\n> > + * \\brief Const iterator for the streams in the configuration\n> > + */\n> > +\n> > +/**\n> > + * \\brief Create an empty camera configuration\n> > + */\n> > +CameraConfiguration::CameraConfiguration()\n> > +\t: order_({}), config_({})\n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\brief Retrieve an iterator to the first element of the streams\n> > + *\n> \n> The standard iterator documentation for begin reports:\n> \"Returns an iterator pointing to the first element in the sequence\"\n> \n> Could we match this?\n> \n> > + * \\return An iterator to the first stream\n> > + */\n> > +std::vector::iterator CameraConfiguration::begin()\n> > +{\n> > +\treturn order_.begin();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Retrieve an iterator to the end of the streams\n> \n> Same as per 'begin()':\n> \n> \"Returns an iterator pointing to the past-the-end element in the sequence\"\n> \n> > + *\n> > + * \\return An iterator to the element following the last stream\n> > + */\n> > +std::vector::iterator CameraConfiguration::end()\n> > +{\n> > +\treturn order_.end();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Retrieve an iterator to the first element of the streams\n> > + *\n> > + * \\return An iterator to the first stream\n> > + */\n> > +std::vector::const_iterator CameraConfiguration::begin() const\n> > +{\n> > +\treturn order_.begin();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Retrieve an iterator to the end of the streams\n> > + *\n> > + * \\return An iterator to the element following the last stream\n> > + */\n> > +std::vector::const_iterator CameraConfiguration::end() const\n> > +{\n> > +\treturn order_.end();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Checks whether the camera configuration is empty\n> > + *\n> > + * \\return True if the configuration is empty\n> > + */\n> > +bool CameraConfiguration::empty() const\n> > +{\n> > +\treturn order_.empty();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Check the number of stream configurations\n> > + *\n> > + * \\return Number of stream configurations\n> > + */\n> > +std::size_t CameraConfiguration::size() const\n> > +{\n> > +\treturn order_.size();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Access the first stream in the configuration\n> > + *\n> > + * \\return The first stream in the configuration\n> > + */\n> > +Stream *CameraConfiguration::front()\n> > +{\n> > +\treturn order_.front();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Retrieve a reference to a stream configuration\n> > + * \\param[in] stream Stream to retrieve configuration for\n> > + *\n> > + * If the camera configuration do not yet contain configuration for the\n> \n> contain a configuration\n> \n> > + * requested stream an empty stream configuration is created and returned.\n> \n> empty one\n> \n> > + *\n> > + * \\return Configuration for the stream\n> \n> The configuration\n> \n> > + */\n> > +StreamConfiguration &CameraConfiguration::operator[](Stream *stream)\n> > +{\n> > +\tif (config_.find(stream) == config_.end())\n> > +\t\torder_.push_back(stream);\n> > +\n> > +\treturn config_[stream];\n> > +}\n> > +\n> \n> I don't know how welcome would the idea of making of this class a\n> full-fledged iterator, but I think increment, decrement and access by\n> integer index should be implemented to make it more useful. Have you\n> considered that?\n\nAs stated above it is already implemented. Well not integer indexed as \nthe whole idea if this object is to make it Stream* indexed but preserve \nthe insertion order when iterating over it to allow mapping stream \nusages to stream configurations.\n\n> \n> Thanks\n> j\n> \n> > /**\n> > * \\class Camera\n> > * \\brief Camera device\n> > --\n> > 2.21.0\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-x232.google.com (mail-lj1-x232.google.com\n\t[IPv6:2a00:1450:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 067E260DB3\n\tfor ;\n\tFri, 5 Apr 2019 12:21:09 +0200 (CEST)","by mail-lj1-x232.google.com with SMTP id r24so4776716ljg.3\n\tfor ;\n\tFri, 05 Apr 2019 03:21:08 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tq6sm446596lfj.36.2019.04.05.03.21.07\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 05 Apr 2019 03:21:07 -0700 (PDT)"],"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=q9++lDOMO00cSFftVOztZ0AWR79VgBMIHB+eTPo+fXA=;\n\tb=QRSQrAEkCC+gNEdppPzzQeeS2yHqRLVkbgL0aX0IlaJ1GYTAJloTvaDADdyKIpYTBO\n\tGWyEeWBYt7k3A5xry2QXUy6C/qHLyp6wcoPxfAYbn7iPJkfPkLRlMlL1eovF6ZEyVtl9\n\ttU6TpILRU3vucwrJIUMVI6kHOTzJLREWDystkNckDU1xfyD4s4CO96v6PIsSnOI95UXs\n\ttymg9hrzy8G0FcbTF4ddM1EAVluiKwVLQc+QBtoovbwQgU+F3l0nAEe43kcZfx3utKSb\n\tssgtvONt7Vlw6FeOH5Nf8CwH43e5ahKXIB0rR6UONSe7I78pBnnkwmkGw6/p0Dvve1HR\n\tWP3g==","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=q9++lDOMO00cSFftVOztZ0AWR79VgBMIHB+eTPo+fXA=;\n\tb=H2p3x5WEoI312SnfYNsKwoCDvcLCbwiTKnL4N2dW5QHbKBsEVZznQO+nvNGb/qcYeN\n\tAQ28LCkAkL2YLLGOGABjE9jqPoLcHDHmpWlXSTjSuuK0hVcr5S7KnmF59rEtFvpeNTI+\n\tmXHQ5L+ZHpKDkb34B6emXMeh8vn1vH/0rGWxLJj0p1ap3YU83sjyeRIjN4pfNTmYnOgH\n\tkpxuvbtNomiD5OtSnPeyMqZNrlySoRvNE/7u+2z/I5gST+9ndC9YeHV7HFFwpJALSjp4\n\t96GPAK4oau4OpBdxZv77CIfo/NN4X/VOz2/9du/q5XFeY2/6quQtjuWlIyBgeX8nO7lT\n\t2SWg==","X-Gm-Message-State":"APjAAAWNWP5tEgcPJw6+EmCDWruNK9I/NaeehVUOCQAsOYa7cNJeAvT6\n\tR0Lqt0W7ejezCFvslZK7IsGlXqhsIqM=","X-Google-Smtp-Source":"APXvYqxVWzWfVXP9LbMIadxC4LmC7+n/s25NCbl1QL6yoy8Zt6d0VlFSRuy66AkedKlRgTtQ4xO2Sg==","X-Received":"by 2002:a2e:87d2:: with SMTP id v18mr6855427ljj.4.1554459668246; \n\tFri, 05 Apr 2019 03:21:08 -0700 (PDT)","Date":"Fri, 5 Apr 2019 12:21:07 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190405102106.GU23466@bigcity.dyn.berto.se>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-8-niklas.soderlund@ragnatech.se>\n\t<20190405094746.s4da4bayokhvnodb@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190405094746.s4da4bayokhvnodb@uno.localdomain>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","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":"Fri, 05 Apr 2019 10:21:09 -0000"}},{"id":1274,"web_url":"https://patchwork.libcamera.org/comment/1274/","msgid":"<20190405111211.di7iecvze3p5r5ge@uno.localdomain>","date":"2019-04-05T11:12:11","subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Fri, Apr 05, 2019 at 12:21:07PM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your feedback. I will just address conceptual thing in this\n> reply and circle back to the nitty-gritty details in an other reply :-)\n>\n> On 2019-04-05 11:47:46 +0200, Jacopo Mondi wrote:\n> > Hi Niklas,\n> > thanks for the patch.\n> >\n> > On Fri, Apr 05, 2019 at 04:02:55AM +0200, Niklas Söderlund wrote:\n> > > To properly support both multiple streams and usages the library must\n> > > provide a method to map the stream usages to the returned stream\n> > > configuration. Add a camera configuration object to handle this mapping.\n> > >\n> > > Applications can iterate over the returned camera configuration to\n> > > retrieve the streams selected by the library in the same order as the\n> > > usages it provided to the library. Further more the application can use\n> >\n> > \"in the same order as it provided usages to the library\"\n> >\n> > furthermore\n> >\n> > > the streams to retrieve there configuration using operator[] of the\n> >\n> > \"a stream pointer to retrieve its configuration\" or\n> > \"the streams to retrieve their configuration\"\n> >\n> > > camera configuration.\n> > >\n> >\n> > This is a nice and clean way to replace the cumbersome map > StreamConfiguration> used by both streamConfiguration() and\n> > configureStream() and makes it easy to access a configuration from a\n> > Stream *, so it's nice. Though, I don't see how this would help in\n> > handling the \"what stream is what\" problem at requestComplete() time.\n> > What's missing is the association with a name/id to a configuration,\n> > is this something planned for later or am I missing how this should be\n> > used?\n>\n> This works today, a dumb way for applications to implement this could\n> look like:\n>\n> /* Setup streams */\n> CameraConfiguration config =\n> camera->streamConfiguration({ Stream::Foo(), Stream::Bar() });\n>\n> camera->configureStreams(config);\n>\n> ...\n>\n> camera->requestCompleted.connect(requestComplete);\n>\n> .. star camera etc...\n>\n>\n> void requestComplete(Request *request, const std::map &buffers)\n> {\n> int i = 0;\n> for (stream : config) {\n> if (buffers.find(stream) != buffer.end())\n> printf(\"Request contains buffer for role position %d\\n\", i);\n>\n> i++\n> }\n> }\n>\n> More likely the application will create its own\n> std::map data structure and populate it\n> once using the loop above.\n\nI see. So this aims to replace the id/name based stream identification\nin general.\n\nDoes this supersedes the idea of having configureStream() assign\nid/names to streams configuration? I sill like the idea of having\napplication assign ids to roles, the id is copied to the returned\nstream configuration assigned to a stream, and copied to the stream at\nconfigureStream() time, so that applications could:\n\n #define STREAM_VF 0\n #define STREAM_STILL 1\n\n CameraConfiguration config = streamConfiguration({STREAM_VF,\n Viewfinder(640x480),\n {STREAM_STILL,\n StillCapture()});\n ...\n\n configureStream(config); // Assign the id in the conf to the\n // stream\n\n ....\n\n void requestComplete(Request *request, const std::map &buffers)\n {\n\n for (auto it : buffers) {\n Stream *s = it.first;\n\n if (s.id == STREAM_VF)\n //display\n if (s.id == STREAM_STILL)\n //save to disk\n }\n }\n\nThey could most probably handle that internally with a map or\nsomething indeed, but this creates a logical connection between the\nintended role and an id applications can control.\n>\n> >\n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > include/libcamera/camera.h | 26 ++++++++\n> > > src/libcamera/camera.cpp | 121 +++++++++++++++++++++++++++++++++++++\n> > > 2 files changed, 147 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > index 0386671c902e55e8..311a51e4070d135c 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -14,6 +14,7 @@\n> > >\n> > > #include \n> > > #include \n> > > +#include \n> > >\n> > > namespace libcamera {\n> > >\n> > > @@ -24,6 +25,31 @@ class Stream;\n> > > class StreamConfiguration;\n> > > class StreamUsage;\n> > >\n> > > +class CameraConfiguration\n> > > +{\n> > > +public:\n> > > +\tusing iterator = std::vector::iterator;\n> > > +\tusing const_iterator = std::vector::const_iterator;\n> >\n> > Have you considered giving this class iterator traits and implement\n> > methods required for at least, say, ForwardIterator?\n> > http://www.cplusplus.com/reference/iterator/ForwardIterator/\n> >\n> > That would make possible for applications to use STL library functions\n> > that operates on iterators on the class. In example, can you use\n> > range-based for loops on this class?\n>\n> This is a full fledge iterator already that support\n> random_access_iterator_tag. It provides the proper begin() and end()\n> functions which proxies the iterators for order_ which is a std::vector\n> :-)\n\nDo you mean the CameraConfiguration class is a\nrandome_access_iterator?\nI see a lot of methods listed here but not supported by the class\nhttp://www.cplusplus.com/reference/iterator/RandomAccessIterator/\n\nIf you forward them to a standard vector, then fine.\n\nAnyway, this is good enough for now most probably.\n\nThanks\n j\n\n>\n> The STL library functions such as ranged based loops works and are\n> already used in the next patch of this series on this data type.\n>\n> >\n> > > +\n> > > +\tCameraConfiguration();\n> > > +\n> > > +\titerator begin();\n> > > +\titerator end();\n> > > +\tconst_iterator begin() const;\n> > > +\tconst_iterator end() const;\n> > > +\n> > > +\tbool empty() const;\n> > > +\tstd::size_t size() const;\n> > > +\n> > > +\tStream *front();\n> > > +\n> > > +\tStreamConfiguration &operator[](Stream *stream);\n> > > +\n> > > +private:\n> > > +\tstd::vector order_;\n> > > +\tstd::map config_;\n> >\n> > I wonder if keeping the Stream * in two separate places might lead to\n> > issues in keeping the two in sync. But these are privates, so it\n> > should be fine...\n> >\n> > > +};\n> > > +\n> > > class Camera final\n> > > {\n> > > public:\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index 63fde0ffc3d02d6c..16162c524297012f 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -39,6 +39,127 @@ namespace libcamera {\n> > >\n> > > LOG_DECLARE_CATEGORY(Camera)\n> > >\n> > > +/**\n> > > + * \\class CameraConfiguration\n> > > + * \\brief Hold configuration for streams of the camera that applications\n> > I would drop what's after the \"of the camera\".\n> > > + * wish to modify and apply.\n> > > + *\n> > > + * The CameraConfiguration is filled with information by the application either\n> > > + * manually or with the streamConfiguration() helper. The helper takes a list\n> > > + * list of usages describing how the application intents to use the camera, the\n> > s/list list/list/\n> > s/intents/intends/\n> > s/use the camera/use the stream/\n> >\n> > > + * application in returns is provided with a default camera configuration it\n> > s/in returns/in return/\n> > > + * can tweak.\n> > > + *\n> > > + * Applications iterates over the CameraConfiguration to discover which streams\n> > s/iterates/iterate/\n> > > + * the camera have selected for its usages and can inspect the configuration\n> > s/the camera have selected for its usage/the camera has associated to a usage/\n> > s/can inspect/can access/\n> > > + * using the operator[].\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\typedef CameraConfiguration::iterator\n> > > + * \\brief Iterator for the streams in the configuration\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\typedef CameraConfiguration::const_iterator\n> > > + * \\brief Const iterator for the streams in the configuration\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Create an empty camera configuration\n> > > + */\n> > > +CameraConfiguration::CameraConfiguration()\n> > > +\t: order_({}), config_({})\n> > > +{\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Retrieve an iterator to the first element of the streams\n> > > + *\n> >\n> > The standard iterator documentation for begin reports:\n> > \"Returns an iterator pointing to the first element in the sequence\"\n> >\n> > Could we match this?\n> >\n> > > + * \\return An iterator to the first stream\n> > > + */\n> > > +std::vector::iterator CameraConfiguration::begin()\n> > > +{\n> > > +\treturn order_.begin();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Retrieve an iterator to the end of the streams\n> >\n> > Same as per 'begin()':\n> >\n> > \"Returns an iterator pointing to the past-the-end element in the sequence\"\n> >\n> > > + *\n> > > + * \\return An iterator to the element following the last stream\n> > > + */\n> > > +std::vector::iterator CameraConfiguration::end()\n> > > +{\n> > > +\treturn order_.end();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Retrieve an iterator to the first element of the streams\n> > > + *\n> > > + * \\return An iterator to the first stream\n> > > + */\n> > > +std::vector::const_iterator CameraConfiguration::begin() const\n> > > +{\n> > > +\treturn order_.begin();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Retrieve an iterator to the end of the streams\n> > > + *\n> > > + * \\return An iterator to the element following the last stream\n> > > + */\n> > > +std::vector::const_iterator CameraConfiguration::end() const\n> > > +{\n> > > +\treturn order_.end();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Checks whether the camera configuration is empty\n> > > + *\n> > > + * \\return True if the configuration is empty\n> > > + */\n> > > +bool CameraConfiguration::empty() const\n> > > +{\n> > > +\treturn order_.empty();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Check the number of stream configurations\n> > > + *\n> > > + * \\return Number of stream configurations\n> > > + */\n> > > +std::size_t CameraConfiguration::size() const\n> > > +{\n> > > +\treturn order_.size();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Access the first stream in the configuration\n> > > + *\n> > > + * \\return The first stream in the configuration\n> > > + */\n> > > +Stream *CameraConfiguration::front()\n> > > +{\n> > > +\treturn order_.front();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Retrieve a reference to a stream configuration\n> > > + * \\param[in] stream Stream to retrieve configuration for\n> > > + *\n> > > + * If the camera configuration do not yet contain configuration for the\n> >\n> > contain a configuration\n> >\n> > > + * requested stream an empty stream configuration is created and returned.\n> >\n> > empty one\n> >\n> > > + *\n> > > + * \\return Configuration for the stream\n> >\n> > The configuration\n> >\n> > > + */\n> > > +StreamConfiguration &CameraConfiguration::operator[](Stream *stream)\n> > > +{\n> > > +\tif (config_.find(stream) == config_.end())\n> > > +\t\torder_.push_back(stream);\n> > > +\n> > > +\treturn config_[stream];\n> > > +}\n> > > +\n> >\n> > I don't know how welcome would the idea of making of this class a\n> > full-fledged iterator, but I think increment, decrement and access by\n> > integer index should be implemented to make it more useful. Have you\n> > considered that?\n>\n> As stated above it is already implemented. Well not integer indexed as\n> the whole idea if this object is to make it Stream* indexed but preserve\n> the insertion order when iterating over it to allow mapping stream\n> usages to stream configurations.\n>\n> >\n> > Thanks\n> > j\n> >\n> > > /**\n> > > * \\class Camera\n> > > * \\brief Camera device\n> > > --\n> > > 2.21.0\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 relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6DD8E60DB3\n\tfor ;\n\tFri, 5 Apr 2019 13:11:26 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id CA2D3FF80D;\n\tFri, 5 Apr 2019 11:11:24 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 5 Apr 2019 13:12:11 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190405111211.di7iecvze3p5r5ge@uno.localdomain>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-8-niklas.soderlund@ragnatech.se>\n\t<20190405094746.s4da4bayokhvnodb@uno.localdomain>\n\t<20190405102106.GU23466@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"2lh2s2zidkrayojk\"","Content-Disposition":"inline","In-Reply-To":"<20190405102106.GU23466@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","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":"Fri, 05 Apr 2019 11:11:26 -0000"}},{"id":1283,"web_url":"https://patchwork.libcamera.org/comment/1283/","msgid":"<20190405120159.GE23466@bigcity.dyn.berto.se>","date":"2019-04-05T12:01:59","subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","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-04-05 13:12:11 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Fri, Apr 05, 2019 at 12:21:07PM +0200, Niklas Söderlund wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for your feedback. I will just address conceptual thing in this\n> > reply and circle back to the nitty-gritty details in an other reply :-)\n> >\n> > On 2019-04-05 11:47:46 +0200, Jacopo Mondi wrote:\n> > > Hi Niklas,\n> > > thanks for the patch.\n> > >\n> > > On Fri, Apr 05, 2019 at 04:02:55AM +0200, Niklas Söderlund wrote:\n> > > > To properly support both multiple streams and usages the library must\n> > > > provide a method to map the stream usages to the returned stream\n> > > > configuration. Add a camera configuration object to handle this mapping.\n> > > >\n> > > > Applications can iterate over the returned camera configuration to\n> > > > retrieve the streams selected by the library in the same order as the\n> > > > usages it provided to the library. Further more the application can use\n> > >\n> > > \"in the same order as it provided usages to the library\"\n> > >\n> > > furthermore\n> > >\n> > > > the streams to retrieve there configuration using operator[] of the\n> > >\n> > > \"a stream pointer to retrieve its configuration\" or\n> > > \"the streams to retrieve their configuration\"\n> > >\n> > > > camera configuration.\n> > > >\n> > >\n> > > This is a nice and clean way to replace the cumbersome map > > StreamConfiguration> used by both streamConfiguration() and\n> > > configureStream() and makes it easy to access a configuration from a\n> > > Stream *, so it's nice. Though, I don't see how this would help in\n> > > handling the \"what stream is what\" problem at requestComplete() time.\n> > > What's missing is the association with a name/id to a configuration,\n> > > is this something planned for later or am I missing how this should be\n> > > used?\n> >\n> > This works today, a dumb way for applications to implement this could\n> > look like:\n> >\n> > /* Setup streams */\n> > CameraConfiguration config =\n> > camera->streamConfiguration({ Stream::Foo(), Stream::Bar() });\n> >\n> > camera->configureStreams(config);\n> >\n> > ...\n> >\n> > camera->requestCompleted.connect(requestComplete);\n> >\n> > .. star camera etc...\n> >\n> >\n> > void requestComplete(Request *request, const std::map &buffers)\n> > {\n> > int i = 0;\n> > for (stream : config) {\n> > if (buffers.find(stream) != buffer.end())\n> > printf(\"Request contains buffer for role position %d\\n\", i);\n> >\n> > i++\n> > }\n> > }\n> >\n> > More likely the application will create its own\n> > std::map data structure and populate it\n> > once using the loop above.\n> \n> I see. So this aims to replace the id/name based stream identification\n> in general.\n\nYes the id/name idea as we discussed yesterday was a hack idea we picked \nto be able to move forward and not the end design of the API. While \nworking on implementing that I grew sick of how ugly it was and did this \ninstead :-)\n\n> \n> Does this supersedes the idea of having configureStream() assign\n> id/names to streams configuration? I sill like the idea of having\n> application assign ids to roles, the id is copied to the returned\n> stream configuration assigned to a stream, and copied to the stream at\n> configureStream() time, so that applications could:\n> \n> #define STREAM_VF 0\n> #define STREAM_STILL 1\n> \n> CameraConfiguration config = streamConfiguration({STREAM_VF,\n> Viewfinder(640x480),\n> {STREAM_STILL,\n> StillCapture()});\n> ...\n> \n> configureStream(config); // Assign the id in the conf to the\n> // stream\n> \n> ....\n> \n> void requestComplete(Request *request, const std::map &buffers)\n> {\n> \n> for (auto it : buffers) {\n> Stream *s = it.first;\n> \n> if (s.id == STREAM_VF)\n> //display\n> if (s.id == STREAM_STILL)\n> //save to disk\n> }\n> }\n> \n> They could most probably handle that internally with a map or\n> something indeed, but this creates a logical connection between the\n> intended role and an id applications can control.\n\nApplications can still do that using my previous example, it just needs \nto create a std::map. The ID we use for streams \nthru out the library is the Stream *. We could add what you suggests as \na convenience for applications but until we become more mature in the \nAPI this solves the problem and adding things on-top feels wrong at this \ntime.\n\nAlso I don't like the example you have above for streamConfiguration(). \nIt's looks to cumbersome to be nice for applications, but I might be \nwrong :-)\n\n> >\n> > >\n> > > > Signed-off-by: Niklas Söderlund \n> > > > ---\n> > > > include/libcamera/camera.h | 26 ++++++++\n> > > > src/libcamera/camera.cpp | 121 +++++++++++++++++++++++++++++++++++++\n> > > > 2 files changed, 147 insertions(+)\n> > > >\n> > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > > index 0386671c902e55e8..311a51e4070d135c 100644\n> > > > --- a/include/libcamera/camera.h\n> > > > +++ b/include/libcamera/camera.h\n> > > > @@ -14,6 +14,7 @@\n> > > >\n> > > > #include \n> > > > #include \n> > > > +#include \n> > > >\n> > > > namespace libcamera {\n> > > >\n> > > > @@ -24,6 +25,31 @@ class Stream;\n> > > > class StreamConfiguration;\n> > > > class StreamUsage;\n> > > >\n> > > > +class CameraConfiguration\n> > > > +{\n> > > > +public:\n> > > > +\tusing iterator = std::vector::iterator;\n> > > > +\tusing const_iterator = std::vector::const_iterator;\n> > >\n> > > Have you considered giving this class iterator traits and implement\n> > > methods required for at least, say, ForwardIterator?\n> > > http://www.cplusplus.com/reference/iterator/ForwardIterator/\n> > >\n> > > That would make possible for applications to use STL library functions\n> > > that operates on iterators on the class. In example, can you use\n> > > range-based for loops on this class?\n> >\n> > This is a full fledge iterator already that support\n> > random_access_iterator_tag. It provides the proper begin() and end()\n> > functions which proxies the iterators for order_ which is a std::vector\n> > :-)\n> \n> Do you mean the CameraConfiguration class is a\n> randome_access_iterator?\n\nNo CameraConfiguration::iterator is.\n\n> I see a lot of methods listed here but not supported by the class\n> http://www.cplusplus.com/reference/iterator/RandomAccessIterator/\n\nThese methods shall be implemented on the iterator not on the object. \nAnd they are implemented for CameraConfiguration::iterator as that is an \nalias for std::vector::iterator, which is a \nRandomAccessIterator.\n\n> \n> If you forward them to a standard vector, then fine.\n> \n> Anyway, this is good enough for now most probably.\n> \n> Thanks\n> j\n> \n> >\n> > The STL library functions such as ranged based loops works and are\n> > already used in the next patch of this series on this data type.\n> >\n> > >\n> > > > +\n> > > > +\tCameraConfiguration();\n> > > > +\n> > > > +\titerator begin();\n> > > > +\titerator end();\n> > > > +\tconst_iterator begin() const;\n> > > > +\tconst_iterator end() const;\n> > > > +\n> > > > +\tbool empty() const;\n> > > > +\tstd::size_t size() const;\n> > > > +\n> > > > +\tStream *front();\n> > > > +\n> > > > +\tStreamConfiguration &operator[](Stream *stream);\n> > > > +\n> > > > +private:\n> > > > +\tstd::vector order_;\n> > > > +\tstd::map config_;\n> > >\n> > > I wonder if keeping the Stream * in two separate places might lead to\n> > > issues in keeping the two in sync. But these are privates, so it\n> > > should be fine...\n> > >\n> > > > +};\n> > > > +\n> > > > class Camera final\n> > > > {\n> > > > public:\n> > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > index 63fde0ffc3d02d6c..16162c524297012f 100644\n> > > > --- a/src/libcamera/camera.cpp\n> > > > +++ b/src/libcamera/camera.cpp\n> > > > @@ -39,6 +39,127 @@ namespace libcamera {\n> > > >\n> > > > LOG_DECLARE_CATEGORY(Camera)\n> > > >\n> > > > +/**\n> > > > + * \\class CameraConfiguration\n> > > > + * \\brief Hold configuration for streams of the camera that applications\n> > > I would drop what's after the \"of the camera\".\n> > > > + * wish to modify and apply.\n> > > > + *\n> > > > + * The CameraConfiguration is filled with information by the application either\n> > > > + * manually or with the streamConfiguration() helper. The helper takes a list\n> > > > + * list of usages describing how the application intents to use the camera, the\n> > > s/list list/list/\n> > > s/intents/intends/\n> > > s/use the camera/use the stream/\n> > >\n> > > > + * application in returns is provided with a default camera configuration it\n> > > s/in returns/in return/\n> > > > + * can tweak.\n> > > > + *\n> > > > + * Applications iterates over the CameraConfiguration to discover which streams\n> > > s/iterates/iterate/\n> > > > + * the camera have selected for its usages and can inspect the configuration\n> > > s/the camera have selected for its usage/the camera has associated to a usage/\n> > > s/can inspect/can access/\n> > > > + * using the operator[].\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\typedef CameraConfiguration::iterator\n> > > > + * \\brief Iterator for the streams in the configuration\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\typedef CameraConfiguration::const_iterator\n> > > > + * \\brief Const iterator for the streams in the configuration\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\brief Create an empty camera configuration\n> > > > + */\n> > > > +CameraConfiguration::CameraConfiguration()\n> > > > +\t: order_({}), config_({})\n> > > > +{\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Retrieve an iterator to the first element of the streams\n> > > > + *\n> > >\n> > > The standard iterator documentation for begin reports:\n> > > \"Returns an iterator pointing to the first element in the sequence\"\n> > >\n> > > Could we match this?\n> > >\n> > > > + * \\return An iterator to the first stream\n> > > > + */\n> > > > +std::vector::iterator CameraConfiguration::begin()\n> > > > +{\n> > > > +\treturn order_.begin();\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Retrieve an iterator to the end of the streams\n> > >\n> > > Same as per 'begin()':\n> > >\n> > > \"Returns an iterator pointing to the past-the-end element in the sequence\"\n> > >\n> > > > + *\n> > > > + * \\return An iterator to the element following the last stream\n> > > > + */\n> > > > +std::vector::iterator CameraConfiguration::end()\n> > > > +{\n> > > > +\treturn order_.end();\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Retrieve an iterator to the first element of the streams\n> > > > + *\n> > > > + * \\return An iterator to the first stream\n> > > > + */\n> > > > +std::vector::const_iterator CameraConfiguration::begin() const\n> > > > +{\n> > > > +\treturn order_.begin();\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Retrieve an iterator to the end of the streams\n> > > > + *\n> > > > + * \\return An iterator to the element following the last stream\n> > > > + */\n> > > > +std::vector::const_iterator CameraConfiguration::end() const\n> > > > +{\n> > > > +\treturn order_.end();\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Checks whether the camera configuration is empty\n> > > > + *\n> > > > + * \\return True if the configuration is empty\n> > > > + */\n> > > > +bool CameraConfiguration::empty() const\n> > > > +{\n> > > > +\treturn order_.empty();\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Check the number of stream configurations\n> > > > + *\n> > > > + * \\return Number of stream configurations\n> > > > + */\n> > > > +std::size_t CameraConfiguration::size() const\n> > > > +{\n> > > > +\treturn order_.size();\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Access the first stream in the configuration\n> > > > + *\n> > > > + * \\return The first stream in the configuration\n> > > > + */\n> > > > +Stream *CameraConfiguration::front()\n> > > > +{\n> > > > +\treturn order_.front();\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\brief Retrieve a reference to a stream configuration\n> > > > + * \\param[in] stream Stream to retrieve configuration for\n> > > > + *\n> > > > + * If the camera configuration do not yet contain configuration for the\n> > >\n> > > contain a configuration\n> > >\n> > > > + * requested stream an empty stream configuration is created and returned.\n> > >\n> > > empty one\n> > >\n> > > > + *\n> > > > + * \\return Configuration for the stream\n> > >\n> > > The configuration\n> > >\n> > > > + */\n> > > > +StreamConfiguration &CameraConfiguration::operator[](Stream *stream)\n> > > > +{\n> > > > +\tif (config_.find(stream) == config_.end())\n> > > > +\t\torder_.push_back(stream);\n> > > > +\n> > > > +\treturn config_[stream];\n> > > > +}\n> > > > +\n> > >\n> > > I don't know how welcome would the idea of making of this class a\n> > > full-fledged iterator, but I think increment, decrement and access by\n> > > integer index should be implemented to make it more useful. Have you\n> > > considered that?\n> >\n> > As stated above it is already implemented. Well not integer indexed as\n> > the whole idea if this object is to make it Stream* indexed but preserve\n> > the insertion order when iterating over it to allow mapping stream\n> > usages to stream configurations.\n> >\n> > >\n> > > Thanks\n> > > j\n> > >\n> > > > /**\n> > > > * \\class Camera\n> > > > * \\brief Camera device\n> > > > --\n> > > > 2.21.0\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 mail-lf1-f45.google.com (mail-lf1-f45.google.com\n\t[209.85.167.45])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F3B4B60DB3\n\tfor ;\n\tFri, 5 Apr 2019 14:03:01 +0200 (CEST)","by mail-lf1-f45.google.com with SMTP id v71so4171331lfa.11\n\tfor ;\n\tFri, 05 Apr 2019 05:03:01 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tl5sm337871lfh.70.2019.04.05.05.02.00\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 05 Apr 2019 05:02:00 -0700 (PDT)"],"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=WhkLOaK4bEjelE5WqLNLlMfcNW4D+AP5eVO3eixTugw=;\n\tb=E/+y49lk2Xp9UbsEULi8vilj7Noj6stuo2PtWkj1zPMGDIl1LhTs8F32KrheIX6QR7\n\tvpWpyI0IULhLAwF1Ust3jLATZN12SzO1ZRQN0O2Bhu8jvkv6iOj0U5V0rBfPOn23jo3z\n\twEvXQHcnazc4PAWnBv5rTQsLxKJf3xoGzz6TN5PdGiWO5uYyJn7R31KIzw6ikfu11n9X\n\t4f/Cz9A3tg8NV7PCZ3WqMOcdodtqC43WjfbZ1WKOh201Qihe3DKzADssDsh49w9JjtU+\n\t7RPVU0U629cUpAurBNzP99wDk022w4OV7FdCI6qH2m/H+lv7oVQepMoSa8Up8Cx85OaB\n\t/L3A==","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=WhkLOaK4bEjelE5WqLNLlMfcNW4D+AP5eVO3eixTugw=;\n\tb=nGtmkeQtzfKTPDDb1OnHGKHWz7Q2lzw6hZ3x6IQD442skWMVaEr9Eb3IuDC9x6OBf3\n\tqTF1TKjok0EHnA6CRQFyWCTg1IIwE3YLep7jkp/QNnvC6wuv4j0SambK3C3tES8Twzgk\n\tvtdldGRainMI2JxG9tt+1y8jbwwl7/ZDE6FTGEfOTMxm5qcV3QwaDSNRcu7vjebpoIbt\n\tH5fKATzq4BbT53pR8AsrhxCaDC547kNWmoByRFol/0JdGhznehJ1vTKOXc/YqbUfhDEa\n\tE2hP5xYXEZBb10tqntXpRkLojuwfL18suL8fxy0Cv3uu6Sae/Ygqz8pgOw1n8evW0XKM\n\tRKTg==","X-Gm-Message-State":"APjAAAVzjLDsBI09SILefpFPq2GKMD2Gks91FdgqgxUGdrRXSEgeQ8+8\n\t8pNwnSG6DgcHGGQSotdOEe6RvAAf7ww=","X-Google-Smtp-Source":"APXvYqxL/2bfxDmE3lqyjlFdgZZ/h4qyid4DP4BOpSOQtlQeu1OixLm2GBMm7tS/e87lARt2KSg6GQ==","X-Received":"by 2002:ac2:5325:: with SMTP id f5mr6876805lfh.77.1554465721253; \n\tFri, 05 Apr 2019 05:02:01 -0700 (PDT)","Date":"Fri, 5 Apr 2019 14:01:59 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190405120159.GE23466@bigcity.dyn.berto.se>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-8-niklas.soderlund@ragnatech.se>\n\t<20190405094746.s4da4bayokhvnodb@uno.localdomain>\n\t<20190405102106.GU23466@bigcity.dyn.berto.se>\n\t<20190405111211.di7iecvze3p5r5ge@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190405111211.di7iecvze3p5r5ge@uno.localdomain>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","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":"Fri, 05 Apr 2019 12:03:02 -0000"}},{"id":1292,"web_url":"https://patchwork.libcamera.org/comment/1292/","msgid":"<20190405154547.GE5184@pendragon.ideasonboard.com>","date":"2019-04-05T15:45:47","subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","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 Fri, Apr 05, 2019 at 01:12:11PM +0200, Jacopo Mondi wrote:\n> On Fri, Apr 05, 2019 at 12:21:07PM +0200, Niklas Söderlund wrote:\n> > On 2019-04-05 11:47:46 +0200, Jacopo Mondi wrote:\n> >> On Fri, Apr 05, 2019 at 04:02:55AM +0200, Niklas Söderlund wrote:\n> >>> To properly support both multiple streams and usages the library must\n\ns/usages/stream usages/\n\n> >>> provide a method to map the stream usages to the returned stream\n> >>> configuration. Add a camera configuration object to handle this mapping.\n\ns/stream configuration/streams configurations/\n\n> >>>\n> >>> Applications can iterate over the returned camera configuration to\n> >>> retrieve the streams selected by the library in the same order as the\n> >>> usages it provided to the library. Further more the application can use\n> >>\n> >> \"in the same order as it provided usages to the library\"\n\nI'd side with Niklas on this one, your version makes \"order\" refer to\n\"provided\", while I think it should refer to \"usages\" (we provide all\nusages in one go).\n\n> >>\n> >> furthermore\n> >>\n> >>> the streams to retrieve there configuration using operator[] of the\n> >>\n> >> \"a stream pointer to retrieve its configuration\" or\n> >> \"the streams to retrieve their configuration\"\n> >>\n> >>> camera configuration.\n> >>>\n> >>\n> >> This is a nice and clean way to replace the cumbersome map >> StreamConfiguration> used by both streamConfiguration() and\n> >> configureStream() and makes it easy to access a configuration from a\n> >> Stream *, so it's nice. Though, I don't see how this would help in\n> >> handling the \"what stream is what\" problem at requestComplete() time.\n> >> What's missing is the association with a name/id to a configuration,\n> >> is this something planned for later or am I missing how this should be\n> >> used?\n> >\n> > This works today, a dumb way for applications to implement this could\n> > look like:\n> >\n> > /* Setup streams */\n> > CameraConfiguration config =\n> > camera->streamConfiguration({ Stream::Foo(), Stream::Bar() });\n> >\n> > camera->configureStreams(config);\n> >\n> > ...\n> >\n> > camera->requestCompleted.connect(requestComplete);\n> >\n> > .. star camera etc...\n> >\n> >\n> > void requestComplete(Request *request, const std::map &buffers)\n> > {\n> > int i = 0;\n> > for (stream : config) {\n> > if (buffers.find(stream) != buffer.end())\n> > printf(\"Request contains buffer for role position %d\\n\", i);\n> >\n> > i++\n> > }\n> > }\n> >\n> > More likely the application will create its own\n> > std::map data structure and populate it\n> > once using the loop above.\n> \n> I see. So this aims to replace the id/name based stream identification\n> in general.\n> \n> Does this supersedes the idea of having configureStream() assign\n> id/names to streams configuration? I sill like the idea of having\n> application assign ids to roles, the id is copied to the returned\n> stream configuration assigned to a stream, and copied to the stream at\n> configureStream() time, so that applications could:\n> \n> #define STREAM_VF 0\n> #define STREAM_STILL 1\n> \n> CameraConfiguration config = streamConfiguration({STREAM_VF,\n> Viewfinder(640x480),\n> {STREAM_STILL,\n> StillCapture()});\n> ...\n> \n> configureStream(config); // Assign the id in the conf to the\n> // stream\n> \n> ....\n> \n> void requestComplete(Request *request, const std::map &buffers)\n> {\n> \n> for (auto it : buffers) {\n> Stream *s = it.first;\n> \n> if (s.id == STREAM_VF)\n> //display\n> if (s.id == STREAM_STILL)\n> //save to disk\n> }\n> }\n> \n> They could most probably handle that internally with a map or\n> something indeed, but this creates a logical connection between the\n> intended role and an id applications can control.\n\nWhile I agree the above is nice, I think the CameraConfiguration class\nis good for now. We may add IDs later if needed, based on how code will\nlook like in our applications.\n\nBy the way, the above application code could also do\n\nconst Stream *streamViewfinder = nullptr;\nconst Stream *streamStill = nullptr;\n\n\t...\n\tCameraConfiguration config = streamConfiguration({Viewfinder(640x480), StillCapture()});\n\tstreamViewfinder = config[0];\n\tstreamStill = config[1];\n\t...\n\n\tvoid requestComplete(Request *request, const std::map &buffers)\n\t{\n\t\tfor (auto it : buffers) {\n\t\t\tStream *s = it.first;\n\n\t\t\tif (s == streamViewfinder)\n\t\t\t\t//display\n\t\t\tif (s == streamStill)\n\t\t\t\t//save to disk\n\t\t}\n\t}\n\nif we added a\n\n\tStream *operator[](int index);\n\noperator to the CameraConfiguration class.\n\n> >>> Signed-off-by: Niklas Söderlund \n> >>> ---\n> >>> include/libcamera/camera.h | 26 ++++++++\n> >>> src/libcamera/camera.cpp | 121 +++++++++++++++++++++++++++++++++++++\n> >>> 2 files changed, 147 insertions(+)\n> >>>\n> >>> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> >>> index 0386671c902e55e8..311a51e4070d135c 100644\n> >>> --- a/include/libcamera/camera.h\n> >>> +++ b/include/libcamera/camera.h\n> >>> @@ -14,6 +14,7 @@\n> >>>\n> >>> #include \n> >>> #include \n> >>> +#include \n\nIs this needed ? I think you only use Stream pointers in this header.\n\n> >>>\n> >>> namespace libcamera {\n> >>>\n> >>> @@ -24,6 +25,31 @@ class Stream;\n> >>> class StreamConfiguration;\n> >>> class StreamUsage;\n> >>>\n> >>> +class CameraConfiguration\n> >>> +{\n> >>> +public:\n> >>> +\tusing iterator = std::vector::iterator;\n> >>> +\tusing const_iterator = std::vector::const_iterator;\n> >>\n> >> Have you considered giving this class iterator traits and implement\n> >> methods required for at least, say, ForwardIterator?\n> >> http://www.cplusplus.com/reference/iterator/ForwardIterator/\n> >>\n> >> That would make possible for applications to use STL library functions\n> >> that operates on iterators on the class. In example, can you use\n> >> range-based for loops on this class?\n> >\n> > This is a full fledge iterator already that support\n> > random_access_iterator_tag. It provides the proper begin() and end()\n> > functions which proxies the iterators for order_ which is a std::vector\n> > :-)\n> \n> Do you mean the CameraConfiguration class is a\n> randome_access_iterator?\n> I see a lot of methods listed here but not supported by the class\n> http://www.cplusplus.com/reference/iterator/RandomAccessIterator/\n> \n> If you forward them to a standard vector, then fine.\n> \n> Anyway, this is good enough for now most probably.\n> \n> > The STL library functions such as ranged based loops works and are\n> > already used in the next patch of this series on this data type.\n> >\n> >>> +\n> >>> +\tCameraConfiguration();\n> >>> +\n> >>> +\titerator begin();\n> >>> +\titerator end();\n> >>> +\tconst_iterator begin() const;\n> >>> +\tconst_iterator end() const;\n> >>> +\n> >>> +\tbool empty() const;\n> >>> +\tstd::size_t size() const;\n> >>> +\n> >>> +\tStream *front();\n> >>> +\n> >>> +\tStreamConfiguration &operator[](Stream *stream);\n> >>> +\n> >>> +private:\n> >>> +\tstd::vector order_;\n> >>> +\tstd::map config_;\n> >>\n> >> I wonder if keeping the Stream * in two separate places might lead to\n> >> issues in keeping the two in sync. But these are privates, so it\n> >> should be fine...\n> >>\n> >>> +};\n> >>> +\n> >>> class Camera final\n> >>> {\n> >>> public:\n> >>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> >>> index 63fde0ffc3d02d6c..16162c524297012f 100644\n> >>> --- a/src/libcamera/camera.cpp\n> >>> +++ b/src/libcamera/camera.cpp\n> >>> @@ -39,6 +39,127 @@ namespace libcamera {\n> >>>\n> >>> LOG_DECLARE_CATEGORY(Camera)\n> >>>\n> >>> +/**\n> >>> + * \\class CameraConfiguration\n> >>> + * \\brief Hold configuration for streams of the camera that applications\n> >> \n> >> I would drop what's after the \"of the camera\".\n> >> \n> >>> + * wish to modify and apply.\n> >>> + *\n> >>> + * The CameraConfiguration is filled with information by the application either\n> >>> + * manually or with the streamConfiguration() helper. The helper takes a list\n> >>> + * list of usages describing how the application intents to use the camera, the\n> >> \n> >> s/list list/list/\n> >> s/intents/intends/\n> >> s/use the camera/use the stream/\n\ns/use the camera/use the streams/\n\nAlthough \"use streams of the camera\" may be better, as the helper\ndoesn't receive streams.\n\n> >>> + * application in returns is provided with a default camera configuration it\n> >> \n> >> s/in returns/in return/\n> >> \n> >>> + * can tweak.\n\ns/tweak/tune/\n\n> >>> + *\n> >>> + * Applications iterates over the CameraConfiguration to discover which streams\n> >> \n> >> s/iterates/iterate/\n> >> \n> >>> + * the camera have selected for its usages and can inspect the configuration\n> >> \n> >> s/the camera have selected for its usage/the camera has associated to a usage/\n\n\"to the usages\"\n\n> >> s/can inspect/can access/\n> >> \n> >>> + * using the operator[].\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\typedef CameraConfiguration::iterator\n> >>> + * \\brief Iterator for the streams in the configuration\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\typedef CameraConfiguration::const_iterator\n> >>> + * \\brief Const iterator for the streams in the configuration\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\brief Create an empty camera configuration\n> >>> + */\n> >>> +CameraConfiguration::CameraConfiguration()\n> >>> +\t: order_({}), config_({})\n> >>> +{\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Retrieve an iterator to the first element of the streams\n> >>> + *\n> >>\n> >> The standard iterator documentation for begin reports:\n> >> \"Returns an iterator pointing to the first element in the sequence\"\n> >>\n> >> Could we match this?\n> >>\n> >>> + * \\return An iterator to the first stream\n> >>> + */\n> >>> +std::vector::iterator CameraConfiguration::begin()\n> >>> +{\n> >>> +\treturn order_.begin();\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Retrieve an iterator to the end of the streams\n> >>\n> >> Same as per 'begin()':\n> >>\n> >> \"Returns an iterator pointing to the past-the-end element in the sequence\"\n> >>\n> >>> + *\n> >>> + * \\return An iterator to the element following the last stream\n> >>> + */\n> >>> +std::vector::iterator CameraConfiguration::end()\n> >>> +{\n> >>> +\treturn order_.end();\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Retrieve an iterator to the first element of the streams\n> >>> + *\n> >>> + * \\return An iterator to the first stream\n> >>> + */\n> >>> +std::vector::const_iterator CameraConfiguration::begin() const\n> >>> +{\n> >>> +\treturn order_.begin();\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Retrieve an iterator to the end of the streams\n> >>> + *\n> >>> + * \\return An iterator to the element following the last stream\n> >>> + */\n> >>> +std::vector::const_iterator CameraConfiguration::end() const\n> >>> +{\n> >>> +\treturn order_.end();\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Checks whether the camera configuration is empty\n\ns/whether/if/\n\nKieran recently taught me that \"whether\" requires two clauses (whether A\nor B).\n\n> >>> + *\n> >>> + * \\return True if the configuration is empty\n> >>> + */\n> >>> +bool CameraConfiguration::empty() const\n> >>> +{\n> >>> +\treturn order_.empty();\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Check the number of stream configurations\n\ns/Check/Retrieve/\n\n> >>> + *\n> >>> + * \\return Number of stream configurations\n> >>> + */\n> >>> +std::size_t CameraConfiguration::size() const\n> >>> +{\n> >>> +\treturn order_.size();\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Access the first stream in the configuration\n> >>> + *\n> >>> + * \\return The first stream in the configuration\n> >>> + */\n> >>> +Stream *CameraConfiguration::front()\n> >>> +{\n> >>> +\treturn order_.front();\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Retrieve a reference to a stream configuration\n> >>> + * \\param[in] stream Stream to retrieve configuration for\n> >>> + *\n> >>> + * If the camera configuration do not yet contain configuration for the\n> >>\n> >> contain a configuration\n> >>\n> >>> + * requested stream an empty stream configuration is created and returned.\n> >>\n> >> empty one\n\n\"If the camera configuration does not yet contain a configuration for\nthe requested stream, create and return an empty stream configuration.\"\n\n> >>\n> >>> + *\n> >>> + * \\return Configuration for the stream\n> >>\n> >> The configuration\n> >>\n> >>> + */\n> >>> +StreamConfiguration &CameraConfiguration::operator[](Stream *stream)\n> >>> +{\n> >>> +\tif (config_.find(stream) == config_.end())\n> >>> +\t\torder_.push_back(stream);\n> >>> +\n> >>> +\treturn config_[stream];\n> >>> +}\n> >>> +\n> >>\n> >> I don't know how welcome would the idea of making of this class a\n> >> full-fledged iterator, but I think increment, decrement and access by\n> >> integer index should be implemented to make it more useful. Have you\n> >> considered that?\n> >\n> > As stated above it is already implemented. Well not integer indexed as\n> > the whole idea if this object is to make it Stream* indexed but preserve\n> > the insertion order when iterating over it to allow mapping stream\n> > usages to stream configurations.\n> >\n> >>> /**\n> >>> * \\class Camera\n> >>> * \\brief Camera device","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2AEBE60DB3\n\tfor ;\n\tFri, 5 Apr 2019 17:46:06 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [109.140.214.47])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5BC89E2;\n\tFri, 5 Apr 2019 17:45:58 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1554479158;\n\tbh=uvhdzZZbLc7b759VQyehkWzw7pThF23jJVsuJb8LsPc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=m0PJc0qtOEsADCWsgrcGQGE8Ntni+kA5fk+Hnp5Z0laJpl8wdGsVPjkPMZvmRxFOp\n\ttNN2KczH71FW9fs8RbpCER62+TRkwDyLfGaseNn5yYgM24DvSMZAkvXmJ2e5tF6HwB\n\tbWXNvnI1Y/+m6sPFDAfvTtGLZnMvjKaeF9gbN2TM=","Date":"Fri, 5 Apr 2019 18:45:47 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","Message-ID":"<20190405154547.GE5184@pendragon.ideasonboard.com>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-8-niklas.soderlund@ragnatech.se>\n\t<20190405094746.s4da4bayokhvnodb@uno.localdomain>\n\t<20190405102106.GU23466@bigcity.dyn.berto.se>\n\t<20190405111211.di7iecvze3p5r5ge@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190405111211.di7iecvze3p5r5ge@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","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":"Fri, 05 Apr 2019 15:46:06 -0000"}},{"id":1294,"web_url":"https://patchwork.libcamera.org/comment/1294/","msgid":"<20190405212023.GA15350@bigcity.dyn.berto.se>","date":"2019-04-05T21:20:23","subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo, Laurent,\n\nThanks for all your feedback, specially the ones improving my \ndocumentation. I have taken all your comments in this area in the next \nversion.\n\nOn 2019-04-05 18:45:47 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Fri, Apr 05, 2019 at 01:12:11PM +0200, Jacopo Mondi wrote:\n> > On Fri, Apr 05, 2019 at 12:21:07PM +0200, Niklas Söderlund wrote:\n> > > On 2019-04-05 11:47:46 +0200, Jacopo Mondi wrote:\n> > >> On Fri, Apr 05, 2019 at 04:02:55AM +0200, Niklas Söderlund wrote:\n> > >>> To properly support both multiple streams and usages the library must\n> \n> s/usages/stream usages/\n> \n> > >>> provide a method to map the stream usages to the returned stream\n> > >>> configuration. Add a camera configuration object to handle this mapping.\n> \n> s/stream configuration/streams configurations/\n> \n> > >>>\n> > >>> Applications can iterate over the returned camera configuration to\n> > >>> retrieve the streams selected by the library in the same order as the\n> > >>> usages it provided to the library. Further more the application can use\n> > >>\n> > >> \"in the same order as it provided usages to the library\"\n> \n> I'd side with Niklas on this one, your version makes \"order\" refer to\n> \"provided\", while I think it should refer to \"usages\" (we provide all\n> usages in one go).\n> \n> > >>\n> > >> furthermore\n> > >>\n> > >>> the streams to retrieve there configuration using operator[] of the\n> > >>\n> > >> \"a stream pointer to retrieve its configuration\" or\n> > >> \"the streams to retrieve their configuration\"\n> > >>\n> > >>> camera configuration.\n> > >>>\n> > >>\n> > >> This is a nice and clean way to replace the cumbersome map > >> StreamConfiguration> used by both streamConfiguration() and\n> > >> configureStream() and makes it easy to access a configuration from a\n> > >> Stream *, so it's nice. Though, I don't see how this would help in\n> > >> handling the \"what stream is what\" problem at requestComplete() time.\n> > >> What's missing is the association with a name/id to a configuration,\n> > >> is this something planned for later or am I missing how this should be\n> > >> used?\n> > >\n> > > This works today, a dumb way for applications to implement this could\n> > > look like:\n> > >\n> > > /* Setup streams */\n> > > CameraConfiguration config =\n> > > camera->streamConfiguration({ Stream::Foo(), Stream::Bar() });\n> > >\n> > > camera->configureStreams(config);\n> > >\n> > > ...\n> > >\n> > > camera->requestCompleted.connect(requestComplete);\n> > >\n> > > .. star camera etc...\n> > >\n> > >\n> > > void requestComplete(Request *request, const std::map &buffers)\n> > > {\n> > > int i = 0;\n> > > for (stream : config) {\n> > > if (buffers.find(stream) != buffer.end())\n> > > printf(\"Request contains buffer for role position %d\\n\", i);\n> > >\n> > > i++\n> > > }\n> > > }\n> > >\n> > > More likely the application will create its own\n> > > std::map data structure and populate it\n> > > once using the loop above.\n> > \n> > I see. So this aims to replace the id/name based stream identification\n> > in general.\n> > \n> > Does this supersedes the idea of having configureStream() assign\n> > id/names to streams configuration? I sill like the idea of having\n> > application assign ids to roles, the id is copied to the returned\n> > stream configuration assigned to a stream, and copied to the stream at\n> > configureStream() time, so that applications could:\n> > \n> > #define STREAM_VF 0\n> > #define STREAM_STILL 1\n> > \n> > CameraConfiguration config = streamConfiguration({STREAM_VF,\n> > Viewfinder(640x480),\n> > {STREAM_STILL,\n> > StillCapture()});\n> > ...\n> > \n> > configureStream(config); // Assign the id in the conf to the\n> > // stream\n> > \n> > ....\n> > \n> > void requestComplete(Request *request, const std::map &buffers)\n> > {\n> > \n> > for (auto it : buffers) {\n> > Stream *s = it.first;\n> > \n> > if (s.id == STREAM_VF)\n> > //display\n> > if (s.id == STREAM_STILL)\n> > //save to disk\n> > }\n> > }\n> > \n> > They could most probably handle that internally with a map or\n> > something indeed, but this creates a logical connection between the\n> > intended role and an id applications can control.\n> \n> While I agree the above is nice, I think the CameraConfiguration class\n> is good for now. We may add IDs later if needed, based on how code will\n> look like in our applications.\n> \n> By the way, the above application code could also do\n> \n> const Stream *streamViewfinder = nullptr;\n> const Stream *streamStill = nullptr;\n> \n> \t...\n> \tCameraConfiguration config = streamConfiguration({Viewfinder(640x480), StillCapture()});\n> \tstreamViewfinder = config[0];\n> \tstreamStill = config[1];\n> \t...\n> \n> \tvoid requestComplete(Request *request, const std::map &buffers)\n> \t{\n> \t\tfor (auto it : buffers) {\n> \t\t\tStream *s = it.first;\n> \n> \t\t\tif (s == streamViewfinder)\n> \t\t\t\t//display\n> \t\t\tif (s == streamStill)\n> \t\t\t\t//save to disk\n> \t\t}\n> \t}\n> \n> if we added a\n> \n> \tStream *operator[](int index);\n\nI like this, will add this for next version.\n\n> \n> operator to the CameraConfiguration class.\n> \n> > >>> Signed-off-by: Niklas Söderlund \n> > >>> ---\n> > >>> include/libcamera/camera.h | 26 ++++++++\n> > >>> src/libcamera/camera.cpp | 121 +++++++++++++++++++++++++++++++++++++\n> > >>> 2 files changed, 147 insertions(+)\n> > >>>\n> > >>> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > >>> index 0386671c902e55e8..311a51e4070d135c 100644\n> > >>> --- a/include/libcamera/camera.h\n> > >>> +++ b/include/libcamera/camera.h\n> > >>> @@ -14,6 +14,7 @@\n> > >>>\n> > >>> #include \n> > >>> #include \n> > >>> +#include \n> \n> Is this needed ? I think you only use Stream pointers in this header.\n\nNo it's not needed. It's a left over from my prototype stage, thanks for \nspotting it.\n\n> \n> > >>>\n> > >>> namespace libcamera {\n> > >>>\n> > >>> @@ -24,6 +25,31 @@ class Stream;\n> > >>> class StreamConfiguration;\n> > >>> class StreamUsage;\n> > >>>\n> > >>> +class CameraConfiguration\n> > >>> +{\n> > >>> +public:\n> > >>> +\tusing iterator = std::vector::iterator;\n> > >>> +\tusing const_iterator = std::vector::const_iterator;\n> > >>\n> > >> Have you considered giving this class iterator traits and implement\n> > >> methods required for at least, say, ForwardIterator?\n> > >> http://www.cplusplus.com/reference/iterator/ForwardIterator/\n> > >>\n> > >> That would make possible for applications to use STL library functions\n> > >> that operates on iterators on the class. In example, can you use\n> > >> range-based for loops on this class?\n> > >\n> > > This is a full fledge iterator already that support\n> > > random_access_iterator_tag. It provides the proper begin() and end()\n> > > functions which proxies the iterators for order_ which is a std::vector\n> > > :-)\n> > \n> > Do you mean the CameraConfiguration class is a\n> > randome_access_iterator?\n> > I see a lot of methods listed here but not supported by the class\n> > http://www.cplusplus.com/reference/iterator/RandomAccessIterator/\n> > \n> > If you forward them to a standard vector, then fine.\n> > \n> > Anyway, this is good enough for now most probably.\n> > \n> > > The STL library functions such as ranged based loops works and are\n> > > already used in the next patch of this series on this data type.\n> > >\n> > >>> +\n> > >>> +\tCameraConfiguration();\n> > >>> +\n> > >>> +\titerator begin();\n> > >>> +\titerator end();\n> > >>> +\tconst_iterator begin() const;\n> > >>> +\tconst_iterator end() const;\n> > >>> +\n> > >>> +\tbool empty() const;\n> > >>> +\tstd::size_t size() const;\n> > >>> +\n> > >>> +\tStream *front();\n> > >>> +\n> > >>> +\tStreamConfiguration &operator[](Stream *stream);\n> > >>> +\n> > >>> +private:\n> > >>> +\tstd::vector order_;\n> > >>> +\tstd::map config_;\n> > >>\n> > >> I wonder if keeping the Stream * in two separate places might lead to\n> > >> issues in keeping the two in sync. But these are privates, so it\n> > >> should be fine...\n> > >>\n> > >>> +};\n> > >>> +\n> > >>> class Camera final\n> > >>> {\n> > >>> public:\n> > >>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > >>> index 63fde0ffc3d02d6c..16162c524297012f 100644\n> > >>> --- a/src/libcamera/camera.cpp\n> > >>> +++ b/src/libcamera/camera.cpp\n> > >>> @@ -39,6 +39,127 @@ namespace libcamera {\n> > >>>\n> > >>> LOG_DECLARE_CATEGORY(Camera)\n> > >>>\n> > >>> +/**\n> > >>> + * \\class CameraConfiguration\n> > >>> + * \\brief Hold configuration for streams of the camera that applications\n> > >> \n> > >> I would drop what's after the \"of the camera\".\n> > >> \n> > >>> + * wish to modify and apply.\n> > >>> + *\n> > >>> + * The CameraConfiguration is filled with information by the application either\n> > >>> + * manually or with the streamConfiguration() helper. The helper takes a list\n> > >>> + * list of usages describing how the application intents to use the camera, the\n> > >> \n> > >> s/list list/list/\n> > >> s/intents/intends/\n> > >> s/use the camera/use the stream/\n> \n> s/use the camera/use the streams/\n> \n> Although \"use streams of the camera\" may be better, as the helper\n> doesn't receive streams.\n> \n> > >>> + * application in returns is provided with a default camera configuration it\n> > >> \n> > >> s/in returns/in return/\n> > >> \n> > >>> + * can tweak.\n> \n> s/tweak/tune/\n> \n> > >>> + *\n> > >>> + * Applications iterates over the CameraConfiguration to discover which streams\n> > >> \n> > >> s/iterates/iterate/\n> > >> \n> > >>> + * the camera have selected for its usages and can inspect the configuration\n> > >> \n> > >> s/the camera have selected for its usage/the camera has associated to a usage/\n> \n> \"to the usages\"\n> \n> > >> s/can inspect/can access/\n> > >> \n> > >>> + * using the operator[].\n> > >>> + */\n> > >>> +\n> > >>> +/**\n> > >>> + * \\typedef CameraConfiguration::iterator\n> > >>> + * \\brief Iterator for the streams in the configuration\n> > >>> + */\n> > >>> +\n> > >>> +/**\n> > >>> + * \\typedef CameraConfiguration::const_iterator\n> > >>> + * \\brief Const iterator for the streams in the configuration\n> > >>> + */\n> > >>> +\n> > >>> +/**\n> > >>> + * \\brief Create an empty camera configuration\n> > >>> + */\n> > >>> +CameraConfiguration::CameraConfiguration()\n> > >>> +\t: order_({}), config_({})\n> > >>> +{\n> > >>> +}\n> > >>> +\n> > >>> +/**\n> > >>> + * \\brief Retrieve an iterator to the first element of the streams\n> > >>> + *\n> > >>\n> > >> The standard iterator documentation for begin reports:\n> > >> \"Returns an iterator pointing to the first element in the sequence\"\n> > >>\n> > >> Could we match this?\n> > >>\n> > >>> + * \\return An iterator to the first stream\n> > >>> + */\n> > >>> +std::vector::iterator CameraConfiguration::begin()\n> > >>> +{\n> > >>> +\treturn order_.begin();\n> > >>> +}\n> > >>> +\n> > >>> +/**\n> > >>> + * \\brief Retrieve an iterator to the end of the streams\n> > >>\n> > >> Same as per 'begin()':\n> > >>\n> > >> \"Returns an iterator pointing to the past-the-end element in the sequence\"\n> > >>\n> > >>> + *\n> > >>> + * \\return An iterator to the element following the last stream\n> > >>> + */\n> > >>> +std::vector::iterator CameraConfiguration::end()\n> > >>> +{\n> > >>> +\treturn order_.end();\n> > >>> +}\n> > >>> +\n> > >>> +/**\n> > >>> + * \\brief Retrieve an iterator to the first element of the streams\n> > >>> + *\n> > >>> + * \\return An iterator to the first stream\n> > >>> + */\n> > >>> +std::vector::const_iterator CameraConfiguration::begin() const\n> > >>> +{\n> > >>> +\treturn order_.begin();\n> > >>> +}\n> > >>> +\n> > >>> +/**\n> > >>> + * \\brief Retrieve an iterator to the end of the streams\n> > >>> + *\n> > >>> + * \\return An iterator to the element following the last stream\n> > >>> + */\n> > >>> +std::vector::const_iterator CameraConfiguration::end() const\n> > >>> +{\n> > >>> +\treturn order_.end();\n> > >>> +}\n> > >>> +\n> > >>> +/**\n> > >>> + * \\brief Checks whether the camera configuration is empty\n> \n> s/whether/if/\n> \n> Kieran recently taught me that \"whether\" requires two clauses (whether A\n> or B).\n> \n> > >>> + *\n> > >>> + * \\return True if the configuration is empty\n> > >>> + */\n> > >>> +bool CameraConfiguration::empty() const\n> > >>> +{\n> > >>> +\treturn order_.empty();\n> > >>> +}\n> > >>> +\n> > >>> +/**\n> > >>> + * \\brief Check the number of stream configurations\n> \n> s/Check/Retrieve/\n> \n> > >>> + *\n> > >>> + * \\return Number of stream configurations\n> > >>> + */\n> > >>> +std::size_t CameraConfiguration::size() const\n> > >>> +{\n> > >>> +\treturn order_.size();\n> > >>> +}\n> > >>> +\n> > >>> +/**\n> > >>> + * \\brief Access the first stream in the configuration\n> > >>> + *\n> > >>> + * \\return The first stream in the configuration\n> > >>> + */\n> > >>> +Stream *CameraConfiguration::front()\n> > >>> +{\n> > >>> +\treturn order_.front();\n> > >>> +}\n> > >>> +\n> > >>> +/**\n> > >>> + * \\brief Retrieve a reference to a stream configuration\n> > >>> + * \\param[in] stream Stream to retrieve configuration for\n> > >>> + *\n> > >>> + * If the camera configuration do not yet contain configuration for the\n> > >>\n> > >> contain a configuration\n> > >>\n> > >>> + * requested stream an empty stream configuration is created and returned.\n> > >>\n> > >> empty one\n> \n> \"If the camera configuration does not yet contain a configuration for\n> the requested stream, create and return an empty stream configuration.\"\n> \n> > >>\n> > >>> + *\n> > >>> + * \\return Configuration for the stream\n> > >>\n> > >> The configuration\n> > >>\n> > >>> + */\n> > >>> +StreamConfiguration &CameraConfiguration::operator[](Stream *stream)\n> > >>> +{\n> > >>> +\tif (config_.find(stream) == config_.end())\n> > >>> +\t\torder_.push_back(stream);\n> > >>> +\n> > >>> +\treturn config_[stream];\n> > >>> +}\n> > >>> +\n> > >>\n> > >> I don't know how welcome would the idea of making of this class a\n> > >> full-fledged iterator, but I think increment, decrement and access by\n> > >> integer index should be implemented to make it more useful. Have you\n> > >> considered that?\n> > >\n> > > As stated above it is already implemented. Well not integer indexed as\n> > > the whole idea if this object is to make it Stream* indexed but preserve\n> > > the insertion order when iterating over it to allow mapping stream\n> > > usages to stream configurations.\n> > >\n> > >>> /**\n> > >>> * \\class Camera\n> > >>> * \\brief Camera device\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com\n\t[IPv6:2a00:1450:4864:20::22b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E53660DB3\n\tfor ;\n\tFri, 5 Apr 2019 23:20:26 +0200 (CEST)","by mail-lj1-x22b.google.com with SMTP id p14so6439590ljg.5\n\tfor ;\n\tFri, 05 Apr 2019 14:20:25 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tv11sm4672097lfn.1.2019.04.05.14.20.23\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 05 Apr 2019 14:20:24 -0700 (PDT)"],"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=rr1S0jdgYBkfzeiaX+g0lKVtTX40nPB7iUvZTybEBvQ=;\n\tb=iAl7QlxYLM0Ds+0gH6tbx9xOenjgFGMjcQTaRY4oGzo6YtofYAg9P57ftY5F7ikoQy\n\tpHkXLeH69U3hD9Wt9w90kLsRqzrGOrv0D9gE0VN3YP6y81iw8rWinW7PF9Lm8XtSAKlB\n\tt/+7XYg+ET7D5NhIzM8DGTICLvZcPOxkn+D0oiCsfot9GB0pxMHdc0A91uBIFpZFPOe0\n\tD9jQueOOkeILPV6oYVZn+CUfjrIK0bV9XjnVxWKHUCYErV6xw+/MRmVMK/Wxbjxd4jhV\n\t+594p/yHNQBAhM7pPhYp4iCLO6rrME84pFaTKs7LH2cBcqe0AtXGWoqsrhtFIu5Rc2eE\n\tf92A==","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=rr1S0jdgYBkfzeiaX+g0lKVtTX40nPB7iUvZTybEBvQ=;\n\tb=Z8yvzRC2k5OYcCVOA446soZlyWGkwt6XZz4t48UCDduqOLKrrCADHArNtnUvYTuUCa\n\t2lHvdtDts4mAndI9VPzd3KNWjPaDvufWuSU23YDO7QQvuP30JlHQzNIOFKwV9L2PPhoV\n\t2Ml8joSL1cE0ZcGihkGR3hR6KgCmuNUEFk435cReLPEN0JHveJvyyGED5k8HZ8sJ50wi\n\t47X+KV6RSIY4DsAb6AoIcCUR2gtCiFS1mc8kZYO/C7Tmcrk4Q62QRWuPrHby37Thjwjg\n\t/Th+41FBf9iOc6cHDo0rGoTSU0dPYxX38EdxWDOKSp03CWbpDpJQ1+4ZndzSyagN05P9\n\tBsFg==","X-Gm-Message-State":"APjAAAWE9BKEKAuaGYf7wtFKbV5F1cYfU0X/CUteaHUmgJr7sLozrU3o\n\tU8H9CVJFiTtC4MvZ4C36tNaxbm65i50=","X-Google-Smtp-Source":"APXvYqwzgl81gEYweTqI73tSkf1+Nk75EkgaH2/PFdCRlf2j5Zm1G93vS+MFpmbLZf8KDv9VF3xEKQ==","X-Received":"by 2002:a2e:9a91:: with SMTP id\n\tp17mr8214819lji.127.1554499225233; \n\tFri, 05 Apr 2019 14:20:25 -0700 (PDT)","Date":"Fri, 5 Apr 2019 23:20:23 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","Message-ID":"<20190405212023.GA15350@bigcity.dyn.berto.se>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-8-niklas.soderlund@ragnatech.se>\n\t<20190405094746.s4da4bayokhvnodb@uno.localdomain>\n\t<20190405102106.GU23466@bigcity.dyn.berto.se>\n\t<20190405111211.di7iecvze3p5r5ge@uno.localdomain>\n\t<20190405154547.GE5184@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190405154547.GE5184@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH v2 7/8] libcamera: camera: Add\n\tCameraConfiguration","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":"Fri, 05 Apr 2019 21:20:26 -0000"}}]