[{"id":21955,"web_url":"https://patchwork.libcamera.org/comment/21955/","msgid":"","date":"2022-01-05T16:12:14","subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for your work.\n\nOn Wed, 5 Jan 2022 at 15:55, David Plowman \nwrote:\n\n> If the sensor exposes the V4L2_CID_NOTIFY_GAINS control, assume it\n> means the sensor wants to be told the latest colour gains.\n>\n> We store whether the control exists and if so its default value, to\n> save us checking for it on every frame.\n>\n> Signed-off-by: David Plowman \n>\n\nReviewed-by: Naushir Patuck \n\n\n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 31 +++++++++++++++++++\n> 1 file changed, 31 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index b5c687da..4adef952 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -264,6 +264,12 @@ public:\n>\n> unsigned int dropFrameCount_;\n>\n> + /*\n> + * If set, this stores the value that represets a gain of one for\n> + * the V4L2_CID_NOTIFY_GAINS control.\n> + */\n> + std::optional notifyGainsUnity_;\n> +\n> private:\n> void checkRequestCompleted();\n> void fillRequestMetadata(const ControlList &bufferControls,\n> @@ -1191,6 +1197,15 @@ int PipelineHandlerRPi::registerCamera(MediaDevice\n> *unicam, MediaDevice *isp)\n> /* Initialize the camera properties. */\n> data->properties_ = data->sensor_->properties();\n>\n> + /*\n> + * The V4L2_CID_NOTIFY_GAINS control, if present, is used to\n> inform the\n> + * sensor of the colour gains. It is defined to be a linear gain\n> where\n> + * the default value represents a gain of exactly one.\n> + */\n> + auto it = data->sensor_->controls().find(V4L2_CID_NOTIFY_GAINS);\n> + if (it != data->sensor_->controls().end())\n> + data->notifyGainsUnity_ = it->second.def().get();\n> +\n> /*\n> * Set a default value for the ScalerCropMaximum property to show\n> * that we support its use, however, initialise it to zero because\n> @@ -1495,6 +1510,22 @@ void RPiCameraData::statsMetadataComplete(uint32_t\n> bufferId, const ControlList &\n> Request *request = requestQueue_.front();\n> request->metadata().merge(controls);\n>\n> + /*\n> + * Inform the sensor of the latest colour gains if it has the\n> + * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is\n> set).\n> + */\n> + if (notifyGainsUnity_ &&\n> controls.contains(libcamera::controls::ColourGains)) {\n> + libcamera::Span colourGains =\n> controls.get(libcamera::controls::ColourGains);\n> + /* The control wants linear gains in the order B, Gb, Gr,\n> R. */\n> + ControlList ctrls(sensor_->controls());\n> + std::array gains = {\n> static_cast(colourGains[1] * *notifyGainsUnity_),\n> + *notifyGainsUnity_,\n> *notifyGainsUnity_,\n> +\n> static_cast(colourGains[0] * *notifyGainsUnity_) };\n> + ctrls.set(V4L2_CID_NOTIFY_GAINS, Span{\n> gains });\n> +\n> + sensor_->setControls(&ctrls);\n> + }\n> +\n> state_ = State::IpaComplete;\n> handleState();\n> }\n> --\n> 2.30.2\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 AC0C3BF415\n\tfor ;\n\tWed, 5 Jan 2022 16:12:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1E7A4608E6;\n\tWed, 5 Jan 2022 17:12:33 +0100 (CET)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F60760868\n\tfor ;\n\tWed, 5 Jan 2022 17:12:31 +0100 (CET)","by mail-lj1-x235.google.com with SMTP id h21so54897832ljh.3\n\tfor ;\n\tWed, 05 Jan 2022 08:12:31 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"e5VesdTV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=l+5aih69X7G4jqVIaDvzI1MLa9KRqyaP2KgiHafv3P0=;\n\tb=e5VesdTVjF3l0KQ6XFBj6bJHqYYnmscuDWcXwubxMWwAGWXr+uT9CMEaYXmJiSlaUe\n\ty/Im3rWYyNM1FJ6fXFlLbvxEkYzgbZMzGeBXCs6MDCYJJNEvWpw2hMwPKyHDZDDTFxQE\n\twnyK/eR7IpqPMezpPFw6IQZwiJ8l5VBt6KLmmC2rYvH/fkaJG6sEuxAE+hCtsEvRA4Ke\n\tqOFsdAAEOCqrGVSBBcEYSgihybleEIW22ls3LmDYMbY6DsypZ7F6N60HwwP5fVA1O616\n\tkIByUKbYFJKz/RRWy8shyisYHI3bNzozp6sTw+jbpRT8rlhHcHm62VM5eNt4dCJXf7we\n\tJSCQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=l+5aih69X7G4jqVIaDvzI1MLa9KRqyaP2KgiHafv3P0=;\n\tb=1QB6EGZqfczdtufkhVcTL12Va0rIa5bp3ZqW5v1u8AojoVNEzJivvm0Mjwx9DpVX6L\n\tXf1xtG6zVHrW5dR2mF6pcLl5BKpUmycWzMJicmlRw+KWRZsI7e7s09C1u/m18TXRMmuQ\n\tkmsrd0YTGf7tVHqOvpOvxirMphhEKAyQa/SpExxGMFnXjvugkp8Iy0e0xQQWPjtr6qJt\n\tf0jBeLtWDJxdTa0IXkIl09acJy1SAtrQy2RisD0QX06Vb8jEO26ClrFxWozhsE88PiVw\n\tH9wcHkPg5JdasSZIlGGdvGCbX+7u5Y6sGI07V532Ihb1tu6sbO+gcBMpE6QgTnvQ5CrR\n\ty5Fg==","X-Gm-Message-State":"AOAM531df1hJI+YhhOYpJne8HACxOFgIEYM36L8dcVzSjcFR+Ww0bL+o\n\tfuXksVkxtz71WBGE/K7FayQ4GcqWj4OjCdJmbMfl3g==","X-Google-Smtp-Source":"ABdhPJwzRAkOyFCxGnFRxLJRGoGeC73AOW6r/sdVMznUojKf98eXJQV6PRokYVGjq+ROevmUqTOVGbZayJKmi89vc3E=","X-Received":"by 2002:a2e:9196:: with SMTP id\n\tf22mr43856873ljg.444.1641399150231; \n\tWed, 05 Jan 2022 08:12:30 -0800 (PST)","MIME-Version":"1.0","References":"<20220105155539.5550-1-david.plowman@raspberrypi.com>\n\t<20220105155539.5550-4-david.plowman@raspberrypi.com>","In-Reply-To":"<20220105155539.5550-4-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Wed, 5 Jan 2022 16:12:14 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"00000000000048d27605d4d80564\"","Subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21957,"web_url":"https://patchwork.libcamera.org/comment/21957/","msgid":"","date":"2022-01-05T19:47:44","subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","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 Wed, Jan 05, 2022 at 03:55:39PM +0000, David Plowman wrote:\n> If the sensor exposes the V4L2_CID_NOTIFY_GAINS control, assume it\n> means the sensor wants to be told the latest colour gains.\n> \n> We store whether the control exists and if so its default value, to\n> save us checking for it on every frame.\n> \n> Signed-off-by: David Plowman \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 31 +++++++++++++++++++\n> 1 file changed, 31 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index b5c687da..4adef952 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -264,6 +264,12 @@ public:\n> \n> \tunsigned int dropFrameCount_;\n> \n> +\t/*\n> +\t * If set, this stores the value that represets a gain of one for\n> +\t * the V4L2_CID_NOTIFY_GAINS control.\n> +\t */\n> +\tstd::optional notifyGainsUnity_;\n\nA plain int32_t would be enough, as a zero value isn't valid. Up to\nyou.\n\n> +\n> private:\n> \tvoid checkRequestCompleted();\n> \tvoid fillRequestMetadata(const ControlList &bufferControls,\n> @@ -1191,6 +1197,15 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)\n> \t/* Initialize the camera properties. */\n> \tdata->properties_ = data->sensor_->properties();\n> \n> +\t/*\n> +\t * The V4L2_CID_NOTIFY_GAINS control, if present, is used to inform the\n> +\t * sensor of the colour gains. It is defined to be a linear gain where\n> +\t * the default value represents a gain of exactly one.\n> +\t */\n> +\tauto it = data->sensor_->controls().find(V4L2_CID_NOTIFY_GAINS);\n> +\tif (it != data->sensor_->controls().end())\n> +\t\tdata->notifyGainsUnity_ = it->second.def().get();\n> +\n> \t/*\n> \t * Set a default value for the ScalerCropMaximum property to show\n> \t * that we support its use, however, initialise it to zero because\n> @@ -1495,6 +1510,22 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> \tRequest *request = requestQueue_.front();\n> \trequest->metadata().merge(controls);\n> \n> +\t/*\n> +\t * Inform the sensor of the latest colour gains if it has the\n> +\t * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).\n> +\t */\n> +\tif (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {\n> +\t\tlibcamera::Span colourGains = controls.get(libcamera::controls::ColourGains);\n> +\t\t/* The control wants linear gains in the order B, Gb, Gr, R. */\n> +\t\tControlList ctrls(sensor_->controls());\n> +\t\tstd::array gains = { static_cast(colourGains[1] * *notifyGainsUnity_),\n> +\t\t\t\t\t\t *notifyGainsUnity_, *notifyGainsUnity_,\n> +\t\t\t\t\t\t static_cast(colourGains[0] * *notifyGainsUnity_) };\n\nLet's shorten the lines a bit:\n\n\t\tstd::array gains{\n\t\t\tstatic_cast(colourGains[1] * *notifyGainsUnity_),\n\t\t\t*notifyGainsUnity_,\n\t\t\t*notifyGainsUnity_,\n\t\t\tstatic_cast(colourGains[0] * *notifyGainsUnity_)\n\t\t};\n\nReviewed-by: Laurent Pinchart \n\n> +\t\tctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> +\n> +\t\tsensor_->setControls(&ctrls);\n> +\t}\n> +\n> \tstate_ = State::IpaComplete;\n> +\t\tctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> +\n> +\t\tsensor_->setControls(&ctrls);\n> +\t}\n> +\n> \tstate_ = State::IpaComplete;\n> \thandleState();\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 00B09BF415\n\tfor ;\n\tWed, 5 Jan 2022 19:47:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 65D74608E6;\n\tWed, 5 Jan 2022 20:47:51 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2E0C60868\n\tfor ;\n\tWed, 5 Jan 2022 20:47:49 +0100 (CET)","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 5D423993;\n\tWed, 5 Jan 2022 20:47:49 +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=\"pdk1u0IZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1641412069;\n\tbh=Eijm3XLn/pjvlnvYZGAwE57zJb/qAX+zKgoJZK2aWCE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pdk1u0IZNtZUw53i/WOiGrdxAoR0LhGWgSeumajvZ5PeoS40KckvsICLo5dyQ8y4G\n\tmFMB9JPbcEFQQAtu5UM2JME3zyWQ+pQ38CMohMu+Alutootu7w6aIF8Nl9Mmx8bWhK\n\tSCc+FEp29aZjE035zJIwg3qRXVWAtpWH67/jabnM=","Date":"Wed, 5 Jan 2022 21:47:44 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20220105155539.5550-1-david.plowman@raspberrypi.com>\n\t<20220105155539.5550-4-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220105155539.5550-4-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21963,"web_url":"https://patchwork.libcamera.org/comment/21963/","msgid":"","date":"2022-01-06T14:26:13","subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nThanks for the review.\n\nOn Wed, 5 Jan 2022 at 19:47, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> Thank you for the patch.\n>\n> On Wed, Jan 05, 2022 at 03:55:39PM +0000, David Plowman wrote:\n> > If the sensor exposes the V4L2_CID_NOTIFY_GAINS control, assume it\n> > means the sensor wants to be told the latest colour gains.\n> >\n> > We store whether the control exists and if so its default value, to\n> > save us checking for it on every frame.\n> >\n> > Signed-off-by: David Plowman \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 31 +++++++++++++++++++\n> > 1 file changed, 31 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index b5c687da..4adef952 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -264,6 +264,12 @@ public:\n> >\n> > unsigned int dropFrameCount_;\n> >\n> > + /*\n> > + * If set, this stores the value that represets a gain of one for\n> > + * the V4L2_CID_NOTIFY_GAINS control.\n> > + */\n> > + std::optional notifyGainsUnity_;\n>\n> A plain int32_t would be enough, as a zero value isn't valid. Up to\n> you.\n\nAnd I was trying so hard to avoid using the value 0 to have a\n\"special\" meaning... :)\n\nI think at this point I'm inclined to leave it as it is, unless anyone\nelse objects, or I might change it if I need to do another revision.\n\n>\n> > +\n> > private:\n> > void checkRequestCompleted();\n> > void fillRequestMetadata(const ControlList &bufferControls,\n> > @@ -1191,6 +1197,15 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)\n> > /* Initialize the camera properties. */\n> > data->properties_ = data->sensor_->properties();\n> >\n> > + /*\n> > + * The V4L2_CID_NOTIFY_GAINS control, if present, is used to inform the\n> > + * sensor of the colour gains. It is defined to be a linear gain where\n> > + * the default value represents a gain of exactly one.\n> > + */\n> > + auto it = data->sensor_->controls().find(V4L2_CID_NOTIFY_GAINS);\n> > + if (it != data->sensor_->controls().end())\n> > + data->notifyGainsUnity_ = it->second.def().get();\n> > +\n> > /*\n> > * Set a default value for the ScalerCropMaximum property to show\n> > * that we support its use, however, initialise it to zero because\n> > @@ -1495,6 +1510,22 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> > Request *request = requestQueue_.front();\n> > request->metadata().merge(controls);\n> >\n> > + /*\n> > + * Inform the sensor of the latest colour gains if it has the\n> > + * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).\n> > + */\n> > + if (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {\n> > + libcamera::Span colourGains = controls.get(libcamera::controls::ColourGains);\n> > + /* The control wants linear gains in the order B, Gb, Gr, R. */\n> > + ControlList ctrls(sensor_->controls());\n> > + std::array gains = { static_cast(colourGains[1] * *notifyGainsUnity_),\n> > + *notifyGainsUnity_, *notifyGainsUnity_,\n> > + static_cast(colourGains[0] * *notifyGainsUnity_) };\n>\n> Let's shorten the lines a bit:\n>\n> std::array gains{\n> static_cast(colourGains[1] * *notifyGainsUnity_),\n> *notifyGainsUnity_,\n> *notifyGainsUnity_,\n> static_cast(colourGains[0] * *notifyGainsUnity_)\n> };\n\nIs that fix-while-applying-able, do you think? Otherwise I'll do\nanother version.\n\nThanks again!\nDavid\n\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > + ctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> > +\n> > + sensor_->setControls(&ctrls);\n> > + }\n> > +\n> > state_ = State::IpaComplete;\n> > + ctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> > +\n> > + sensor_->setControls(&ctrls);\n> > + }\n> > +\n> > state_ = State::IpaComplete;\n> > handleState();\n> > }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 F1458BE080\n\tfor ;\n\tThu, 6 Jan 2022 14:26:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 33AE360219;\n\tThu, 6 Jan 2022 15:26:27 +0100 (CET)","from mail-wr1-x434.google.com (mail-wr1-x434.google.com\n\t[IPv6:2a00:1450:4864:20::434])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 91E5260219\n\tfor ;\n\tThu, 6 Jan 2022 15:26:25 +0100 (CET)","by mail-wr1-x434.google.com with SMTP id h23so5167331wrc.1\n\tfor ;\n\tThu, 06 Jan 2022 06:26:25 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"qSAcvWgH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=qrOANoS9q9/jwaAkOG9LidiQiusAFrVWrgQ/RV/UIgY=;\n\tb=qSAcvWgHWIjz/V7EClJ7+iGqyax6ohxKH9cK1un/6Pp7iGRZ1TacWlozn28MJofkap\n\tbb7B50uoDTFm+FXuuP9STKy8ohKEA5LsgHkIOfsze4T8TL/4QItqgL+lgzioTkX5/gJZ\n\t2hsAFKmt2bcmfoE29+fcfkbnpmxYk8FXWASDFrQR82PSzyXAzPrqgUV7yefpty4OZjxj\n\tQs4dxPHZkKpTut/F/ZRCpBMsQ32TLMGpTNT9gbAHZ4vhAVMHFSJSKBSlHd4pHdQHbPF/\n\tiD8QLSklEDrV3qzBQX5JHaNeeu5lecwt2Q0xMTddqwPFZ41IRntEB2dGohbT0eOd1VgR\n\t1QXw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=qrOANoS9q9/jwaAkOG9LidiQiusAFrVWrgQ/RV/UIgY=;\n\tb=OK7M+lcrRskLufQ9+6mZ7UAjZtOKmC1JUCTGPs8m2gyGZSkeejrKsbKNyNHW+yzgY5\n\t0SgOjWzt5R1mqD1GHopxMDD/SL4YMZJOAlIoG850tATXMzkXZUgR7x2Dws9GjU1fcVXx\n\tiZAvlKrorqSUFW2nCFfHotq0QZnAHfUezFNIClhfYJJTGgGCErvwcklhbIdD35l0VwnI\n\tqyy14EOJIF8phzzuvAhqhU1XXAq7cdTDsnbvo9/UZ7a1laonTXBL+0DtI5u2Qg0u0l6i\n\t5EJS21AvFkomFopjs+HVZu0XhxhzSos8Z6s+jECxjIGDIZeNHnzszDJuba5Yx/eV+2Rw\n\tFJQg==","X-Gm-Message-State":"AOAM530o9uOK0c3D+HWJpivLWSfbvmOd8JPqh4nAY/JWsWeeMpGEc8dz\n\tkMaRIFVlgH5zKmi/3i8BNdfh+GOxXfJDXZb2foOV8IizHMh4AA==","X-Google-Smtp-Source":"ABdhPJzg58NZfD51zCKSZM8qdfbg74WVr6FAxWe14Z7woMSVf9s18Nipj9gNHMzVjGSjVSOplw4JjXg/AX4SjqFe05g=","X-Received":"by 2002:a05:6000:18a5:: with SMTP id\n\tb5mr50604076wri.24.1641479184995; \n\tThu, 06 Jan 2022 06:26:24 -0800 (PST)","MIME-Version":"1.0","References":"<20220105155539.5550-1-david.plowman@raspberrypi.com>\n\t<20220105155539.5550-4-david.plowman@raspberrypi.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Thu, 6 Jan 2022 14:26:13 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21964,"web_url":"https://patchwork.libcamera.org/comment/21964/","msgid":"","date":"2022-01-06T14:37:28","subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Thu, Jan 06, 2022 at 02:26:13PM +0000, David Plowman wrote:\n> On Wed, 5 Jan 2022 at 19:47, Laurent Pinchart wrote:\n> > On Wed, Jan 05, 2022 at 03:55:39PM +0000, David Plowman wrote:\n> > > If the sensor exposes the V4L2_CID_NOTIFY_GAINS control, assume it\n> > > means the sensor wants to be told the latest colour gains.\n> > >\n> > > We store whether the control exists and if so its default value, to\n> > > save us checking for it on every frame.\n> > >\n> > > Signed-off-by: David Plowman \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 31 +++++++++++++++++++\n> > > 1 file changed, 31 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index b5c687da..4adef952 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -264,6 +264,12 @@ public:\n> > >\n> > > unsigned int dropFrameCount_;\n> > >\n> > > + /*\n> > > + * If set, this stores the value that represets a gain of one for\n> > > + * the V4L2_CID_NOTIFY_GAINS control.\n> > > + */\n> > > + std::optional notifyGainsUnity_;\n> >\n> > A plain int32_t would be enough, as a zero value isn't valid. Up to\n> > you.\n> \n> And I was trying so hard to avoid using the value 0 to have a\n> \"special\" meaning... :)\n\nI thought std::optional was nice in that it avoided considering any\nspecific value as special, but then realized that 0 is invalid anyway.\n\n> I think at this point I'm inclined to leave it as it is, unless anyone\n> else objects, or I might change it if I need to do another revision.\n\nFine with me.\n\n> > > +\n> > > private:\n> > > void checkRequestCompleted();\n> > > void fillRequestMetadata(const ControlList &bufferControls,\n> > > @@ -1191,6 +1197,15 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)\n> > > /* Initialize the camera properties. */\n> > > data->properties_ = data->sensor_->properties();\n> > >\n> > > + /*\n> > > + * The V4L2_CID_NOTIFY_GAINS control, if present, is used to inform the\n> > > + * sensor of the colour gains. It is defined to be a linear gain where\n> > > + * the default value represents a gain of exactly one.\n> > > + */\n> > > + auto it = data->sensor_->controls().find(V4L2_CID_NOTIFY_GAINS);\n> > > + if (it != data->sensor_->controls().end())\n> > > + data->notifyGainsUnity_ = it->second.def().get();\n> > > +\n> > > /*\n> > > * Set a default value for the ScalerCropMaximum property to show\n> > > * that we support its use, however, initialise it to zero because\n> > > @@ -1495,6 +1510,22 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> > > Request *request = requestQueue_.front();\n> > > request->metadata().merge(controls);\n> > >\n> > > + /*\n> > > + * Inform the sensor of the latest colour gains if it has the\n> > > + * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).\n> > > + */\n> > > + if (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {\n> > > + libcamera::Span colourGains = controls.get(libcamera::controls::ColourGains);\n> > > + /* The control wants linear gains in the order B, Gb, Gr, R. */\n> > > + ControlList ctrls(sensor_->controls());\n> > > + std::array gains = { static_cast(colourGains[1] * *notifyGainsUnity_),\n> > > + *notifyGainsUnity_, *notifyGainsUnity_,\n> > > + static_cast(colourGains[0] * *notifyGainsUnity_) };\n> >\n> > Let's shorten the lines a bit:\n> >\n> > std::array gains{\n> > static_cast(colourGains[1] * *notifyGainsUnity_),\n> > *notifyGainsUnity_,\n> > *notifyGainsUnity_,\n> > static_cast(colourGains[0] * *notifyGainsUnity_)\n> > };\n> \n> Is that fix-while-applying-able, do you think? Otherwise I'll do\n> another version.\n\nI can handle this locally, no problem.\n\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > + ctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> > > +\n> > > + sensor_->setControls(&ctrls);\n> > > + }\n> > > +\n> > > state_ = State::IpaComplete;\n> > > + ctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> > > +\n> > > + sensor_->setControls(&ctrls);\n> > > + }\n> > > +\n> > > state_ = State::IpaComplete;\n> > > handleState();\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 351F8BF415\n\tfor ;\n\tThu, 6 Jan 2022 14:37:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7135460925;\n\tThu, 6 Jan 2022 15:37:39 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7161260219\n\tfor ;\n\tThu, 6 Jan 2022 15:37:37 +0100 (CET)","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 D72A911FE;\n\tThu, 6 Jan 2022 15:37:36 +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=\"G4AEeRU8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1641479857;\n\tbh=ZiYY5m210odTSyFL5aCNOYOCC8RNrstAB4LcHU1RW94=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=G4AEeRU8zwpps1SZ8ylAFU03hP4cLWnfmX4uozP2mmxYVYZUPvPQxIU8aQ0DJQDdC\n\tA4p+yJ3QPT/ScwyxrVsuWOYZBCpOmuiWL0R5wAQdREUt3mYQzfsyuLoHtq9ZmXsjew\n\tPHDo+hPED1dwqdKOc0ZVYLfagnkvsB/reu2sz7G0=","Date":"Thu, 6 Jan 2022 16:37:28 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20220105155539.5550-1-david.plowman@raspberrypi.com>\n\t<20220105155539.5550-4-david.plowman@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21965,"web_url":"https://patchwork.libcamera.org/comment/21965/","msgid":"","date":"2022-01-06T14:39:16","subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Thanks, Laurent!\n\nOn Thu, 6 Jan 2022 at 14:37, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> On Thu, Jan 06, 2022 at 02:26:13PM +0000, David Plowman wrote:\n> > On Wed, 5 Jan 2022 at 19:47, Laurent Pinchart wrote:\n> > > On Wed, Jan 05, 2022 at 03:55:39PM +0000, David Plowman wrote:\n> > > > If the sensor exposes the V4L2_CID_NOTIFY_GAINS control, assume it\n> > > > means the sensor wants to be told the latest colour gains.\n> > > >\n> > > > We store whether the control exists and if so its default value, to\n> > > > save us checking for it on every frame.\n> > > >\n> > > > Signed-off-by: David Plowman \n> > > > ---\n> > > > .../pipeline/raspberrypi/raspberrypi.cpp | 31 +++++++++++++++++++\n> > > > 1 file changed, 31 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index b5c687da..4adef952 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -264,6 +264,12 @@ public:\n> > > >\n> > > > unsigned int dropFrameCount_;\n> > > >\n> > > > + /*\n> > > > + * If set, this stores the value that represets a gain of one for\n> > > > + * the V4L2_CID_NOTIFY_GAINS control.\n> > > > + */\n> > > > + std::optional notifyGainsUnity_;\n> > >\n> > > A plain int32_t would be enough, as a zero value isn't valid. Up to\n> > > you.\n> >\n> > And I was trying so hard to avoid using the value 0 to have a\n> > \"special\" meaning... :)\n>\n> I thought std::optional was nice in that it avoided considering any\n> specific value as special, but then realized that 0 is invalid anyway.\n>\n> > I think at this point I'm inclined to leave it as it is, unless anyone\n> > else objects, or I might change it if I need to do another revision.\n>\n> Fine with me.\n>\n> > > > +\n> > > > private:\n> > > > void checkRequestCompleted();\n> > > > void fillRequestMetadata(const ControlList &bufferControls,\n> > > > @@ -1191,6 +1197,15 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)\n> > > > /* Initialize the camera properties. */\n> > > > data->properties_ = data->sensor_->properties();\n> > > >\n> > > > + /*\n> > > > + * The V4L2_CID_NOTIFY_GAINS control, if present, is used to inform the\n> > > > + * sensor of the colour gains. It is defined to be a linear gain where\n> > > > + * the default value represents a gain of exactly one.\n> > > > + */\n> > > > + auto it = data->sensor_->controls().find(V4L2_CID_NOTIFY_GAINS);\n> > > > + if (it != data->sensor_->controls().end())\n> > > > + data->notifyGainsUnity_ = it->second.def().get();\n> > > > +\n> > > > /*\n> > > > * Set a default value for the ScalerCropMaximum property to show\n> > > > * that we support its use, however, initialise it to zero because\n> > > > @@ -1495,6 +1510,22 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> > > > Request *request = requestQueue_.front();\n> > > > request->metadata().merge(controls);\n> > > >\n> > > > + /*\n> > > > + * Inform the sensor of the latest colour gains if it has the\n> > > > + * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).\n> > > > + */\n> > > > + if (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {\n> > > > + libcamera::Span colourGains = controls.get(libcamera::controls::ColourGains);\n> > > > + /* The control wants linear gains in the order B, Gb, Gr, R. */\n> > > > + ControlList ctrls(sensor_->controls());\n> > > > + std::array gains = { static_cast(colourGains[1] * *notifyGainsUnity_),\n> > > > + *notifyGainsUnity_, *notifyGainsUnity_,\n> > > > + static_cast(colourGains[0] * *notifyGainsUnity_) };\n> > >\n> > > Let's shorten the lines a bit:\n> > >\n> > > std::array gains{\n> > > static_cast(colourGains[1] * *notifyGainsUnity_),\n> > > *notifyGainsUnity_,\n> > > *notifyGainsUnity_,\n> > > static_cast(colourGains[0] * *notifyGainsUnity_)\n> > > };\n> >\n> > Is that fix-while-applying-able, do you think? Otherwise I'll do\n> > another version.\n>\n> I can handle this locally, no problem.\n>\n> > > Reviewed-by: Laurent Pinchart \n> > >\n> > > > + ctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> > > > +\n> > > > + sensor_->setControls(&ctrls);\n> > > > + }\n> > > > +\n> > > > state_ = State::IpaComplete;\n> > > > + ctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> > > > +\n> > > > + sensor_->setControls(&ctrls);\n> > > > + }\n> > > > +\n> > > > state_ = State::IpaComplete;\n> > > > handleState();\n> > > > }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 24ABBBF415\n\tfor ;\n\tThu, 6 Jan 2022 14:39:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6FD1F604F8;\n\tThu, 6 Jan 2022 15:39:29 +0100 (CET)","from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com\n\t[IPv6:2a00:1450:4864:20::32b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A4DF760219\n\tfor ;\n\tThu, 6 Jan 2022 15:39:27 +0100 (CET)","by mail-wm1-x32b.google.com with SMTP id e5so1933575wmq.1\n\tfor ;\n\tThu, 06 Jan 2022 06:39:27 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"hhFnCW6Q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=Mp/dzszA83ZKLsZVBnaPjlcoWTsHcnYc/ox8GD4bXqI=;\n\tb=hhFnCW6QXL71NGWlY1N6SPoljaad8xLHKyiShMdfOXcJexb31vlO1J88KbV8aiLiJz\n\tdetiUVkQEzAzHiRwNvhPw1sqz4uaRm/ZrkjPhzCcFIJat7XcYJ8JKmN0YwYZAMMQhmXn\n\tFV1QKAy7mxIhi+uvsGU1ZZ9+rt63qmmz/q3WGWE5hzuvhvuoqgJiZLfeeOpOq7bB7ak/\n\tRmhE80GmcqugiF03kJOsCTD0v8/ZZfqzB6ZwJMILUFUBbqnYbpdE8RPXP9mM1Z5aglZb\n\tjA0TkQWLCZlma/q3VRji9mtYaqeSDiFGZ7Y4i19K9u/06Aj3Isi/6QN2hKFD8ka5LVSr\n\tvXmw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Mp/dzszA83ZKLsZVBnaPjlcoWTsHcnYc/ox8GD4bXqI=;\n\tb=3KOPjF1vdzFrFNhbW5JbJdU/4VGXqNVOVI4F6qs60hhLssOXxwjt+QVrpuHyFuEiBJ\n\tRyHfBn6/9sZj5e06GPPJA8m0IOVFFMLU8g3wvgMjJxzm+wulTqj8ho8zGZ87fFoTBgC0\n\tEkAza76bSPuPlNtgFJmWFxd4+D299Y+V00AVlMUH/CKpaAtm59BKzdO6j2Y5UdrrAFE9\n\tThB9EWLwI+yBDWNs2HZEVvARvLUtYTK1/FUKZs3u5BMceSe8w+b+phWU5MPl8caaam2W\n\tJKZ+xjwqAjmuUzDfNn7gwHi6Clhsgu7MA9CA17bv5l9Z2KMVBR9ukulhvIuoMzHxv7KB\n\tbIyw==","X-Gm-Message-State":"AOAM530UpPQFZ5BVFasCKnxHuCdYRaEHhSLHvbUwgHg2cZSmAtl5VegQ\n\tXAMNFTG/VkB8Siq/GvHQyB/g3tr6xV7ahYOS6Gjz1g5xn/lBig==","X-Google-Smtp-Source":"ABdhPJz91xvzBqG6rbKhIOUvVjzBex3e4+zs+iCB3o2Fh+1T9WtpZvGKs/O7DP959zkGt09h5gP4QqQdKrjMjN8pwtQ=","X-Received":"by 2002:a05:600c:40d6:: with SMTP id\n\tm22mr7268821wmh.163.1641479967239; \n\tThu, 06 Jan 2022 06:39:27 -0800 (PST)","MIME-Version":"1.0","References":"<20220105155539.5550-1-david.plowman@raspberrypi.com>\n\t<20220105155539.5550-4-david.plowman@raspberrypi.com>\n\t\n\t\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Thu, 6 Jan 2022 14:39:16 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 3/3] libcamera: pipeline:\n\traspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]