[{"id":12622,"web_url":"https://patchwork.libcamera.org/comment/12622/","msgid":"<20200921111232.v5strpp7l2x65bxy@uno.localdomain>","date":"2020-09-21T11:12:32","subject":"Re: [libcamera-devel] [RFC PATCH 1/4] libcamera: Add\n\tSensorOutputSize property","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Mon, Sep 07, 2020 at 05:44:47PM +0100, David Plowman wrote:\n> The SensorOutputSize camera property changes with the selected camera\n> mode, so it must be updated when a new mode is chosen.\n\nnitpicking: I would make a separate patch that adds the property\ndefinition.\n\n> ---\n> src/libcamera/camera_sensor.cpp | 3 +++\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 2 ++\n> src/libcamera/property_ids.yaml | 6 ++++++\n> 3 files changed, 11 insertions(+)\n>\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index d2679a4..b1c218e 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -249,6 +249,9 @@ int CameraSensor::init()\n> \t\tpropertyValue = 0;\n> \tproperties_.set(properties::Rotation, propertyValue);\n>\n> +\t/* The SensorOutputSize is known once the camera mode is chosen. */\n> +\tproperties_.set(properties::SensorOutputSize, Size(0, 0));\n> +\n\nWouldn't it be better to initialize the property reading the format applied\nto the sensor instead of reporting (0,0) ?\n\nTrue that before any configuration have been applied, it has a limited\nvalue knowing it, but I feel it's still safer.\n\n\n> \t/* Enumerate, sort and cache media bus codes and sizes. */\n> \tformats_ = subdev_->formats(pad_);\n> \tif (formats_.empty()) {\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index ce43af3..7f151cb 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -646,6 +646,8 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \tLOG(RPI, Info) << \"Sensor: \" << camera->id()\n> \t\t << \" - Selected mode: \" << sensorFormat.toString();\n>\n> +\tdata->properties_.set(properties::SensorOutputSize, sensorFormat.size);\n> +\n\nI guess this is fine for now. We left the option of augmenting\nCameraConfiguration with properties dandling for quite some time now.\n\nIt's my personal opinion this is acceptable at the moment and we\nshould not block this feature, but once we have augmented CameraConfiguration\nwith properties this one shall be set at validate() time\nit should be moved there. Could you add:\n\n \\todo Move this property to CameraConfiguration when that\n feature will be made available and set it validate() time\n?\n\n\n> \t/*\n> \t * This format may be reset on start() if the bayer order has changed\n> \t * because of flips in the sensor.\n> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> index 74ad019..ed1df19 100644\n> --- a/src/libcamera/property_ids.yaml\n> +++ b/src/libcamera/property_ids.yaml\n> @@ -640,4 +640,10 @@ controls:\n> \\todo Rename this property to ActiveAreas once we will have property\n> categories (i.e. Properties::PixelArray::ActiveAreas)\n>\n> + - SensorOutputSize:\n> + type: Size\n> + description: |\n> + The size, in pixels, of the image being output by the sensor in this\n> + camera mode.\n> +\n\nA Size or a rectangle reporting the offset from the active pixel\narray top-left corner ?\n\nReporting a Rectangle might help calculating the FOV, but it's\nprobably not enough wihtout reporting information on the sensor\nprocessing pipeline (analog crop, scaling/binning etc) all information\nwe currently report to IPAs through CameraSensorInfo. I guess for the\ntime being it's fine just reporting the Size to keep things simple...\n\nAnother question is, for the purpose of this property, the here\nreported size might be obtained by cropping a larger frame in the\nCSI-2 receiver so 'Sensor' might be slightly mis-leading in the name.\nExpanding the documentation might help clarify this.\n\n The size, in pixels of the image being used to produce the\n desired output streams. The image size might correspond to the\n size of the frames produced by the image sensor or might take\n into account additional cropping (or even re-scaling)\n performed by the CSI-2 receiver to adjust the sensor frame\n size to conform to the output images ratio. This property\n might be used to implement digital zoom.\n\nTake anything you consider meaningful and fix anything that sounds\nweird in English :)\n\nThanks\n j\n\n\n> ...\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","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 22626C3B5B\n\tfor ;\n\tMon, 21 Sep 2020 11:08:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DF4E262FD6;\n\tMon, 21 Sep 2020 13:08:40 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 163C860576\n\tfor ;\n\tMon, 21 Sep 2020 13:08:40 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id A90891C0005;\n\tMon, 21 Sep 2020 11:08:39 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 21 Sep 2020 13:12:32 +0200","From":"Jacopo Mondi ","To":"David Plowman ","Message-ID":"<20200921111232.v5strpp7l2x65bxy@uno.localdomain>","References":"<20200907164450.13082-1-david.plowman@raspberrypi.com>\n\t<20200907164450.13082-2-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200907164450.13082-2-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 1/4] libcamera: Add\n\tSensorOutputSize 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\" "}}]