[{"id":11247,"web_url":"https://patchwork.libcamera.org/comment/11247/","msgid":"<20200708145943.GD20298@pendragon.ideasonboard.com>","date":"2020-07-08T14:59:43","subject":"Re: [libcamera-devel] [PATCH v4 08/21] v4l2: v4l2_camera_proxy: Use\n\tstream config in tryFormat","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Wed, Jul 08, 2020 at 10:44:04PM +0900, Paul Elder wrote:\n> For handling try_fmt, the values should be filled in by validating the\n> stream configuration, and not by recalculating them or manually checking\n> against the cached list of formats and sizes. Use\n> V4L2Camera::validateConfiguration to obtain size, format, stride, and\n> frameSize values. Also, implement this function in V4L2Camera.\n\nMaybe \"Add a new V4L2Camera::validateConfiguration() function to\nvalidate a configuration, and use it to obtain ...\" ?\n\nAs the pipeline handlers don't fill stride and frame size consistently\nat this point, this may cause a bit of a regression. I would shuffle the\npatches in order to \n\n- extend PixelFormatInfo\n- add stride and frameSize to StreamConfiguration\n- update the pipeline handlers\n- use all this in the V4L2 compat layer\n\nThis should hopefully not cause any conflict.\n\n> Signed-off-by: Paul Elder \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> Changes in v4:\n> - clean up code a bit\n> - squash the implementation and usage\n> \n> New in v3\n> ---\n> src/v4l2/v4l2_camera.cpp | 20 ++++++++++++++++++++\n> src/v4l2/v4l2_camera.h | 3 +++\n> src/v4l2/v4l2_camera_proxy.cpp | 23 +++++++++--------------\n> 3 files changed, 32 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> index ffc1230..9f81c97 100644\n> --- a/src/v4l2/v4l2_camera.cpp\n> +++ b/src/v4l2/v4l2_camera.cpp\n> @@ -138,6 +138,26 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut,\n> \treturn 0;\n> }\n> \n> +int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,\n> +\t\t\t\t const Size &size,\n> +\t\t\t\t StreamConfiguration *streamConfigOut)\n> +{\n> +\tstd::unique_ptr config =\n> +\t\tcamera_->generateConfiguration({ StreamRole::Viewfinder });\n> +\tStreamConfiguration &cfg = config->at(0);\n> +\tcfg.size = size;\n> +\tcfg.pixelFormat = pixelFormat;\n> +\tcfg.bufferCount = 1;\n> +\n> +\tCameraConfiguration::Status validation = config->validate();\n> +\tif (validation == CameraConfiguration::Invalid)\n> +\t\treturn -EINVAL;\n> +\n> +\t*streamConfigOut = cfg;\n> +\n> +\treturn 0;\n> +}\n> +\n> int V4L2Camera::allocBuffers(unsigned int count)\n> {\n> \tStream *stream = *camera_->streams().begin();\n> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h\n> index 515e906..be6c4e1 100644\n> --- a/src/v4l2/v4l2_camera.h\n> +++ b/src/v4l2/v4l2_camera.h\n> @@ -47,6 +47,9 @@ public:\n> \tint configure(StreamConfiguration *streamConfigOut,\n> \t\t const Size &size, const PixelFormat &pixelformat,\n> \t\t unsigned int bufferCount);\n> +\tint validateConfiguration(const PixelFormat &pixelformat,\n> +\t\t\t\t const Size &size,\n> +\t\t\t\t StreamConfiguration *streamConfigOut);\n> \n> \tint allocBuffers(unsigned int count);\n> \tvoid freeBuffers();\n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 305ede0..5c92ff7 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -300,24 +300,19 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)\n> {\n> \tV4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);\n> \tPixelFormat format = PixelFormatInfo::info(v4l2Format).format;\n> -\tconst std::vector &formats =\n> -\t\tstreamConfig_.formats().pixelformats();\n> -\tif (std::find(formats.begin(), formats.end(), format) == formats.end())\n> -\t\tformat = streamConfig_.formats().pixelformats()[0];\n> -\n> \tSize size(arg->fmt.pix.width, arg->fmt.pix.height);\n> -\tconst std::vector &sizes = streamConfig_.formats().sizes(format);\n> -\tif (std::find(sizes.begin(), sizes.end(), size) == sizes.end())\n> -\t\tsize = streamConfig_.formats().sizes(format)[0];\n> \n> -\tconst PixelFormatInfo &formatInfo = PixelFormatInfo::info(format);\n> +\tStreamConfiguration config;\n> +\tvcam_->validateConfiguration(format, size, &config);\n> +\n> +\tconst PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);\n> \n> -\targ->fmt.pix.width = size.width;\n> -\targ->fmt.pix.height = size.height;\n> -\targ->fmt.pix.pixelformat = formatInfo.v4l2Format;\n> +\targ->fmt.pix.width = config.size.width;\n> +\targ->fmt.pix.height = config.size.height;\n> +\targ->fmt.pix.pixelformat = info.v4l2Format;\n> \targ->fmt.pix.field = V4L2_FIELD_NONE;\n> -\targ->fmt.pix.bytesperline = formatInfo.stride(size.width, 0);\n> -\targ->fmt.pix.sizeimage = formatInfo.frameSize(size);\n> +\targ->fmt.pix.bytesperline = config.stride;\n> +\targ->fmt.pix.sizeimage = config.frameSize;\n> \targ->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;\n> \targ->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;\n> \targ->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 62F10BD792\n\tfor ;\n\tWed, 8 Jul 2020 14:59:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DCCDE61153;\n\tWed, 8 Jul 2020 16:59:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 54147610F4\n\tfor ;\n\tWed, 8 Jul 2020 16:59:49 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B9503543;\n\tWed, 8 Jul 2020 16:59:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uUK/0tCl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594220388;\n\tbh=7rTQssaObFO+ggqrcJS7AL4GZ8vvHaWLvoxDJTngmtg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uUK/0tClGlm1PUePqXDNHS+YNgPpSXJSFq+TX9FfN6stmPaC1Sq5aarGvBFmRXMYg\n\tYAwjZH8h7iX5Zb5SfBbyrcYn3+P7VhUjmSQsbyL4AHTdBe8KZG+jMU4h8gnfZq5z4L\n\t4caxtKPW5KngeNDd0t8ZgevFpa53jSoTMMEyJKKc=","Date":"Wed, 8 Jul 2020 17:59:43 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20200708145943.GD20298@pendragon.ideasonboard.com>","References":"<20200708134417.67747-1-paul.elder@ideasonboard.com>\n\t<20200708134417.67747-9-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200708134417.67747-9-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 08/21] v4l2: v4l2_camera_proxy: Use\n\tstream config in tryFormat","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11261,"web_url":"https://patchwork.libcamera.org/comment/11261/","msgid":"<20200708212010.xbw6mva6qex4l46s@uno.localdomain>","date":"2020-07-08T21:20:10","subject":"Re: [libcamera-devel] [PATCH v4 08/21] v4l2: v4l2_camera_proxy: Use\n\tstream config in tryFormat","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Wed, Jul 08, 2020 at 10:44:04PM +0900, Paul Elder wrote:\n> For handling try_fmt, the values should be filled in by validating the\n> stream configuration, and not by recalculating them or manually checking\n> against the cached list of formats and sizes. Use\n> V4L2Camera::validateConfiguration to obtain size, format, stride, and\n> frameSize values. Also, implement this function in V4L2Camera.\n>\n> Signed-off-by: Paul Elder \n>\n> ---\n> Changes in v4:\n> - clean up code a bit\n> - squash the implementation and usage\n>\n> New in v3\n> ---\n> src/v4l2/v4l2_camera.cpp | 20 ++++++++++++++++++++\n> src/v4l2/v4l2_camera.h | 3 +++\n> src/v4l2/v4l2_camera_proxy.cpp | 23 +++++++++--------------\n> 3 files changed, 32 insertions(+), 14 deletions(-)\n>\n> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> index ffc1230..9f81c97 100644\n> --- a/src/v4l2/v4l2_camera.cpp\n> +++ b/src/v4l2/v4l2_camera.cpp\n> @@ -138,6 +138,26 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut,\n> \treturn 0;\n> }\n>\n> +int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,\n\nI'm not familiar with this part of the code, nor I have a full\nunderstanding of the series yet, so maybe there's alreay a\ngenerateConfiguration() or has been introduced, but this function\nseems to that does not validate a configuration with filled fields,\nbut rather generates a valid one.\n\nIf its purpose is to validate a configuration it should then take a\nconst StreamConfiguration & and return its status, or an error code.\n\nIf it should instead generate a valid StreamConfiguration I would\nrename that and make it return a StreamConfiguration\n\n> +\t\t\t\t const Size &size,\n> +\t\t\t\t StreamConfiguration *streamConfigOut)\n> +{\n> +\tstd::unique_ptr config =\n> +\t\tcamera_->generateConfiguration({ StreamRole::Viewfinder });\n> +\tStreamConfiguration &cfg = config->at(0);\n> +\tcfg.size = size;\n> +\tcfg.pixelFormat = pixelFormat;\n> +\tcfg.bufferCount = 1;\n> +\n> +\tCameraConfiguration::Status validation = config->validate();\n> +\tif (validation == CameraConfiguration::Invalid)\n> +\t\treturn -EINVAL;\n> +\n> +\t*streamConfigOut = cfg;\n> +\n> +\treturn 0;\n> +}\n> +\n> int V4L2Camera::allocBuffers(unsigned int count)\n> {\n> \tStream *stream = *camera_->streams().begin();\n> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h\n> index 515e906..be6c4e1 100644\n> --- a/src/v4l2/v4l2_camera.h\n> +++ b/src/v4l2/v4l2_camera.h\n> @@ -47,6 +47,9 @@ public:\n> \tint configure(StreamConfiguration *streamConfigOut,\n> \t\t const Size &size, const PixelFormat &pixelformat,\n> \t\t unsigned int bufferCount);\n> +\tint validateConfiguration(const PixelFormat &pixelformat,\n> +\t\t\t\t const Size &size,\n> +\t\t\t\t StreamConfiguration *streamConfigOut);\n>\n> \tint allocBuffers(unsigned int count);\n> \tvoid freeBuffers();\n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 305ede0..5c92ff7 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -300,24 +300,19 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)\n> {\n> \tV4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);\n> \tPixelFormat format = PixelFormatInfo::info(v4l2Format).format;\n> -\tconst std::vector &formats =\n> -\t\tstreamConfig_.formats().pixelformats();\n> -\tif (std::find(formats.begin(), formats.end(), format) == formats.end())\n> -\t\tformat = streamConfig_.formats().pixelformats()[0];\n> -\n> \tSize size(arg->fmt.pix.width, arg->fmt.pix.height);\n> -\tconst std::vector &sizes = streamConfig_.formats().sizes(format);\n> -\tif (std::find(sizes.begin(), sizes.end(), size) == sizes.end())\n> -\t\tsize = streamConfig_.formats().sizes(format)[0];\n>\n> -\tconst PixelFormatInfo &formatInfo = PixelFormatInfo::info(format);\n> +\tStreamConfiguration config;\n> +\tvcam_->validateConfiguration(format, size, &config);\n\nShouldn't you check if it's valid ? maybe it's done later on.\n\n> +\n> +\tconst PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);\n>\n> -\targ->fmt.pix.width = size.width;\n> -\targ->fmt.pix.height = size.height;\n> -\targ->fmt.pix.pixelformat = formatInfo.v4l2Format;\n> +\targ->fmt.pix.width = config.size.width;\n> +\targ->fmt.pix.height = config.size.height;\n> +\targ->fmt.pix.pixelformat = info.v4l2Format;\n> \targ->fmt.pix.field = V4L2_FIELD_NONE;\n> -\targ->fmt.pix.bytesperline = formatInfo.stride(size.width, 0);\n> -\targ->fmt.pix.sizeimage = formatInfo.frameSize(size);\n> +\targ->fmt.pix.bytesperline = config.stride;\n> +\targ->fmt.pix.sizeimage = config.frameSize;\n> \targ->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;\n> \targ->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;\n> \targ->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 29FE6BD790\n\tfor ;\n\tWed, 8 Jul 2020 21:16:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AB1FB61171;\n\tWed, 8 Jul 2020 23:16:38 +0200 (CEST)","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 4E4EE61163\n\tfor ;\n\tWed, 8 Jul 2020 23:16:37 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 997AE240003;\n\tWed, 8 Jul 2020 21:16:36 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 8 Jul 2020 23:20:10 +0200","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20200708212010.xbw6mva6qex4l46s@uno.localdomain>","References":"<20200708134417.67747-1-paul.elder@ideasonboard.com>\n\t<20200708134417.67747-9-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200708134417.67747-9-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 08/21] v4l2: v4l2_camera_proxy: Use\n\tstream config in tryFormat","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11276,"web_url":"https://patchwork.libcamera.org/comment/11276/","msgid":"<20200709081116.GD66701@pyrite.rasen.tech>","date":"2020-07-09T08:11:16","subject":"Re: [libcamera-devel] [PATCH v4 08/21] v4l2: v4l2_camera_proxy: Use\n\tstream config in tryFormat","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the review.\n\nOn Wed, Jul 08, 2020 at 11:20:10PM +0200, Jacopo Mondi wrote:\n> Hi Paul,\n> \n> On Wed, Jul 08, 2020 at 10:44:04PM +0900, Paul Elder wrote:\n> > For handling try_fmt, the values should be filled in by validating the\n> > stream configuration, and not by recalculating them or manually checking\n> > against the cached list of formats and sizes. Use\n> > V4L2Camera::validateConfiguration to obtain size, format, stride, and\n> > frameSize values. Also, implement this function in V4L2Camera.\n> >\n> > Signed-off-by: Paul Elder \n> >\n> > ---\n> > Changes in v4:\n> > - clean up code a bit\n> > - squash the implementation and usage\n> >\n> > New in v3\n> > ---\n> > src/v4l2/v4l2_camera.cpp | 20 ++++++++++++++++++++\n> > src/v4l2/v4l2_camera.h | 3 +++\n> > src/v4l2/v4l2_camera_proxy.cpp | 23 +++++++++--------------\n> > 3 files changed, 32 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> > index ffc1230..9f81c97 100644\n> > --- a/src/v4l2/v4l2_camera.cpp\n> > +++ b/src/v4l2/v4l2_camera.cpp\n> > @@ -138,6 +138,26 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut,\n> > \treturn 0;\n> > }\n> >\n> > +int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,\n> \n> I'm not familiar with this part of the code, nor I have a full\n> understanding of the series yet, so maybe there's alreay a\n> generateConfiguration() or has been introduced, but this function\n> seems to that does not validate a configuration with filled fields,\n> but rather generates a valid one.\n\nWhen the V4L2Camera is open()ed, it generates a configuration, to save\nthe current configuration of the V4L2Camera/Camera. After we supported\nmultiple open, the V4L2Camera is only open()ed once, and V4L2CameraProxy\nkeeps references for different file open()s, and V4L2CompatManager keeps\nreferences for dup()s. Of course, if all files and dupped fds are\nclosed then the V4L2Camera is also close()d, and the next open() will\nalso open() the V4L2Camera.\n\n> If its purpose is to validate a configuration it should then take a\n> const StreamConfiguration & and return its status, or an error code.\n> \n> If it should instead generate a valid StreamConfiguration I would\n> rename that and make it return a StreamConfiguration\n\nThe purpose of this function is, basically, to implement try_fmt. That's\nwhy it takes a size and a pixelformat as input, and uses those to create\na configuration, and simply validates it.\n\nPerhaps the function isn't named very well? The caller still needs to\nprovide and output StreamConfiguration though, since it needs to extract\nthe validated size and pixelformat, as well as the stride and frameSize,\nso the StreamConfiguration can't be completely abstracted away from the\ncaller. In any case, the V4L2CameraProxy deals with StreamConfiguration\nelsewhere too, so there isn't much motivation to partition them.\n\n> > +\t\t\t\t const Size &size,\n> > +\t\t\t\t StreamConfiguration *streamConfigOut)\n> > +{\n> > +\tstd::unique_ptr config =\n> > +\t\tcamera_->generateConfiguration({ StreamRole::Viewfinder });\n> > +\tStreamConfiguration &cfg = config->at(0);\n> > +\tcfg.size = size;\n> > +\tcfg.pixelFormat = pixelFormat;\n> > +\tcfg.bufferCount = 1;\n> > +\n> > +\tCameraConfiguration::Status validation = config->validate();\n> > +\tif (validation == CameraConfiguration::Invalid)\n> > +\t\treturn -EINVAL;\n> > +\n> > +\t*streamConfigOut = cfg;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > int V4L2Camera::allocBuffers(unsigned int count)\n> > {\n> > \tStream *stream = *camera_->streams().begin();\n> > diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h\n> > index 515e906..be6c4e1 100644\n> > --- a/src/v4l2/v4l2_camera.h\n> > +++ b/src/v4l2/v4l2_camera.h\n> > @@ -47,6 +47,9 @@ public:\n> > \tint configure(StreamConfiguration *streamConfigOut,\n> > \t\t const Size &size, const PixelFormat &pixelformat,\n> > \t\t unsigned int bufferCount);\n> > +\tint validateConfiguration(const PixelFormat &pixelformat,\n> > +\t\t\t\t const Size &size,\n> > +\t\t\t\t StreamConfiguration *streamConfigOut);\n> >\n> > \tint allocBuffers(unsigned int count);\n> > \tvoid freeBuffers();\n> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > index 305ede0..5c92ff7 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > @@ -300,24 +300,19 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)\n> > {\n> > \tV4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);\n> > \tPixelFormat format = PixelFormatInfo::info(v4l2Format).format;\n> > -\tconst std::vector &formats =\n> > -\t\tstreamConfig_.formats().pixelformats();\n> > -\tif (std::find(formats.begin(), formats.end(), format) == formats.end())\n> > -\t\tformat = streamConfig_.formats().pixelformats()[0];\n> > -\n> > \tSize size(arg->fmt.pix.width, arg->fmt.pix.height);\n> > -\tconst std::vector &sizes = streamConfig_.formats().sizes(format);\n> > -\tif (std::find(sizes.begin(), sizes.end(), size) == sizes.end())\n> > -\t\tsize = streamConfig_.formats().sizes(format)[0];\n> >\n> > -\tconst PixelFormatInfo &formatInfo = PixelFormatInfo::info(format);\n> > +\tStreamConfiguration config;\n> > +\tvcam_->validateConfiguration(format, size, &config);\n> \n> Shouldn't you check if it's valid ? maybe it's done later on.\n\nHmmmmm.... yeah I think I should. Invalid means it failed to adjust, so\nin that case it's fair for try_fmt to return -EINVAL, right.\n\n> > +\n> > +\tconst PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);\n> >\n> > -\targ->fmt.pix.width = size.width;\n> > -\targ->fmt.pix.height = size.height;\n> > -\targ->fmt.pix.pixelformat = formatInfo.v4l2Format;\n> > +\targ->fmt.pix.width = config.size.width;\n> > +\targ->fmt.pix.height = config.size.height;\n> > +\targ->fmt.pix.pixelformat = info.v4l2Format;\n> > \targ->fmt.pix.field = V4L2_FIELD_NONE;\n> > -\targ->fmt.pix.bytesperline = formatInfo.stride(size.width, 0);\n> > -\targ->fmt.pix.sizeimage = formatInfo.frameSize(size);\n> > +\targ->fmt.pix.bytesperline = config.stride;\n> > +\targ->fmt.pix.sizeimage = config.frameSize;\n> > \targ->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;\n> > \targ->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;\n> > \targ->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;\n\n\nThanks,\n\nPaul","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D3DDCBD790\n\tfor ;\n\tThu, 9 Jul 2020 08:11:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A107661184;\n\tThu, 9 Jul 2020 10:11:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A621661158\n\tfor ;\n\tThu, 9 Jul 2020 10:11:24 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 35889289;\n\tThu, 9 Jul 2020 10:11:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"b6UHDmw/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594282284;\n\tbh=FC+7fZPasFtkWSVlOXYB0I2w5XKx+O4Ll0osRvccYww=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b6UHDmw//kb+9LyZFFyZOc64fVoGp3ew768p32Xls3EvBedBBSWJG6BPmPDyOlsaY\n\tc62ABJF4AEddrY5bkvFRDmtTVWHbRe+br3t5cSWHRwemS+NLETiK1ScYrxk+799ouR\n\toFrqkH9b3s17HScAdmo+pmLSjrAqeEGx7gARVPAY=","Date":"Thu, 9 Jul 2020 17:11:16 +0900","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi ","Message-ID":"<20200709081116.GD66701@pyrite.rasen.tech>","References":"<20200708134417.67747-1-paul.elder@ideasonboard.com>\n\t<20200708134417.67747-9-paul.elder@ideasonboard.com>\n\t<20200708212010.xbw6mva6qex4l46s@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200708212010.xbw6mva6qex4l46s@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v4 08/21] v4l2: v4l2_camera_proxy: Use\n\tstream config in tryFormat","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]