[{"id":30199,"web_url":"https://patchwork.libcamera.org/comment/30199/","msgid":"<76khnfc3ftz26xlutuuncxhq4rjxewx3taxh7olvon3t4zh2n6@w6razlmjmq2u>","date":"2024-07-02T08:56:59","subject":"Re: [PATCH 0/5] ipa: Add black level to camera sensor helpers","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Mon, Jul 01, 2024 at 04:38:23PM GMT, Stefan Klug wrote:\n> For the tuning process, the sensor black level needs to be known. Many\n> sensors have either a fixed black level, or the current sensor kernel\n> driver sets a fixed value. It therefore makes sense to keep that\n\nWhere do kernel driver handles BLC and how ?\n\n> information inside libcamera. When the need arises to be more flexible\n> we can extend the sensor drivers to report black level information. This\n> series adds black level support to the camera sensor helpers and the\n> corresponding metadata to the rkisp1 ipa.\n\nI always considered the camera_sensor_properties database a better\nplace where to store immutable properties like this one, but the\ndatabase is not accessibile by IPA. Have you gone for\nCameraSensorHelper for this reason ?\n\nHow does this play with the idea of moving CameraSensorHelpers on the\npipeline side (something on our roadmap since a long time ?). Will we\nneed in this case to anyway pass sensor's data from pipeline to the\nIPA ?\n\n>\n> My tuning series will be rebased on top of this one.\n>\n> Best regards,\n> Stefan\n>\n> Stefan Klug (5):\n> ipa: libipa: Add black levels to camera sensor helper\n> ipa: rkisp1: Move camHelper into IPAContext\n> ipa: rkisp1: blc: Query black levels from camera sensor helper\n> ipa: rkisp1: blc: Report sensor black levels in metadata\n> ipa: rkisp1: data: Update tuning files for imx219 and imx258\n>\n> src/ipa/libipa/camera_sensor_helper.cpp | 18 ++++++++++\n> src/ipa/libipa/camera_sensor_helper.h | 6 ++++\n> src/ipa/rkisp1/algorithms/blc.cpp | 47 ++++++++++++++++++++++---\n> src/ipa/rkisp1/algorithms/blc.h | 5 ++-\n> src/ipa/rkisp1/data/imx219.yaml | 4 ---\n> src/ipa/rkisp1/data/imx258.yaml | 1 +\n> src/ipa/rkisp1/data/uncalibrated.yaml | 1 +\n> src/ipa/rkisp1/ipa_context.h | 4 +++\n> src/ipa/rkisp1/rkisp1.cpp | 26 +++++++-------\n> 9 files changed, 89 insertions(+), 23 deletions(-)\n>\n> --\n> 2.43.0\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 01F6CBD87C\n\tfor ;\n\tTue, 2 Jul 2024 08:57:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E056862E23;\n\tTue, 2 Jul 2024 10:57:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C737619C8\n\tfor ;\n\tTue, 2 Jul 2024 10:57:03 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A603E5A4;\n\tTue, 2 Jul 2024 10:56:35 +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=\"NkTvF2/T\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719910595;\n\tbh=hvRAIf+WvM2b0E+Uxif0mu2+pgTiNTfgWqm2eHk9irk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NkTvF2/TY8Qfu9xrgbDoKHjspT+5jLny6HftHXVwSvvMCA2LeMnC91+qOgawr0TMF\n\tbbOkTzYEmdKYV955VhKVcs1EzlG2iif9vf2SwRVynCm0/hS7/nH/FKaCwDPLbCu7yu\n\tXwt8pK8UykcToT6qJP3vieAB2LEQlt3PFy8JKA+M=","Date":"Tue, 2 Jul 2024 10:56:59 +0200","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/5] ipa: Add black level to camera sensor helpers","Message-ID":"<76khnfc3ftz26xlutuuncxhq4rjxewx3taxh7olvon3t4zh2n6@w6razlmjmq2u>","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240701144122.3418955-1-stefan.klug@ideasonboard.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":30215,"web_url":"https://patchwork.libcamera.org/comment/30215/","msgid":"<20240702125759.GO30900@pendragon.ideasonboard.com>","date":"2024-07-02T12:57:59","subject":"Re: [PATCH 0/5] ipa: Add black level to camera sensor helpers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jul 02, 2024 at 10:56:59AM +0200, Jacopo Mondi wrote:\n> Hi Stefan\n> \n> On Mon, Jul 01, 2024 at 04:38:23PM GMT, Stefan Klug wrote:\n> > For the tuning process, the sensor black level needs to be known. Many\n> > sensors have either a fixed black level, or the current sensor kernel\n> > driver sets a fixed value. It therefore makes sense to keep that\n> \n> Where do kernel driver handles BLC and how ?\n\nCurrently they either leave the corresponding registers to their\npower-on default values, or program them with fixed values. I'm not\naware of any driver in mainline that makes the data pedestal\nconfigurable from userspace.\n\n> > information inside libcamera. When the need arises to be more flexible\n> > we can extend the sensor drivers to report black level information. This\n> > series adds black level support to the camera sensor helpers and the\n> > corresponding metadata to the rkisp1 ipa.\n\nIt could be useful to explain a bit better that we're covering the data\npedestal only at this point, and how that differs from the black level.\nIdeally that information should be captured somewhere in libcamera.\nMaybe in the documentation of the CameraSensorHelper::blackLevels()\nfunction ?\n\n> I always considered the camera_sensor_properties database a better\n> place where to store immutable properties like this one, but the\n> database is not accessibile by IPA. Have you gone for\n> CameraSensorHelper for this reason ?\n\nI told Stefan to go for CameraSensorHelper for that reason, yes.\nEventually the two sets of helpers should be merged into one.\n\n> How does this play with the idea of moving CameraSensorHelpers on the\n> pipeline side (something on our roadmap since a long time ?). Will we\n> need in this case to anyway pass sensor's data from pipeline to the\n> IPA ?\n\nI'd rather address it at that point, with a solution that covers all the\ninformation the IPA module needs, instead of adding a partial\nimplementation today that we would need to rework tomorrow.\n\n> > My tuning series will be rebased on top of this one.\n> >\n> > Best regards,\n> > Stefan\n> >\n> > Stefan Klug (5):\n> > ipa: libipa: Add black levels to camera sensor helper\n> > ipa: rkisp1: Move camHelper into IPAContext\n> > ipa: rkisp1: blc: Query black levels from camera sensor helper\n> > ipa: rkisp1: blc: Report sensor black levels in metadata\n> > ipa: rkisp1: data: Update tuning files for imx219 and imx258\n> >\n> > src/ipa/libipa/camera_sensor_helper.cpp | 18 ++++++++++\n> > src/ipa/libipa/camera_sensor_helper.h | 6 ++++\n> > src/ipa/rkisp1/algorithms/blc.cpp | 47 ++++++++++++++++++++++---\n> > src/ipa/rkisp1/algorithms/blc.h | 5 ++-\n> > src/ipa/rkisp1/data/imx219.yaml | 4 ---\n> > src/ipa/rkisp1/data/imx258.yaml | 1 +\n> > src/ipa/rkisp1/data/uncalibrated.yaml | 1 +\n> > src/ipa/rkisp1/ipa_context.h | 4 +++\n> > src/ipa/rkisp1/rkisp1.cpp | 26 +++++++-------\n> > 9 files changed, 89 insertions(+), 23 deletions(-)\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 45B4EBD87C\n\tfor ;\n\tTue, 2 Jul 2024 12:58:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5CCE262C96;\n\tTue, 2 Jul 2024 14:58:23 +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 A4FC7619CC\n\tfor ;\n\tTue, 2 Jul 2024 14:58:20 +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 0499B5A4;\n\tTue, 2 Jul 2024 14:57:52 +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=\"s/d6rsRr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719925073;\n\tbh=/mRSXA6cWpbFFsCubmNxyafolYi304A2um4nOnAAnnk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=s/d6rsRrPOi6PvV8pM0UxzoPeiln+2WFl9SFrcK/L7kUNfS2CIIzyCkXhx0HGBbtG\n\tnVqpEt60wvRCkrzbws6xF0gDJLOftUB3uEpKlJgmERVT3FkwtDxiwPAn4Ujy4P4wtv\n\tMRWBp1uRftl8+unw5w8lUsl58nH3Sm8kQ5FDJmgc=","Date":"Tue, 2 Jul 2024 15:57:59 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/5] ipa: Add black level to camera sensor helpers","Message-ID":"<20240702125759.GO30900@pendragon.ideasonboard.com>","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<76khnfc3ftz26xlutuuncxhq4rjxewx3taxh7olvon3t4zh2n6@w6razlmjmq2u>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<76khnfc3ftz26xlutuuncxhq4rjxewx3taxh7olvon3t4zh2n6@w6razlmjmq2u>","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\" "}}]