[{"id":16655,"web_url":"https://patchwork.libcamera.org/comment/16655/","msgid":"<20210427103146.xgwm7p2phia76wlr@uno.localdomain>","date":"2021-04-27T10:31:46","subject":"Re: [libcamera-devel] [RFC PATCH v2 09/12] pipeline: ipu3: Add\n\tcontrols for FULL compliance","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Thu, Apr 22, 2021 at 06:40:59PM +0900, Paul Elder wrote:\n> Add controls to IPU3Controls that are necessary for FULL compliance.\n>\n> Signed-off-by: Paul Elder \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 19 +++++++++++++++++++\n> 1 file changed, 19 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 28e849a4..70a5e9ce 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -48,8 +48,27 @@ static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64;\n> static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32;\n> static constexpr Size IPU3ViewfinderSize(1280, 720);\n>\n> +const std::array IPU3NoiseReductionModeValues = {\n> +\tstatic_cast(controls::draft::NoiseReductionModeOff),\n> +\tstatic_cast(controls::draft::NoiseReductionModeFast),\n> +\tstatic_cast(controls::draft::NoiseReductionModeHighQuality),\n> +};\n> +\n> static const ControlInfoMap::Map IPU3Controls = {\n> \t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n> +\t{ &controls::draft::BlackLevelLocked, ControlInfo(false, true) },\n> +\t{ &controls::AeLocked, ControlInfo(false, true) },\n> +\t{ &controls::draft::AePrecaptureTrigger,\n> +\t\tControlInfo(controls::draft::AePrecaptureTriggerValues) },\n> +\t{ &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> +\t{ &controls::AwbLocked, ControlInfo(false, true) },\n> +\t{ &controls::draft::EdgeMode,\n> +\t\tControlInfo(controls::draft::EdgeModeValues) },\n> +\t{ &controls::draft::NoiseReductionMode,\n> +\t\tControlInfo(IPU3NoiseReductionModeValues) },\n\nInteresting you've ruled out IPU3NoiseReductionModeZSL.\nWhat if we plumb an IPA that supports that ? We have two IPAs for\nIPU3, depending on which one is in use we might have different\nfeatures supported. Going forward, do we need the IPA capabilities to\nbe discoverable ?\n\n> +\t{ &controls::draft::SensorSensitivity, ControlInfo(32, 2400) },\n\nThis should probably come from the sensor.\n\nThe following V4L2 control is described as:\n\nV4L2_CID_ISO_SENSITIVITY (integer menu)\nDetermines ISO equivalent of an image sensor indicating the sensor’s\nsensitivity to light. The numbers are expressed in arithmetic scale,\nas per ISO 12232:2006 standard, where doubling the sensor sensitivity\nis represented by doubling the numerical ISO value. Applications\nshould interpret the values as standard ISO values multiplied by 1000,\ne.g. control value 800 stands for ISO 0.8. Drivers will usually\nsupport only a subset of standard ISO values. The effect of setting\nthis control while the V4L2_CID_ISO_SENSITIVITY_AUTO control is set to\na value other than V4L2_CID_ISO_SENSITIVITY_MANUAL is undefined,\ndrivers should ignore such requests.\n\nIt's good the standard the control refers to is the same as\nandroid.sensor.sensitivity control:\n\nThe sensitivity is the standard ISO sensitivity value,as defined in ISO 12232:2006.\n\nAnyway, that's more a policy decision. Do we require/mandate that\ncontrol in sensor drivers (keeping in mind we keep rising the bar for\ndrivers to qualify libcamera-compatible) or do we go through the\nsensor database ?\n\nThanks\n j\n\n> +\t{ &controls::draft::TonemapMode,\n> +\t\tControlInfo(controls::draft::TonemapModeValues) },\n> };\n>\n> class IPU3CameraData : public CameraData\n> --\n> 2.27.0\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 11C39BDE19\n\tfor ;\n\tTue, 27 Apr 2021 10:31:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6262B68882;\n\tTue, 27 Apr 2021 12:31:06 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 69CD2602C2\n\tfor ;\n\tTue, 27 Apr 2021 12:31:05 +0200 (CEST)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id B817D4000F;\n\tTue, 27 Apr 2021 10:31:04 +0000 (UTC)"],"X-Originating-IP":"93.61.96.190","Date":"Tue, 27 Apr 2021 12:31:46 +0200","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20210427103146.xgwm7p2phia76wlr@uno.localdomain>","References":"<20210422094102.371772-1-paul.elder@ideasonboard.com>\n\t<20210422094102.371772-10-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210422094102.371772-10-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 09/12] pipeline: ipu3: Add\n\tcontrols for FULL compliance","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16667,"web_url":"https://patchwork.libcamera.org/comment/16667/","msgid":"<20210428102401.GK195599@pyrite.rasen.tech>","date":"2021-04-28T10:24:01","subject":"Re: [libcamera-devel] [RFC PATCH v2 09/12] pipeline: ipu3: Add\n\tcontrols for FULL compliance","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Apr 27, 2021 at 12:31:46PM +0200, Jacopo Mondi wrote:\n> Hi Paul,\n> \n> On Thu, Apr 22, 2021 at 06:40:59PM +0900, Paul Elder wrote:\n> > Add controls to IPU3Controls that are necessary for FULL compliance.\n> >\n> > Signed-off-by: Paul Elder \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 19 +++++++++++++++++++\n> > 1 file changed, 19 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 28e849a4..70a5e9ce 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -48,8 +48,27 @@ static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64;\n> > static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32;\n> > static constexpr Size IPU3ViewfinderSize(1280, 720);\n> >\n> > +const std::array IPU3NoiseReductionModeValues = {\n> > +\tstatic_cast(controls::draft::NoiseReductionModeOff),\n> > +\tstatic_cast(controls::draft::NoiseReductionModeFast),\n> > +\tstatic_cast(controls::draft::NoiseReductionModeHighQuality),\n> > +};\n> > +\n> > static const ControlInfoMap::Map IPU3Controls = {\n> > \t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n> > +\t{ &controls::draft::BlackLevelLocked, ControlInfo(false, true) },\n> > +\t{ &controls::AeLocked, ControlInfo(false, true) },\n> > +\t{ &controls::draft::AePrecaptureTrigger,\n> > +\t\tControlInfo(controls::draft::AePrecaptureTriggerValues) },\n> > +\t{ &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> > +\t{ &controls::AwbLocked, ControlInfo(false, true) },\n> > +\t{ &controls::draft::EdgeMode,\n> > +\t\tControlInfo(controls::draft::EdgeModeValues) },\n> > +\t{ &controls::draft::NoiseReductionMode,\n> > +\t\tControlInfo(IPU3NoiseReductionModeValues) },\n> \n> Interesting you've ruled out IPU3NoiseReductionModeZSL.\n> What if we plumb an IPA that supports that ? We have two IPAs for\n\nCTS complains if you don't support reprocessing but do support noise\nreduction ZSL.\n\n> IPU3, depending on which one is in use we might have different\n> features supported. Going forward, do we need the IPA capabilities to\n> be discoverable ?\n\nI think that would be better, yeah.\n\n> \n> > +\t{ &controls::draft::SensorSensitivity, ControlInfo(32, 2400) },\n> \n> This should probably come from the sensor.\n> \n> The following V4L2 control is described as:\n> \n> V4L2_CID_ISO_SENSITIVITY (integer menu)\n> Determines ISO equivalent of an image sensor indicating the sensor’s\n> sensitivity to light. The numbers are expressed in arithmetic scale,\n> as per ISO 12232:2006 standard, where doubling the sensor sensitivity\n> is represented by doubling the numerical ISO value. Applications\n> should interpret the values as standard ISO values multiplied by 1000,\n> e.g. control value 800 stands for ISO 0.8. Drivers will usually\n> support only a subset of standard ISO values. The effect of setting\n> this control while the V4L2_CID_ISO_SENSITIVITY_AUTO control is set to\n> a value other than V4L2_CID_ISO_SENSITIVITY_MANUAL is undefined,\n> drivers should ignore such requests.\n> \n> It's good the standard the control refers to is the same as\n> android.sensor.sensitivity control:\n> \n> The sensitivity is the standard ISO sensitivity value,as defined in ISO 12232:2006.\n> \n> Anyway, that's more a policy decision. Do we require/mandate that\n> control in sensor drivers (keeping in mind we keep rising the bar for\n> drivers to qualify libcamera-compatible) or do we go through the\n> sensor database ?\n\nIf the sensor driver doesn't support it, then we could just drop to\nLIMITED hardware level. This is only needed for FULL. So for libcamera\ncompatibility I think it's not an issue.\n\n\nPaul\n\n> \n> > +\t{ &controls::draft::TonemapMode,\n> > +\t\tControlInfo(controls::draft::TonemapModeValues) },\n> > };\n> >\n> > class IPU3CameraData : public CameraData\n> > --\n> > 2.27.0\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 AD9D3BDE44\n\tfor ;\n\tWed, 28 Apr 2021 10:24:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E90B7688B9;\n\tWed, 28 Apr 2021 12:24: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 A06FD602D1\n\tfor ;\n\tWed, 28 Apr 2021 12:24:08 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 216042CF;\n\tWed, 28 Apr 2021 12:24:06 +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=\"gGgQoMic\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619605448;\n\tbh=mqtiVkaWBi1ivkGKfwZDmwIespbtJt2XUj2hvNNqKQk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gGgQoMice/XKOByZbi7Tfghmllm3dOTMoyDzvorBuUE34dZchXztyfaoFgnpRuz2t\n\tn1r8xF8UYUfqpNNfiDYfWAQ3VJdKxC5yLa2G3eZCZ3s10Eg6ZNGkytzmFb+WBTfX9D\n\tPLMujjiPCTVyOl0CZfB8wm/BdRx4D7IrxnAJK2Fg=","Date":"Wed, 28 Apr 2021 19:24:01 +0900","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi ","Message-ID":"<20210428102401.GK195599@pyrite.rasen.tech>","References":"<20210422094102.371772-1-paul.elder@ideasonboard.com>\n\t<20210422094102.371772-10-paul.elder@ideasonboard.com>\n\t<20210427103146.xgwm7p2phia76wlr@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210427103146.xgwm7p2phia76wlr@uno.localdomain>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 09/12] pipeline: ipu3: Add\n\tcontrols for FULL compliance","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]