[{"id":31477,"web_url":"https://patchwork.libcamera.org/comment/31477/","msgid":"","date":"2024-10-01T06:26:57","subject":"Re: [PATCH v2 4/7] pipeline: rpi: Introduce CameraData::CropParams","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Mon, Sep 30, 2024 at 03:14:12PM GMT, Naushir Patuck wrote:\n> In preparation for assigning separate crop windows for each stream, add\n> a new CropParams structure that stores the existing ispCrop_ and\n> ispMinCropSize_ as fields. Use a new std::map to store a CropParams\n> structure where the map key is the index of the stream configuration in\n> the CameraConfiguration vector.\n>\n> At preset, only a single CropParams structure will be set at key == 0 to\n> preserve the existing crop handling logic.\n>\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: Laurent Pinchart \n> ---\n> .../pipeline/rpi/common/pipeline_base.cpp | 28 +++++++++++++------\n> .../pipeline/rpi/common/pipeline_base.h | 21 ++++++++++++--\n> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 12 ++++++--\n> 3 files changed, 47 insertions(+), 14 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 2de6111bacfd..220c7b962280 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -533,6 +533,7 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> \t * Platform specific internal stream configuration. This also assigns\n> \t * external streams which get configured below.\n> \t */\n> +\tdata->cropParams_.clear();\n> \tret = data->platformConfigure(rpiConfig);\n> \tif (ret)\n> \t\treturn ret;\n> @@ -561,10 +562,14 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> \tfor (auto const &c : result.controlInfo)\n> \t\tctrlMap.emplace(c.first, c.second);\n>\n> -\t/* Add the ScalerCrop control limits based on the current mode. */\n> -\tRectangle ispMinCrop = data->scaleIspCrop(Rectangle(data->ispMinCropSize_));\n> -\tctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,\n> -\t\t\t\t\t\t data->scaleIspCrop(data->ispCrop_));\n> +\tconst auto cropParamsIt = data->cropParams_.find(0);\n> +\tif (cropParamsIt != data->cropParams_.end()) {\n> +\t\tconst CameraData::CropParams &cropParams = cropParamsIt->second;\n> +\t\t/* Add the ScalerCrop control limits based on the current mode. */\n> +\t\tRectangle ispMinCrop = data->scaleIspCrop(Rectangle(cropParams.ispMinCropSize));\n> +\t\tctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,\n> +\t\t\t\t\t\t\t data->scaleIspCrop(cropParams.ispCrop));\n> +\t}\n>\n> \tdata->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());\n>\n> @@ -1291,8 +1296,10 @@ Rectangle CameraData::scaleIspCrop(const Rectangle &ispCrop) const\n> void CameraData::applyScalerCrop(const ControlList &controls)\n> {\n> \tconst auto &scalerCrop = controls.get(controls::ScalerCrop);\n> -\tif (scalerCrop) {\n> +\tconst auto cropParamsIt = cropParams_.find(0);\n> +\tif (scalerCrop && cropParamsIt != cropParams_.end()) {\n> \t\tRectangle nativeCrop = *scalerCrop;\n> +\t\tCropParams &cropParams = cropParamsIt->second;\n>\n> \t\tif (!nativeCrop.width || !nativeCrop.height)\n> \t\t\tnativeCrop = { 0, 0, 1, 1 };\n> @@ -1308,12 +1315,12 @@ void CameraData::applyScalerCrop(const ControlList &controls)\n> \t\t * 2. With the same mid-point, if possible.\n> \t\t * 3. But it can't go outside the sensor area.\n> \t\t */\n> -\t\tSize minSize = ispMinCropSize_.expandedToAspectRatio(nativeCrop.size());\n> +\t\tSize minSize = cropParams.ispMinCropSize.expandedToAspectRatio(nativeCrop.size());\n> \t\tSize size = ispCrop.size().expandedTo(minSize);\n> \t\tispCrop = size.centeredTo(ispCrop.center()).enclosedIn(Rectangle(sensorInfo_.outputSize));\n>\n> -\t\tif (ispCrop != ispCrop_) {\n> -\t\t\tispCrop_ = ispCrop;\n> +\t\tif (ispCrop != cropParams.ispCrop) {\n> +\t\t\tcropParams.ispCrop = ispCrop;\n> \t\t\tplatformSetIspCrop(ispCrop);\n\nWith the question from the previous patch clarified\n\nReviewed-by: Jacopo Mondi \n\n> \t\t}\n> \t}\n> @@ -1471,7 +1478,10 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request\n> \trequest->metadata().set(controls::SensorTimestamp,\n> \t\t\t\tbufferControls.get(controls::SensorTimestamp).value_or(0));\n>\n> -\trequest->metadata().set(controls::ScalerCrop, scaleIspCrop(ispCrop_));\n> +\tconst auto cropParamsIt = cropParams_.find(0);\n> +\tif (cropParamsIt != cropParams_.end())\n> +\t\trequest->metadata().set(controls::ScalerCrop,\n> +\t\t\t\t\tscaleIspCrop(cropParamsIt->second.ispCrop));\n> }\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index d65b695c30b5..75babbe17f31 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -133,8 +133,25 @@ public:\n>\n> \t/* For handling digital zoom. */\n> \tIPACameraSensorInfo sensorInfo_;\n> -\tRectangle ispCrop_; /* crop in ISP (camera mode) pixels */\n> -\tSize ispMinCropSize_;\n> +\n> +\tstruct CropParams {\n> +\t\tCropParams(Rectangle ispCrop_, Size ispMinCropSize_)\n> +\t\t\t: ispCrop(ispCrop_), ispMinCropSize(ispMinCropSize_)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tCropParams()\n> +\t\t{\n> +\t\t}\n> +\n> +\t\t/* Crop in ISP (camera mode) pixels */\n> +\t\tRectangle ispCrop;\n> +\t\t/* Minimum crop size in ISP output pixels */\n> +\t\tSize ispMinCropSize;\n> +\t};\n> +\n> +\t/* Mapping of CropParams keyed by the output stream order in CameraConfiguration */\n> +\tstd::map cropParams_;\n>\n> \tunsigned int dropFrameCount_;\n>\n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index 0ea032293bc9..8080f55a1cf4 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -702,13 +702,19 @@ int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfi\n> \t/* Figure out the smallest selection the ISP will allow. */\n> \tRectangle testCrop(0, 0, 1, 1);\n> \tisp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);\n> -\tispMinCropSize_ = testCrop.size();\n>\n> \t/* Adjust aspect ratio by providing crops on the input image. */\n> \tSize size = unicamFormat.size.boundedToAspectRatio(maxSize);\n> -\tispCrop_ = size.centeredTo(Rectangle(unicamFormat.size).center());\n> +\tRectangle ispCrop = size.centeredTo(Rectangle(unicamFormat.size).center());\n>\n> -\tplatformSetIspCrop(ispCrop_);\n> +\tplatformSetIspCrop(ispCrop);\n> +\t/*\n> +\t * Set the scaler crop to the value we are using (scaled to native sensor\n> +\t * coordinates).\n> +\t */\n> +\tcropParams_.emplace(std::piecewise_construct,\n> +\t\t\t std::forward_as_tuple(0),\n> +\t\t\t std::forward_as_tuple(ispCrop, testCrop.size()));\n>\n> \treturn 0;\n> }\n> --\n> 2.34.1\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 50A97C3257\n\tfor ;\n\tTue, 1 Oct 2024 06:27:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0656563517;\n\tTue, 1 Oct 2024 08:27:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0691C62C91\n\tfor ;\n\tTue, 1 Oct 2024 08:27:02 +0200 (CEST)","from ideasonboard.com (unknown [5.179.150.95])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 656F72E3;\n\tTue, 1 Oct 2024 08:25:30 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lIbOmPyQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727763930;\n\tbh=99a60yDiZLrPZUXuwT9ga79y8HGTOT5LtGr/yZdgA/o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lIbOmPyQAXak3XWGTU00ZXZUH6CJznoyfUBw+bt0737a0ZBgQ/IfAYI/aDzXCEJru\n\tHDdakJsTtp5/a3bv2sm4yYhy3BSXoN3r7H8t1g//ACcUgPIYDFZfI+xw5eo2wKzmrN\n\tskXC/fp2x4aH8oHEvlllQF2Eh/MfHVqjsE6CXutk=","Date":"Tue, 1 Oct 2024 08:26:57 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org, \n\tLaurent Pinchart ","Subject":"Re: [PATCH v2 4/7] pipeline: rpi: Introduce CameraData::CropParams","Message-ID":"","References":"<20240930141415.8857-1-naush@raspberrypi.com>\n\t<20240930141415.8857-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240930141415.8857-5-naush@raspberrypi.com>","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":31478,"web_url":"https://patchwork.libcamera.org/comment/31478/","msgid":"","date":"2024-10-01T06:30:29","subject":"Re: [PATCH v2 4/7] pipeline: rpi: Introduce CameraData::CropParams","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush,\n one more nit\n\nOn Mon, Sep 30, 2024 at 03:14:12PM GMT, Naushir Patuck wrote:\n> In preparation for assigning separate crop windows for each stream, add\n> a new CropParams structure that stores the existing ispCrop_ and\n> ispMinCropSize_ as fields. Use a new std::map to store a CropParams\n> structure where the map key is the index of the stream configuration in\n> the CameraConfiguration vector.\n>\n> At preset, only a single CropParams structure will be set at key == 0 to\n> preserve the existing crop handling logic.\n>\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: Laurent Pinchart \n> ---\n> .../pipeline/rpi/common/pipeline_base.cpp | 28 +++++++++++++------\n> .../pipeline/rpi/common/pipeline_base.h | 21 ++++++++++++--\n> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 12 ++++++--\n> 3 files changed, 47 insertions(+), 14 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 2de6111bacfd..220c7b962280 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -533,6 +533,7 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> \t * Platform specific internal stream configuration. This also assigns\n> \t * external streams which get configured below.\n> \t */\n> +\tdata->cropParams_.clear();\n> \tret = data->platformConfigure(rpiConfig);\n> \tif (ret)\n> \t\treturn ret;\n> @@ -561,10 +562,14 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> \tfor (auto const &c : result.controlInfo)\n> \t\tctrlMap.emplace(c.first, c.second);\n>\n> -\t/* Add the ScalerCrop control limits based on the current mode. */\n> -\tRectangle ispMinCrop = data->scaleIspCrop(Rectangle(data->ispMinCropSize_));\n> -\tctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,\n> -\t\t\t\t\t\t data->scaleIspCrop(data->ispCrop_));\n> +\tconst auto cropParamsIt = data->cropParams_.find(0);\n> +\tif (cropParamsIt != data->cropParams_.end()) {\n> +\t\tconst CameraData::CropParams &cropParams = cropParamsIt->second;\n> +\t\t/* Add the ScalerCrop control limits based on the current mode. */\n> +\t\tRectangle ispMinCrop = data->scaleIspCrop(Rectangle(cropParams.ispMinCropSize));\n> +\t\tctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,\n> +\t\t\t\t\t\t\t data->scaleIspCrop(cropParams.ispCrop));\n> +\t}\n>\n> \tdata->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());\n>\n> @@ -1291,8 +1296,10 @@ Rectangle CameraData::scaleIspCrop(const Rectangle &ispCrop) const\n> void CameraData::applyScalerCrop(const ControlList &controls)\n> {\n> \tconst auto &scalerCrop = controls.get(controls::ScalerCrop);\n> -\tif (scalerCrop) {\n> +\tconst auto cropParamsIt = cropParams_.find(0);\n> +\tif (scalerCrop && cropParamsIt != cropParams_.end()) {\n> \t\tRectangle nativeCrop = *scalerCrop;\n> +\t\tCropParams &cropParams = cropParamsIt->second;\n>\n> \t\tif (!nativeCrop.width || !nativeCrop.height)\n> \t\t\tnativeCrop = { 0, 0, 1, 1 };\n> @@ -1308,12 +1315,12 @@ void CameraData::applyScalerCrop(const ControlList &controls)\n> \t\t * 2. With the same mid-point, if possible.\n> \t\t * 3. But it can't go outside the sensor area.\n> \t\t */\n> -\t\tSize minSize = ispMinCropSize_.expandedToAspectRatio(nativeCrop.size());\n> +\t\tSize minSize = cropParams.ispMinCropSize.expandedToAspectRatio(nativeCrop.size());\n> \t\tSize size = ispCrop.size().expandedTo(minSize);\n> \t\tispCrop = size.centeredTo(ispCrop.center()).enclosedIn(Rectangle(sensorInfo_.outputSize));\n>\n> -\t\tif (ispCrop != ispCrop_) {\n> -\t\t\tispCrop_ = ispCrop;\n> +\t\tif (ispCrop != cropParams.ispCrop) {\n> +\t\t\tcropParams.ispCrop = ispCrop;\n> \t\t\tplatformSetIspCrop(ispCrop);\n> \t\t}\n> \t}\n> @@ -1471,7 +1478,10 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request\n> \trequest->metadata().set(controls::SensorTimestamp,\n> \t\t\t\tbufferControls.get(controls::SensorTimestamp).value_or(0));\n>\n> -\trequest->metadata().set(controls::ScalerCrop, scaleIspCrop(ispCrop_));\n> +\tconst auto cropParamsIt = cropParams_.find(0);\n> +\tif (cropParamsIt != cropParams_.end())\n> +\t\trequest->metadata().set(controls::ScalerCrop,\n> +\t\t\t\t\tscaleIspCrop(cropParamsIt->second.ispCrop));\n> }\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index d65b695c30b5..75babbe17f31 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -133,8 +133,25 @@ public:\n>\n> \t/* For handling digital zoom. */\n> \tIPACameraSensorInfo sensorInfo_;\n> -\tRectangle ispCrop_; /* crop in ISP (camera mode) pixels */\n> -\tSize ispMinCropSize_;\n> +\n> +\tstruct CropParams {\n> +\t\tCropParams(Rectangle ispCrop_, Size ispMinCropSize_)\n> +\t\t\t: ispCrop(ispCrop_), ispMinCropSize(ispMinCropSize_)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tCropParams()\n> +\t\t{\n> +\t\t}\n\nYou probably don't need this ?\n\nThanks\n j\n\n> +\n> +\t\t/* Crop in ISP (camera mode) pixels */\n> +\t\tRectangle ispCrop;\n> +\t\t/* Minimum crop size in ISP output pixels */\n> +\t\tSize ispMinCropSize;\n> +\t};\n> +\n> +\t/* Mapping of CropParams keyed by the output stream order in CameraConfiguration */\n> +\tstd::map cropParams_;\n>\n> \tunsigned int dropFrameCount_;\n>\n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index 0ea032293bc9..8080f55a1cf4 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -702,13 +702,19 @@ int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfi\n> \t/* Figure out the smallest selection the ISP will allow. */\n> \tRectangle testCrop(0, 0, 1, 1);\n> \tisp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);\n> -\tispMinCropSize_ = testCrop.size();\n>\n> \t/* Adjust aspect ratio by providing crops on the input image. */\n> \tSize size = unicamFormat.size.boundedToAspectRatio(maxSize);\n> -\tispCrop_ = size.centeredTo(Rectangle(unicamFormat.size).center());\n> +\tRectangle ispCrop = size.centeredTo(Rectangle(unicamFormat.size).center());\n>\n> -\tplatformSetIspCrop(ispCrop_);\n> +\tplatformSetIspCrop(ispCrop);\n> +\t/*\n> +\t * Set the scaler crop to the value we are using (scaled to native sensor\n> +\t * coordinates).\n> +\t */\n> +\tcropParams_.emplace(std::piecewise_construct,\n> +\t\t\t std::forward_as_tuple(0),\n> +\t\t\t std::forward_as_tuple(ispCrop, testCrop.size()));\n>\n> \treturn 0;\n> }\n> --\n> 2.34.1\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 2479DBD80A\n\tfor ;\n\tTue, 1 Oct 2024 06:30:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 116D36350E;\n\tTue, 1 Oct 2024 08:30:37 +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 4ECA962C91\n\tfor ;\n\tTue, 1 Oct 2024 08:30:35 +0200 (CEST)","from ideasonboard.com (unknown [5.179.150.95])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5BE33824;\n\tTue, 1 Oct 2024 08:29:03 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BccVNeOM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727764143;\n\tbh=85MP2i4NO2J4kEwZjhOlRmpi1lzKlwiNxZXVmLIAIkQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BccVNeOMLptZJvrSG7A+hRX75MPQWnk/6Inl6drLVoWmPFtIJmuJ32eviT4fMDTrU\n\tOp5SUT2oQSRwWsAEzhKBCA/1MukBj4V/oseZ/Y6igACaoT1gXjGzlKQLF1SfzeX6PK\n\tiX7UROEVeSPjyq2BdMJfn0kHQg12h9VmevIO4IlI=","Date":"Tue, 1 Oct 2024 08:30:29 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org, \n\tLaurent Pinchart ","Subject":"Re: [PATCH v2 4/7] pipeline: rpi: Introduce CameraData::CropParams","Message-ID":"","References":"<20240930141415.8857-1-naush@raspberrypi.com>\n\t<20240930141415.8857-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240930141415.8857-5-naush@raspberrypi.com>","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":31498,"web_url":"https://patchwork.libcamera.org/comment/31498/","msgid":"","date":"2024-10-01T11:12:46","subject":"Re: [PATCH v2 4/7] pipeline: rpi: Introduce CameraData::CropParams","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nOn Tue, 1 Oct 2024 at 07:30, Jacopo Mondi wrote:\n>\n> Hi Naush,\n> one more nit\n>\n> On Mon, Sep 30, 2024 at 03:14:12PM GMT, Naushir Patuck wrote:\n> > In preparation for assigning separate crop windows for each stream, add\n> > a new CropParams structure that stores the existing ispCrop_ and\n> > ispMinCropSize_ as fields. Use a new std::map to store a CropParams\n> > structure where the map key is the index of the stream configuration in\n> > the CameraConfiguration vector.\n> >\n> > At preset, only a single CropParams structure will be set at key == 0 to\n> > preserve the existing crop handling logic.\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: Laurent Pinchart \n> > ---\n> > .../pipeline/rpi/common/pipeline_base.cpp | 28 +++++++++++++------\n> > .../pipeline/rpi/common/pipeline_base.h | 21 ++++++++++++--\n> > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 12 ++++++--\n> > 3 files changed, 47 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index 2de6111bacfd..220c7b962280 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -533,6 +533,7 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> > * Platform specific internal stream configuration. This also assigns\n> > * external streams which get configured below.\n> > */\n> > + data->cropParams_.clear();\n> > ret = data->platformConfigure(rpiConfig);\n> > if (ret)\n> > return ret;\n> > @@ -561,10 +562,14 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> > for (auto const &c : result.controlInfo)\n> > ctrlMap.emplace(c.first, c.second);\n> >\n> > - /* Add the ScalerCrop control limits based on the current mode. */\n> > - Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(data->ispMinCropSize_));\n> > - ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,\n> > - data->scaleIspCrop(data->ispCrop_));\n> > + const auto cropParamsIt = data->cropParams_.find(0);\n> > + if (cropParamsIt != data->cropParams_.end()) {\n> > + const CameraData::CropParams &cropParams = cropParamsIt->second;\n> > + /* Add the ScalerCrop control limits based on the current mode. */\n> > + Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(cropParams.ispMinCropSize));\n> > + ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,\n> > + data->scaleIspCrop(cropParams.ispCrop));\n> > + }\n> >\n> > data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());\n> >\n> > @@ -1291,8 +1296,10 @@ Rectangle CameraData::scaleIspCrop(const Rectangle &ispCrop) const\n> > void CameraData::applyScalerCrop(const ControlList &controls)\n> > {\n> > const auto &scalerCrop = controls.get(controls::ScalerCrop);\n> > - if (scalerCrop) {\n> > + const auto cropParamsIt = cropParams_.find(0);\n> > + if (scalerCrop && cropParamsIt != cropParams_.end()) {\n> > Rectangle nativeCrop = *scalerCrop;\n> > + CropParams &cropParams = cropParamsIt->second;\n> >\n> > if (!nativeCrop.width || !nativeCrop.height)\n> > nativeCrop = { 0, 0, 1, 1 };\n> > @@ -1308,12 +1315,12 @@ void CameraData::applyScalerCrop(const ControlList &controls)\n> > * 2. With the same mid-point, if possible.\n> > * 3. But it can't go outside the sensor area.\n> > */\n> > - Size minSize = ispMinCropSize_.expandedToAspectRatio(nativeCrop.size());\n> > + Size minSize = cropParams.ispMinCropSize.expandedToAspectRatio(nativeCrop.size());\n> > Size size = ispCrop.size().expandedTo(minSize);\n> > ispCrop = size.centeredTo(ispCrop.center()).enclosedIn(Rectangle(sensorInfo_.outputSize));\n> >\n> > - if (ispCrop != ispCrop_) {\n> > - ispCrop_ = ispCrop;\n> > + if (ispCrop != cropParams.ispCrop) {\n> > + cropParams.ispCrop = ispCrop;\n> > platformSetIspCrop(ispCrop);\n> > }\n> > }\n> > @@ -1471,7 +1478,10 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request\n> > request->metadata().set(controls::SensorTimestamp,\n> > bufferControls.get(controls::SensorTimestamp).value_or(0));\n> >\n> > - request->metadata().set(controls::ScalerCrop, scaleIspCrop(ispCrop_));\n> > + const auto cropParamsIt = cropParams_.find(0);\n> > + if (cropParamsIt != cropParams_.end())\n> > + request->metadata().set(controls::ScalerCrop,\n> > + scaleIspCrop(cropParamsIt->second.ispCrop));\n> > }\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > index d65b695c30b5..75babbe17f31 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > @@ -133,8 +133,25 @@ public:\n> >\n> > /* For handling digital zoom. */\n> > IPACameraSensorInfo sensorInfo_;\n> > - Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */\n> > - Size ispMinCropSize_;\n> > +\n> > + struct CropParams {\n> > + CropParams(Rectangle ispCrop_, Size ispMinCropSize_)\n> > + : ispCrop(ispCrop_), ispMinCropSize(ispMinCropSize_)\n> > + {\n> > + }\n> > +\n> > + CropParams()\n> > + {\n> > + }\n>\n> You probably don't need this ?\n\nYes, I can get rid of this. I will have to change from cropParams_[i]\nto cropParams_.at(i) in the code since it's a std::map. But I think\nthat's a good idea anyway!\n\nRegards,\nNaush\n\n>\n> Thanks\n> j\n>\n> > +\n> > + /* Crop in ISP (camera mode) pixels */\n> > + Rectangle ispCrop;\n> > + /* Minimum crop size in ISP output pixels */\n> > + Size ispMinCropSize;\n> > + };\n> > +\n> > + /* Mapping of CropParams keyed by the output stream order in CameraConfiguration */\n> > + std::map cropParams_;\n> >\n> > unsigned int dropFrameCount_;\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > index 0ea032293bc9..8080f55a1cf4 100644\n> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > @@ -702,13 +702,19 @@ int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfi\n> > /* Figure out the smallest selection the ISP will allow. */\n> > Rectangle testCrop(0, 0, 1, 1);\n> > isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);\n> > - ispMinCropSize_ = testCrop.size();\n> >\n> > /* Adjust aspect ratio by providing crops on the input image. */\n> > Size size = unicamFormat.size.boundedToAspectRatio(maxSize);\n> > - ispCrop_ = size.centeredTo(Rectangle(unicamFormat.size).center());\n> > + Rectangle ispCrop = size.centeredTo(Rectangle(unicamFormat.size).center());\n> >\n> > - platformSetIspCrop(ispCrop_);\n> > + platformSetIspCrop(ispCrop);\n> > + /*\n> > + * Set the scaler crop to the value we are using (scaled to native sensor\n> > + * coordinates).\n> > + */\n> > + cropParams_.emplace(std::piecewise_construct,\n> > + std::forward_as_tuple(0),\n> > + std::forward_as_tuple(ispCrop, testCrop.size()));\n> >\n> > return 0;\n> > }\n> > --\n> > 2.34.1\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 354A5C3257\n\tfor ;\n\tTue, 1 Oct 2024 11:13:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 27105618D7;\n\tTue, 1 Oct 2024 13:13:25 +0200 (CEST)","from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com\n\t[IPv6:2a00:1450:4864:20::42b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22DDA60553\n\tfor ;\n\tTue, 1 Oct 2024 13:13:23 +0200 (CEST)","by mail-wr1-x42b.google.com with SMTP id\n\tffacd0b85a97d-37cdc7bd941so197934f8f.0\n\tfor ;\n\tTue, 01 Oct 2024 04:13:23 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"YEz3HZBi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1727781202; x=1728386002;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=6+m4XADpFozTC8ZFpMq4ZoV6aRWnGIMjBHP6vx/TqlQ=;\n\tb=YEz3HZBiNwir1FBACKkfiavPSpb5DukXd4QU938HmkZ2/fpaPDNYpmGRGjI9WWOdS/\n\tHrW/VQCidFue1QCGdxg/BbBokvlOvorNbEd19io48KP71wl3DSQNeziQ/bkMR0196kgm\n\tkD9LLXFDrsIfCnZlEiTmElaB8sV+9zuZvGGHGSYtIAUHEyCdQWCwz3vdSWn0F5rHWUiL\n\tfYvvQaQaBmE4QdND07LfjtCcylUhB8w2UyhrLpWNdrgBOgwiavugv4Q8mTZt3DhiDAzr\n\t3mr4v+zu5IycUnm7aGpCep+TpT/FLdYDimqNO5zNOKprlxhGs6FRG0IeTtGKb6mIpHuk\n\tVk3w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1727781202; x=1728386002;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=6+m4XADpFozTC8ZFpMq4ZoV6aRWnGIMjBHP6vx/TqlQ=;\n\tb=JbgLsYe65da9nD6CGBb59n97kbGxX155mxqGAqYTIvPAcSxRIzAPaWDO/FSiqmY3gb\n\tbzDTe9wupa0IA1x+JSQsX/XgoJRS1xDaQIkkPNg1kNOLPdMzxdGWI0hSDVzzWU6gQC7r\n\t2PH/mSrggMlIdxSju0Tjb3coX4gpRC2TAlIjz7+FMPi0IkTWxrNX5kzzSgkjrZVFhgwZ\n\t17gXSzIzrKbyZEI5ERFJqd9RSTmZVk51rIkf9mYHFSlbonMIYEy9I+TNDuKPwpICTlzS\n\t9bZ4zpYhpmEP1C4B/EozAMqA1vOgCc2jGqaA1ECfb4pKmngfd9vu3iFQLvnwRJoESVTt\n\t9q7w==","X-Gm-Message-State":"AOJu0YzI/jDTaD+7gEGLNnoq0IU27XIsLlnKDpMQXw77k6T6krowy3yy\n\tRFDnF2WwYY4Pfsel4ENrrzmCun2rg0PW/7X5zoetxvtVQlo3lgyks7YTVV4Dr1fCNhDmOXlWjsi\n\tkEBRHbEx2jrB3/HjYCQ7snqET8bTQBSglXweVyg==","X-Google-Smtp-Source":"AGHT+IEIDVAENhRfeCxjWjTfkii4WBsjjGatPbtJOBEJGyWdiGde/OTlKHbqAq0EpSmPUwb9Nty+CxyXfAdFmr0rdFY=","X-Received":"by 2002:a05:6000:184b:b0:378:c6f5:9e61 with SMTP id\n\tffacd0b85a97d-37cde3b4aa3mr3528681f8f.2.1727781202384;\n\tTue, 01 Oct 2024 04:13:22 -0700 (PDT)","MIME-Version":"1.0","References":"<20240930141415.8857-1-naush@raspberrypi.com>\n\t<20240930141415.8857-5-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 1 Oct 2024 12:12:46 +0100","Message-ID":"","Subject":"Re: [PATCH v2 4/7] pipeline: rpi: Introduce CameraData::CropParams","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, \n\tLaurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","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":31510,"web_url":"https://patchwork.libcamera.org/comment/31510/","msgid":"","date":"2024-10-01T18:23:02","subject":"Re: [PATCH v2 4/7] pipeline: rpi: Introduce CameraData::CropParams","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Tue, Oct 01, 2024 at 12:12:46PM GMT, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> On Tue, 1 Oct 2024 at 07:30, Jacopo Mondi wrote:\n> >\n> > Hi Naush,\n> > one more nit\n> >\n> > On Mon, Sep 30, 2024 at 03:14:12PM GMT, Naushir Patuck wrote:\n> > > In preparation for assigning separate crop windows for each stream, add\n> > > a new CropParams structure that stores the existing ispCrop_ and\n> > > ispMinCropSize_ as fields. Use a new std::map to store a CropParams\n> > > structure where the map key is the index of the stream configuration in\n> > > the CameraConfiguration vector.\n> > >\n> > > At preset, only a single CropParams structure will be set at key == 0 to\n> > > preserve the existing crop handling logic.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > Reviewed-by: Laurent Pinchart \n> > > ---\n> > > .../pipeline/rpi/common/pipeline_base.cpp | 28 +++++++++++++------\n> > > .../pipeline/rpi/common/pipeline_base.h | 21 ++++++++++++--\n> > > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 12 ++++++--\n> > > 3 files changed, 47 insertions(+), 14 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > index 2de6111bacfd..220c7b962280 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > @@ -533,6 +533,7 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> > > * Platform specific internal stream configuration. This also assigns\n> > > * external streams which get configured below.\n> > > */\n> > > + data->cropParams_.clear();\n> > > ret = data->platformConfigure(rpiConfig);\n> > > if (ret)\n> > > return ret;\n> > > @@ -561,10 +562,14 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> > > for (auto const &c : result.controlInfo)\n> > > ctrlMap.emplace(c.first, c.second);\n> > >\n> > > - /* Add the ScalerCrop control limits based on the current mode. */\n> > > - Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(data->ispMinCropSize_));\n> > > - ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,\n> > > - data->scaleIspCrop(data->ispCrop_));\n> > > + const auto cropParamsIt = data->cropParams_.find(0);\n> > > + if (cropParamsIt != data->cropParams_.end()) {\n> > > + const CameraData::CropParams &cropParams = cropParamsIt->second;\n> > > + /* Add the ScalerCrop control limits based on the current mode. */\n> > > + Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(cropParams.ispMinCropSize));\n> > > + ctrlMap[&controls::ScalerCrop] = ControlInfo(ispMinCrop, data->sensorInfo_.analogCrop,\n> > > + data->scaleIspCrop(cropParams.ispCrop));\n> > > + }\n> > >\n> > > data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());\n> > >\n> > > @@ -1291,8 +1296,10 @@ Rectangle CameraData::scaleIspCrop(const Rectangle &ispCrop) const\n> > > void CameraData::applyScalerCrop(const ControlList &controls)\n> > > {\n> > > const auto &scalerCrop = controls.get(controls::ScalerCrop);\n> > > - if (scalerCrop) {\n> > > + const auto cropParamsIt = cropParams_.find(0);\n> > > + if (scalerCrop && cropParamsIt != cropParams_.end()) {\n> > > Rectangle nativeCrop = *scalerCrop;\n> > > + CropParams &cropParams = cropParamsIt->second;\n> > >\n> > > if (!nativeCrop.width || !nativeCrop.height)\n> > > nativeCrop = { 0, 0, 1, 1 };\n> > > @@ -1308,12 +1315,12 @@ void CameraData::applyScalerCrop(const ControlList &controls)\n> > > * 2. With the same mid-point, if possible.\n> > > * 3. But it can't go outside the sensor area.\n> > > */\n> > > - Size minSize = ispMinCropSize_.expandedToAspectRatio(nativeCrop.size());\n> > > + Size minSize = cropParams.ispMinCropSize.expandedToAspectRatio(nativeCrop.size());\n> > > Size size = ispCrop.size().expandedTo(minSize);\n> > > ispCrop = size.centeredTo(ispCrop.center()).enclosedIn(Rectangle(sensorInfo_.outputSize));\n> > >\n> > > - if (ispCrop != ispCrop_) {\n> > > - ispCrop_ = ispCrop;\n> > > + if (ispCrop != cropParams.ispCrop) {\n> > > + cropParams.ispCrop = ispCrop;\n> > > platformSetIspCrop(ispCrop);\n> > > }\n> > > }\n> > > @@ -1471,7 +1478,10 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request\n> > > request->metadata().set(controls::SensorTimestamp,\n> > > bufferControls.get(controls::SensorTimestamp).value_or(0));\n> > >\n> > > - request->metadata().set(controls::ScalerCrop, scaleIspCrop(ispCrop_));\n> > > + const auto cropParamsIt = cropParams_.find(0);\n> > > + if (cropParamsIt != cropParams_.end())\n> > > + request->metadata().set(controls::ScalerCrop,\n> > > + scaleIspCrop(cropParamsIt->second.ispCrop));\n> > > }\n> > >\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > > index d65b695c30b5..75babbe17f31 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > > @@ -133,8 +133,25 @@ public:\n> > >\n> > > /* For handling digital zoom. */\n> > > IPACameraSensorInfo sensorInfo_;\n> > > - Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */\n> > > - Size ispMinCropSize_;\n> > > +\n> > > + struct CropParams {\n> > > + CropParams(Rectangle ispCrop_, Size ispMinCropSize_)\n> > > + : ispCrop(ispCrop_), ispMinCropSize(ispMinCropSize_)\n> > > + {\n> > > + }\n> > > +\n> > > + CropParams()\n> > > + {\n> > > + }\n> >\n> > You probably don't need this ?\n>\n> Yes, I can get rid of this. I will have to change from cropParams_[i]\n> to cropParams_.at(i) in the code since it's a std::map. But I think\n> that's a good idea anyway!\n\nAh right\n\n../src/libcamera/pipeline/rpi/common/pipeline_base.cpp:577:74: required from here\n 577 | Rectangle ispMinCrop = data->scaleIspCrop(Rectangle(data->cropParams_[0].ispMinCropSize));\n | ^\n/usr/include/c++/14/tuple:2888:9: error: no matching function for call to ‘libcamera::RPi::CameraData::CropParams::CropParams()’\n 2888 | second(std::forward<_Args2>(std::get<_Indexes2>(__tuple2))...)\n | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\nThis will prevent erroneously creating non-initialized instances.\nHowever std::vector::at() might throw an exception\n\nMaybe\n\t\tCropParams() = default;\n\nit's not that bad after all :)\n\n\n>\n> Regards,\n> Naush\n>\n> >\n> > Thanks\n> > j\n> >\n> > > +\n> > > + /* Crop in ISP (camera mode) pixels */\n> > > + Rectangle ispCrop;\n> > > + /* Minimum crop size in ISP output pixels */\n> > > + Size ispMinCropSize;\n> > > + };\n> > > +\n> > > + /* Mapping of CropParams keyed by the output stream order in CameraConfiguration */\n> > > + std::map cropParams_;\n> > >\n> > > unsigned int dropFrameCount_;\n> > >\n> > > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > index 0ea032293bc9..8080f55a1cf4 100644\n> > > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > @@ -702,13 +702,19 @@ int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfi\n> > > /* Figure out the smallest selection the ISP will allow. */\n> > > Rectangle testCrop(0, 0, 1, 1);\n> > > isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop);\n> > > - ispMinCropSize_ = testCrop.size();\n> > >\n> > > /* Adjust aspect ratio by providing crops on the input image. */\n> > > Size size = unicamFormat.size.boundedToAspectRatio(maxSize);\n> > > - ispCrop_ = size.centeredTo(Rectangle(unicamFormat.size).center());\n> > > + Rectangle ispCrop = size.centeredTo(Rectangle(unicamFormat.size).center());\n> > >\n> > > - platformSetIspCrop(ispCrop_);\n> > > + platformSetIspCrop(ispCrop);\n> > > + /*\n> > > + * Set the scaler crop to the value we are using (scaled to native sensor\n> > > + * coordinates).\n> > > + */\n> > > + cropParams_.emplace(std::piecewise_construct,\n> > > + std::forward_as_tuple(0),\n> > > + std::forward_as_tuple(ispCrop, testCrop.size()));\n> > >\n> > > return 0;\n> > > }\n> > > --\n> > > 2.34.1\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 B4294C3257\n\tfor ;\n\tTue, 1 Oct 2024 18:23:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5794E63512;\n\tTue, 1 Oct 2024 20:23:09 +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 4F06D60553\n\tfor ;\n\tTue, 1 Oct 2024 20:23:07 +0200 (CEST)","from ideasonboard.com (unknown [5.77.89.72])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E74C8A1A;\n\tTue, 1 Oct 2024 20:21:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gsXjmohN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727806895;\n\tbh=gbIHO0nUkn0drs7V3rZIcVs6NLLtAKv1aUZlBMqNZrY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gsXjmohNJzmBvMofi/awE0GYgYgrMzPIUqDI5e9MmzUoMuwURu/jThfHPCcQvWI+j\n\tL31efUjZQyETTRMpGMgOxlPvibwlmtxJpoY1uTiRwlw/+8/eAyD5R250mMO2Ddm2vE\n\tGhE/rDhmBz6f808Muz87R9n36vUV/JPSlDTjuLAM=","Date":"Tue, 1 Oct 2024 20:23:02 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart ","Subject":"Re: [PATCH v2 4/7] pipeline: rpi: Introduce CameraData::CropParams","Message-ID":"","References":"<20240930141415.8857-1-naush@raspberrypi.com>\n\t<20240930141415.8857-5-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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\" "}}]