[{"id":21677,"web_url":"https://patchwork.libcamera.org/comment/21677/","msgid":"<163897214186.1970692.12032974764666967064@Monstersaurus>","date":"2021-12-08T14:02:21","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2021-12-01 15:23:06)\n> From: Benjamin Schaaf \n> \n> Use the list of supported ranges from the video format to configure the\n> stream and subdev instead of the capture size, allowing streams to be\n> configured below the maximum resolution.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=96\n> Signed-off-by: Benjamin Schaaf \n> ---\n> Sent on behalf of Benjamin as posted at https://bugs.libcamera.org/attachment.cgi?id=33\n> \n> src/libcamera/pipeline/simple/simple.cpp | 34 ++++++++++++++++--------\n> 1 file changed, 23 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 701fb4be0b71..a597e27f6139 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -213,7 +213,7 @@ public:\n> PixelFormat captureFormat;\n> Size captureSize;\n> std::vector outputFormats;\n> - SizeRange outputSizes;\n> + std::vector outputSizes;\n> };\n> \n> std::vector streams_;\n> @@ -492,10 +492,10 @@ int SimpleCameraData::init()\n> \n> if (!converter_) {\n> config.outputFormats = { pixelFormat };\n> - config.outputSizes = config.captureSize;\n> + config.outputSizes = videoFormat.second;\n> } else {\n> config.outputFormats = converter_->formats(pixelFormat);\n> - config.outputSizes = converter_->sizes(format.size);\n> + config.outputSizes = { converter_->sizes(format.size) };\n> }\n> \n> configs_.push_back(config);\n> @@ -804,17 +804,23 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> status = Adjusted;\n> }\n> \n> - if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> + auto sizeIt = std::find_if(pipeConfig_->outputSizes.begin(),\n> + pipeConfig_->outputSizes.end(),\n> + [&](SizeRange r) { return r.contains(cfg.size); });\n> + if (sizeIt == pipeConfig_->outputSizes.end()) {\n> LOG(SimplePipeline, Debug)\n> << \"Adjusting size from \" << cfg.size.toString()\n> << \" to \" << pipeConfig_->captureSize.toString();\n> - cfg.size = pipeConfig_->captureSize;\n> +\n> + cfg.size = pipeConfig_->outputSizes[0].max;\n> status = Adjusted;\n> +\n> + /* \\todo Create a libcamera core class to group format and size */\n> + if (cfg.size != pipeConfig_->captureSize)\n> + needConversion_ = true;\n> }\n> \n> - /* \\todo Create a libcamera core class to group format and size */\n> - if (cfg.pixelFormat != pipeConfig_->captureFormat ||\n> - cfg.size != pipeConfig_->captureSize)\n> + if (cfg.pixelFormat != pipeConfig_->captureFormat)\n> needConversion_ = true;\n> \n> /* Set the stride, frameSize and bufferCount. */\n> @@ -907,8 +913,14 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> if (ret < 0)\n> return ret;\n> \n\nIt took me a moment to realize what this code is doing. A block comment\nwould help\n\n\t/*\n\t * Iterate the requested streams and identify the largest\n\t * frame size. Use that to configure the capture device.\n\t */\n\n> + Size size;\n\nI think it would help to call this something more specific, perhaps sensorSize;\n\n> + for (unsigned int i = 0; i < config->size(); ++i) {\n> + StreamConfiguration &cfg = config->at(i);\n> + size.expandTo(cfg.size);\n> + }\n\nWhat would happen if the user requests a stream larger than the\nSensor/capture device can produce (and expects a convertor to upscale\nit?)\n\nMaybe this is as simple as restricting it a max of\ndata->sensor_->resolution().\n\nBut I'm not sure this is quite right ;-(\n\nIt worries me that this might be 'changing' the configuration after the\nvalidation process, which we must make sure we don't do.\n\n\n> +\n> const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();\n> - V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };\n> + V4L2SubdeviceFormat format{ pipeConfig->code, size };\n> \n> ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n> if (ret < 0)\n> @@ -919,7 +931,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \n> V4L2DeviceFormat captureFormat;\n> captureFormat.fourcc = videoFormat;\n> - captureFormat.size = pipeConfig->captureSize;\n> + captureFormat.size = size;\n\nI'm having a hard time to grasp if the core requirements are being met\nthough.\n\nWe have two devices that could be configured in 3 different ways:\n\n1)\n ┌────────────┐\n │ video_ ├──► Stream0\n └────────────┘\n\n2)\n ┌────────────┐ ┌────────────┐\n │ video_ ├──►│ convertor_ ├──► Stream0\n └────────────┘ └────────────┘\n\n3)\n ┌────────────┐ ┌────────────┐\n │ video_ ├──►│ convertor_ ├──► Stream1\n └───────┬────┘ └────────────┘\n │\n └► stream0\n\n\nI believe the use case you are extending is 1), so that it can expose\nmultiple stream sizes from the device.\n\nI am worried that it might be breaking use case 2) though, as we must\nmake sure the video_ is configured to a format that's compatible there,\nwhile the Stream0 produces the (possibly format converted, and rescaled)\noutput from the convertor_.\n\nDuring validate we need to:\n\nCheck the number of streams:\n If 2: we're using a convertor.\n video_ and convertor_ should be configured as set in those streams\n explicitly, or fail, (or return adjusted).\n\n\n if 1 stream:\n *1 iterate supported frame sizes from the video_\n *2 choose closest match\n\n set the convertor (if available) to try to process any remaining change.\n\n\nFrom what I understand, *1 currently without this patch is simply taking\nthe biggest size supported, and there is no *2, so those are the parts\nthat this patch is adding.\n\nIs that close?\n\nThe important part might be how we convey between validate() and\nconfigure() which mode (1,2,3) we're in and ensure that we do not make\nany changes to that during configure.\n\n--\nKieran\n\n\n\n> ret = video->setFormat(&captureFormat);\n> if (ret)\n> @@ -932,7 +944,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> }\n> \n> if (captureFormat.fourcc != videoFormat ||\n> - captureFormat.size != pipeConfig->captureSize) {\n> + captureFormat.size != size) {\n> LOG(SimplePipeline, Error)\n> << \"Unable to configure capture in \"\n> << pipeConfig->captureSize.toString() << \"-\"\n> -- \n> 2.30.2\n>","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 75CCEBDB13\n\tfor ;\n\tWed, 8 Dec 2021 14:02:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6C5906086A;\n\tWed, 8 Dec 2021 15:02:26 +0100 (CET)","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 09D9D60225\n\tfor ;\n\tWed, 8 Dec 2021 15:02:25 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9C82F8BB;\n\tWed, 8 Dec 2021 15:02:24 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"W3FKOgvx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638972144;\n\tbh=6m7+IykBcKqdNLKSL1xarniVhvTlzZUUi/2HvaMc1fo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=W3FKOgvxc/GKVxIXkWu31mGff2XkF8QrjSZgombuS4Bc3dxLRUU0HXdAvHK5GYDKN\n\t3kjKrhwPA3NmVo0DzLkHvyIFf9vkfL0IbHZM40bg38oqysIJ4HrkYxZQkqMjXTB/gk\n\ttJb5d43Pq+V3Uk2ecHvGpi9THh6tONYfj2HbU6kg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211201152306.3401143-1-kieran.bingham@ideasonboard.com>","References":"<20211201152306.3401143-1-kieran.bingham@ideasonboard.com>","From":"Kieran Bingham ","To":"libcamera devel ","Date":"Wed, 08 Dec 2021 14:02:21 +0000","Message-ID":"<163897214186.1970692.12032974764666967064@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","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":"Benjamin Schaaf ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21731,"web_url":"https://patchwork.libcamera.org/comment/21731/","msgid":"","date":"2021-12-10T00:52:34","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Benjamin,\n\nThank you for the patch.\n\nOn Wed, Dec 08, 2021 at 02:02:21PM +0000, Kieran Bingham wrote:\n> Quoting Kieran Bingham (2021-12-01 15:23:06)\n> > From: Benjamin Schaaf \n> > \n> > Use the list of supported ranges from the video format to configure the\n> > stream and subdev instead of the capture size, allowing streams to be\n> > configured below the maximum resolution.\n> > \n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=96\n> > Signed-off-by: Benjamin Schaaf \n> > ---\n> > Sent on behalf of Benjamin as posted at https://bugs.libcamera.org/attachment.cgi?id=33\n> > \n> > src/libcamera/pipeline/simple/simple.cpp | 34 ++++++++++++++++--------\n> > 1 file changed, 23 insertions(+), 11 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 701fb4be0b71..a597e27f6139 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -213,7 +213,7 @@ public:\n> > PixelFormat captureFormat;\n> > Size captureSize;\n> > std::vector outputFormats;\n> > - SizeRange outputSizes;\n> > + std::vector outputSizes;\n> > };\n> > \n> > std::vector streams_;\n> > @@ -492,10 +492,10 @@ int SimpleCameraData::init()\n> > \n> > if (!converter_) {\n> > config.outputFormats = { pixelFormat };\n> > - config.outputSizes = config.captureSize;\n> > + config.outputSizes = videoFormat.second;\n\nI think this will be a problem. videoFormat.second comes from the\nenumeration of supported sizes by the video node, but doesn't take into\naccount what is produced by the pipeline. It only exposes the intrinsic\ncapabilities of the video node. I'm pretty sure this will break other\nsupported platforms.\n\nI assume your platform doesn't have a converter. How do you get support\nfor lower resolutions, are they generated directly by the sensor, or do\nyou have a scaler in the pipeline ?\n\n> > } else {\n> > config.outputFormats = converter_->formats(pixelFormat);\n> > - config.outputSizes = converter_->sizes(format.size);\n> > + config.outputSizes = { converter_->sizes(format.size) };\n> > }\n> > \n> > configs_.push_back(config);\n> > @@ -804,17 +804,23 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > status = Adjusted;\n> > }\n> > \n> > - if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> > + auto sizeIt = std::find_if(pipeConfig_->outputSizes.begin(),\n> > + pipeConfig_->outputSizes.end(),\n> > + [&](SizeRange r) { return r.contains(cfg.size); });\n> > + if (sizeIt == pipeConfig_->outputSizes.end()) {\n> > LOG(SimplePipeline, Debug)\n> > << \"Adjusting size from \" << cfg.size.toString()\n> > << \" to \" << pipeConfig_->captureSize.toString();\n> > - cfg.size = pipeConfig_->captureSize;\n> > +\n> > + cfg.size = pipeConfig_->outputSizes[0].max;\n> > status = Adjusted;\n> > +\n> > + /* \\todo Create a libcamera core class to group format and size */\n> > + if (cfg.size != pipeConfig_->captureSize)\n> > + needConversion_ = true;\n> > }\n> > \n> > - /* \\todo Create a libcamera core class to group format and size */\n> > - if (cfg.pixelFormat != pipeConfig_->captureFormat ||\n> > - cfg.size != pipeConfig_->captureSize)\n> > + if (cfg.pixelFormat != pipeConfig_->captureFormat)\n> > needConversion_ = true;\n> > \n> > /* Set the stride, frameSize and bufferCount. */\n> > @@ -907,8 +913,14 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > if (ret < 0)\n> > return ret;\n> > \n> \n> It took me a moment to realize what this code is doing. A block comment\n> would help\n> \n> \t/*\n> \t * Iterate the requested streams and identify the largest\n> \t * frame size. Use that to configure the capture device.\n> \t */\n> \n> > + Size size;\n> \n> I think it would help to call this something more specific, perhaps sensorSize;\n> \n> > + for (unsigned int i = 0; i < config->size(); ++i) {\n> > + StreamConfiguration &cfg = config->at(i);\n> > + size.expandTo(cfg.size);\n> > + }\n> \n> What would happen if the user requests a stream larger than the\n> Sensor/capture device can produce (and expects a convertor to upscale\n> it?)\n> \n> Maybe this is as simple as restricting it a max of\n> data->sensor_->resolution().\n> \n> But I'm not sure this is quite right ;-(\n> \n> It worries me that this might be 'changing' the configuration after the\n> validation process, which we must make sure we don't do.\n> \n> \n> > +\n> > const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();\n> > - V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };\n> > + V4L2SubdeviceFormat format{ pipeConfig->code, size };\n> > \n> > ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n> > if (ret < 0)\n> > @@ -919,7 +931,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > \n> > V4L2DeviceFormat captureFormat;\n> > captureFormat.fourcc = videoFormat;\n> > - captureFormat.size = pipeConfig->captureSize;\n> > + captureFormat.size = size;\n> \n> I'm having a hard time to grasp if the core requirements are being met\n> though.\n> \n> We have two devices that could be configured in 3 different ways:\n> \n> 1)\n> ┌────────────┐\n> │ video_ ├──► Stream0\n> └────────────┘\n> \n> 2)\n> ┌────────────┐ ┌────────────┐\n> │ video_ ├──►│ convertor_ ├──► Stream0\n> └────────────┘ └────────────┘\n> \n> 3)\n> ┌────────────┐ ┌────────────┐\n> │ video_ ├──►│ convertor_ ├──► Stream1\n> └───────┬────┘ └────────────┘\n> │\n> └► stream0\n> \n> \n> I believe the use case you are extending is 1), so that it can expose\n> multiple stream sizes from the device.\n> \n> I am worried that it might be breaking use case 2) though, as we must\n> make sure the video_ is configured to a format that's compatible there,\n> while the Stream0 produces the (possibly format converted, and rescaled)\n> output from the convertor_.\n> \n> During validate we need to:\n> \n> Check the number of streams:\n> If 2: we're using a convertor.\n> video_ and convertor_ should be configured as set in those streams\n> explicitly, or fail, (or return adjusted).\n> \n> \n> if 1 stream:\n> *1 iterate supported frame sizes from the video_\n> *2 choose closest match\n> \n> set the convertor (if available) to try to process any remaining change.\n> \n> \n> From what I understand, *1 currently without this patch is simply taking\n> the biggest size supported, and there is no *2, so those are the parts\n> that this patch is adding.\n> \n> Is that close?\n> \n> The important part might be how we convey between validate() and\n> configure() which mode (1,2,3) we're in and ensure that we do not make\n> any changes to that during configure.\n> \n> > ret = video->setFormat(&captureFormat);\n> > if (ret)\n> > @@ -932,7 +944,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > }\n> > \n> > if (captureFormat.fourcc != videoFormat ||\n> > - captureFormat.size != pipeConfig->captureSize) {\n> > + captureFormat.size != size) {\n> > LOG(SimplePipeline, Error)\n> > << \"Unable to configure capture in \"\n> > << pipeConfig->captureSize.toString() << \"-\"","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 2114DBF415\n\tfor ;\n\tFri, 10 Dec 2021 00:53:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 48E1B6087A;\n\tFri, 10 Dec 2021 01:53:05 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 359B860113\n\tfor ;\n\tFri, 10 Dec 2021 01:53:04 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8BB1790E;\n\tFri, 10 Dec 2021 01:53:03 +0100 (CET)"],"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=\"i6oY8eA0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1639097583;\n\tbh=E5p3paKhb7Q7ibwVBB/zggppAdpl+EmtqIo7yP+bL1M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=i6oY8eA0Un4lUr51xW9Y5u4F9ID1b3PMLeNB1OrPRx+9sdI2f61geMB+jApeQsdXp\n\t+fPy53hq+cB8l1pxdGVOiMMH+hSUMW/EZTDbWg9ywLBvzf/jhBBOna7QJnHODaGlqp\n\tkgl9XLAMfIvJ5bfIsS4l+hRqTVTGAhIsMBViV7pM=","Date":"Fri, 10 Dec 2021 02:52:34 +0200","From":"Laurent Pinchart ","To":"Benjamin Schaaf ","Message-ID":"","References":"<20211201152306.3401143-1-kieran.bingham@ideasonboard.com>\n\t<163897214186.1970692.12032974764666967064@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<163897214186.1970692.12032974764666967064@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21735,"web_url":"https://patchwork.libcamera.org/comment/21735/","msgid":"","date":"2021-12-10T11:35:49","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","submitter":{"id":108,"url":"https://patchwork.libcamera.org/api/people/108/","name":"Benjamin Schaaf","email":"ben.schaaf@gmail.com"},"content":"On Fri, Dec 10, 2021 at 11:53 AM Laurent Pinchart\n wrote:\n>\n> Hi Benjamin,\n>\n> Thank you for the patch.\n>\n> On Wed, Dec 08, 2021 at 02:02:21PM +0000, Kieran Bingham wrote:\n> > Quoting Kieran Bingham (2021-12-01 15:23:06)\n> > > From: Benjamin Schaaf \n> > >\n> > > Use the list of supported ranges from the video format to configure the\n> > > stream and subdev instead of the capture size, allowing streams to be\n> > > configured below the maximum resolution.\n> > >\n> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=96\n> > > Signed-off-by: Benjamin Schaaf \n> > > ---\n> > > Sent on behalf of Benjamin as posted at https://bugs.libcamera.org/attachment.cgi?id=33\n> > >\n> > > src/libcamera/pipeline/simple/simple.cpp | 34 ++++++++++++++++--------\n> > > 1 file changed, 23 insertions(+), 11 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 701fb4be0b71..a597e27f6139 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -213,7 +213,7 @@ public:\n> > > PixelFormat captureFormat;\n> > > Size captureSize;\n> > > std::vector outputFormats;\n> > > - SizeRange outputSizes;\n> > > + std::vector outputSizes;\n> > > };\n> > >\n> > > std::vector streams_;\n> > > @@ -492,10 +492,10 @@ int SimpleCameraData::init()\n> > >\n> > > if (!converter_) {\n> > > config.outputFormats = { pixelFormat };\n> > > - config.outputSizes = config.captureSize;\n> > > + config.outputSizes = videoFormat.second;\n>\n> I think this will be a problem. videoFormat.second comes from the\n> enumeration of supported sizes by the video node, but doesn't take into\n> account what is produced by the pipeline. It only exposes the intrinsic\n> capabilities of the video node. I'm pretty sure this will break other\n> supported platforms.\n>\n> I assume your platform doesn't have a converter. How do you get support\n> for lower resolutions, are they generated directly by the sensor, or do\n> you have a scaler in the pipeline ?\n\nThe sensor supports arbitrary resolutions with a built-in scaler.\n\n> > > } else {\n> > > config.outputFormats = converter_->formats(pixelFormat);\n> > > - config.outputSizes = converter_->sizes(format.size);\n> > > + config.outputSizes = { converter_->sizes(format.size) };\n> > > }\n> > >\n> > > configs_.push_back(config);\n> > > @@ -804,17 +804,23 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > > status = Adjusted;\n> > > }\n> > >\n> > > - if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> > > + auto sizeIt = std::find_if(pipeConfig_->outputSizes.begin(),\n> > > + pipeConfig_->outputSizes.end(),\n> > > + [&](SizeRange r) { return r.contains(cfg.size); });\n> > > + if (sizeIt == pipeConfig_->outputSizes.end()) {\n> > > LOG(SimplePipeline, Debug)\n> > > << \"Adjusting size from \" << cfg.size.toString()\n> > > << \" to \" << pipeConfig_->captureSize.toString();\n> > > - cfg.size = pipeConfig_->captureSize;\n> > > +\n> > > + cfg.size = pipeConfig_->outputSizes[0].max;\n> > > status = Adjusted;\n> > > +\n> > > + /* \\todo Create a libcamera core class to group format and size */\n> > > + if (cfg.size != pipeConfig_->captureSize)\n> > > + needConversion_ = true;\n> > > }\n> > >\n> > > - /* \\todo Create a libcamera core class to group format and size */\n> > > - if (cfg.pixelFormat != pipeConfig_->captureFormat ||\n> > > - cfg.size != pipeConfig_->captureSize)\n> > > + if (cfg.pixelFormat != pipeConfig_->captureFormat)\n> > > needConversion_ = true;\n> > >\n> > > /* Set the stride, frameSize and bufferCount. */\n> > > @@ -907,8 +913,14 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > > if (ret < 0)\n> > > return ret;\n> > >\n> >\n> > It took me a moment to realize what this code is doing. A block comment\n> > would help\n> >\n> > /*\n> > * Iterate the requested streams and identify the largest\n> > * frame size. Use that to configure the capture device.\n> > */\n> >\n> > > + Size size;\n> >\n> > I think it would help to call this something more specific, perhaps sensorSize;\n> >\n> > > + for (unsigned int i = 0; i < config->size(); ++i) {\n> > > + StreamConfiguration &cfg = config->at(i);\n> > > + size.expandTo(cfg.size);\n> > > + }\n> >\n> > What would happen if the user requests a stream larger than the\n> > Sensor/capture device can produce (and expects a convertor to upscale\n> > it?)\n> >\n> > Maybe this is as simple as restricting it a max of\n> > data->sensor_->resolution().\n> >\n> > But I'm not sure this is quite right ;-(\n> >\n> > It worries me that this might be 'changing' the configuration after the\n> > validation process, which we must make sure we don't do.\n> >\n> >\n> > > +\n> > > const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();\n> > > - V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };\n> > > + V4L2SubdeviceFormat format{ pipeConfig->code, size };\n> > >\n> > > ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n> > > if (ret < 0)\n> > > @@ -919,7 +931,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > >\n> > > V4L2DeviceFormat captureFormat;\n> > > captureFormat.fourcc = videoFormat;\n> > > - captureFormat.size = pipeConfig->captureSize;\n> > > + captureFormat.size = size;\n> >\n> > I'm having a hard time to grasp if the core requirements are being met\n> > though.\n> >\n> > We have two devices that could be configured in 3 different ways:\n> >\n> > 1)\n> > ┌────────────┐\n> > │ video_ ├──► Stream0\n> > └────────────┘\n> >\n> > 2)\n> > ┌────────────┐ ┌────────────┐\n> > │ video_ ├──►│ convertor_ ├──► Stream0\n> > └────────────┘ └────────────┘\n> >\n> > 3)\n> > ┌────────────┐ ┌────────────┐\n> > │ video_ ├──►│ convertor_ ├──► Stream1\n> > └───────┬────┘ └────────────┘\n> > │\n> > └► stream0\n> >\n> >\n> > I believe the use case you are extending is 1), so that it can expose\n> > multiple stream sizes from the device.\n> >\n> > I am worried that it might be breaking use case 2) though, as we must\n> > make sure the video_ is configured to a format that's compatible there,\n> > while the Stream0 produces the (possibly format converted, and rescaled)\n> > output from the convertor_.\n> >\n> > During validate we need to:\n> >\n> > Check the number of streams:\n> > If 2: we're using a convertor.\n> > video_ and convertor_ should be configured as set in those streams\n> > explicitly, or fail, (or return adjusted).\n> >\n> >\n> > if 1 stream:\n> > *1 iterate supported frame sizes from the video_\n> > *2 choose closest match\n> >\n> > set the convertor (if available) to try to process any remaining change.\n> >\n> >\n> > From what I understand, *1 currently without this patch is simply taking\n> > the biggest size supported, and there is no *2, so those are the parts\n> > that this patch is adding.\n> >\n> > Is that close?\n> >\n> > The important part might be how we convey between validate() and\n> > configure() which mode (1,2,3) we're in and ensure that we do not make\n> > any changes to that during configure.\n> >\n> > > ret = video->setFormat(&captureFormat);\n> > > if (ret)\n> > > @@ -932,7 +944,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > > }\n> > >\n> > > if (captureFormat.fourcc != videoFormat ||\n> > > - captureFormat.size != pipeConfig->captureSize) {\n> > > + captureFormat.size != size) {\n> > > LOG(SimplePipeline, Error)\n> > > << \"Unable to configure capture in \"\n> > > << pipeConfig->captureSize.toString() << \"-\"\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 A3EA0BDB13\n\tfor ;\n\tFri, 10 Dec 2021 11:36:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F2A9760894;\n\tFri, 10 Dec 2021 12:36:03 +0100 (CET)","from mail-yb1-xb2d.google.com (mail-yb1-xb2d.google.com\n\t[IPv6:2607:f8b0:4864:20::b2d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AFD8D6087E\n\tfor ;\n\tFri, 10 Dec 2021 12:36:01 +0100 (CET)","by mail-yb1-xb2d.google.com with SMTP id v203so20626918ybe.6\n\tfor ;\n\tFri, 10 Dec 2021 03:36:01 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"fD7wQ1Oz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=sMh4090tglI1BZTj2ECuDD7qGZWoqTI94XNr4qNhiRI=;\n\tb=fD7wQ1Ozix0BiBsOxegV1Ts0Qh1PIdlSS83s5o2vB+zjHOzQ/YRrmGsxLbpeAWkfM+\n\tMFTo9YkQw7GYD1rLFuZxNUr9eanZuLjQFNFO9WB765AObmFbRwvEFIUBdW2agBJXwewT\n\tP6P564YapGK5EpiDy01l4pmOLaRJ2aBxsipv+2wVWHqydd3D/30lnSVtscPSbVJ9v0t2\n\tiOVA1ne1TBQmpuVakHZW/fv227Ct/H0p7CcBQKlope02U545L19Yo7enK1HQ/oVL2MBS\n\tYL4oJrXA+AvA2y+S5y1c5WccSdL70w7MyatWbjuE2SJIMKFMKjWf77mfT1Sqb1BBkyE/\n\twtJw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=sMh4090tglI1BZTj2ECuDD7qGZWoqTI94XNr4qNhiRI=;\n\tb=SSFhWW9cCTAnL4Yd77pGODP826x0E5cuJG3duhXIRTRvLhprugtqVieJ0XgKblwc6g\n\t6idWdlZGuwSNDMupBLT6o5b+07t3KRIzs02GP4Q4lk7Nvz+GdV5yTh2EmwPBnKF+gwTo\n\tTQIjVYf3zBn3Hhtny5C6OxhkFdr0/4r9Hxq/FcChGCidqkjNgv9hT6TJo3BmpTbQHrIq\n\tRbDyHbsbq5HtGpHOM5PFqQDhIn2hR0q3ISKqw4ksCqFeei7wc6Oas8G36D4EGuaCQ0Tc\n\t9/SEvfmYAvE2MBSdoZrs45Em0wUfxWv5/akd8u9xTHiDFA4UahZk4uEtKhJ6VmioH503\n\t0RJw==","X-Gm-Message-State":"AOAM530zjRjQ93T5rrra38+YBwkC5nB+HGBPp9EgP3pKdf8m3QaNVKdg\n\tEkjO9B+L3exxQqWgYw3hJXYqUM+J/l5DdXbw5Rvho/bP","X-Google-Smtp-Source":"ABdhPJzyBabFAPWCWHS0MlgLRySlR8fn/uLs3xFXSvUkdfw9JXdzIeMQQ9X3RPEIlkhH93cW+0gfKeysWfaDmXrqMjk=","X-Received":"by 2002:a25:28c6:: with SMTP id\n\to189mr13026240ybo.462.1639136160272; \n\tFri, 10 Dec 2021 03:36:00 -0800 (PST)","MIME-Version":"1.0","References":"<20211201152306.3401143-1-kieran.bingham@ideasonboard.com>\n\t<163897214186.1970692.12032974764666967064@Monstersaurus>\n\t","In-Reply-To":"","From":"Benjamin Schaaf ","Date":"Fri, 10 Dec 2021 22:35:49 +1100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21768,"web_url":"https://patchwork.libcamera.org/comment/21768/","msgid":"","date":"2021-12-13T12:28:22","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","submitter":{"id":108,"url":"https://patchwork.libcamera.org/api/people/108/","name":"Benjamin Schaaf","email":"ben.schaaf@gmail.com"},"content":"On Thu, Dec 9, 2021 at 1:02 AM Kieran Bingham\n wrote:\n>\n> Quoting Kieran Bingham (2021-12-01 15:23:06)\n> > From: Benjamin Schaaf \n> >\n> > Use the list of supported ranges from the video format to configure the\n> > stream and subdev instead of the capture size, allowing streams to be\n> > configured below the maximum resolution.\n> >\n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=96\n> > Signed-off-by: Benjamin Schaaf \n> > ---\n> > Sent on behalf of Benjamin as posted at https://bugs.libcamera.org/attachment.cgi?id=33\n> >\n> > src/libcamera/pipeline/simple/simple.cpp | 34 ++++++++++++++++--------\n> > 1 file changed, 23 insertions(+), 11 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 701fb4be0b71..a597e27f6139 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -213,7 +213,7 @@ public:\n> > PixelFormat captureFormat;\n> > Size captureSize;\n> > std::vector outputFormats;\n> > - SizeRange outputSizes;\n> > + std::vector outputSizes;\n> > };\n> >\n> > std::vector streams_;\n> > @@ -492,10 +492,10 @@ int SimpleCameraData::init()\n> >\n> > if (!converter_) {\n> > config.outputFormats = { pixelFormat };\n> > - config.outputSizes = config.captureSize;\n> > + config.outputSizes = videoFormat.second;\n> > } else {\n> > config.outputFormats = converter_->formats(pixelFormat);\n> > - config.outputSizes = converter_->sizes(format.size);\n> > + config.outputSizes = { converter_->sizes(format.size) };\n> > }\n> >\n> > configs_.push_back(config);\n> > @@ -804,17 +804,23 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > status = Adjusted;\n> > }\n> >\n> > - if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> > + auto sizeIt = std::find_if(pipeConfig_->outputSizes.begin(),\n> > + pipeConfig_->outputSizes.end(),\n> > + [&](SizeRange r) { return r.contains(cfg.size); });\n> > + if (sizeIt == pipeConfig_->outputSizes.end()) {\n> > LOG(SimplePipeline, Debug)\n> > << \"Adjusting size from \" << cfg.size.toString()\n> > << \" to \" << pipeConfig_->captureSize.toString();\n> > - cfg.size = pipeConfig_->captureSize;\n> > +\n> > + cfg.size = pipeConfig_->outputSizes[0].max;\n> > status = Adjusted;\n> > +\n> > + /* \\todo Create a libcamera core class to group format and size */\n> > + if (cfg.size != pipeConfig_->captureSize)\n> > + needConversion_ = true;\n> > }\n> >\n> > - /* \\todo Create a libcamera core class to group format and size */\n> > - if (cfg.pixelFormat != pipeConfig_->captureFormat ||\n> > - cfg.size != pipeConfig_->captureSize)\n> > + if (cfg.pixelFormat != pipeConfig_->captureFormat)\n> > needConversion_ = true;\n> >\n> > /* Set the stride, frameSize and bufferCount. */\n> > @@ -907,8 +913,14 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > if (ret < 0)\n> > return ret;\n> >\n>\n> It took me a moment to realize what this code is doing. A block comment\n> would help\n>\n> /*\n> * Iterate the requested streams and identify the largest\n> * frame size. Use that to configure the capture device.\n> */\n>\n> > + Size size;\n>\n> I think it would help to call this something more specific, perhaps sensorSize;\n>\n> > + for (unsigned int i = 0; i < config->size(); ++i) {\n> > + StreamConfiguration &cfg = config->at(i);\n> > + size.expandTo(cfg.size);\n> > + }\n>\n> What would happen if the user requests a stream larger than the\n> Sensor/capture device can produce (and expects a convertor to upscale\n> it?)\n>\n> Maybe this is as simple as restricting it a max of\n> data->sensor_->resolution().\n>\n> But I'm not sure this is quite right ;-(\n>\n> It worries me that this might be 'changing' the configuration after the\n> validation process, which we must make sure we don't do.\n>\n>\n> > +\n> > const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();\n> > - V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };\n> > + V4L2SubdeviceFormat format{ pipeConfig->code, size };\n> >\n> > ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n> > if (ret < 0)\n> > @@ -919,7 +931,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> >\n> > V4L2DeviceFormat captureFormat;\n> > captureFormat.fourcc = videoFormat;\n> > - captureFormat.size = pipeConfig->captureSize;\n> > + captureFormat.size = size;\n>\n> I'm having a hard time to grasp if the core requirements are being met\n> though.\n>\n> We have two devices that could be configured in 3 different ways:\n>\n> 1)\n> ┌────────────┐\n> │ video_ ├──► Stream0\n> └────────────┘\n>\n> 2)\n> ┌────────────┐ ┌────────────┐\n> │ video_ ├──►│ convertor_ ├──► Stream0\n> └────────────┘ └────────────┘\n>\n> 3)\n> ┌────────────┐ ┌────────────┐\n> │ video_ ├──►│ convertor_ ├──► Stream1\n> └───────┬────┘ └────────────┘\n> │\n> └► stream0\n>\n>\n> I believe the use case you are extending is 1), so that it can expose\n> multiple stream sizes from the device.\n>\n> I am worried that it might be breaking use case 2) though, as we must\n> make sure the video_ is configured to a format that's compatible there,\n> while the Stream0 produces the (possibly format converted, and rescaled)\n> output from the convertor_.\n>\n> During validate we need to:\n>\n> Check the number of streams:\n> If 2: we're using a convertor.\n> video_ and convertor_ should be configured as set in those streams\n> explicitly, or fail, (or return adjusted).\n>\n>\n> if 1 stream:\n> *1 iterate supported frame sizes from the video_\n> *2 choose closest match\n>\n> set the convertor (if available) to try to process any remaining change.\n>\n>\n> From what I understand, *1 currently without this patch is simply taking\n> the biggest size supported, and there is no *2, so those are the parts\n> that this patch is adding.\n>\n> Is that close?\n\nWith the converter there seems to be a disconnect between two\ndifferent functions of `SimpleCameraConfiguration::validate`: Some\napplications will want to use the closest native resolution of the\ncamera, whereas others will want the converter to match whatever\nresolution is requested. There's no way to specify which with the\ncurrent API. What's the solution here?\n\n> The important part might be how we convey between validate() and\n> configure() which mode (1,2,3) we're in and ensure that we do not make\n> any changes to that during configure.\n>\n> --\n> Kieran\n>\n>\n>\n> > ret = video->setFormat(&captureFormat);\n> > if (ret)\n> > @@ -932,7 +944,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > }\n> >\n> > if (captureFormat.fourcc != videoFormat ||\n> > - captureFormat.size != pipeConfig->captureSize) {\n> > + captureFormat.size != size) {\n> > LOG(SimplePipeline, Error)\n> > << \"Unable to configure capture in \"\n> > << pipeConfig->captureSize.toString() << \"-\"\n> > --\n> > 2.30.2\n> >","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 9EB61BDB13\n\tfor ;\n\tMon, 13 Dec 2021 12:28:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B609B60868;\n\tMon, 13 Dec 2021 13:28:36 +0100 (CET)","from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com\n\t[IPv6:2607:f8b0:4864:20::b32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BD7AF60868\n\tfor ;\n\tMon, 13 Dec 2021 13:28:34 +0100 (CET)","by mail-yb1-xb32.google.com with SMTP id v138so37909550ybb.8\n\tfor ;\n\tMon, 13 Dec 2021 04:28:34 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"qkRoQZWC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=CgjFy2hRhdufjApWBmW+vUt3eepWUjEaVnq4dY0pHJk=;\n\tb=qkRoQZWCxR9iEKaCiXvAYGRiMaYHD+Wh1TH3v1feqDkQQxccSHvLQRka/hDbur62N4\n\tlm67DjvUAGV5iGU0ZwJEPilyQlDITfrckyaNc3s6ZfY16Of7d35e5HC8vFMDmhVXZtF4\n\tmiHS2tNR08sh3Hn9lyZPkdsZTexdELrh3WYSE99ACuUnE+CGGaE8L/kQHJeqcPzBG4S8\n\tCMjPC55wSSpjFZ9JbpjgiKQQZgXuGcs/Lw+fuSIn/Q83XD8jcBzCYlvCvB3RZy9LpkxL\n\tma2Tdb2Kf1zPVP8xIC5tlKypmZDiCPhl0QWpo+w7lThjMLUBhAJG2V+7xhCUkctm90UX\n\tLR7A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=CgjFy2hRhdufjApWBmW+vUt3eepWUjEaVnq4dY0pHJk=;\n\tb=zbPVEsSDEV+zlpswfY0UY+OBxNf+uA3nDhwaPnXGoPQttEKA7Q8TkrCwO6m1gGbu4Q\n\tC2mlvxAMJ/CiJKLaeIccBvNNEoiwZzDwO7ouJfRdtlfv7vwTy38eOjiSTHis3mePqBHo\n\trEwj63EFHjpF/KQ/uYjy5C1zG8HsheX4456vD8Ha+yL0STy2Vm+jcsqg3k03Umlmhaws\n\tsquedQUOrzspYGaIfv3CpRv4iPcbOWetTVmxWyikSp7ieqWhQAhVlxog3b8NGPXLz5Dy\n\tb/RodxoB2ioe3JiLbr4aJuQDDTAJT3imfUyE/TEMih5exEMDhn8Gn1cbC06Jtw6qRpCE\n\tGN3w==","X-Gm-Message-State":"AOAM5321QxGTXrlP56VQvJUPLTZMSEtB04jB5IRSDI5X9QxZI9gbRFL8\n\tu2kPd0M0zKCGcJanhM/tP4ttajArNc+gUnJ8gXs=","X-Google-Smtp-Source":"ABdhPJzP2g3VcOiyaIJ+s2OZfmEyIsq0649G+1mxPOUgRP5Ur8Pso+RZxnIY8L7uUdFgWodavjvOBjsWTdjJpWGYyrc=","X-Received":"by 2002:a25:830f:: with SMTP id\n\ts15mr32247720ybk.549.1639398513200; \n\tMon, 13 Dec 2021 04:28:33 -0800 (PST)","MIME-Version":"1.0","References":"<20211201152306.3401143-1-kieran.bingham@ideasonboard.com>\n\t<163897214186.1970692.12032974764666967064@Monstersaurus>","In-Reply-To":"<163897214186.1970692.12032974764666967064@Monstersaurus>","From":"Benjamin Schaaf ","Date":"Mon, 13 Dec 2021 23:28:22 +1100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21823,"web_url":"https://patchwork.libcamera.org/comment/21823/","msgid":"<20211217194326.65d40c55.dorota.czaplejewicz@puri.sm>","date":"2021-12-17T18:43:26","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","submitter":{"id":96,"url":"https://patchwork.libcamera.org/api/people/96/","name":"Dorota Czaplejewicz","email":"dorota.czaplejewicz@puri.sm"},"content":"Hi,\n\nI think I know how to solve this, and I've been working on it today.\n\nThere's an open question that Benjamin posed: what's the policy for choosing size adjustments?\nI think at this stage, that question is best answered by choosing an arbitrary policy and dedicating the effort to get this to work at all. Then to thinking about exposing new APIs.\n\nNow, i have immediate questions about the convertor setup, before I submit my take on the same patch.\n\n> I'm having a hard time to grasp if the core requirements are being met\n> though.\n> \n> We have two devices that could be configured in 3 different ways:\n> \n> 1)\n> ┌────────────┐\n> │ video_ ├──► Stream0\n> └────────────┘\n> \n> 2)\n> ┌────────────┐ ┌────────────┐\n> │ video_ ├──►│ convertor_ ├──► Stream0\n> └────────────┘ └────────────┘\n> \n> 3)\n> ┌────────────┐ ┌────────────┐\n> │ video_ ├──►│ convertor_ ├──► Stream1\n> └───────┬────┘ └────────────┘\n> │\n> └► stream0\n> \n\nIs this last picture really correct? I was under the impression that the sensor is expected to create only one stream, and all extra streams are handled by the converter.\nA comment in camera.cpp says:\n\n> * It is possible to produce up to one stream without conversion\n\nThere's also the question about how independent the different streams can be. It's undocumented, but SimpleCameraData::Configuration seems to define the range of configurations which can be simultaneously deployed on multiple streams.\n\n```\nstruct Configuration {\n\t...\n\tstd::vector outputFormats;\n\tSizeRange outputSizes;\n}\n```\n\nReading through the validate() code, any combination of those will be accepted, no matter how many streams.\n\nIs that the intended function of this structure?\n\nOriginally I was under an impression that it merely enumerated supported modes, and that replacing the outputFormats collection with multiple Configurations, each with a single outputFormat would not change any functionality. But now I think this is actually overloaded with converter configuration.\n\nI don't want to mess up the structuring of this code, so I'm going to hold on until I get an answer to this question.\n\nThanks,\nDorota","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 7CA37BDB13\n\tfor ;\n\tFri, 17 Dec 2021 18:45:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1039608A2;\n\tFri, 17 Dec 2021 19:45:09 +0100 (CET)","from comms.puri.sm (comms.puri.sm [159.203.221.185])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3AF8660114\n\tfor ;\n\tFri, 17 Dec 2021 19:45:08 +0100 (CET)","from localhost (localhost [127.0.0.1])\n\tby comms.puri.sm (Postfix) with ESMTP id 55536E10FA\n\tfor ;\n\tFri, 17 Dec 2021 10:44:36 -0800 (PST)","from comms.puri.sm ([127.0.0.1])\n\tby localhost (comms.puri.sm [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id BLDRuETWCLHZ for ; \n\tFri, 17 Dec 2021 10:44:35 -0800 (PST)"],"Date":"Fri, 17 Dec 2021 19:43:26 +0100","From":"Dorota Czaplejewicz ","To":"libcamera-devel@lists.libcamera.org","Message-ID":"<20211217194326.65d40c55.dorota.czaplejewicz@puri.sm>","Organization":"Purism","In-Reply-To":"","MIME-Version":"1.0","Content-Type":"multipart/signed; boundary=\"Sig_/1BoDNiW.cxLEHCYpX/m1bLa\";\n\tprotocol=\"application/pgp-signature\"; micalg=pgp-sha256","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21829,"web_url":"https://patchwork.libcamera.org/comment/21829/","msgid":"<164001815747.2512616.134177853124079992@Monstersaurus>","date":"2021-12-20T16:35:57","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Dorota Czaplejewicz (2021-12-17 18:43:26)\n> Hi,\n> \n> I think I know how to solve this, and I've been working on it today.\n> \n> There's an open question that Benjamin posed: what's the policy for choosing size adjustments?\n\nThe tricky part is being able to expose what the hardware can provide,\nand validate that the configuration requested by an application can be\nsupported by the underlying hardware.\n\nIf the configuration can not be supported, then the configuration should\nbe modified - \"Adjusted\" to fit the capabilities by any means the\npipeline handler sees fit to do so.\n\nThe configure() operation will call validate() and it is only allowed to\nconfigure if validate() completes without making any adjustments.\n\n\nTrying to go back to what I think is the 'open question':\n\n With the converter there seems to be a disconnect between two\n different functions of `SimpleCameraConfiguration::validate`: Some\n applications will want to use the closest native resolution of the\n camera, whereas others will want the converter to match whatever\n resolution is requested. There's no way to specify which with the\n current API. What's the solution here?\n\n\nI believe we've handled this on the Raspberry Pi by exposing the native\nresoltions of the sensor in the 'Raw' stream. (Which is the stream\ndirectly out of the video_ node in this case.\n\n\n> I think at this stage, that question is best answered by choosing an\n> arbitrary policy and dedicating the effort to get this to work at all.\n\nI'm not quite sure what this implies. What arbitrary policy do you have\nin mind?\n\n> Then to thinking about exposing new APIs.\n> \n> Now, i have immediate questions about the convertor setup, before I\n> submit my take on the same patch.\n> \n> > I'm having a hard time to grasp if the core requirements are being met\n> > though.\n> > \n> > We have two devices that could be configured in 3 different ways:\n> > \n> > 1)\n> > ┌────────────┐\n> > │ video_ ├──► Stream0\n> > └────────────┘\n> > \n> > 2)\n> > ┌────────────┐ ┌────────────┐\n> > │ video_ ├──►│ convertor_ ├──► Stream0\n> > └────────────┘ └────────────┘\n> > \n> > 3)\n> > ┌────────────┐ ┌────────────┐\n> > │ video_ ├──►│ convertor_ ├──► Stream1\n> > └───────┬────┘ └────────────┘\n> > │\n> > └► stream0\n> > \n> \n> Is this last picture really correct? I was under the impression that\n> the sensor is expected to create only one stream, and all extra\n> streams are handled by the converter. A comment in camera.cpp says:\n> \n> > * It is possible to produce up to one stream without conversion\n\nCorrect, that is use case 1.\n\nPerhaps it would be better to draw 3) as:\n\n\n ┌───────────────────► stream0\n │\n ┌────────────┐ │ ┌────────────┐\n │ video_ ├─┴──►│ convertor_ ├──► stream1\n └────────────┘ └────────────┘\n\n\n> There's also the question about how independent the different streams\n> can be. It's undocumented, but SimpleCameraData::Configuration seems\n> to define the range of configurations which can be simultaneously\n> deployed on multiple streams.\n\nThe only way two streams could be handled here, is by the buffer being\npassed from the video_ to convertor_ ... and when the convertor is\nfinished reading, it can be passed to the application in the request.\n\nSo the application would see two streams.\n\n video_ output (exactly as is passed into the convertor_)\n convertor_ output.\n\n\nNote when I say 'can be passed' - that doesn't mean it is currently\nimplemented. It means it could be implemented, if we need to expose\naccess to the 'raw' (pre-converted) frames.\n\n\nNote also that the documentation for the simple pipeline handler states:\n\n * Format Conversion and Scaling\n * -----------------------------\n *\n * The capture pipeline isn't expected to include a scaler, and if a scaler is\n * available, it is ignored when configuring the pipeline.\n\n\nSo this current activity is about changing that as I understand it. \n\n \n> ```\n> struct Configuration {\n> ...\n> std::vector outputFormats;\n> SizeRange outputSizes;\n> }\n> ```\n> \n> Reading through the validate() code, any combination of those will be accepted, no matter how many streams.\n> \n> Is that the intended function of this structure?\n\nIt is the job of the pipeline handler to validate the configuration is\nacceptable and functional on the hardware.\n\nSo in this case, it's up to the simple pipeline handler to ensure that a\nCameraConfiguration (with it's stream configurations) passed in can be\nconfigured.\n\nIf the existing code base is not doing that, then it should be updated.\n\n\nBut that struct Configuration is internal to the SimplePipeline handler,\nso it is not exposed to applications anyway, and doesn't need\n'validation'.\n\nIt is the CameraConfiguration that must be validated.\n\n\nAnd the number of streams there is certainly limited by the code:\n\n SimpleCameraConfiguration::validate()\n {\n ...\n\n\t/* Cap the number of entries to the available streams. */\n\tif (config_.size() > data_->streams_.size()) {\n\t\tconfig_.resize(data_->streams_.size());\n\t\tstatus = Adjusted;\n\t}\n\n\t...\n }\n\nI would assume/expect that data_->streams_ is always restricted to a\nsingle stream at the moment, but I have not confirmed this.\n\n\n> Originally I was under an impression that it merely enumerated\n> supported modes, and that replacing the outputFormats collection with\n> multiple Configurations, each with a single outputFormat would not\n> change any functionality. But now I think this is actually overloaded\n> with converter configuration.\n\nI'm not sure I fully understand here, There is a defined configuration\nprocess... somewhat off the top of my head as a high-level view:\n\n # Optionally specify roles to prefill\n c = Camera->generateConfiguration({Raw, Viewfinder})\n\n # Set any parameters desired on the stream Configuration (stream0)\n c.at(0).size = { 1920, 1080 };\n\n # Set our viewFinder size\n c.at(1).size = { 640, 480 };\n\n # Validate it. If it can't be handled, the pipeline handler will update\n # to what can be.\n ret = c->validate();\n\n if (ret == Adjusted) {\n\t# The pipeline handler made changes, we might want to check, or\n\t# we can just accept it.\n\t# If we don't like what was returned, we can change it again,\n\t# but we should re-validate it before calling configure - or the\n\t# configure could simply fail.\n }\n\n # The now adjusted configuration should be guaranteed to provide a\n # useable configuration.\n ret = Camera.configure(c);\n\n> I don't want to mess up the structuring of this code, so I'm going to\n> hold on until I get an answer to this question.\n\nThat's understandable, I suspect there are a few more design questions\nto solve along the way too.\n\n--\nKieran\n\n\n> Thanks,\n> Dorota","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 E06B2BE080\n\tfor ;\n\tMon, 20 Dec 2021 16:36:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3600560868;\n\tMon, 20 Dec 2021 17:36:02 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9626260115\n\tfor ;\n\tMon, 20 Dec 2021 17:36:00 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C74FFEF;\n\tMon, 20 Dec 2021 17:36:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lKbqKzNV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1640018160;\n\tbh=UCKoriO+6tRWIAbagd5hZ3f2xFDU1EyFG184g8UN4Qo=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=lKbqKzNVvsvCdssS2abVE4ZM/SuoV1KzoSNmiVL6n/BUx0cyDqDrCvohUaqFSYnS6\n\tl97YenPHIByIqfZwYYp3kF7XzJtdXOaoWzYOn19LjP1I/I/7auGSl0C8LDzO52WMny\n\tKaz9Yh5BcgOvkMMmKBdFIAs651V0ntEkM5YapiBg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211217194326.65d40c55.dorota.czaplejewicz@puri.sm>","References":"<20211217194326.65d40c55.dorota.czaplejewicz@puri.sm>","From":"Kieran Bingham ","To":"Dorota Czaplejewicz ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 20 Dec 2021 16:35:57 +0000","Message-ID":"<164001815747.2512616.134177853124079992@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22620,"web_url":"https://patchwork.libcamera.org/comment/22620/","msgid":"","date":"2022-04-06T12:23:56","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Reviving a very old thread (time flies...)\n\nOn Mon, Dec 20, 2021 at 04:35:57PM +0000, Kieran Bingham wrote:\n> Quoting Dorota Czaplejewicz (2021-12-17 18:43:26)\n> > Hi,\n> > \n> > I think I know how to solve this, and I've been working on it today.\n> > \n> > There's an open question that Benjamin posed: what's the policy for\n> > choosing size adjustments?\n> \n> The tricky part is being able to expose what the hardware can provide,\n> and validate that the configuration requested by an application can be\n> supported by the underlying hardware.\n> \n> If the configuration can not be supported, then the configuration should\n> be modified - \"Adjusted\" to fit the capabilities by any means the\n> pipeline handler sees fit to do so.\n> \n> The configure() operation will call validate() and it is only allowed to\n> configure if validate() completes without making any adjustments.\n> \n> \n> Trying to go back to what I think is the 'open question':\n> \n> With the converter there seems to be a disconnect between two\n> different functions of `SimpleCameraConfiguration::validate`: Some\n> applications will want to use the closest native resolution of the\n> camera, whereas others will want the converter to match whatever\n> resolution is requested. There's no way to specify which with the\n> current API. What's the solution here?\n> \n> \n> I believe we've handled this on the Raspberry Pi by exposing the native\n> resoltions of the sensor in the 'Raw' stream. (Which is the stream\n> directly out of the video_ node in this case.\n\nThe simple pipeline handler doesn't deal with raw streams in this case\n(it can capture raw data, but when a converter is involved, the sensor\noutput has to be RGB or YUV, and is thus not raw in the traditional\nsense), but conceptually, we have one stream coming from the camera\nreceiver, and a set of additional streams generated by the converter.\nThe same concept is thus applicable, but would require the ability to\nexpose some more information about streams to differentiate \"direct\" and\n\"post-processed\" streams. I think that's a direction we should take\nanyway.\n\n> > I think at this stage, that question is best answered by choosing an\n> > arbitrary policy and dedicating the effort to get this to work at all.\n> \n> I'm not quite sure what this implies. What arbitrary policy do you have\n> in mind?\n> \n> > Then to thinking about exposing new APIs.\n> > \n> > Now, i have immediate questions about the convertor setup, before I\n> > submit my take on the same patch.\n> > \n> > > I'm having a hard time to grasp if the core requirements are being met\n> > > though.\n> > > \n> > > We have two devices that could be configured in 3 different ways:\n> > > \n> > > 1)\n> > > ┌────────────┐\n> > > │ video_ ├──► Stream0\n> > > └────────────┘\n> > > \n> > > 2)\n> > > ┌────────────┐ ┌────────────┐\n> > > │ video_ ├──►│ convertor_ ├──► Stream0\n> > > └────────────┘ └────────────┘\n> > > \n> > > 3)\n> > > ┌────────────┐ ┌────────────┐\n> > > │ video_ ├──►│ convertor_ ├──► Stream1\n> > > └───────┬────┘ └────────────┘\n> > > │\n> > > └► stream0\n> > > \n> > \n> > Is this last picture really correct? I was under the impression that\n> > the sensor is expected to create only one stream, and all extra\n> > streams are handled by the converter. A comment in camera.cpp says:\n> > \n> > > * It is possible to produce up to one stream without conversion\n> \n> Correct, that is use case 1.\n> \n> Perhaps it would be better to draw 3) as:\n> \n> \n> ┌───────────────────► stream0\n> │\n> ┌────────────┐ │ ┌────────────┐\n> │ video_ ├─┴──►│ convertor_ ├──► stream1\n> └────────────┘ └────────────┘\n\nNote that there is no direct hardware connection between video_ and\nconvertor_, there's a memory buffer in-between.\n\n> > There's also the question about how independent the different streams\n> > can be. It's undocumented, but SimpleCameraData::Configuration seems\n> > to define the range of configurations which can be simultaneously\n> > deployed on multiple streams.\n> \n> The only way two streams could be handled here, is by the buffer being\n> passed from the video_ to convertor_ ... and when the convertor is\n> finished reading, it can be passed to the application in the request.\n> \n> So the application would see two streams.\n> \n> video_ output (exactly as is passed into the convertor_)\n> convertor_ output.\n> \n> \n> Note when I say 'can be passed' - that doesn't mean it is currently\n> implemented. It means it could be implemented, if we need to expose\n> access to the 'raw' (pre-converted) frames.\n> \n> \n> Note also that the documentation for the simple pipeline handler states:\n> \n> * Format Conversion and Scaling\n> * -----------------------------\n> *\n> * The capture pipeline isn't expected to include a scaler, and if a scaler is\n> * available, it is ignored when configuring the pipeline.\n> \n> \n> So this current activity is about changing that as I understand it. \n\nAnd to add more fun, we can have two scalers in the capture pipeline,\none in the sensor (commonly through binning/skipping, but sometimes with\na real scaler as well) and one in the SoC.\n\n> > ```\n> > struct Configuration {\n> > ...\n> > std::vector outputFormats;\n> > SizeRange outputSizes;\n> > }\n> > ```\n> > \n> > Reading through the validate() code, any combination of those will\n> > be accepted, no matter how many streams.\n> > \n> > Is that the intended function of this structure?\n> \n> It is the job of the pipeline handler to validate the configuration is\n> acceptable and functional on the hardware.\n> \n> So in this case, it's up to the simple pipeline handler to ensure that a\n> CameraConfiguration (with it's stream configurations) passed in can be\n> configured.\n> \n> If the existing code base is not doing that, then it should be updated.\n> \n> \n> But that struct Configuration is internal to the SimplePipeline handler,\n> so it is not exposed to applications anyway, and doesn't need\n> 'validation'.\n> \n> It is the CameraConfiguration that must be validated.\n> \n> \n> And the number of streams there is certainly limited by the code:\n> \n> SimpleCameraConfiguration::validate()\n> {\n> ...\n> \n> \t/* Cap the number of entries to the available streams. */\n> \tif (config_.size() > data_->streams_.size()) {\n> \t\tconfig_.resize(data_->streams_.size());\n> \t\tstatus = Adjusted;\n> \t}\n> \n> \t...\n> }\n> \n> I would assume/expect that data_->streams_ is always restricted to a\n> single stream at the moment, but I have not confirmed this.\n\nYes and no. The number of streams comes from the pipeline\nSimplePipelineInfo structure, and is a static property of a platform.\nThe only platform that supports a converter today in the master branch\nis imx7 (which covers some i.MX8 SoCs as well), and it is limited to one\nstream, but we could conceptually have more.\n\n> > Originally I was under an impression that it merely enumerated\n> > supported modes, and that replacing the outputFormats collection with\n> > multiple Configurations, each with a single outputFormat would not\n> > change any functionality. But now I think this is actually overloaded\n> > with converter configuration.\n> \n> I'm not sure I fully understand here, There is a defined configuration\n> process... somewhat off the top of my head as a high-level view:\n> \n> # Optionally specify roles to prefill\n> c = Camera->generateConfiguration({Raw, Viewfinder})\n> \n> # Set any parameters desired on the stream Configuration (stream0)\n> c.at(0).size = { 1920, 1080 };\n> \n> # Set our viewFinder size\n> c.at(1).size = { 640, 480 };\n> \n> # Validate it. If it can't be handled, the pipeline handler will update\n> # to what can be.\n> ret = c->validate();\n> \n> if (ret == Adjusted) {\n> \t# The pipeline handler made changes, we might want to check, or\n> \t# we can just accept it.\n> \t# If we don't like what was returned, we can change it again,\n> \t# but we should re-validate it before calling configure - or the\n> \t# configure could simply fail.\n> }\n> \n> # The now adjusted configuration should be guaranteed to provide a\n> # useable configuration.\n> ret = Camera.configure(c);\n> \n> > I don't want to mess up the structuring of this code, so I'm going to\n> > hold on until I get an answer to this question.\n> \n> That's understandable, I suspect there are a few more design questions\n> to solve along the way too.\n\nFYI, I've resumed working on this.","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 E9A3EC3256\n\tfor ;\n\tWed, 6 Apr 2022 12:24:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 685F565643;\n\tWed, 6 Apr 2022 14:24:01 +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 327BB6563F\n\tfor ;\n\tWed, 6 Apr 2022 14:24:00 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 90AFDDEF;\n\tWed, 6 Apr 2022 14:23:59 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649247841;\n\tbh=0HDXH3QjqfF1io0GMF1KwwayJl/cqTe4I8v+2f9iaVI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Psi1RiwRnGgW6CYOyNtxIQfnDwmFAhMjeU0j43pTArUnEXT94wucDNJUuSUeIiUC6\n\tP2M0Gk3qKc0LmdqeoHC4F3rIyvOxCfdqJpSITtcvOiLHK3BaHAzryGRdPOvcIHFf2H\n\t0Wn6Tz8HVOt9OnE9+Qva5x+Z+j5ZmZtdkOwDXDG2Wbuv2yWY9SvlI0Mpn5eZJik5rG\n\tmjpbYy3Nu5u3QIOe+75gh61wdq4J4HxAC8iNk7LYZYW/KaY278Juhup5E4V97AQJgv\n\tjJUGC6z0TbWJ96vcDStx/yi5C4dFUnqS/YzBOyCEa+go+2jQ8n3XtT57OYf8SIy8ar\n\t9cTaKaL2vB7DQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649247839;\n\tbh=0HDXH3QjqfF1io0GMF1KwwayJl/cqTe4I8v+2f9iaVI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uTwdTEZ5XX95t3XiYRS7l5FN7ZCaOOiUzr3TQ/2qM7P9AsK91wf9esO6xA6MNSQMx\n\tN/aRqSebacQHQRgeLAhnzD0qaU8AAfgBKIalBvL3AR4JitONifhvuFombiFagvQCnI\n\tyNzZTJTcLpT0nVxB64d5RFOuy4tGksOde9AtWJnA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uTwdTEZ5\"; dkim-atps=neutral","Date":"Wed, 6 Apr 2022 15:23:56 +0300","To":"Kieran Bingham ","Message-ID":"","References":"<20211217194326.65d40c55.dorota.czaplejewicz@puri.sm>\n\t<164001815747.2512616.134177853124079992@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<164001815747.2512616.134177853124079992@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: Support\n\toutput size ranges","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Dorota Czaplejewicz ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]