[{"id":13397,"web_url":"https://patchwork.libcamera.org/comment/13397/","msgid":"<20201022052340.GK3942@pendragon.ideasonboard.com>","date":"2020-10-22T05:23:40","subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: Initialise the\n\tScalerCropMaximum property","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Mon, Oct 19, 2020 at 01:51:53PM +0100, David Plowman wrote:\n> Add a default initialisation according to the sensor resolution,\n> though it will need updating when the camera mode changes.\n> \n> Signed-off-by: David Plowman \n> ---\n> src/libcamera/camera_sensor.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 78c7ceec..ae25c5c6 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -279,6 +279,12 @@ int CameraSensor::init()\n> \t */\n> \tresolution_ = sizes_.back();\n> \n> +\t/*\n> +\t * Set a default value for the ScalerCropMaximum, though it will have to\n> +\t * be updated when new camera modes are chosen.\n> +\t */\n> +\tproperties_.set(properties::ScalerCropMaximum, Rectangle{ 0, 0, resolution_ });\n\nI wonder if we actually need this. The properties of the CameraSensor\nclass itself are not updated by the pipeline handler, which updates the\nproperties stored in CameraData instead, so this only serves as a way to\ninitialize the ScalerCropMaximum property. It's not even required in\norder to be able to update the property at configure time, as calling\n\n\tdata->properties_.set(properties::ScalerCropMaximum, ...);\n\nworks without having to set it here.\n\nFurthermore, wouldn't it make sense to only report the property when the\npipeline handler supports digital zoom ? If so, I think it could be best\nto initialize ScalerCropMaximum in the pipeline handler instead. This\ncould be done in PipelineHandlerRPi::match(), right after\n\n\tdata->properties_ = data->sensor_->properties();\n\n> +\n> \treturn 0;\n> }\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 A1D07C3D3C\n\tfor ;\n\tThu, 22 Oct 2020 05:24:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 031DA61059;\n\tThu, 22 Oct 2020 07:24:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AE8EF6034F\n\tfor ;\n\tThu, 22 Oct 2020 07:24:26 +0200 (CEST)","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 2444153;\n\tThu, 22 Oct 2020 07:24:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sn4TDYv2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603344266;\n\tbh=2tHN6xBYKg0zIb3odU3ff0PAf+IdjIbx2c6DfH/OkSs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sn4TDYv2DMlD3UTtYpW4eOvYPiA8LmLgR4Gjw1SOyOm5NL5doMfct0nvphZGhWXbl\n\tb1zmh2cZWFNoAXSfz7oMMpXQ5/A/cmnteMPxhQ3JjN2veOzSnuuWESx5O8LW9sHRd+\n\twVPK2ZxMoLcVY3RwPL3oxQXoli2a8om5mkHfvhOs=","Date":"Thu, 22 Oct 2020 08:23:40 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"<20201022052340.GK3942@pendragon.ideasonboard.com>","References":"<20201019125156.26751-1-david.plowman@raspberrypi.com>\n\t<20201019125156.26751-3-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201019125156.26751-3-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: Initialise the\n\tScalerCropMaximum property","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]