[{"id":31979,"web_url":"https://patchwork.libcamera.org/comment/31979/","msgid":"<87zfmkio08.fsf@redhat.com>","date":"2024-10-31T10:07:51","subject":"Re: [PATCH RFC 0/3] Pass sensor delays from rkisp1 IPA to the\n\tpipeline handler","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Mikhail,\n\nthank you for the patches.\n\nMikhail Rudenko <mike.rudenko@gmail.com> writes:\n\n> Hi,\n>\n> At present, rkisp1 uses hardcoded sensor control delays. In the case\n> when they do not match real sensor delays, the AGC algorithm operates\n> in a suboptimal mode, leading to gain/exposure oscillations on capture\n> start and slower convergence.\n>\n> This series does 3 things to fix this situation. \n\nNice, later, it could be fixed for other pipelines too.\n\n> First, it adds an IPC structure for sensor control delays. Second, it\n> adds sensorDelays() method to the CameraSensorHelper and overrides it\n> where sensor-specific delays are known. And finally, it replaces\n> hardcoded delays in rkisp1 pipeline handler with those obtained from\n> the CameraSensorHelper via IPA.\n>\n> I'm not completely sure this is the best solution from the\n> architecture viewpoint, thus \"RFC\". Any feedback is welcome!\n\nThe approach looks good to me, I'm just not sure about the way\nIPASensorDelays is passed around in the 3rd patch.  In any case, let's\nsee what the maintainers or others think.\n\n> Mikhail Rudenko (3):\n>   ipa: core: add IPASensorDelays structure\n>   libcamera: libipa: camera_sensor: Add sensorDelays method\n>   ipa: rkisp1: Pass sensor delays from IPA to pipeline handler\n>\n>  include/libcamera/ipa/core.mojom         | 35 ++++++++++++++\n>  include/libcamera/ipa/rkisp1.mojom       |  3 +-\n>  src/ipa/libipa/camera_sensor_helper.cpp  | 59 ++++++++++++++++++++++++\n>  src/ipa/libipa/camera_sensor_helper.h    |  5 ++\n>  src/ipa/rkisp1/rkisp1.cpp                |  8 +++-\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 27 +++++------\n>  6 files changed, 118 insertions(+), 19 deletions(-)\n>\n> --\n> 2.46.0","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 CE36FC3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 31 Oct 2024 10:08:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C92F8653A8;\n\tThu, 31 Oct 2024 11:08:03 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D8BB9618BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 31 Oct 2024 11:08:00 +0100 (CET)","from mail-lf1-f70.google.com (mail-lf1-f70.google.com\n\t[209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-277-UeneButXO1COEx9CR505jA-1; Thu, 31 Oct 2024 06:07:55 -0400","by mail-lf1-f70.google.com with SMTP id\n\t2adb3069b0e04-539e7dc83ecso450172e87.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 31 Oct 2024 03:07:54 -0700 (PDT)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-381c116ad6fsm1618675f8f.98.2024.10.31.03.07.51\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 31 Oct 2024 03:07:52 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"ch7xfMKP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1730369279;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=y55qLnLiyEd/ntmdLbR1T4dr1WlFhPGh3Ne+bzkwkwU=;\n\tb=ch7xfMKPn9PJsJaTxS8knqNv8rKs2bOcupdgLgoGtA1FHq945I5vl29SikDllyYOEHKQlB\n\tl9w+d68DjpK8USrlAxZcssU9U0iFJNOrrgnInxH2wzZQ1yk6JzAgxQNAPY3EjotMGCkrpd\n\tIWzt25FKCWpc8rzbO516j5itX/Z4rrA=","X-MC-Unique":"UeneButXO1COEx9CR505jA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1730369273; x=1730974073;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=y55qLnLiyEd/ntmdLbR1T4dr1WlFhPGh3Ne+bzkwkwU=;\n\tb=a5xW3z+Qf2sbEKSUj8IiChV2L0nG8i7F9u1efxAJjHLx8BXB+04K1DzjRdZMZ+20Nh\n\t+CdjR2ianS8DiXGBCOrawaPo/m37meE9dPbDpYFn5vMr8tUAQO9eXHQPHIt2TBh2rllP\n\tsnay6YFAk4M1WI5EEngiIY0eLlctB1PwROiPhWYDTOZJa4JFIrPHjre30/6BrJGT9snJ\n\tZA/1egMEZ8aFrMfYMX40nqXuYDdLR3za96l7TOutDgIFb65jb8s/kaFbkxRnKP8PCdIa\n\tNwZQ1IfFFBXU3RAYBkiWhq27OjXlHGvS/FuYRfVgccfF5s8EVrZC0Cks7G4095S3rL7Y\n\tj95Q==","X-Gm-Message-State":"AOJu0YwF9kqlIBUhjplzgsgrkkc1Hv/NyBQ3lOMpkzp1/RJdBNnYK4ss\n\tI+J0QXZACZ7q7sTj077pUbmoSGb28qyZRXnYLkGr7Q1Kx4qebhN1gEcCFYkoPZEDnTyNARgNmjK\n\tjiMZOZHPUZYFBzSJqkDm+h5BK5AOZFOtU4u8+WUzzTQfAfuylanyDWFKhyWIsyVt+IkG0FbY=","X-Received":["by 2002:a05:6512:2204:b0:539:e65a:8a7c with SMTP id\n\t2adb3069b0e04-53b34c995camr7388761e87.58.1730369273306; \n\tThu, 31 Oct 2024 03:07:53 -0700 (PDT)","by 2002:a05:6512:2204:b0:539:e65a:8a7c with SMTP id\n\t2adb3069b0e04-53b34c995camr7388744e87.58.1730369272828; \n\tThu, 31 Oct 2024 03:07:52 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IH4XZvZ9pbHGNPQPHaAL2l90CfLbQHP9f4a3KlariD3X+iQaSDm/EgrZDQcZExXC7zzdSrUBw==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Mikhail Rudenko <mike.rudenko@gmail.com>","Cc":"libcamera-devel@lists.libcamera.org,  Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>,  Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>","Subject":"Re: [PATCH RFC 0/3] Pass sensor delays from rkisp1 IPA to the\n\tpipeline handler","In-Reply-To":"<20241028173659.247353-1-mike.rudenko@gmail.com> (Mikhail\n\tRudenko's message of \"Mon, 28 Oct 2024 20:36:56 +0300\")","References":"<20241028173659.247353-1-mike.rudenko@gmail.com>","Date":"Thu, 31 Oct 2024 11:07:51 +0100","Message-ID":"<87zfmkio08.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31988,"web_url":"https://patchwork.libcamera.org/comment/31988/","msgid":"<a8fe9ef7-3194-4d31-9ddb-db5feb3c45c7@ideasonboard.com>","date":"2024-10-31T13:49:23","subject":"Re: [PATCH RFC 0/3] Pass sensor delays from rkisp1 IPA to the\n\tpipeline handler","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Mikhail - thanks for the RFC\n\nOn 28/10/2024 17:36, Mikhail Rudenko wrote:\n> Hi,\n>\n> At present, rkisp1 uses hardcoded sensor control delays. In the case\n> when they do not match real sensor delays, the AGC algorithm operates\n> in a suboptimal mode, leading to gain/exposure oscillations on capture\n> start and slower convergence.\n>\n> This series does 3 things to fix this situation. First, it adds an IPC\n> structure for sensor control delays. Second, it adds sensorDelays()\n> method to the CameraSensorHelper and overrides it where\n> sensor-specific delays are known. And finally, it replaces hardcoded\n> delays in rkisp1 pipeline handler with those obtained from the\n> CameraSensorHelper via IPA.\n>\n> I'm not completely sure this is the best solution from the\n> architecture viewpoint, thus \"RFC\". Any feedback is welcome!\n\n\nI had a similar patch locally, but I decided to swap it in the end so that the delays are defined as \nmembers of the CameraSensorProperties class, which is available from the PipelineHandlers already - \nso setting the delays in the rkisp1 (and all the other...) pipeline handlers becomes:\n\n\n     /* Initialize the camera properties. */\n     data->properties_ = data->sensor_->properties();\n\n     unsigned int gainDelay = data->properties_.get(properties::GainDelay).value();\n     unsigned int exposureDelay = data->properties_.get(properties::ExposureDelay).value();\n     std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {\n         { V4L2_CID_ANALOGUE_GAIN, { gainDelay, false } },\n         { V4L2_CID_EXPOSURE, { exposureDelay, false } },\n     };\n\n\nUnless the delays are going to also be needed by the IPA module I think that's probably a bit \ncleaner - I can share the patches to show the full changeset. Do you anticipate the IPA's needing \nthe delay values?\n\n\nThanks\n\nDan\n\n\n>\n> Mikhail Rudenko (3):\n>    ipa: core: add IPASensorDelays structure\n>    libcamera: libipa: camera_sensor: Add sensorDelays method\n>    ipa: rkisp1: Pass sensor delays from IPA to pipeline handler\n>\n>   include/libcamera/ipa/core.mojom         | 35 ++++++++++++++\n>   include/libcamera/ipa/rkisp1.mojom       |  3 +-\n>   src/ipa/libipa/camera_sensor_helper.cpp  | 59 ++++++++++++++++++++++++\n>   src/ipa/libipa/camera_sensor_helper.h    |  5 ++\n>   src/ipa/rkisp1/rkisp1.cpp                |  8 +++-\n>   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 27 +++++------\n>   6 files changed, 118 insertions(+), 19 deletions(-)\n>\n> --\n> 2.46.0","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 8FC62C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 31 Oct 2024 13:49:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C39E1653A6;\n\tThu, 31 Oct 2024 14:49:27 +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 CFE9860360\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 31 Oct 2024 14:49:25 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D555E842\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 31 Oct 2024 14:49:21 +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=\"iNOOQhef\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730382561;\n\tbh=T9xh3iqPp53RDl4AUqbZIY6AaIh9a9aK3SjeaQDHP7I=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=iNOOQhefXnkyRwlD7qdteCCW+u8IreNFRiC/XbY4fik6hieBETWIyIVwdlZh0B8dx\n\tb7vQzEVqEvJYAoywJhu9LTvHsjsZmrKsL6p++6VZA7TlfJ5H6pBxvM4pkoImWUvxR0\n\taiURcKolp4PRI8D2oNiw/oVGfTWKenAxAjG48N9E=","Message-ID":"<a8fe9ef7-3194-4d31-9ddb-db5feb3c45c7@ideasonboard.com>","Date":"Thu, 31 Oct 2024 13:49:23 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH RFC 0/3] Pass sensor delays from rkisp1 IPA to the\n\tpipeline handler","To":"libcamera-devel@lists.libcamera.org","References":"<20241028173659.247353-1-mike.rudenko@gmail.com>","Content-Language":"en-US","From":"Dan Scally <dan.scally@ideasonboard.com>","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20241028173659.247353-1-mike.rudenko@gmail.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31990,"web_url":"https://patchwork.libcamera.org/comment/31990/","msgid":"<878qu4pbfl.fsf@gmail.com>","date":"2024-10-31T14:25:37","subject":"Re: [PATCH RFC 0/3] Pass sensor delays from rkisp1 IPA to the\n\tpipeline handler","submitter":{"id":146,"url":"https://patchwork.libcamera.org/api/people/146/","name":"Mikhail Rudenko","email":"mike.rudenko@gmail.com"},"content":"Hi Dan!\n\nOn 2024-10-31 at 13:49 GMT, Dan Scally <dan.scally@ideasonboard.com> wrote:\n\n> Hi Mikhail - thanks for the RFC\n>\n> On 28/10/2024 17:36, Mikhail Rudenko wrote:\n>> Hi,\n>>\n>> At present, rkisp1 uses hardcoded sensor control delays. In the case\n>> when they do not match real sensor delays, the AGC algorithm operates\n>> in a suboptimal mode, leading to gain/exposure oscillations on capture\n>> start and slower convergence.\n>>\n>> This series does 3 things to fix this situation. First, it adds an IPC\n>> structure for sensor control delays. Second, it adds sensorDelays()\n>> method to the CameraSensorHelper and overrides it where\n>> sensor-specific delays are known. And finally, it replaces hardcoded\n>> delays in rkisp1 pipeline handler with those obtained from the\n>> CameraSensorHelper via IPA.\n>>\n>> I'm not completely sure this is the best solution from the\n>> architecture viewpoint, thus \"RFC\". Any feedback is welcome!\n>\n>\n> I had a similar patch locally, but I decided to swap it in the end so\n> that the delays are defined as members of the CameraSensorProperties\n> class, which is available from the PipelineHandlers already - so\n> setting the delays in the rkisp1 (and all the other...) pipeline\n> handlers becomes:\n>\n>\n>     /* Initialize the camera properties. */\n>     data->properties_ = data->sensor_->properties();\n>\n>     unsigned int gainDelay = data->properties_.get(properties::GainDelay).value();\n>     unsigned int exposureDelay = data->properties_.get(properties::ExposureDelay).value();\n>     std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {\n>         { V4L2_CID_ANALOGUE_GAIN, { gainDelay, false } },\n>         { V4L2_CID_EXPOSURE, { exposureDelay, false } },\n>     };\n\nI've used the approach taken by RPi PH/IPA. I also had a thought that\nCameraSensorProperties could be a better place to store the delay. It's\nactually the question I wanted to raise when posting this RFC :)\n\n> Unless the delays are going to also be needed by the IPA module I\n> think that's probably a bit cleaner - I can share the patches to show\n> the full changeset. Do you anticipate the IPA's needing the delay\n> values?\n\nI can't speak for all the IPAs, but as of rkisp1 I see no need for\nexplicit control delays in IPA. Delayed control values are already\npassed by the PH in processStatsBuffer and that looks sufficient.\n\nI think your approach is better overall, so please post your\nseries. Let's see what the maintainers say.\n\n>\n> Thanks\n>\n> Dan\n>\n>\n>>\n>> Mikhail Rudenko (3):\n>>    ipa: core: add IPASensorDelays structure\n>>    libcamera: libipa: camera_sensor: Add sensorDelays method\n>>    ipa: rkisp1: Pass sensor delays from IPA to pipeline handler\n>>\n>>   include/libcamera/ipa/core.mojom         | 35 ++++++++++++++\n>>   include/libcamera/ipa/rkisp1.mojom       |  3 +-\n>>   src/ipa/libipa/camera_sensor_helper.cpp  | 59 ++++++++++++++++++++++++\n>>   src/ipa/libipa/camera_sensor_helper.h    |  5 ++\n>>   src/ipa/rkisp1/rkisp1.cpp                |  8 +++-\n>>   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 27 +++++------\n>>   6 files changed, 118 insertions(+), 19 deletions(-)\n>>\n>> --\n>> 2.46.0\n\n\n--\nBest regards,\nMikhail Rudenko","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 EABA1C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 31 Oct 2024 14:57:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 114FF653A1;\n\tThu, 31 Oct 2024 15:57:44 +0100 (CET)","from mail-qv1-xf35.google.com (mail-qv1-xf35.google.com\n\t[IPv6:2607:f8b0:4864:20::f35])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8389860360\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 31 Oct 2024 15:57:42 +0100 (CET)","by mail-qv1-xf35.google.com with SMTP id\n\t6a1803df08f44-6cbe90c7c95so6982076d6.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 31 Oct 2024 07:57:42 -0700 (PDT)","from razdolb ([77.83.199.21]) by smtp.gmail.com with ESMTPSA id\n\t6a1803df08f44-6d353fc6d07sm8583766d6.44.2024.10.31.07.57.38\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 31 Oct 2024 07:57:40 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"WUzZePX4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1730386661; x=1730991461;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:mime-version:message-id:in-reply-to:date\n\t:subject:cc:to:from:user-agent:references:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=LwpZ2Qvqb0z+WlcPyvUnhJC4olGMpqtokBr1kGBTDhQ=;\n\tb=WUzZePX4DCkF04ZOI+vtVL44goO7XG8YJd7ADC6Eln4RrmdpAleDQxroaNlFwWqn3c\n\tuNYCUoaICU92w3WOspHREj4q45+BMIjs/QqBYASoeyeE5LFS5F9iswZ7gF++rD/LnnA6\n\tBUu+Z2/7C5NTmnEzZzca3y4KU0wX8c2H0CLbBRo2zCzzNNKHa6qiZuMDZQs9D5WLPu2P\n\tlVdnw2JJa8Steg7zWJIOfL2r0wBS5lKObQKU9WjG/H58Xw+mmPJE44+nO/zAzQh+pqCj\n\tsaONTIh+id8/1jqZ1I79tbFm/UM1M6eRwsbihKd90wJS58lNKUKWNQrsggo1b4mxG3uX\n\taFyA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1730386661; x=1730991461;\n\th=content-transfer-encoding:mime-version:message-id:in-reply-to:date\n\t:subject:cc:to:from:user-agent:references:x-gm-message-state:from:to\n\t:cc:subject:date:message-id:reply-to;\n\tbh=LwpZ2Qvqb0z+WlcPyvUnhJC4olGMpqtokBr1kGBTDhQ=;\n\tb=i099ZWlbxzPVFf7PnvRvY7rS3stGYqIUxgC+hh/Mw9l00pNh6f1ducc1v/I4p9IsUF\n\th9d60OD2eZknr72JcAOddW2hRSmiv47sAkbAWTRPQWAHO7BH6oYWAa+fpdtl8yWJHQjE\n\t9N/M50ON+HcxGI/tG0npdT/GQwoplozARubwNVTVEWURQq67ne2LTY848Hen4rYP5o5k\n\tZtTdDL4O1Cmxt3Kl4fRZpZdmbwidYT+H2B5kvunnC9dyOgK//8swjys8Xbd3leRmc0/t\n\ta4d8Kfh8BhUmNYBrvCmy4f8+cvZPqOI2jWEgHQpu4MNJFMuBHH9XiiaXW5HDa2QD525x\n\tY3gA==","X-Gm-Message-State":"AOJu0YzAHluWczzxxhuMjCq45FVnx0kaH6p3w4vWBKahi2alQhybuTKk\n\tClStIZ9g7d/1jAB8xeCcIUMNJGkPTrLpioKkvlwGXt2wXtppU+DMDs+7o6Rl","X-Google-Smtp-Source":"AGHT+IGe7c+DLdBatgIaZ9JooKcNf/brwg6O4ydTLhzyzJmB1IxqbzZZebdIoSft88dfbVt7CLcsUQ==","X-Received":"by 2002:a05:6214:53c9:b0:6cb:c9e0:c299 with SMTP id\n\t6a1803df08f44-6d1856f7c3cmr293139346d6.27.1730386660934; \n\tThu, 31 Oct 2024 07:57:40 -0700 (PDT)","References":"<20241028173659.247353-1-mike.rudenko@gmail.com>\n\t<a8fe9ef7-3194-4d31-9ddb-db5feb3c45c7@ideasonboard.com>","User-agent":"mu4e 1.10.8; emacs 29.4.50","From":"Mikhail Rudenko <mike.rudenko@gmail.com>","To":"Dan Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH RFC 0/3] Pass sensor delays from rkisp1 IPA to the\n\tpipeline handler","Date":"Thu, 31 Oct 2024 17:25:37 +0300","In-reply-to":"<a8fe9ef7-3194-4d31-9ddb-db5feb3c45c7@ideasonboard.com>","Message-ID":"<878qu4pbfl.fsf@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]