[{"id":4162,"web_url":"https://patchwork.libcamera.org/comment/4162/","msgid":"<20200321115546.33cco4naxal64nhl@uno.localdomain>","date":"2020-03-21T11:56:12","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kaaira,\n\nOn Sat, Mar 21, 2020 at 03:20:20AM +0530, Kaaira Gupta wrote:\n> Use of default constructor StreamConfiguration() in deprecated, hence\n> make it private. Make Stream class a friend of StreamConfiguration as it\n> uses the default constructor.\n>\n> Replace default constructor StreamConfiguration() by it's parametered\n\ns/parametered/parametrized ?\n\n> counterpart by using StreamFormats in generateConfiguration() in rkisp1\n>\n> Signed-off-by: Kaaira Gupta \n> ---\n> WIP:\n> \t- It fails to build as other pipelines still use default\n> \t constructor.\n> \t- Even rkisp1 fails as it uses default constructor in its\n> \t validate() function.\n\nThe compiler points to\n\n config_.resize(1);\n ^\n../include/libcamera/stream.h:54:2: note: declared private here\n StreamConfiguration();\n\nThe std::vector::resize() documentation says\nIf the current size is greater than count, the container is reduced to its first count elements.\n\nSo I guess the compiler is just worried about the possibility that the\ncurrent size is smaller than the resize(n) argument and a new instance\nshould be created, so it would need to access the default constructor.\n\nThe most trivial option here is to manually delete the exceeding\ninstances if vector.size() > 1 and create a new one using the\nparametrized constructor.\n\n> \t- I have taken care of generateConfiguration() only.\n>\n> include/libcamera/stream.h | 4 ++-\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 37 ++++++++++++++++--------\n> src/libcamera/stream.cpp | 9 ------\n> 3 files changed, 28 insertions(+), 22 deletions(-)\n>\n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index b1441f8..c19aed6 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -37,7 +37,6 @@ private:\n> };\n>\n> struct StreamConfiguration {\n> -\tStreamConfiguration();\n> \tStreamConfiguration(const StreamFormats &formats);\n>\n> \tPixelFormat pixelFormat;\n> @@ -52,8 +51,11 @@ struct StreamConfiguration {\n> \tstd::string toString() const;\n>\n> private:\n> +\tStreamConfiguration();\n> \tStream *stream_;\n> \tStreamFormats formats_;\n> +\n> +\tfriend class Stream;\n> };\n>\n> enum StreamRole {\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 2f909ce..bf97b53 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -218,6 +218,21 @@ private:\n> \tCamera *activeCamera_;\n> };\n>\n> +namespace {\n> +\n> +static const std::array formats{\n> +\tPixelFormat(DRM_FORMAT_YUYV),\n> +\tPixelFormat(DRM_FORMAT_YVYU),\n> +\tPixelFormat(DRM_FORMAT_VYUY),\n> +\tPixelFormat(DRM_FORMAT_NV16),\n> +\tPixelFormat(DRM_FORMAT_NV61),\n> +\tPixelFormat(DRM_FORMAT_NV21),\n> +\tPixelFormat(DRM_FORMAT_NV12),\n> +\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> +};\n> +\n> +} /* namespace */\n> +\n> RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n> \t: pipe_(dynamic_cast(pipe))\n> {\n> @@ -430,17 +445,6 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n>\n> CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> {\n> -\tstatic const std::array formats{\n> -\t\tPixelFormat(DRM_FORMAT_YUYV),\n> -\t\tPixelFormat(DRM_FORMAT_YVYU),\n> -\t\tPixelFormat(DRM_FORMAT_VYUY),\n> -\t\tPixelFormat(DRM_FORMAT_NV16),\n> -\t\tPixelFormat(DRM_FORMAT_NV61),\n> -\t\tPixelFormat(DRM_FORMAT_NV21),\n> -\t\tPixelFormat(DRM_FORMAT_NV12),\n> -\t\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> -\t};\n> -\n> \tconst CameraSensor *sensor = data_->sensor_;\n> \tStatus status = Valid;\n>\n> @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> \tif (roles.empty())\n> \t\treturn config;\n>\n> -\tStreamConfiguration cfg{};\n> +\tstd::map> pixelformats;\n> +\n> +\tfor (PixelFormat pixelformat : formats) {\n> +\t\tstd::vector sizes{\n> +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n\nI might have missed where these sizes come from.\n\n> +\t\t};\n> +\t\tpixelformats[pixelformat] = sizes;\n\nSo you have an hardcoded list of PixelFormats in the pipeline handler.\nAnd you got a map of V4L2PixelFormats from the video device.\nWhat I would do is going through all the PixelFormats, get their\nV4L2PixelFormat counterpart, access the map using that as key to\nretrive the size vector, and associated them with the PixelFormat you\nstarted with.\n\nI don't think it is necessary to make sure all the V4L2PixelFormat\nobtained from the PixelFormat array are valid, as the pipeline handler\nshould know that.\n\nThanks\n j\n\n\n> +\t}\n> +\tStreamFormats format(pixelformats);\n> +\tStreamConfiguration cfg(format);\n> \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> \tcfg.size = data->sensor_->resolution();\n>\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index ea3d214..96b3dc5 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> * configured for a single video stream.\n> */\n>\n> -/**\n> - * \\todo This method is deprecated and should be removed once all pipeline\n> - * handlers provied StreamFormats.\n> - */\n> -StreamConfiguration::StreamConfiguration()\n> -\t: pixelFormat(0), stream_(nullptr)\n> -{\n> -}\n> -\n> /**\n> * \\brief Construct a configuration with stream formats\n> */\n> --\n> 2.17.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 B9EFB6041A\n\tfor ;\n\tSat, 21 Mar 2020 12:53:16 +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 7A448E0003;\n\tSat, 21 Mar 2020 11:53:14 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sat, 21 Mar 2020 12:56:12 +0100","From":"Jacopo Mondi ","To":"Kaaira Gupta ","Cc":"libcamera-devel@lists.libcamera.org, kieran.bingham@ideasonboard.com,\n\tHelen Koike ,\n\tVaishali Thakkar ","Message-ID":"<20200321115546.33cco4naxal64nhl@uno.localdomain>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 11:53:16 -0000"}},{"id":4163,"web_url":"https://patchwork.libcamera.org/comment/4163/","msgid":"<20200321120246.shf3v464dgb4qdud@uno.localdomain>","date":"2020-03-21T12:02:46","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi again,\n\nOn Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n> Hi Kaaira,\n>\n> On Sat, Mar 21, 2020 at 03:20:20AM +0530, Kaaira Gupta wrote:\n> > Use of default constructor StreamConfiguration() in deprecated, hence\n> > make it private. Make Stream class a friend of StreamConfiguration as it\n> > uses the default constructor.\n> >\n> > Replace default constructor StreamConfiguration() by it's parametered\n>\n> s/parametered/parametrized ?\n>\n> > counterpart by using StreamFormats in generateConfiguration() in rkisp1\n> >\n> > Signed-off-by: Kaaira Gupta \n> > ---\n> > WIP:\n> > \t- It fails to build as other pipelines still use default\n> > \t constructor.\n> > \t- Even rkisp1 fails as it uses default constructor in its\n> > \t validate() function.\n>\n> The compiler points to\n>\n> config_.resize(1);\n> ^\n> ../include/libcamera/stream.h:54:2: note: declared private here\n> StreamConfiguration();\n>\n> The std::vector::resize() documentation says\n> If the current size is greater than count, the container is reduced to its first count elements.\n>\n> So I guess the compiler is just worried about the possibility that the\n> current size is smaller than the resize(n) argument and a new instance\n> should be created, so it would need to access the default constructor.\n>\n> The most trivial option here is to manually delete the exceeding\n> instances if vector.size() > 1 and create a new one using the\n> parametrized constructor.\n\nSorry this is confused.\n\nDelete the exceeding entries if the size of the vector is larger and\ncreate a new instance manually if the vector is empty (which can't\nhappen as it is caught above :).\n\n>\n> > \t- I have taken care of generateConfiguration() only.\n> >\n> > include/libcamera/stream.h | 4 ++-\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 37 ++++++++++++++++--------\n> > src/libcamera/stream.cpp | 9 ------\n> > 3 files changed, 28 insertions(+), 22 deletions(-)\n> >\n> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > index b1441f8..c19aed6 100644\n> > --- a/include/libcamera/stream.h\n> > +++ b/include/libcamera/stream.h\n> > @@ -37,7 +37,6 @@ private:\n> > };\n> >\n> > struct StreamConfiguration {\n> > -\tStreamConfiguration();\n> > \tStreamConfiguration(const StreamFormats &formats);\n> >\n> > \tPixelFormat pixelFormat;\n> > @@ -52,8 +51,11 @@ struct StreamConfiguration {\n> > \tstd::string toString() const;\n> >\n> > private:\n> > +\tStreamConfiguration();\n> > \tStream *stream_;\n> > \tStreamFormats formats_;\n> > +\n> > +\tfriend class Stream;\n> > };\n> >\n> > enum StreamRole {\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 2f909ce..bf97b53 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -218,6 +218,21 @@ private:\n> > \tCamera *activeCamera_;\n> > };\n> >\n> > +namespace {\n> > +\n> > +static const std::array formats{\n> > +\tPixelFormat(DRM_FORMAT_YUYV),\n> > +\tPixelFormat(DRM_FORMAT_YVYU),\n> > +\tPixelFormat(DRM_FORMAT_VYUY),\n> > +\tPixelFormat(DRM_FORMAT_NV16),\n> > +\tPixelFormat(DRM_FORMAT_NV61),\n> > +\tPixelFormat(DRM_FORMAT_NV21),\n> > +\tPixelFormat(DRM_FORMAT_NV12),\n> > +\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > +};\n> > +\n> > +} /* namespace */\n> > +\n> > RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n> > \t: pipe_(dynamic_cast(pipe))\n> > {\n> > @@ -430,17 +445,6 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> >\n> > CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > {\n> > -\tstatic const std::array formats{\n> > -\t\tPixelFormat(DRM_FORMAT_YUYV),\n> > -\t\tPixelFormat(DRM_FORMAT_YVYU),\n> > -\t\tPixelFormat(DRM_FORMAT_VYUY),\n> > -\t\tPixelFormat(DRM_FORMAT_NV16),\n> > -\t\tPixelFormat(DRM_FORMAT_NV61),\n> > -\t\tPixelFormat(DRM_FORMAT_NV21),\n> > -\t\tPixelFormat(DRM_FORMAT_NV12),\n> > -\t\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > -\t};\n> > -\n> > \tconst CameraSensor *sensor = data_->sensor_;\n> > \tStatus status = Valid;\n> >\n> > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > \tif (roles.empty())\n> > \t\treturn config;\n> >\n> > -\tStreamConfiguration cfg{};\n> > +\tstd::map> pixelformats;\n> > +\n> > +\tfor (PixelFormat pixelformat : formats) {\n> > +\t\tstd::vector sizes{\n> > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n>\n> I might have missed where these sizes come from.\n>\n> > +\t\t};\n> > +\t\tpixelformats[pixelformat] = sizes;\n>\n> So you have an hardcoded list of PixelFormats in the pipeline handler.\n> And you got a map of V4L2PixelFormats from the video device.\n> What I would do is going through all the PixelFormats, get their\n> V4L2PixelFormat counterpart, access the map using that as key to\n> retrive the size vector, and associated them with the PixelFormat you\n> started with.\n>\n> I don't think it is necessary to make sure all the V4L2PixelFormat\n> obtained from the PixelFormat array are valid, as the pipeline handler\n> should know that.\n>\n> Thanks\n> j\n>\n>\n> > +\t}\n> > +\tStreamFormats format(pixelformats);\n> > +\tStreamConfiguration cfg(format);\n> > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > \tcfg.size = data->sensor_->resolution();\n> >\n> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > index ea3d214..96b3dc5 100644\n> > --- a/src/libcamera/stream.cpp\n> > +++ b/src/libcamera/stream.cpp\n> > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > * configured for a single video stream.\n> > */\n> >\n> > -/**\n> > - * \\todo This method is deprecated and should be removed once all pipeline\n> > - * handlers provied StreamFormats.\n> > - */\n> > -StreamConfiguration::StreamConfiguration()\n> > -\t: pixelFormat(0), stream_(nullptr)\n> > -{\n> > -}\n> > -\n> > /**\n> > * \\brief Construct a configuration with stream formats\n> > */\n> > --\n> > 2.17.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\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 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 E3E9D6041A\n\tfor ;\n\tSat, 21 Mar 2020 12:59:53 +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 DE802C000D;\n\tSat, 21 Mar 2020 11:59:49 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sat, 21 Mar 2020 13:02:46 +0100","From":"Jacopo Mondi ","To":"Kaaira Gupta ","Cc":"Vaishali Thakkar ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200321120246.shf3v464dgb4qdud@uno.localdomain>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200321115546.33cco4naxal64nhl@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 11:59:54 -0000"}},{"id":4164,"web_url":"https://patchwork.libcamera.org/comment/4164/","msgid":"<20200321140922.GA8952@kaaira-HP-Pavilion-Notebook>","date":"2020-03-21T14:09:23","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":39,"url":"https://patchwork.libcamera.org/api/people/39/","name":"Kaaira Gupta","email":"kgupta@es.iitr.ac.in"},"content":"On Sat, Mar 21, 2020 at 01:02:46PM +0100, Jacopo Mondi wrote:\n> Hi again,\n> \n> On Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n> > Hi Kaaira,\n> >\n> > On Sat, Mar 21, 2020 at 03:20:20AM +0530, Kaaira Gupta wrote:\n> > > Use of default constructor StreamConfiguration() in deprecated, hence\n> > > make it private. Make Stream class a friend of StreamConfiguration as it\n> > > uses the default constructor.\n> > >\n> > > Replace default constructor StreamConfiguration() by it's parametered\n> >\n> > s/parametered/parametrized ?\n> >\n> > > counterpart by using StreamFormats in generateConfiguration() in rkisp1\n> > >\n> > > Signed-off-by: Kaaira Gupta \n> > > ---\n> > > WIP:\n> > > \t- It fails to build as other pipelines still use default\n> > > \t constructor.\n> > > \t- Even rkisp1 fails as it uses default constructor in its\n> > > \t validate() function.\n> >\n> > The compiler points to\n> >\n> > config_.resize(1);\n> > ^\n> > ../include/libcamera/stream.h:54:2: note: declared private here\n> > StreamConfiguration();\n> >\n> > The std::vector::resize() documentation says\n> > If the current size is greater than count, the container is reduced to its first count elements.\n> >\n> > So I guess the compiler is just worried about the possibility that the\n> > current size is smaller than the resize(n) argument and a new instance\n> > should be created, so it would need to access the default constructor.\n> >\n> > The most trivial option here is to manually delete the exceeding\n> > instances if vector.size() > 1 and create a new one using the\n> > parametrized constructor.\n> \n> Sorry this is confused.\n> \n> Delete the exceeding entries if the size of the vector is larger and\n> create a new instance manually if the vector is empty (which can't\n> happen as it is caught above :).\n\nCan you please elaborate it a bit more? I don't catch it when you say\nthat \"which can't happen as it is caught above\", what exactly are you\ntalking about? \nSorry if this seems trivial but I am confused of what you are trying to\nget me to do here.\n\nthanks!\n\n> \n> >\n> > > \t- I have taken care of generateConfiguration() only.\n> > >\n> > > include/libcamera/stream.h | 4 ++-\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 37 ++++++++++++++++--------\n> > > src/libcamera/stream.cpp | 9 ------\n> > > 3 files changed, 28 insertions(+), 22 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > > index b1441f8..c19aed6 100644\n> > > --- a/include/libcamera/stream.h\n> > > +++ b/include/libcamera/stream.h\n> > > @@ -37,7 +37,6 @@ private:\n> > > };\n> > >\n> > > struct StreamConfiguration {\n> > > -\tStreamConfiguration();\n> > > \tStreamConfiguration(const StreamFormats &formats);\n> > >\n> > > \tPixelFormat pixelFormat;\n> > > @@ -52,8 +51,11 @@ struct StreamConfiguration {\n> > > \tstd::string toString() const;\n> > >\n> > > private:\n> > > +\tStreamConfiguration();\n> > > \tStream *stream_;\n> > > \tStreamFormats formats_;\n> > > +\n> > > +\tfriend class Stream;\n> > > };\n> > >\n> > > enum StreamRole {\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 2f909ce..bf97b53 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -218,6 +218,21 @@ private:\n> > > \tCamera *activeCamera_;\n> > > };\n> > >\n> > > +namespace {\n> > > +\n> > > +static const std::array formats{\n> > > +\tPixelFormat(DRM_FORMAT_YUYV),\n> > > +\tPixelFormat(DRM_FORMAT_YVYU),\n> > > +\tPixelFormat(DRM_FORMAT_VYUY),\n> > > +\tPixelFormat(DRM_FORMAT_NV16),\n> > > +\tPixelFormat(DRM_FORMAT_NV61),\n> > > +\tPixelFormat(DRM_FORMAT_NV21),\n> > > +\tPixelFormat(DRM_FORMAT_NV12),\n> > > +\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > > +};\n> > > +\n> > > +} /* namespace */\n> > > +\n> > > RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n> > > \t: pipe_(dynamic_cast(pipe))\n> > > {\n> > > @@ -430,17 +445,6 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> > >\n> > > CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > {\n> > > -\tstatic const std::array formats{\n> > > -\t\tPixelFormat(DRM_FORMAT_YUYV),\n> > > -\t\tPixelFormat(DRM_FORMAT_YVYU),\n> > > -\t\tPixelFormat(DRM_FORMAT_VYUY),\n> > > -\t\tPixelFormat(DRM_FORMAT_NV16),\n> > > -\t\tPixelFormat(DRM_FORMAT_NV61),\n> > > -\t\tPixelFormat(DRM_FORMAT_NV21),\n> > > -\t\tPixelFormat(DRM_FORMAT_NV12),\n> > > -\t\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > > -\t};\n> > > -\n> > > \tconst CameraSensor *sensor = data_->sensor_;\n> > > \tStatus status = Valid;\n> > >\n> > > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > \tif (roles.empty())\n> > > \t\treturn config;\n> > >\n> > > -\tStreamConfiguration cfg{};\n> > > +\tstd::map> pixelformats;\n> > > +\n> > > +\tfor (PixelFormat pixelformat : formats) {\n> > > +\t\tstd::vector sizes{\n> > > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n> >\n> > I might have missed where these sizes come from.\n> >\n> > > +\t\t};\n> > > +\t\tpixelformats[pixelformat] = sizes;\n> >\n> > So you have an hardcoded list of PixelFormats in the pipeline handler.\n> > And you got a map of V4L2PixelFormats from the video device.\n> > What I would do is going through all the PixelFormats, get their\n> > V4L2PixelFormat counterpart, access the map using that as key to\n> > retrive the size vector, and associated them with the PixelFormat you\n> > started with.\n> >\n> > I don't think it is necessary to make sure all the V4L2PixelFormat\n> > obtained from the PixelFormat array are valid, as the pipeline handler\n> > should know that.\n> >\n> > Thanks\n> > j\n> >\n> >\n> > > +\t}\n> > > +\tStreamFormats format(pixelformats);\n> > > +\tStreamConfiguration cfg(format);\n> > > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > > \tcfg.size = data->sensor_->resolution();\n> > >\n> > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > index ea3d214..96b3dc5 100644\n> > > --- a/src/libcamera/stream.cpp\n> > > +++ b/src/libcamera/stream.cpp\n> > > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > > * configured for a single video stream.\n> > > */\n> > >\n> > > -/**\n> > > - * \\todo This method is deprecated and should be removed once all pipeline\n> > > - * handlers provied StreamFormats.\n> > > - */\n> > > -StreamConfiguration::StreamConfiguration()\n> > > -\t: pixelFormat(0), stream_(nullptr)\n> > > -{\n> > > -}\n> > > -\n> > > /**\n> > > * \\brief Construct a configuration with stream formats\n> > > */\n> > > --\n> > > 2.17.1\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\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-pj1-x1043.google.com (mail-pj1-x1043.google.com\n\t[IPv6:2607:f8b0:4864:20::1043])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 98FEA6041A\n\tfor ;\n\tSat, 21 Mar 2020 15:09:31 +0100 (CET)","by mail-pj1-x1043.google.com with SMTP id ng8so3784490pjb.2\n\tfor ;\n\tSat, 21 Mar 2020 07:09:31 -0700 (PDT)","from kaaira-HP-Pavilion-Notebook ([103.113.213.154])\n\tby smtp.gmail.com with ESMTPSA id\n\tf8sm8790401pfq.178.2020.03.21.07.09.27\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSat, 21 Mar 2020 07:09:28 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=es-iitr-ac-in.20150623.gappssmtp.com; s=20150623;\n\th=from:date:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=Us254lE3zuTr4pO/vqtb6mSL8m0rdD3SIgViZ/JIzao=;\n\tb=iv077BQFdc79hhOFOpPzxtyKoNZkDy5EaBSB/uRAq0MfYjCElBNXinpdaQla0jV+Bh\n\tuoF347olfI0iJ2cqHJ2uzxws8qvu2nlTxsfPPagKsy+QXsrQhItTBU0Bj2vUXEJJXGcf\n\to/wJJYiOxs4kz7HUdYhbAMfBdu3N+K5PpOFWL6dNkpNNWlZ1vd+9KJ9QqPTCuP2+9vDr\n\tsG91FhMhZtvojtk7WpLHFetn4QJwMldWIrorexikOv8zlu6xvXFqefoHv3wII9eoG3R0\n\ttjyUKhFW+J4VDK+Gp47Pyo6ISC9Ns2PxD++edPMpLD5Wwg2/lqZJ8kILA6v61+r0sk7p\n\tDYWQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:date:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=Us254lE3zuTr4pO/vqtb6mSL8m0rdD3SIgViZ/JIzao=;\n\tb=ZacD2hzSwuVkVInnxsJxUkRleg0+R6unbqRIJogoaEAg6MWI+4u0ny7GvxrNuXve5M\n\tBOGd/MHLF3r3wm34U6J0awXTljyB/ZaoAUg3IoxnkW8RaRbgtoQuvdu5ju6WYAJF6d8v\n\tMRa3FYIZExwqTLqPYcFfSOnu8UdaY7a2HpRWSjkRco5IAyL16mmkc5xAa2N+4FndROei\n\tUylU4Dqs5lJP9CEY/ogjk2cjS+Ox01jO3cLjZjUH4U2sjGQvkzlIgHFnFJaTEnOQ5qV7\n\toDOy/asnkIOjYiY/trNmSFby0r4kLd3DnVU2rxsa2vohX74yYPAzR88OhR2Iu5WVPxtp\n\tFkpQ==","X-Gm-Message-State":"ANhLgQ1zxyMNzN0Hd+XjSwDXikFQ2ktTewyd8tFqpgSHhgYa2J95QbWt\n\tsH5X55dsMti2HwLFa2RzYeFTkQ==","X-Google-Smtp-Source":"ADFU+vvC28TPYCFS78OVFPC0fkTR5szYOutiP/jQNbp7g0+AW3O0E7DCpIWvHeTq5pzN4N6TdG699Q==","X-Received":"by 2002:a17:90a:b94a:: with SMTP id\n\tf10mr14885748pjw.62.1584799769529; \n\tSat, 21 Mar 2020 07:09:29 -0700 (PDT)","From":"Kaaira Gupta ","X-Google-Original-From":"Kaaira Gupta ","Date":"Sat, 21 Mar 2020 19:39:23 +0530","To":"Jacopo Mondi ","Cc":"Kaaira Gupta ,\n\tVaishali Thakkar ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200321140922.GA8952@kaaira-HP-Pavilion-Notebook>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>\n\t<20200321120246.shf3v464dgb4qdud@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200321120246.shf3v464dgb4qdud@uno.localdomain>","User-Agent":"Mutt/1.9.4 (2018-02-28)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 14:09:32 -0000"}},{"id":4165,"web_url":"https://patchwork.libcamera.org/comment/4165/","msgid":"<20200321143538.GB8952@kaaira-HP-Pavilion-Notebook>","date":"2020-03-21T14:35:38","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":39,"url":"https://patchwork.libcamera.org/api/people/39/","name":"Kaaira Gupta","email":"kgupta@es.iitr.ac.in"},"content":"On Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n> Hi Kaaira,\n> \n> On Sat, Mar 21, 2020 at 03:20:20AM +0530, Kaaira Gupta wrote:\n> > Use of default constructor StreamConfiguration() in deprecated, hence\n> > make it private. Make Stream class a friend of StreamConfiguration as it\n> > uses the default constructor.\n> >\n> > Replace default constructor StreamConfiguration() by it's parametered\n> \n> s/parametered/parametrized ?\n> \n> > counterpart by using StreamFormats in generateConfiguration() in rkisp1\n> >\n> > Signed-off-by: Kaaira Gupta \n> > ---\n> > WIP:\n> > \t- It fails to build as other pipelines still use default\n> > \t constructor.\n> > \t- Even rkisp1 fails as it uses default constructor in its\n> > \t validate() function.\n> \n> The compiler points to\n> \n> config_.resize(1);\n> ^\n> ../include/libcamera/stream.h:54:2: note: declared private here\n> StreamConfiguration();\n> \n> The std::vector::resize() documentation says\n> If the current size is greater than count, the container is reduced to its first count elements.\n> \n> So I guess the compiler is just worried about the possibility that the\n> current size is smaller than the resize(n) argument and a new instance\n> should be created, so it would need to access the default constructor.\n> \n> The most trivial option here is to manually delete the exceeding\n> instances if vector.size() > 1 and create a new one using the\n> parametrized constructor.\n> \n> > \t- I have taken care of generateConfiguration() only.\n> >\n> > include/libcamera/stream.h | 4 ++-\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 37 ++++++++++++++++--------\n> > src/libcamera/stream.cpp | 9 ------\n> > 3 files changed, 28 insertions(+), 22 deletions(-)\n> >\n> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > index b1441f8..c19aed6 100644\n> > --- a/include/libcamera/stream.h\n> > +++ b/include/libcamera/stream.h\n> > @@ -37,7 +37,6 @@ private:\n> > };\n> >\n> > struct StreamConfiguration {\n> > -\tStreamConfiguration();\n> > \tStreamConfiguration(const StreamFormats &formats);\n> >\n> > \tPixelFormat pixelFormat;\n> > @@ -52,8 +51,11 @@ struct StreamConfiguration {\n> > \tstd::string toString() const;\n> >\n> > private:\n> > +\tStreamConfiguration();\n> > \tStream *stream_;\n> > \tStreamFormats formats_;\n> > +\n> > +\tfriend class Stream;\n> > };\n> >\n> > enum StreamRole {\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 2f909ce..bf97b53 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -218,6 +218,21 @@ private:\n> > \tCamera *activeCamera_;\n> > };\n> >\n> > +namespace {\n> > +\n> > +static const std::array formats{\n> > +\tPixelFormat(DRM_FORMAT_YUYV),\n> > +\tPixelFormat(DRM_FORMAT_YVYU),\n> > +\tPixelFormat(DRM_FORMAT_VYUY),\n> > +\tPixelFormat(DRM_FORMAT_NV16),\n> > +\tPixelFormat(DRM_FORMAT_NV61),\n> > +\tPixelFormat(DRM_FORMAT_NV21),\n> > +\tPixelFormat(DRM_FORMAT_NV12),\n> > +\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > +};\n> > +\n> > +} /* namespace */\n> > +\n> > RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n> > \t: pipe_(dynamic_cast(pipe))\n> > {\n> > @@ -430,17 +445,6 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> >\n> > CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > {\n> > -\tstatic const std::array formats{\n> > -\t\tPixelFormat(DRM_FORMAT_YUYV),\n> > -\t\tPixelFormat(DRM_FORMAT_YVYU),\n> > -\t\tPixelFormat(DRM_FORMAT_VYUY),\n> > -\t\tPixelFormat(DRM_FORMAT_NV16),\n> > -\t\tPixelFormat(DRM_FORMAT_NV61),\n> > -\t\tPixelFormat(DRM_FORMAT_NV21),\n> > -\t\tPixelFormat(DRM_FORMAT_NV12),\n> > -\t\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > -\t};\n> > -\n> > \tconst CameraSensor *sensor = data_->sensor_;\n> > \tStatus status = Valid;\n> >\n> > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > \tif (roles.empty())\n> > \t\treturn config;\n> >\n> > -\tStreamConfiguration cfg{};\n> > +\tstd::map> pixelformats;\n> > +\n> > +\tfor (PixelFormat pixelformat : formats) {\n> > +\t\tstd::vector sizes{\n> > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n> \n> I might have missed where these sizes come from.\n\nI got these values from here, in the handler()\n\tcfg.size.width = std::max(32U, std::min(4416U, cfg.size.width));\n\tcfg.size.height = std::max(16U, std::min(3312U, cfg.size.height));\n\nThat is how I defined them.\n\n> \n> > +\t\t};\n> > +\t\tpixelformats[pixelformat] = sizes;\n> \n> So you have an hardcoded list of PixelFormats in the pipeline handler.\n> And you got a map of V4L2PixelFormats from the video device.\n> What I would do is going through all the PixelFormats, get their\n> V4L2PixelFormat counterpart, access the map using that as key to\n> retrive the size vector, and associated them with the PixelFormat you\n> started with.\n\nPlease read the above logic behind the hardcoded values once and let me\nknow if I still have to map the sizes this way?\n\nThanks!\n\n> \n> I don't think it is necessary to make sure all the V4L2PixelFormat\n> obtained from the PixelFormat array are valid, as the pipeline handler\n> should know that.\n> \n> Thanks\n> j\n> \n> \n> > +\t}\n> > +\tStreamFormats format(pixelformats);\n> > +\tStreamConfiguration cfg(format);\n> > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > \tcfg.size = data->sensor_->resolution();\n> >\n> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > index ea3d214..96b3dc5 100644\n> > --- a/src/libcamera/stream.cpp\n> > +++ b/src/libcamera/stream.cpp\n> > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > * configured for a single video stream.\n> > */\n> >\n> > -/**\n> > - * \\todo This method is deprecated and should be removed once all pipeline\n> > - * handlers provied StreamFormats.\n> > - */\n> > -StreamConfiguration::StreamConfiguration()\n> > -\t: pixelFormat(0), stream_(nullptr)\n> > -{\n> > -}\n> > -\n> > /**\n> > * \\brief Construct a configuration with stream formats\n> > */\n> > --\n> > 2.17.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-pg1-x544.google.com (mail-pg1-x544.google.com\n\t[IPv6:2607:f8b0:4864:20::544])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4306F6041A\n\tfor ;\n\tSat, 21 Mar 2020 15:35:48 +0100 (CET)","by mail-pg1-x544.google.com with SMTP id k191so3453978pgc.13\n\tfor ;\n\tSat, 21 Mar 2020 07:35:48 -0700 (PDT)","from kaaira-HP-Pavilion-Notebook ([103.113.213.154])\n\tby smtp.gmail.com with ESMTPSA id\n\tna18sm7897761pjb.31.2020.03.21.07.35.42\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSat, 21 Mar 2020 07:35:45 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=es-iitr-ac-in.20150623.gappssmtp.com; s=20150623;\n\th=from:date:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=q5WwOVediNzt4G+E8OfBrUd66MLxBA7UTQ5wN6HM7do=;\n\tb=z2IXSCRZ4XU3UDzrBGPwexl5RI+JFaBfOqDNsSEQ0lyeRzul35ql8XEXy4c4iA85ws\n\tAOQsgV+4TGuPxgwJ5MpIxv0zxfiu6WsBQvcIm213/fbihtUF+Kvu8/WsWwrhlo01NnoJ\n\tmnG1YB/qn9ek8KD6L92IOZ2I6wA0BOSGe98XLfHVp1CAJXoG1dc4GBuORDse3CF4RLc0\n\tFOUw15LCfPBPLCHWHYGoEbqvyx5jjdpFvCDsuTq1INc4Fgu1uwriAToV4NhVssKOuxTu\n\tDCtDVN2H5bae/2BZwTQ1dMqvYh0mIfjBzAZO3ATvILRN8nzcgnoSSdOjUILh1052Tci3\n\tFqJQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:date:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=q5WwOVediNzt4G+E8OfBrUd66MLxBA7UTQ5wN6HM7do=;\n\tb=bKO6YIR3oAEtDmMKDtvxqXGAjFLKW+5mpOMW2A+aur1H8NDL3R9wQUkQpy1Zgpk0ke\n\tUi4faqLEumFNx96FGwgHJkPJllolyFM1B4yz99Io1gTFWwjYqtXu1UwPqCvK9/kwbFRS\n\t1ahAjWR0KzzJB2IsB429dtA37cu6zTn8Y8EWc70AywzgrWWFAA3J2lOLAt8s+ZUfhI+R\n\tnJe3qndWgRJy8Gi6mDMjYjBAt1hAnsHuZK+KSYMwpsML1UOdDbIx4Rm/2ZfN/bRVvzks\n\t293dLw+OD3ACtPlbIKIJ+CH3T4LVbcyAiwFIN8WQ0aRNchiSDZpMoJ341N/WRS+ieW7t\n\tV+Yw==","X-Gm-Message-State":"ANhLgQ1LIXuBn33w5svjhktdy9hx016xfqEzHzdGdV2iC9fICZx3VqHQ\n\tuut6flgP8rl73NVIwGfHhIQgqw==","X-Google-Smtp-Source":"ADFU+vvR7SD6ufd5+fRQYJhT8IAL+USEhUrBxNcUNqyovplbgNl7zp6wCzhWIzx280mPZoo6D+vw0g==","X-Received":"by 2002:a62:342:: with SMTP id 63mr16147157pfd.19.1584801346110; \n\tSat, 21 Mar 2020 07:35:46 -0700 (PDT)","From":"Kaaira Gupta ","X-Google-Original-From":"Kaaira Gupta ","Date":"Sat, 21 Mar 2020 20:05:38 +0530","To":"Jacopo Mondi ","Cc":"Kaaira Gupta , libcamera-devel@lists.libcamera.org,\n\tkieran.bingham@ideasonboard.com,\n\tHelen Koike , \n\tVaishali Thakkar ","Message-ID":"<20200321143538.GB8952@kaaira-HP-Pavilion-Notebook>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200321115546.33cco4naxal64nhl@uno.localdomain>","User-Agent":"Mutt/1.9.4 (2018-02-28)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 14:35:48 -0000"}},{"id":4166,"web_url":"https://patchwork.libcamera.org/comment/4166/","msgid":"<20200321153826.p675qwhp6dfllo7o@uno.localdomain>","date":"2020-03-21T15:38:26","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kaaira,\n\nOn Sat, Mar 21, 2020 at 07:39:23PM +0530, Kaaira Gupta wrote:\n> On Sat, Mar 21, 2020 at 01:02:46PM +0100, Jacopo Mondi wrote:\n> > Hi again,\n> >\n> > On Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n> > > Hi Kaaira,\n> > >\n> > > On Sat, Mar 21, 2020 at 03:20:20AM +0530, Kaaira Gupta wrote:\n> > > > Use of default constructor StreamConfiguration() in deprecated, hence\n> > > > make it private. Make Stream class a friend of StreamConfiguration as it\n> > > > uses the default constructor.\n> > > >\n> > > > Replace default constructor StreamConfiguration() by it's parametered\n> > >\n> > > s/parametered/parametrized ?\n> > >\n> > > > counterpart by using StreamFormats in generateConfiguration() in rkisp1\n> > > >\n> > > > Signed-off-by: Kaaira Gupta \n> > > > ---\n> > > > WIP:\n> > > > \t- It fails to build as other pipelines still use default\n> > > > \t constructor.\n> > > > \t- Even rkisp1 fails as it uses default constructor in its\n> > > > \t validate() function.\n> > >\n> > > The compiler points to\n> > >\n> > > config_.resize(1);\n> > > ^\n> > > ../include/libcamera/stream.h:54:2: note: declared private here\n> > > StreamConfiguration();\n> > >\n> > > The std::vector::resize() documentation says\n> > > If the current size is greater than count, the container is reduced to its first count elements.\n> > >\n> > > So I guess the compiler is just worried about the possibility that the\n> > > current size is smaller than the resize(n) argument and a new instance\n> > > should be created, so it would need to access the default constructor.\n> > >\n> > > The most trivial option here is to manually delete the exceeding\n> > > instances if vector.size() > 1 and create a new one using the\n> > > parametrized constructor.\n> >\n> > Sorry this is confused.\n> >\n> > Delete the exceeding entries if the size of the vector is larger and\n> > create a new instance manually if the vector is empty (which can't\n> > happen as it is caught above :).\n>\n> Can you please elaborate it a bit more? I don't catch it when you say\n> that \"which can't happen as it is caught above\", what exactly are you\n> talking about?\n> Sorry if this seems trivial but I am confused of what you are trying to\n> get me to do here.\n\nNo worries, I have indeed confused you.\n\nMy point was that the compiler points to std::vector::resize() and,\neven if it can't happen that in this code path that a new element has\nto be created, I assume the resize implementation calls the StreamConfiguration\ndefault constructor in case the vector has to be enlarged, hence the error.\n\nThat call to default constructor can't happen here, as we check for\nthe vector not to be empty just before calling resize, and as we\nresize to 1, the only option left to enter the here below second if\nbranch is that (config_.size() > 1) returns true.\n\n\tif (config_.empty())\n\t\treturn Invalid;\n\n\t/* Cap the number of entries to the available streams. */\n\tif (config_.size() > 1) {\n\t\tconfig_.resize(1);\n\t\tstatus = Adjusted;\n\t}\n\nAs long as this pipeline handler only support a single stream and its\nvalide() implementation only accept a single StreamConfiguration,\nI would just erase all the entries beyond the first one to\nshrink the vector size to 1 and release the memory containing the\nexceeding entries.\n\nTo sum it up I would got for\n\n\tif (config_.empty())\n\t\treturn Invalid;\n\n\t/* Cap the number of entries to the available streams. */\n\tif (config_.size() > 1) {\n\t\tconfig_.erase(config_.begin() + 1, config_.end());\n\t\tstatus = Adjusted;\n\t}\n\nThat should make sure there are no calls to the class default\nconstructor in any code path.\n\nAlso, please note you removed the default constructor implementation\nin stream.cpp, and you will likely get a linking error. You rightfully\nchanged the method visibility to private, but its implementation has\nto be defined somewhere.\n\nHope it clears it up\nThanks\n j\n\n>\n> thanks!\n>\n> >\n> > >\n> > > > \t- I have taken care of generateConfiguration() only.\n> > > >\n> > > > include/libcamera/stream.h | 4 ++-\n> > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 37 ++++++++++++++++--------\n> > > > src/libcamera/stream.cpp | 9 ------\n> > > > 3 files changed, 28 insertions(+), 22 deletions(-)\n> > > >\n> > > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > > > index b1441f8..c19aed6 100644\n> > > > --- a/include/libcamera/stream.h\n> > > > +++ b/include/libcamera/stream.h\n> > > > @@ -37,7 +37,6 @@ private:\n> > > > };\n> > > >\n> > > > struct StreamConfiguration {\n> > > > -\tStreamConfiguration();\n> > > > \tStreamConfiguration(const StreamFormats &formats);\n> > > >\n> > > > \tPixelFormat pixelFormat;\n> > > > @@ -52,8 +51,11 @@ struct StreamConfiguration {\n> > > > \tstd::string toString() const;\n> > > >\n> > > > private:\n> > > > +\tStreamConfiguration();\n> > > > \tStream *stream_;\n> > > > \tStreamFormats formats_;\n> > > > +\n> > > > +\tfriend class Stream;\n> > > > };\n> > > >\n> > > > enum StreamRole {\n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > index 2f909ce..bf97b53 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > @@ -218,6 +218,21 @@ private:\n> > > > \tCamera *activeCamera_;\n> > > > };\n> > > >\n> > > > +namespace {\n> > > > +\n> > > > +static const std::array formats{\n> > > > +\tPixelFormat(DRM_FORMAT_YUYV),\n> > > > +\tPixelFormat(DRM_FORMAT_YVYU),\n> > > > +\tPixelFormat(DRM_FORMAT_VYUY),\n> > > > +\tPixelFormat(DRM_FORMAT_NV16),\n> > > > +\tPixelFormat(DRM_FORMAT_NV61),\n> > > > +\tPixelFormat(DRM_FORMAT_NV21),\n> > > > +\tPixelFormat(DRM_FORMAT_NV12),\n> > > > +\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > > > +};\n> > > > +\n> > > > +} /* namespace */\n> > > > +\n> > > > RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n> > > > \t: pipe_(dynamic_cast(pipe))\n> > > > {\n> > > > @@ -430,17 +445,6 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> > > >\n> > > > CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > > {\n> > > > -\tstatic const std::array formats{\n> > > > -\t\tPixelFormat(DRM_FORMAT_YUYV),\n> > > > -\t\tPixelFormat(DRM_FORMAT_YVYU),\n> > > > -\t\tPixelFormat(DRM_FORMAT_VYUY),\n> > > > -\t\tPixelFormat(DRM_FORMAT_NV16),\n> > > > -\t\tPixelFormat(DRM_FORMAT_NV61),\n> > > > -\t\tPixelFormat(DRM_FORMAT_NV21),\n> > > > -\t\tPixelFormat(DRM_FORMAT_NV12),\n> > > > -\t\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > > > -\t};\n> > > > -\n> > > > \tconst CameraSensor *sensor = data_->sensor_;\n> > > > \tStatus status = Valid;\n> > > >\n> > > > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > > \tif (roles.empty())\n> > > > \t\treturn config;\n> > > >\n> > > > -\tStreamConfiguration cfg{};\n> > > > +\tstd::map> pixelformats;\n> > > > +\n> > > > +\tfor (PixelFormat pixelformat : formats) {\n> > > > +\t\tstd::vector sizes{\n> > > > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n> > >\n> > > I might have missed where these sizes come from.\n> > >\n> > > > +\t\t};\n> > > > +\t\tpixelformats[pixelformat] = sizes;\n> > >\n> > > So you have an hardcoded list of PixelFormats in the pipeline handler.\n> > > And you got a map of V4L2PixelFormats from the video device.\n> > > What I would do is going through all the PixelFormats, get their\n> > > V4L2PixelFormat counterpart, access the map using that as key to\n> > > retrive the size vector, and associated them with the PixelFormat you\n> > > started with.\n> > >\n> > > I don't think it is necessary to make sure all the V4L2PixelFormat\n> > > obtained from the PixelFormat array are valid, as the pipeline handler\n> > > should know that.\n> > >\n> > > Thanks\n> > > j\n> > >\n> > >\n> > > > +\t}\n> > > > +\tStreamFormats format(pixelformats);\n> > > > +\tStreamConfiguration cfg(format);\n> > > > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > > > \tcfg.size = data->sensor_->resolution();\n> > > >\n> > > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > > index ea3d214..96b3dc5 100644\n> > > > --- a/src/libcamera/stream.cpp\n> > > > +++ b/src/libcamera/stream.cpp\n> > > > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > > > * configured for a single video stream.\n> > > > */\n> > > >\n> > > > -/**\n> > > > - * \\todo This method is deprecated and should be removed once all pipeline\n> > > > - * handlers provied StreamFormats.\n> > > > - */\n> > > > -StreamConfiguration::StreamConfiguration()\n> > > > -\t: pixelFormat(0), stream_(nullptr)\n> > > > -{\n> > > > -}\n> > > > -\n> > > > /**\n> > > > * \\brief Construct a configuration with stream formats\n> > > > */\n> > > > --\n> > > > 2.17.1\n> > > >\n> > > > _______________________________________________\n> > > > libcamera-devel mailing list\n> > > > libcamera-devel@lists.libcamera.org\n> > > > https://lists.libcamera.org/listinfo/libcamera-devel\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 relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4758E6041A\n\tfor ;\n\tSat, 21 Mar 2020 16:35:33 +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 relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 71FA0240007;\n\tSat, 21 Mar 2020 15:35:29 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sat, 21 Mar 2020 16:38:26 +0100","From":"Jacopo Mondi ","To":"Kaaira Gupta ","Cc":"Vaishali Thakkar ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200321153826.p675qwhp6dfllo7o@uno.localdomain>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>\n\t<20200321120246.shf3v464dgb4qdud@uno.localdomain>\n\t<20200321140922.GA8952@kaaira-HP-Pavilion-Notebook>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200321140922.GA8952@kaaira-HP-Pavilion-Notebook>","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 15:35:33 -0000"}},{"id":4167,"web_url":"https://patchwork.libcamera.org/comment/4167/","msgid":"<20200321153635.GC8952@kaaira-HP-Pavilion-Notebook>","date":"2020-03-21T15:36:35","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":39,"url":"https://patchwork.libcamera.org/api/people/39/","name":"Kaaira Gupta","email":"kgupta@es.iitr.ac.in"},"content":"On Sat, Mar 21, 2020 at 01:02:46PM +0100, Jacopo Mondi wrote:\n> Hi again,\n> \n> On Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n> > Hi Kaaira,\n> >\n> > On Sat, Mar 21, 2020 at 03:20:20AM +0530, Kaaira Gupta wrote:\n> > > Use of default constructor StreamConfiguration() in deprecated, hence\n> > > make it private. Make Stream class a friend of StreamConfiguration as it\n> > > uses the default constructor.\n> > >\n> > > Replace default constructor StreamConfiguration() by it's parametered\n> >\n> > s/parametered/parametrized ?\n> >\n> > > counterpart by using StreamFormats in generateConfiguration() in rkisp1\n> > >\n> > > Signed-off-by: Kaaira Gupta \n> > > ---\n> > > WIP:\n> > > \t- It fails to build as other pipelines still use default\n> > > \t constructor.\n> > > \t- Even rkisp1 fails as it uses default constructor in its\n> > > \t validate() function.\n> >\n> > The compiler points to\n> >\n> > config_.resize(1);\n> > ^\n> > ../include/libcamera/stream.h:54:2: note: declared private here\n> > StreamConfiguration();\n> >\n> > The std::vector::resize() documentation says\n> > If the current size is greater than count, the container is reduced to its first count elements.\n> >\n> > So I guess the compiler is just worried about the possibility that the\n> > current size is smaller than the resize(n) argument and a new instance\n> > should be created, so it would need to access the default constructor.\n\nI don't think this is the case. We are already checking if the size>1,\nand only if it is, we are resizing it.\nI think the problem is with \n```StreamConfiguration &cfg = config_[0];```\nwhere we are calling the default constructor. &Format must be passed\nhere.\n\n> >\n> > The most trivial option here is to manually delete the exceeding\n> > instances if vector.size() > 1 and create a new one using the\n> > parametrized constructor.\n> \n> Sorry this is confused.\n> \n> Delete the exceeding entries if the size of the vector is larger and\n> create a new instance manually if the vector is empty (which can't\n> happen as it is caught above :).\n> \n> >\n> > > \t- I have taken care of generateConfiguration() only.\n> > >\n> > > include/libcamera/stream.h | 4 ++-\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 37 ++++++++++++++++--------\n> > > src/libcamera/stream.cpp | 9 ------\n> > > 3 files changed, 28 insertions(+), 22 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > > index b1441f8..c19aed6 100644\n> > > --- a/include/libcamera/stream.h\n> > > +++ b/include/libcamera/stream.h\n> > > @@ -37,7 +37,6 @@ private:\n> > > };\n> > >\n> > > struct StreamConfiguration {\n> > > -\tStreamConfiguration();\n> > > \tStreamConfiguration(const StreamFormats &formats);\n> > >\n> > > \tPixelFormat pixelFormat;\n> > > @@ -52,8 +51,11 @@ struct StreamConfiguration {\n> > > \tstd::string toString() const;\n> > >\n> > > private:\n> > > +\tStreamConfiguration();\n> > > \tStream *stream_;\n> > > \tStreamFormats formats_;\n> > > +\n> > > +\tfriend class Stream;\n> > > };\n> > >\n> > > enum StreamRole {\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 2f909ce..bf97b53 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -218,6 +218,21 @@ private:\n> > > \tCamera *activeCamera_;\n> > > };\n> > >\n> > > +namespace {\n> > > +\n> > > +static const std::array formats{\n> > > +\tPixelFormat(DRM_FORMAT_YUYV),\n> > > +\tPixelFormat(DRM_FORMAT_YVYU),\n> > > +\tPixelFormat(DRM_FORMAT_VYUY),\n> > > +\tPixelFormat(DRM_FORMAT_NV16),\n> > > +\tPixelFormat(DRM_FORMAT_NV61),\n> > > +\tPixelFormat(DRM_FORMAT_NV21),\n> > > +\tPixelFormat(DRM_FORMAT_NV12),\n> > > +\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > > +};\n> > > +\n> > > +} /* namespace */\n> > > +\n> > > RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n> > > \t: pipe_(dynamic_cast(pipe))\n> > > {\n> > > @@ -430,17 +445,6 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> > >\n> > > CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > {\n> > > -\tstatic const std::array formats{\n> > > -\t\tPixelFormat(DRM_FORMAT_YUYV),\n> > > -\t\tPixelFormat(DRM_FORMAT_YVYU),\n> > > -\t\tPixelFormat(DRM_FORMAT_VYUY),\n> > > -\t\tPixelFormat(DRM_FORMAT_NV16),\n> > > -\t\tPixelFormat(DRM_FORMAT_NV61),\n> > > -\t\tPixelFormat(DRM_FORMAT_NV21),\n> > > -\t\tPixelFormat(DRM_FORMAT_NV12),\n> > > -\t\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n> > > -\t};\n> > > -\n> > > \tconst CameraSensor *sensor = data_->sensor_;\n> > > \tStatus status = Valid;\n> > >\n> > > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > \tif (roles.empty())\n> > > \t\treturn config;\n> > >\n> > > -\tStreamConfiguration cfg{};\n> > > +\tstd::map> pixelformats;\n> > > +\n> > > +\tfor (PixelFormat pixelformat : formats) {\n> > > +\t\tstd::vector sizes{\n> > > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n> >\n> > I might have missed where these sizes come from.\n> >\n> > > +\t\t};\n> > > +\t\tpixelformats[pixelformat] = sizes;\n> >\n> > So you have an hardcoded list of PixelFormats in the pipeline handler.\n> > And you got a map of V4L2PixelFormats from the video device.\n> > What I would do is going through all the PixelFormats, get their\n> > V4L2PixelFormat counterpart, access the map using that as key to\n> > retrive the size vector, and associated them with the PixelFormat you\n> > started with.\n> >\n> > I don't think it is necessary to make sure all the V4L2PixelFormat\n> > obtained from the PixelFormat array are valid, as the pipeline handler\n> > should know that.\n> >\n> > Thanks\n> > j\n> >\n> >\n> > > +\t}\n> > > +\tStreamFormats format(pixelformats);\n> > > +\tStreamConfiguration cfg(format);\n> > > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > > \tcfg.size = data->sensor_->resolution();\n> > >\n> > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > index ea3d214..96b3dc5 100644\n> > > --- a/src/libcamera/stream.cpp\n> > > +++ b/src/libcamera/stream.cpp\n> > > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > > * configured for a single video stream.\n> > > */\n> > >\n> > > -/**\n> > > - * \\todo This method is deprecated and should be removed once all pipeline\n> > > - * handlers provied StreamFormats.\n> > > - */\n> > > -StreamConfiguration::StreamConfiguration()\n> > > -\t: pixelFormat(0), stream_(nullptr)\n> > > -{\n> > > -}\n> > > -\n> > > /**\n> > > * \\brief Construct a configuration with stream formats\n> > > */\n> > > --\n> > > 2.17.1\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\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-pj1-x1041.google.com (mail-pj1-x1041.google.com\n\t[IPv6:2607:f8b0:4864:20::1041])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C90CD6041A\n\tfor ;\n\tSat, 21 Mar 2020 16:36:44 +0100 (CET)","by mail-pj1-x1041.google.com with SMTP id ng8so3875086pjb.2\n\tfor ;\n\tSat, 21 Mar 2020 08:36:44 -0700 (PDT)","from kaaira-HP-Pavilion-Notebook ([103.113.213.154])\n\tby smtp.gmail.com with ESMTPSA id\n\te28sm8172370pgn.21.2020.03.21.08.36.39\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSat, 21 Mar 2020 08:36:41 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=es-iitr-ac-in.20150623.gappssmtp.com; s=20150623;\n\th=from:date:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=Drnv2ijZH6JYKOmeksQPsxt55C1sj/vihagV7qVSQSM=;\n\tb=MRfkaYAyrJ+vFwYDNMsRDquexh0Ka/xWQB8W32uNENtjBr/beuW2hgIFJiLW4Bgw0w\n\tq1sw1ZjoONE9RukDRLTe6aLe3rUlgtQmKp2B1xpXSH0SLWZncxkaJ5FMqddAi8jAvbJx\n\tKv9g/4ALoqHqHLBFKFW1Zdl2iGvS1N8WK3am7KkZ0OSSh6vZjtmsQ09uptECnjnCMiNg\n\tvO2U8TxyPj4lYk89lqS/CXnvMdI/YLKJDX6vqAHefUtk8hh9zKqM6jiILBYwkRQgD5Yo\n\txoe322uh4BE/RICIpfSgqI5TkobLY406XbPlgTPelvE0nZwrZOCkdfDTbiM8jzJo1192\n\tJu/Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:date:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=Drnv2ijZH6JYKOmeksQPsxt55C1sj/vihagV7qVSQSM=;\n\tb=pMMnlJYQHEppVXPtTfM1+8mQHTk/n1f99EZrB0JrYhf632exYtF0ahqet0qJ084I0F\n\tfWpg9CpIfKIQGwoN26IauLpfew7fIB7hnXyVZPrqdbyvpICwgAu4zSwPsII5sqvQUQ/z\n\tsgoFcYo1n3QTTiK3NplcrBo6gAdPjPXnqC/aXgpP+MeZnAzkxhmHmdZSxWxyjP24qSNo\n\teFBpm3RNigjIOesa2dnmWPD48UM8kZyoAaMDKbOKA+1+AVL6pB2qMYNOkXNSkzxQygE8\n\tseUTK9XJVYhkD7FEvgC2qnwZ0GK+No7z0nHBfqNPiBxd87fNIf1xfkMW0n/Wpi0SSiTu\n\tBv2w==","X-Gm-Message-State":"ANhLgQ0yzlNtl6wFAwRYjGlJAFsdsj9hQggh2zZbuTejq++QDiU+2AFS\n\tWZkW/CY12ZTjWH/XtgpqnAwOLCWU+aE=","X-Google-Smtp-Source":"ADFU+vtHQy62ZzYYnDozegAvacObnvF0ncKsB1SASyG5vubAuqfi0cjN3B0iSxS9Xo346X8cGOYQOQ==","X-Received":"by 2002:a17:90a:c583:: with SMTP id\n\tl3mr1766354pjt.84.1584805002307; \n\tSat, 21 Mar 2020 08:36:42 -0700 (PDT)","From":"Kaaira Gupta ","X-Google-Original-From":"Kaaira Gupta ","Date":"Sat, 21 Mar 2020 21:06:35 +0530","To":"Jacopo Mondi ","Cc":"Kaaira Gupta ,\n\tVaishali Thakkar ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200321153635.GC8952@kaaira-HP-Pavilion-Notebook>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>\n\t<20200321120246.shf3v464dgb4qdud@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200321120246.shf3v464dgb4qdud@uno.localdomain>","User-Agent":"Mutt/1.9.4 (2018-02-28)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 15:36:46 -0000"}},{"id":4168,"web_url":"https://patchwork.libcamera.org/comment/4168/","msgid":"<20200321162856.gsuetc5urt7iadlq@uno.localdomain>","date":"2020-03-21T16:28:56","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kaaira,\n\nOn Sat, Mar 21, 2020 at 08:05:38PM +0530, Kaaira Gupta wrote:\n> On Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n\n[snip]\n\n> > >\n> > > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > \tif (roles.empty())\n> > > \t\treturn config;\n> > >\n> > > -\tStreamConfiguration cfg{};\n> > > +\tstd::map> pixelformats;\n> > > +\n> > > +\tfor (PixelFormat pixelformat : formats) {\n> > > +\t\tstd::vector sizes{\n> > > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n> >\n> > I might have missed where these sizes come from.\n>\n> I got these values from here, in the handler()\n\nvalidate() ?\n\n> \tcfg.size.width = std::max(32U, std::min(4416U, cfg.size.width));\n> \tcfg.size.height = std::max(16U, std::min(3312U, cfg.size.height));\n>\n> That is how I defined them.\n>\n\nOh, sorry missed those. I'm not an expert of the platform, but from\nthat piece of code I get it's lecit to assume for all pixel formats\nthe valid size range is the one you reported.\n\n\n> >\n> > > +\t\t};\n> > > +\t\tpixelformats[pixelformat] = sizes;\n> >\n> > So you have an hardcoded list of PixelFormats in the pipeline handler.\n> > And you got a map of V4L2PixelFormats from the video device.\n> > What I would do is going through all the PixelFormats, get their\n> > V4L2PixelFormat counterpart, access the map using that as key to\n> > retrive the size vector, and associated them with the PixelFormat you\n> > started with.\n>\n> Please read the above logic behind the hardcoded values once and let me\n> know if I still have to map the sizes this way?\n\nNo, I think it's fine the way it is, but I would wait for Niklas or\nLaurent to confirm this as they know this platform better.\n\nIn the meantime, and that's for exercize, not because it's so critical\nat this point, be aware that this goes through 2 copies, if I'm not\nmistaken\n\n std::vector sizes{\n SizeRange{ { 32, 16 }, { 4416, 3312 } }\n };\n pixelformats[pixelformat] = sizes;\n\nYou construct a SizeRange, then copy it inside the vector, than copy\nthe vector inside the map. The compiler might optimize the fist copy,\nbut I think the second one stays.\n\nI think this could be reduced to\n\n\t\tSizeRange range({32, 16}, {4416, 3312});\n\t\tpixelformats.emplace(std::piecewise_construct,\n\t\t\t\t std::forward_as_tuple(pixelformat),\n\t\t\t\t std::forward_as_tuple(1, range));\n\nAnd someone which is more fluent in C++-voodoo than me could even be\nable to construct the Sizerange in place in the vector instead\nhaving to create an instance explicitly and copy it as I'm doing.\n\nThis saves you the copy of of the vector in the map, by constructing the\nelement in the map directly by forwarding the (1, range) arguments to the\nvector constructor. The gain is minumum but not so long ago the first\ntime I saw a similar construct I was a bit puzzled, so I hope this\ncould help decoding other parts of the library where we use it.\n\nThanks\n j\n\n>\n> Thanks!\n>\n> >\n> > I don't think it is necessary to make sure all the V4L2PixelFormat\n> > obtained from the PixelFormat array are valid, as the pipeline handler\n> > should know that.\n> >\n> > Thanks\n> > j\n> >\n> >\n> > > +\t}\n> > > +\tStreamFormats format(pixelformats);\n> > > +\tStreamConfiguration cfg(format);\n> > > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > > \tcfg.size = data->sensor_->resolution();\n> > >\n> > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > index ea3d214..96b3dc5 100644\n> > > --- a/src/libcamera/stream.cpp\n> > > +++ b/src/libcamera/stream.cpp\n> > > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > > * configured for a single video stream.\n> > > */\n> > >\n> > > -/**\n> > > - * \\todo This method is deprecated and should be removed once all pipeline\n> > > - * handlers provied StreamFormats.\n> > > - */\n> > > -StreamConfiguration::StreamConfiguration()\n> > > -\t: pixelFormat(0), stream_(nullptr)\n> > > -{\n> > > -}\n> > > -\n> > > /**\n> > > * \\brief Construct a configuration with stream formats\n> > > */\n> > > --\n> > > 2.17.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 relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 698CB6041A\n\tfor ;\n\tSat, 21 Mar 2020 17:26:06 +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 relay10.mail.gandi.net (Postfix) with ESMTPSA id CE0A7240005;\n\tSat, 21 Mar 2020 16:25:59 +0000 (UTC)"],"Date":"Sat, 21 Mar 2020 17:28:56 +0100","From":"Jacopo Mondi ","To":"Kaaira Gupta ","Cc":"libcamera-devel@lists.libcamera.org, kieran.bingham@ideasonboard.com,\n\tHelen Koike ,\n\tVaishali Thakkar ","Message-ID":"<20200321162856.gsuetc5urt7iadlq@uno.localdomain>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>\n\t<20200321143538.GB8952@kaaira-HP-Pavilion-Notebook>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200321143538.GB8952@kaaira-HP-Pavilion-Notebook>","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 16:26:06 -0000"}},{"id":4170,"web_url":"https://patchwork.libcamera.org/comment/4170/","msgid":"<20200321171008.GE8952@kaaira-HP-Pavilion-Notebook>","date":"2020-03-21T17:10:08","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":39,"url":"https://patchwork.libcamera.org/api/people/39/","name":"Kaaira Gupta","email":"kgupta@es.iitr.ac.in"},"content":"On Sat, Mar 21, 2020 at 05:28:56PM +0100, Jacopo Mondi wrote:\n> Hi Kaaira,\n> \n> On Sat, Mar 21, 2020 at 08:05:38PM +0530, Kaaira Gupta wrote:\n> > On Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n> \n> [snip]\n> \n> > > >\n> > > > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > > \tif (roles.empty())\n> > > > \t\treturn config;\n> > > >\n> > > > -\tStreamConfiguration cfg{};\n> > > > +\tstd::map> pixelformats;\n> > > > +\n> > > > +\tfor (PixelFormat pixelformat : formats) {\n> > > > +\t\tstd::vector sizes{\n> > > > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n> > >\n> > > I might have missed where these sizes come from.\n> >\n> > I got these values from here, in the handler()\n> \n> validate() ?\n\nYes, sorry.\n\n> \n> > \tcfg.size.width = std::max(32U, std::min(4416U, cfg.size.width));\n> > \tcfg.size.height = std::max(16U, std::min(3312U, cfg.size.height));\n> >\n> > That is how I defined them.\n> >\n> \n> Oh, sorry missed those. I'm not an expert of the platform, but from\n> that piece of code I get it's lecit to assume for all pixel formats\n> the valid size range is the one you reported.\n> \n> \n> > >\n> > > > +\t\t};\n> > > > +\t\tpixelformats[pixelformat] = sizes;\n> > >\n> > > So you have an hardcoded list of PixelFormats in the pipeline handler.\n> > > And you got a map of V4L2PixelFormats from the video device.\n> > > What I would do is going through all the PixelFormats, get their\n> > > V4L2PixelFormat counterpart, access the map using that as key to\n> > > retrive the size vector, and associated them with the PixelFormat you\n> > > started with.\n> >\n> > Please read the above logic behind the hardcoded values once and let me\n> > know if I still have to map the sizes this way?\n> \n> No, I think it's fine the way it is, but I would wait for Niklas or\n> Laurent to confirm this as they know this platform better.\n\nOkay!\n\n> \n> In the meantime, and that's for exercize, not because it's so critical\n> at this point, be aware that this goes through 2 copies, if I'm not\n> mistaken\n> \n> std::vector sizes{\n> SizeRange{ { 32, 16 }, { 4416, 3312 } }\n> };\n> pixelformats[pixelformat] = sizes;\n> \n> You construct a SizeRange, then copy it inside the vector, than copy\n> the vector inside the map. The compiler might optimize the fist copy,\n> but I think the second one stays.\n> \n> I think this could be reduced to\n> \n> \t\tSizeRange range({32, 16}, {4416, 3312});\n> \t\tpixelformats.emplace(std::piecewise_construct,\n> \t\t\t\t std::forward_as_tuple(pixelformat),\n> \t\t\t\t std::forward_as_tuple(1, range));\n> \n> And someone which is more fluent in C++-voodoo than me could even be\n> able to construct the Sizerange in place in the vector instead\n> having to create an instance explicitly and copy it as I'm doing.\n\nI'll try that for sure\n\n> \n> This saves you the copy of of the vector in the map, by constructing the\n> element in the map directly by forwarding the (1, range) arguments to the\n> vector constructor. The gain is minumum but not so long ago the first\n> time I saw a similar construct I was a bit puzzled, so I hope this\n> could help decoding other parts of the library where we use it.\n\nThankyou so much for this help, it would surely have taken me time to\ndecode it myself as I am more fluent with Java and Python. Had C++ as a\ncourse in just one sem..\n\nAlso, as for the error the compiler throws in validate(), if you get\ntime please check my mails regarding that.\n\nThanks!\n\n> \n> Thanks\n> j\n> \n> >\n> > Thanks!\n> >\n> > >\n> > > I don't think it is necessary to make sure all the V4L2PixelFormat\n> > > obtained from the PixelFormat array are valid, as the pipeline handler\n> > > should know that.\n> > >\n> > > Thanks\n> > > j\n> > >\n> > >\n> > > > +\t}\n> > > > +\tStreamFormats format(pixelformats);\n> > > > +\tStreamConfiguration cfg(format);\n> > > > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > > > \tcfg.size = data->sensor_->resolution();\n> > > >\n> > > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > > index ea3d214..96b3dc5 100644\n> > > > --- a/src/libcamera/stream.cpp\n> > > > +++ b/src/libcamera/stream.cpp\n> > > > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > > > * configured for a single video stream.\n> > > > */\n> > > >\n> > > > -/**\n> > > > - * \\todo This method is deprecated and should be removed once all pipeline\n> > > > - * handlers provied StreamFormats.\n> > > > - */\n> > > > -StreamConfiguration::StreamConfiguration()\n> > > > -\t: pixelFormat(0), stream_(nullptr)\n> > > > -{\n> > > > -}\n> > > > -\n> > > > /**\n> > > > * \\brief Construct a configuration with stream formats\n> > > > */\n> > > > --\n> > > > 2.17.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-pf1-x443.google.com (mail-pf1-x443.google.com\n\t[IPv6:2607:f8b0:4864:20::443])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B35AC60415\n\tfor ;\n\tSat, 21 Mar 2020 18:10:17 +0100 (CET)","by mail-pf1-x443.google.com with SMTP id z65so5048149pfz.8\n\tfor ;\n\tSat, 21 Mar 2020 10:10:17 -0700 (PDT)","from kaaira-HP-Pavilion-Notebook ([103.113.213.154])\n\tby smtp.gmail.com with ESMTPSA id\n\ty28sm8926719pfp.128.2020.03.21.10.10.12\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSat, 21 Mar 2020 10:10:15 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=es-iitr-ac-in.20150623.gappssmtp.com; s=20150623;\n\th=from:date:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=zJaMc1HUIL5sHut5NvLeOxaXa5J/atSUl1uxi+JJlvg=;\n\tb=GDo+7UnhC6m8hx8xUsp2xJs4mIPIdeRES64KEbVG4k7bv5m9+F2s/fMd7mASm1yALx\n\tvcnfwFpdNZYA/G0hOSxnlEd4lDLHoDMOe64DJO0CXW1O3ZY1uO89pktdZ1H+3+o21eAc\n\tfZ1rPxV5x/K/xfkZHmGUrbeV0Iw4+cNJobeiZ1XoYY9GSnNHW2YtGXAetKOA53nHT5oo\n\tanInsJgf26WlsSI4dOKzGxHVjJGNRhzGYD+N2dlNmkTMXV004v02E4g0ZSesImpbecHY\n\tDjVIZXopw6uBPTmbRCVTm52OlxMRB5qBLbEi89qukaT5Bu4TxmEKknsGrDUd0hhckRJi\n\tQwuw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:date:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=zJaMc1HUIL5sHut5NvLeOxaXa5J/atSUl1uxi+JJlvg=;\n\tb=QWe0KQAkFEEU3DXEhSXnrKG/gmGEj+wdGV/Xq+n+krTkyU64/HOQ0+mLxKb8wj9VXK\n\tZ7rFU+wUC5khQ1akbpwwfVmhQd7lsU2ZvAoonCpBIualuB67kfeXsDyJMKrTxiMuk8KB\n\t/f8g3pMFMOBv0x3QBHD3BIHpTpwTlImM8UgGpWgydxugKOx6SXDXdvQ7QJ7WFU922adZ\n\t68gdO0iCPV52Kxp6lCF26rGsyPoNlOA/DiMm6EZUDxeCDFk+p0E1239QNjoO+2hwe0lU\n\tx3J7jRlHgDtAePJryNruAJCEVt6z6A1cQb75x3VsL50H8rWD+T8+dT6EZLWIYrZCeAGW\n\tiI1Q==","X-Gm-Message-State":"ANhLgQ0gkAB7mQSFLitiJr4GsOazfiT1GG3ewXmQHQLYis1CyhetMmqc\n\tk5M761o3wRAlldGEVZv5p59Nvg==","X-Google-Smtp-Source":"ADFU+vuSNnYxLQLynSdy7E4XFu32kXjLiRL8lr8uIETyo8n8wSelJ08YGyEGGCpu6h+zQsBbgmBPKA==","X-Received":"by 2002:a62:502:: with SMTP id 2mr16169723pff.324.1584810616128; \n\tSat, 21 Mar 2020 10:10:16 -0700 (PDT)","From":"Kaaira Gupta ","X-Google-Original-From":"Kaaira Gupta ","Date":"Sat, 21 Mar 2020 22:40:08 +0530","To":"Jacopo Mondi ","Cc":"Kaaira Gupta , libcamera-devel@lists.libcamera.org,\n\tkieran.bingham@ideasonboard.com,\n\tHelen Koike , \n\tVaishali Thakkar ","Message-ID":"<20200321171008.GE8952@kaaira-HP-Pavilion-Notebook>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>\n\t<20200321143538.GB8952@kaaira-HP-Pavilion-Notebook>\n\t<20200321162856.gsuetc5urt7iadlq@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200321162856.gsuetc5urt7iadlq@uno.localdomain>","User-Agent":"Mutt/1.9.4 (2018-02-28)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 17:10:17 -0000"}},{"id":4171,"web_url":"https://patchwork.libcamera.org/comment/4171/","msgid":"<20200321172754.bvabz2wg3bc2m4by@uno.localdomain>","date":"2020-03-21T17:27:54","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kaaira,\n\nOn Sat, Mar 21, 2020 at 10:40:08PM +0530, Kaaira Gupta wrote:\n> On Sat, Mar 21, 2020 at 05:28:56PM +0100, Jacopo Mondi wrote:\n> > Hi Kaaira,\n> >\n> > On Sat, Mar 21, 2020 at 08:05:38PM +0530, Kaaira Gupta wrote:\n> > > On Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n> >\n> > [snip]\n> >\n> > > > >\n> > > > > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > > > \tif (roles.empty())\n> > > > > \t\treturn config;\n> > > > >\n> > > > > -\tStreamConfiguration cfg{};\n> > > > > +\tstd::map> pixelformats;\n> > > > > +\n> > > > > +\tfor (PixelFormat pixelformat : formats) {\n> > > > > +\t\tstd::vector sizes{\n> > > > > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n> > > >\n> > > > I might have missed where these sizes come from.\n> > >\n> > > I got these values from here, in the handler()\n> >\n> > validate() ?\n>\n> Yes, sorry.\n>\n> >\n> > > \tcfg.size.width = std::max(32U, std::min(4416U, cfg.size.width));\n> > > \tcfg.size.height = std::max(16U, std::min(3312U, cfg.size.height));\n> > >\n> > > That is how I defined them.\n> > >\n> >\n> > Oh, sorry missed those. I'm not an expert of the platform, but from\n> > that piece of code I get it's lecit to assume for all pixel formats\n> > the valid size range is the one you reported.\n> >\n> >\n> > > >\n> > > > > +\t\t};\n> > > > > +\t\tpixelformats[pixelformat] = sizes;\n> > > >\n> > > > So you have an hardcoded list of PixelFormats in the pipeline handler.\n> > > > And you got a map of V4L2PixelFormats from the video device.\n> > > > What I would do is going through all the PixelFormats, get their\n> > > > V4L2PixelFormat counterpart, access the map using that as key to\n> > > > retrive the size vector, and associated them with the PixelFormat you\n> > > > started with.\n> > >\n> > > Please read the above logic behind the hardcoded values once and let me\n> > > know if I still have to map the sizes this way?\n> >\n> > No, I think it's fine the way it is, but I would wait for Niklas or\n> > Laurent to confirm this as they know this platform better.\n>\n> Okay!\n>\n> >\n> > In the meantime, and that's for exercize, not because it's so critical\n> > at this point, be aware that this goes through 2 copies, if I'm not\n> > mistaken\n> >\n> > std::vector sizes{\n> > SizeRange{ { 32, 16 }, { 4416, 3312 } }\n> > };\n> > pixelformats[pixelformat] = sizes;\n> >\n> > You construct a SizeRange, then copy it inside the vector, than copy\n> > the vector inside the map. The compiler might optimize the fist copy,\n> > but I think the second one stays.\n> >\n> > I think this could be reduced to\n> >\n> > \t\tSizeRange range({32, 16}, {4416, 3312});\n> > \t\tpixelformats.emplace(std::piecewise_construct,\n> > \t\t\t\t std::forward_as_tuple(pixelformat),\n> > \t\t\t\t std::forward_as_tuple(1, range));\n> >\n> > And someone which is more fluent in C++-voodoo than me could even be\n> > able to construct the Sizerange in place in the vector instead\n> > having to create an instance explicitly and copy it as I'm doing.\n>\n> I'll try that for sure\n>\n> >\n> > This saves you the copy of of the vector in the map, by constructing the\n> > element in the map directly by forwarding the (1, range) arguments to the\n> > vector constructor. The gain is minumum but not so long ago the first\n> > time I saw a similar construct I was a bit puzzled, so I hope this\n> > could help decoding other parts of the library where we use it.\n>\n> Thankyou so much for this help, it would surely have taken me time to\n> decode it myself as I am more fluent with Java and Python. Had C++ as a\n> course in just one sem..\n>\n> Also, as for the error the compiler throws in validate(), if you get\n> time please check my mails regarding that.\n>\n\nI did, I'm not sure you have read the clarification were I suggested to\nreplace resize() with erase() :)\n\n> Thanks!\n>\n> >\n> > Thanks\n> > j\n> >\n> > >\n> > > Thanks!\n> > >\n> > > >\n> > > > I don't think it is necessary to make sure all the V4L2PixelFormat\n> > > > obtained from the PixelFormat array are valid, as the pipeline handler\n> > > > should know that.\n> > > >\n> > > > Thanks\n> > > > j\n> > > >\n> > > >\n> > > > > +\t}\n> > > > > +\tStreamFormats format(pixelformats);\n> > > > > +\tStreamConfiguration cfg(format);\n> > > > > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > > > > \tcfg.size = data->sensor_->resolution();\n> > > > >\n> > > > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > > > index ea3d214..96b3dc5 100644\n> > > > > --- a/src/libcamera/stream.cpp\n> > > > > +++ b/src/libcamera/stream.cpp\n> > > > > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > > > > * configured for a single video stream.\n> > > > > */\n> > > > >\n> > > > > -/**\n> > > > > - * \\todo This method is deprecated and should be removed once all pipeline\n> > > > > - * handlers provied StreamFormats.\n> > > > > - */\n> > > > > -StreamConfiguration::StreamConfiguration()\n> > > > > -\t: pixelFormat(0), stream_(nullptr)\n> > > > > -{\n> > > > > -}\n> > > > > -\n> > > > > /**\n> > > > > * \\brief Construct a configuration with stream formats\n> > > > > */\n> > > > > --\n> > > > > 2.17.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 relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 931A760415\n\tfor ;\n\tSat, 21 Mar 2020 18:24:58 +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 relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 7A8F9240005;\n\tSat, 21 Mar 2020 17:24:56 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sat, 21 Mar 2020 18:27:54 +0100","From":"Jacopo Mondi ","To":"Kaaira Gupta ","Cc":"libcamera-devel@lists.libcamera.org, kieran.bingham@ideasonboard.com,\n\tHelen Koike ,\n\tVaishali Thakkar ","Message-ID":"<20200321172754.bvabz2wg3bc2m4by@uno.localdomain>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>\n\t<20200321143538.GB8952@kaaira-HP-Pavilion-Notebook>\n\t<20200321162856.gsuetc5urt7iadlq@uno.localdomain>\n\t<20200321171008.GE8952@kaaira-HP-Pavilion-Notebook>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200321171008.GE8952@kaaira-HP-Pavilion-Notebook>","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 17:24:58 -0000"}},{"id":4172,"web_url":"https://patchwork.libcamera.org/comment/4172/","msgid":"<20200321175948.GF8952@kaaira-HP-Pavilion-Notebook>","date":"2020-03-21T17:59:48","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":39,"url":"https://patchwork.libcamera.org/api/people/39/","name":"Kaaira Gupta","email":"kgupta@es.iitr.ac.in"},"content":"On Sat, Mar 21, 2020 at 06:27:54PM +0100, Jacopo Mondi wrote:\n> Hi Kaaira,\n> \n> On Sat, Mar 21, 2020 at 10:40:08PM +0530, Kaaira Gupta wrote:\n> > On Sat, Mar 21, 2020 at 05:28:56PM +0100, Jacopo Mondi wrote:\n> > > Hi Kaaira,\n> > >\n> > > On Sat, Mar 21, 2020 at 08:05:38PM +0530, Kaaira Gupta wrote:\n> > > > On Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n> > >\n> > > [snip]\n> > >\n> > > > > >\n> > > > > > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > > > > \tif (roles.empty())\n> > > > > > \t\treturn config;\n> > > > > >\n> > > > > > -\tStreamConfiguration cfg{};\n> > > > > > +\tstd::map> pixelformats;\n> > > > > > +\n> > > > > > +\tfor (PixelFormat pixelformat : formats) {\n> > > > > > +\t\tstd::vector sizes{\n> > > > > > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n> > > > >\n> > > > > I might have missed where these sizes come from.\n> > > >\n> > > > I got these values from here, in the handler()\n> > >\n> > > validate() ?\n> >\n> > Yes, sorry.\n> >\n> > >\n> > > > \tcfg.size.width = std::max(32U, std::min(4416U, cfg.size.width));\n> > > > \tcfg.size.height = std::max(16U, std::min(3312U, cfg.size.height));\n> > > >\n> > > > That is how I defined them.\n> > > >\n> > >\n> > > Oh, sorry missed those. I'm not an expert of the platform, but from\n> > > that piece of code I get it's lecit to assume for all pixel formats\n> > > the valid size range is the one you reported.\n> > >\n> > >\n> > > > >\n> > > > > > +\t\t};\n> > > > > > +\t\tpixelformats[pixelformat] = sizes;\n> > > > >\n> > > > > So you have an hardcoded list of PixelFormats in the pipeline handler.\n> > > > > And you got a map of V4L2PixelFormats from the video device.\n> > > > > What I would do is going through all the PixelFormats, get their\n> > > > > V4L2PixelFormat counterpart, access the map using that as key to\n> > > > > retrive the size vector, and associated them with the PixelFormat you\n> > > > > started with.\n> > > >\n> > > > Please read the above logic behind the hardcoded values once and let me\n> > > > know if I still have to map the sizes this way?\n> > >\n> > > No, I think it's fine the way it is, but I would wait for Niklas or\n> > > Laurent to confirm this as they know this platform better.\n> >\n> > Okay!\n> >\n> > >\n> > > In the meantime, and that's for exercize, not because it's so critical\n> > > at this point, be aware that this goes through 2 copies, if I'm not\n> > > mistaken\n> > >\n> > > std::vector sizes{\n> > > SizeRange{ { 32, 16 }, { 4416, 3312 } }\n> > > };\n> > > pixelformats[pixelformat] = sizes;\n> > >\n> > > You construct a SizeRange, then copy it inside the vector, than copy\n> > > the vector inside the map. The compiler might optimize the fist copy,\n> > > but I think the second one stays.\n> > >\n> > > I think this could be reduced to\n> > >\n> > > \t\tSizeRange range({32, 16}, {4416, 3312});\n> > > \t\tpixelformats.emplace(std::piecewise_construct,\n> > > \t\t\t\t std::forward_as_tuple(pixelformat),\n> > > \t\t\t\t std::forward_as_tuple(1, range));\n> > >\n> > > And someone which is more fluent in C++-voodoo than me could even be\n> > > able to construct the Sizerange in place in the vector instead\n> > > having to create an instance explicitly and copy it as I'm doing.\n> >\n> > I'll try that for sure\n> >\n> > >\n> > > This saves you the copy of of the vector in the map, by constructing the\n> > > element in the map directly by forwarding the (1, range) arguments to the\n> > > vector constructor. The gain is minumum but not so long ago the first\n> > > time I saw a similar construct I was a bit puzzled, so I hope this\n> > > could help decoding other parts of the library where we use it.\n> >\n> > Thankyou so much for this help, it would surely have taken me time to\n> > decode it myself as I am more fluent with Java and Python. Had C++ as a\n> > course in just one sem..\n> >\n> > Also, as for the error the compiler throws in validate(), if you get\n> > time please check my mails regarding that.\n> >\n> \n> I did, I'm not sure you have read the clarification were I suggested to\n> replace resize() with erase() :)\n\nYes I did, and yes using clear() worked. But I was confused as to why the compiler\nwould worry about the size being less than the resize size when we\nalready check for it before sending it to resize. But now I understand\nthat it's the property of resize() to check for both larger and smaller\nsize whether or not we check beforehand. Hence, it's clear to me now.\n\nThanks\n\n> \n> > Thanks!\n> >\n> > >\n> > > Thanks\n> > > j\n> > >\n> > > >\n> > > > Thanks!\n> > > >\n> > > > >\n> > > > > I don't think it is necessary to make sure all the V4L2PixelFormat\n> > > > > obtained from the PixelFormat array are valid, as the pipeline handler\n> > > > > should know that.\n> > > > >\n> > > > > Thanks\n> > > > > j\n> > > > >\n> > > > >\n> > > > > > +\t}\n> > > > > > +\tStreamFormats format(pixelformats);\n> > > > > > +\tStreamConfiguration cfg(format);\n> > > > > > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > > > > > \tcfg.size = data->sensor_->resolution();\n> > > > > >\n> > > > > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > > > > index ea3d214..96b3dc5 100644\n> > > > > > --- a/src/libcamera/stream.cpp\n> > > > > > +++ b/src/libcamera/stream.cpp\n> > > > > > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > > > > > * configured for a single video stream.\n> > > > > > */\n> > > > > >\n> > > > > > -/**\n> > > > > > - * \\todo This method is deprecated and should be removed once all pipeline\n> > > > > > - * handlers provied StreamFormats.\n> > > > > > - */\n> > > > > > -StreamConfiguration::StreamConfiguration()\n> > > > > > -\t: pixelFormat(0), stream_(nullptr)\n> > > > > > -{\n> > > > > > -}\n> > > > > > -\n> > > > > > /**\n> > > > > > * \\brief Construct a configuration with stream formats\n> > > > > > */\n> > > > > > --\n> > > > > > 2.17.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-pg1-x541.google.com (mail-pg1-x541.google.com\n\t[IPv6:2607:f8b0:4864:20::541])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C734560415\n\tfor ;\n\tSat, 21 Mar 2020 18:59:58 +0100 (CET)","by mail-pg1-x541.google.com with SMTP id d17so4232379pgo.0\n\tfor ;\n\tSat, 21 Mar 2020 10:59:58 -0700 (PDT)","from kaaira-HP-Pavilion-Notebook ([103.113.213.154])\n\tby smtp.gmail.com with ESMTPSA id\n\to65sm2366903pfg.187.2020.03.21.10.59.53\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSat, 21 Mar 2020 10:59:55 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=es-iitr-ac-in.20150623.gappssmtp.com; s=20150623;\n\th=from:date:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=KwJhEE85u6K/5OcKYpG8h++5URRhVfY8vU+VGCtkT+4=;\n\tb=Sa3WBX5Bq+S+H3sKJ6xGqzkWX85J9gZCCVs8XDfRDb3G5PNIMcq7sAlDoffLMwO0tx\n\tyYsw300+Ou4014Tpc5WI424m3paIk6cKKH4huL7yBGqcsezFZyJSLRt5BG8NXRMWjZ0o\n\t+UvEvUj2E1p/qUN1J09hTab3twfjb9ALQzzwwUTJfbzwU/uw4vjofa+DMcn53FNQGO+z\n\tGwIBNmjO+Tl7ES7YXfQIhWKgx9AdhIeIJ5/yR4oL7wZY99Dkq6W65vBRDIvjSUrfxkNQ\n\t4fenPbm0CKKluZyMZF4/JVUGZ5cPeQjtmE4on0KMZ/znEj7vjDZv1xsDBFe0+S1B22YJ\n\t7jww==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:date:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=KwJhEE85u6K/5OcKYpG8h++5URRhVfY8vU+VGCtkT+4=;\n\tb=eVEDKYXVAkDcGhhWDKvyyFUmpboeVTE4dgeXuzEwTuQJ2CHyylo/aRNn1uG2Gn6ST9\n\txBmLxvNS0LoQudEzum0MsZkB6LD7upzs0qghkcYvmdVrKr4UiTmLCi1YefcihXb0voW5\n\ty6UGZ4Pre1O5N/FAXcm0GBGAUqLQA3N9eM0KW3TPUY1Uu05bBdVXQV/UYFuw1o7A9rsK\n\t6hQDfnWgf9AMc9yugU7oTjEBDd+3lrp1bhUw7tUb86qPA+bNCzbvPs4VpfRe1oIHsdy/\n\to02070heb9LsDgAnEPQRDrnHmGc2fteO3uHZ6Zgc7TTauYaH1fv7p+iUIYSEZh0CTshH\n\tTtwQ==","X-Gm-Message-State":"ANhLgQ2xd1BUfVB11yr4yWiTrAtEBd51W2bzT1LCokxAsVTYxAqgXTQe\n\t5Cq7luRVQMo4HgqPJNhLQ8q09w==","X-Google-Smtp-Source":"ADFU+vvBx6ywTfSHHSE7viIRR4hg9uqDAdYFsDNahblTKrhG72Cmd9SX4w4u2aTrYZCrT9Mw9NakWQ==","X-Received":"by 2002:a63:1e44:: with SMTP id\n\tp4mr14532191pgm.367.1584813596779; \n\tSat, 21 Mar 2020 10:59:56 -0700 (PDT)","From":"Kaaira Gupta ","X-Google-Original-From":"Kaaira Gupta ","Date":"Sat, 21 Mar 2020 23:29:48 +0530","To":"Jacopo Mondi ","Cc":"Kaaira Gupta , libcamera-devel@lists.libcamera.org,\n\tkieran.bingham@ideasonboard.com,\n\tHelen Koike , \n\tVaishali Thakkar ","Message-ID":"<20200321175948.GF8952@kaaira-HP-Pavilion-Notebook>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>\n\t<20200321143538.GB8952@kaaira-HP-Pavilion-Notebook>\n\t<20200321162856.gsuetc5urt7iadlq@uno.localdomain>\n\t<20200321171008.GE8952@kaaira-HP-Pavilion-Notebook>\n\t<20200321172754.bvabz2wg3bc2m4by@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200321172754.bvabz2wg3bc2m4by@uno.localdomain>","User-Agent":"Mutt/1.9.4 (2018-02-28)","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 18:00:02 -0000"}},{"id":4173,"web_url":"https://patchwork.libcamera.org/comment/4173/","msgid":"<20200321182935.mjymbqduutztecra@uno.localdomain>","date":"2020-03-21T18:30:14","subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kaaira,\n\nOn Sat, Mar 21, 2020 at 11:29:48PM +0530, Kaaira Gupta wrote:\n> On Sat, Mar 21, 2020 at 06:27:54PM +0100, Jacopo Mondi wrote:\n> > Hi Kaaira,\n> >\n> > On Sat, Mar 21, 2020 at 10:40:08PM +0530, Kaaira Gupta wrote:\n> > > On Sat, Mar 21, 2020 at 05:28:56PM +0100, Jacopo Mondi wrote:\n> > > > Hi Kaaira,\n> > > >\n> > > > On Sat, Mar 21, 2020 at 08:05:38PM +0530, Kaaira Gupta wrote:\n> > > > > On Sat, Mar 21, 2020 at 12:56:12PM +0100, Jacopo Mondi wrote:\n> > > >\n> > > > [snip]\n> > > >\n> > > > > > >\n> > > > > > > @@ -537,7 +541,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > > > > > \tif (roles.empty())\n> > > > > > > \t\treturn config;\n> > > > > > >\n> > > > > > > -\tStreamConfiguration cfg{};\n> > > > > > > +\tstd::map> pixelformats;\n> > > > > > > +\n> > > > > > > +\tfor (PixelFormat pixelformat : formats) {\n> > > > > > > +\t\tstd::vector sizes{\n> > > > > > > +\t\t\tSizeRange{ { 32, 16 }, { 4416, 3312 } }\n> > > > > >\n> > > > > > I might have missed where these sizes come from.\n> > > > >\n> > > > > I got these values from here, in the handler()\n> > > >\n> > > > validate() ?\n> > >\n> > > Yes, sorry.\n> > >\n> > > >\n> > > > > \tcfg.size.width = std::max(32U, std::min(4416U, cfg.size.width));\n> > > > > \tcfg.size.height = std::max(16U, std::min(3312U, cfg.size.height));\n> > > > >\n> > > > > That is how I defined them.\n> > > > >\n> > > >\n> > > > Oh, sorry missed those. I'm not an expert of the platform, but from\n> > > > that piece of code I get it's lecit to assume for all pixel formats\n> > > > the valid size range is the one you reported.\n> > > >\n> > > >\n> > > > > >\n> > > > > > > +\t\t};\n> > > > > > > +\t\tpixelformats[pixelformat] = sizes;\n> > > > > >\n> > > > > > So you have an hardcoded list of PixelFormats in the pipeline handler.\n> > > > > > And you got a map of V4L2PixelFormats from the video device.\n> > > > > > What I would do is going through all the PixelFormats, get their\n> > > > > > V4L2PixelFormat counterpart, access the map using that as key to\n> > > > > > retrive the size vector, and associated them with the PixelFormat you\n> > > > > > started with.\n> > > > >\n> > > > > Please read the above logic behind the hardcoded values once and let me\n> > > > > know if I still have to map the sizes this way?\n> > > >\n> > > > No, I think it's fine the way it is, but I would wait for Niklas or\n> > > > Laurent to confirm this as they know this platform better.\n> > >\n> > > Okay!\n> > >\n> > > >\n> > > > In the meantime, and that's for exercize, not because it's so critical\n> > > > at this point, be aware that this goes through 2 copies, if I'm not\n> > > > mistaken\n> > > >\n> > > > std::vector sizes{\n> > > > SizeRange{ { 32, 16 }, { 4416, 3312 } }\n> > > > };\n> > > > pixelformats[pixelformat] = sizes;\n> > > >\n> > > > You construct a SizeRange, then copy it inside the vector, than copy\n> > > > the vector inside the map. The compiler might optimize the fist copy,\n> > > > but I think the second one stays.\n> > > >\n> > > > I think this could be reduced to\n> > > >\n> > > > \t\tSizeRange range({32, 16}, {4416, 3312});\n> > > > \t\tpixelformats.emplace(std::piecewise_construct,\n> > > > \t\t\t\t std::forward_as_tuple(pixelformat),\n> > > > \t\t\t\t std::forward_as_tuple(1, range));\n> > > >\n> > > > And someone which is more fluent in C++-voodoo than me could even be\n> > > > able to construct the Sizerange in place in the vector instead\n> > > > having to create an instance explicitly and copy it as I'm doing.\n> > >\n> > > I'll try that for sure\n> > >\n> > > >\n> > > > This saves you the copy of of the vector in the map, by constructing the\n> > > > element in the map directly by forwarding the (1, range) arguments to the\n> > > > vector constructor. The gain is minumum but not so long ago the first\n> > > > time I saw a similar construct I was a bit puzzled, so I hope this\n> > > > could help decoding other parts of the library where we use it.\n> > >\n> > > Thankyou so much for this help, it would surely have taken me time to\n> > > decode it myself as I am more fluent with Java and Python. Had C++ as a\n> > > course in just one sem..\n> > >\n> > > Also, as for the error the compiler throws in validate(), if you get\n> > > time please check my mails regarding that.\n> > >\n> >\n> > I did, I'm not sure you have read the clarification were I suggested to\n> > replace resize() with erase() :)\n>\n> Yes I did, and yes using clear() worked. But I was confused as to why the compiler\n> would worry about the size being less than the resize size when we\n> already check for it before sending it to resize. But now I understand\n> that it's the property of resize() to check for both larger and smaller\n> size whether or not we check beforehand. Hence, it's clear to me now.\n>\n\nGreat! be aware that clear() != erase()\n\n void clear();\n (until C++11)\n Erases all elements from the container. After this call, size() returns zero.\n\nThis is not what you want, I'm sure :)\n\nThanks\n j\n\n> Thanks\n>\n> >\n> > > Thanks!\n> > >\n> > > >\n> > > > Thanks\n> > > > j\n> > > >\n> > > > >\n> > > > > Thanks!\n> > > > >\n> > > > > >\n> > > > > > I don't think it is necessary to make sure all the V4L2PixelFormat\n> > > > > > obtained from the PixelFormat array are valid, as the pipeline handler\n> > > > > > should know that.\n> > > > > >\n> > > > > > Thanks\n> > > > > > j\n> > > > > >\n> > > > > >\n> > > > > > > +\t}\n> > > > > > > +\tStreamFormats format(pixelformats);\n> > > > > > > +\tStreamConfiguration cfg(format);\n> > > > > > > \tcfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12);\n> > > > > > > \tcfg.size = data->sensor_->resolution();\n> > > > > > >\n> > > > > > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > > > > > index ea3d214..96b3dc5 100644\n> > > > > > > --- a/src/libcamera/stream.cpp\n> > > > > > > +++ b/src/libcamera/stream.cpp\n> > > > > > > @@ -274,15 +274,6 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const\n> > > > > > > * configured for a single video stream.\n> > > > > > > */\n> > > > > > >\n> > > > > > > -/**\n> > > > > > > - * \\todo This method is deprecated and should be removed once all pipeline\n> > > > > > > - * handlers provied StreamFormats.\n> > > > > > > - */\n> > > > > > > -StreamConfiguration::StreamConfiguration()\n> > > > > > > -\t: pixelFormat(0), stream_(nullptr)\n> > > > > > > -{\n> > > > > > > -}\n> > > > > > > -\n> > > > > > > /**\n> > > > > > > * \\brief Construct a configuration with stream formats\n> > > > > > > */\n> > > > > > > --\n> > > > > > > 2.17.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 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 4D6CE60415\n\tfor ;\n\tSat, 21 Mar 2020 19:27:20 +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 0D4BDC0007;\n\tSat, 21 Mar 2020 18:27:17 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sat, 21 Mar 2020 19:30:14 +0100","From":"Jacopo Mondi ","To":"Kaaira Gupta ","Cc":"libcamera-devel@lists.libcamera.org, kieran.bingham@ideasonboard.com,\n\tHelen Koike ,\n\tVaishali Thakkar ","Message-ID":"<20200321182935.mjymbqduutztecra@uno.localdomain>","References":"<20200320215020.GA26347@kaaira-HP-Pavilion-Notebook>\n\t<20200321115546.33cco4naxal64nhl@uno.localdomain>\n\t<20200321143538.GB8952@kaaira-HP-Pavilion-Notebook>\n\t<20200321162856.gsuetc5urt7iadlq@uno.localdomain>\n\t<20200321171008.GE8952@kaaira-HP-Pavilion-Notebook>\n\t<20200321172754.bvabz2wg3bc2m4by@uno.localdomain>\n\t<20200321175948.GF8952@kaaira-HP-Pavilion-Notebook>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200321175948.GF8952@kaaira-HP-Pavilion-Notebook>","Subject":"Re: [libcamera-devel] [PATCH LIBCAMERA WIP] libcamera: rkisp1: Use\n\tparametered constructor instead of default","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 21 Mar 2020 18:27:20 -0000"}}]