[{"id":23573,"web_url":"https://patchwork.libcamera.org/comment/23573/","msgid":"<165607406117.1149771.550158354264835957@Monstersaurus>","date":"2022-06-24T12:34:21","subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Use\n\tstd::optional in DeviceStatus","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2022-06-24 08:35:28)\n> Switch the aperture, lens_position, and flash_intensity fields in the\n> DeviceStatus structure to use std::optional instead of using invalid default\n> values.\n\n\\o/\n\n\nReviewed-by: Kieran Bingham \n\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/controller/device_status.cpp | 14 ++++++++++----\n> src/ipa/raspberrypi/controller/device_status.h | 9 ++++-----\n> src/ipa/raspberrypi/controller/rpi/lux.cpp | 5 ++---\n> 3 files changed, 16 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/device_status.cpp b/src/ipa/raspberrypi/controller/device_status.cpp\n> index 05897fc15b50..a389c40dafed 100644\n> --- a/src/ipa/raspberrypi/controller/device_status.cpp\n> +++ b/src/ipa/raspberrypi/controller/device_status.cpp\n> @@ -12,10 +12,16 @@ std::ostream &operator<<(std::ostream &out, const DeviceStatus &d)\n> {\n> out << \"Exposure: \" << d.shutter_speed\n> << \" Frame length: \" << d.frame_length\n> - << \" Gain: \" << d.analogue_gain\n> - << \" Aperture: \" << d.aperture\n> - << \" Lens: \" << d.lens_position\n> - << \" Flash: \" << d.flash_intensity;\n> + << \" Gain: \" << d.analogue_gain;\n> +\n> + if (d.aperture)\n> + out << \" Aperture: \" << *d.aperture;\n> +\n> + if (d.lens_position)\n> + out << \" Lens: \" << *d.lens_position;\n> +\n> + if (d.flash_intensity)\n> + out << \" Flash: \" << *d.flash_intensity;\n> \n> if (d.sensor_temperature)\n> out << \" Temperature: \" << *d.sensor_temperature;\n> diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h\n> index eca3bf4b042e..b33f0d093ff3 100644\n> --- a/src/ipa/raspberrypi/controller/device_status.h\n> +++ b/src/ipa/raspberrypi/controller/device_status.h\n> @@ -19,8 +19,7 @@\n> struct DeviceStatus {\n> DeviceStatus()\n> : shutter_speed(std::chrono::seconds(0)), frame_length(0),\n> - analogue_gain(0.0), lens_position(0.0), aperture(0.0),\n> - flash_intensity(0.0)\n> + analogue_gain(0.0)\n> {\n> }\n> \n> @@ -32,11 +31,11 @@ struct DeviceStatus {\n> uint32_t frame_length;\n> double analogue_gain;\n> /* 1.0/distance-in-metres, or 0 if unknown */\n> - double lens_position;\n> + std::optional lens_position;\n> /* 1/f so that brightness quadruples when this doubles, or 0 if unknown */\n> - double aperture;\n> + std::optional aperture;\n> /* proportional to brightness with 0 = no flash, 1 = maximum flash */\n> - double flash_intensity;\n> + std::optional flash_intensity;\n> /* Sensor reported temperature value (in degrees) */\n> std::optional sensor_temperature;\n> };\n> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> index f77e9140ac10..643fb8fbaac6 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> @@ -63,9 +63,8 @@ void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> DeviceStatus device_status;\n> if (image_metadata->Get(\"device.status\", device_status) == 0) {\n> double current_gain = device_status.analogue_gain;\n> - double current_aperture = device_status.aperture;\n> - if (current_aperture == 0)\n> - current_aperture = current_aperture_;\n> + double current_aperture = device_status.aperture ? *device_status.aperture\n> + : current_aperture_;\n> uint64_t sum = 0;\n> uint32_t num = 0;\n> uint32_t *bin = stats->hist[0].g_hist;\n> -- \n> 2.25.1\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 A28EABE173\n\tfor ;\n\tFri, 24 Jun 2022 12:34:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 547266559A;\n\tFri, 24 Jun 2022 14:34:25 +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 D0AD7600EC\n\tfor ;\n\tFri, 24 Jun 2022 14:34:23 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8655E47C;\n\tFri, 24 Jun 2022 14:34:23 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656074065;\n\tbh=KvrmPNBbKHG1PQbE3Im6cIFXWxB84TGW9l7TPWoxinU=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=yewzSgmCTb0vo8zB+6LnFhj5sX5GMUqAsqYidxzCtJa4+vHZpsnvFFVZlOiTdS8wU\n\tBCpReADdOAi2B80jIqQron/xc4ToS8gABJnEOdjSVM9JUQLdVPVC1YlWd7YXWbU0A0\n\tck5McJhRSVy8VfHOAVk6E7dVkL6cYhXSElFYQBT78n9LH1VqVov4RfV+vgGvkQfgbs\n\toQPUAa+jxRRw/tG3Amq9g2p6/D2YVyLrejngi1Gi6T0cc74D4uZHvJmUSrlcI/XjOd\n\tI3KpV4kwFjG5xIo/XvCCXju7W3PzvZQV0CgmCJONltBaf2mtIPFdNWLZ/ecP4ixs6p\n\tcl9MHap7MB1MA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656074063;\n\tbh=KvrmPNBbKHG1PQbE3Im6cIFXWxB84TGW9l7TPWoxinU=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=iQttGlt3BPopHJlHHBO4JvfWucCMhci7OeY/tFbnaDniR4MfQOYPL4i2SNdLuJWvg\n\tfj6MYo3AQLAmHIAQp+B5qRUSfKQcOyRKOtyc3hwCYqFP9I1MOEf1/8+9RZRw09BD2V\n\tmfubAiOZPDggJqQKSJLa2ZBEUgN3pZvkpDg2FSr0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"iQttGlt3\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220624073528.26670-5-naush@raspberrypi.com>","References":"<20220624073528.26670-1-naush@raspberrypi.com>\n\t<20220624073528.26670-5-naush@raspberrypi.com>","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 24 Jun 2022 13:34:21 +0100","Message-ID":"<165607406117.1149771.550158354264835957@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Use\n\tstd::optional in DeviceStatus","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23637,"web_url":"https://patchwork.libcamera.org/comment/23637/","msgid":"","date":"2022-06-28T08:35:00","subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Use\n\tstd::optional in DeviceStatus","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for this, I think it's a good improvement!\n\nOn Fri, 24 Jun 2022 at 13:34, Kieran Bingham via libcamera-devel\n wrote:\n>\n> Quoting Naushir Patuck via libcamera-devel (2022-06-24 08:35:28)\n> > Switch the aperture, lens_position, and flash_intensity fields in the\n> > DeviceStatus structure to use std::optional instead of using invalid default\n> > values.\n>\n> \\o/\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n> >\n> > Signed-off-by: Naushir Patuck \n\nI suppose the only thing I wonder about is that once upon a time we\nhad a convention that .h files could be included in a C program, and\n.hpp was for C++ only headers. But obviously libcamera doesn't do that\nso I guess it is indeed time to consign that one to the dustbin of\nhistory. Is it a bit weird now that we have both? I don't think I'm\nbothered. So it's a \\o/ from me too.\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> > ---\n> > src/ipa/raspberrypi/controller/device_status.cpp | 14 ++++++++++----\n> > src/ipa/raspberrypi/controller/device_status.h | 9 ++++-----\n> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 5 ++---\n> > 3 files changed, 16 insertions(+), 12 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/device_status.cpp b/src/ipa/raspberrypi/controller/device_status.cpp\n> > index 05897fc15b50..a389c40dafed 100644\n> > --- a/src/ipa/raspberrypi/controller/device_status.cpp\n> > +++ b/src/ipa/raspberrypi/controller/device_status.cpp\n> > @@ -12,10 +12,16 @@ std::ostream &operator<<(std::ostream &out, const DeviceStatus &d)\n> > {\n> > out << \"Exposure: \" << d.shutter_speed\n> > << \" Frame length: \" << d.frame_length\n> > - << \" Gain: \" << d.analogue_gain\n> > - << \" Aperture: \" << d.aperture\n> > - << \" Lens: \" << d.lens_position\n> > - << \" Flash: \" << d.flash_intensity;\n> > + << \" Gain: \" << d.analogue_gain;\n> > +\n> > + if (d.aperture)\n> > + out << \" Aperture: \" << *d.aperture;\n> > +\n> > + if (d.lens_position)\n> > + out << \" Lens: \" << *d.lens_position;\n> > +\n> > + if (d.flash_intensity)\n> > + out << \" Flash: \" << *d.flash_intensity;\n> >\n> > if (d.sensor_temperature)\n> > out << \" Temperature: \" << *d.sensor_temperature;\n> > diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h\n> > index eca3bf4b042e..b33f0d093ff3 100644\n> > --- a/src/ipa/raspberrypi/controller/device_status.h\n> > +++ b/src/ipa/raspberrypi/controller/device_status.h\n> > @@ -19,8 +19,7 @@\n> > struct DeviceStatus {\n> > DeviceStatus()\n> > : shutter_speed(std::chrono::seconds(0)), frame_length(0),\n> > - analogue_gain(0.0), lens_position(0.0), aperture(0.0),\n> > - flash_intensity(0.0)\n> > + analogue_gain(0.0)\n> > {\n> > }\n> >\n> > @@ -32,11 +31,11 @@ struct DeviceStatus {\n> > uint32_t frame_length;\n> > double analogue_gain;\n> > /* 1.0/distance-in-metres, or 0 if unknown */\n> > - double lens_position;\n> > + std::optional lens_position;\n> > /* 1/f so that brightness quadruples when this doubles, or 0 if unknown */\n> > - double aperture;\n> > + std::optional aperture;\n> > /* proportional to brightness with 0 = no flash, 1 = maximum flash */\n> > - double flash_intensity;\n> > + std::optional flash_intensity;\n> > /* Sensor reported temperature value (in degrees) */\n> > std::optional sensor_temperature;\n> > };\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > index f77e9140ac10..643fb8fbaac6 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > @@ -63,9 +63,8 @@ void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> > DeviceStatus device_status;\n> > if (image_metadata->Get(\"device.status\", device_status) == 0) {\n> > double current_gain = device_status.analogue_gain;\n> > - double current_aperture = device_status.aperture;\n> > - if (current_aperture == 0)\n> > - current_aperture = current_aperture_;\n> > + double current_aperture = device_status.aperture ? *device_status.aperture\n> > + : current_aperture_;\n> > uint64_t sum = 0;\n> > uint32_t num = 0;\n> > uint32_t *bin = stats->hist[0].g_hist;\n> > --\n> > 2.25.1\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 27B49BD808\n\tfor ;\n\tTue, 28 Jun 2022 08:35:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D7DAA65635;\n\tTue, 28 Jun 2022 10:35:12 +0200 (CEST)","from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com\n\t[IPv6:2a00:1450:4864:20::62c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 377EF6559A\n\tfor ;\n\tTue, 28 Jun 2022 10:35:11 +0200 (CEST)","by mail-ej1-x62c.google.com with SMTP id mf9so24324580ejb.0\n\tfor ;\n\tTue, 28 Jun 2022 01:35:11 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656405312;\n\tbh=piL1wi33Rf9HtSny4oeej8CtpphO8JXj+hmLOy3ZQ9M=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=IITPL29X2mcpYR1lKjhHRCfuAr8cOeeuiipstM7Xnx1kqAJVOTJ0R3fWBJhz8So3a\n\taDt89H0ZiJLkqXs2slE4pAWTRpERdruh6WUBu0+YlrH92ht2sklqfhh041hG7ioaRt\n\twYr2oJ+V1cgsQ42CbYyIL9F9gbxlSkW7AOpdMlpuIHqWWlu7FbGfaLMgWZItzgKfDI\n\tVK1fGu5065MbYmh3mc1bLecQS/KrU8LeYV+skoD1DjcQvwdIOYsdtiL5AK8PkhQrzi\n\teIkNWeEy/uyqlGNwnS6XrNmdYAQFCbr9atlKKmldwjksFyadrVfu33vGlZbbZomycT\n\tU7t1h4V0uejJA==","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=R3BVEx1Mu3RSfGDZKoYKkh+zJ231DGuHKGKyszcFyp0=;\n\tb=K6nAuQbkjvjejhKzerzZy396Ak2bFe3uqotgCGZ4VeWYTNnJLGRS3mxSBJnlXvgwYp\n\tO6d4u/ap639GtgdFN/9+ZbjUN9jF/SijNQuG1L4VkUIZbOiKpZM3yOfnS2Vy3RgwhI3S\n\tOreP1rOuAxK19yHpACt6bg3pDSoD0C1xj2PXcgnoHbrNps446jYtG680R6m7c+GgRzgl\n\tBL9z3zw7a82BXhNUahvURBEyOn4Gob5bc1JY5W8SXcgSbyYr57WctUmIdVSVCBvXxscH\n\tV4hmNAEw2XLnxaKlJ5t5W65gZnciyqvHXuoebiO4wPrTOrNMS3yf5155jOBvb1kHyoeP\n\tb+DA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"K6nAuQbk\"; dkim-atps=neutral","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=R3BVEx1Mu3RSfGDZKoYKkh+zJ231DGuHKGKyszcFyp0=;\n\tb=dfSG0X73luCTe9j6m1US3OHdHhdZF+9PFrXWWgOJGjwLBTy2PGcL9f+7GUtphrAMk7\n\tk1+DB943h2+iSirJnf4P7fKCywn8yvpMSENNlwbYtJu6amfKXMRK/j+qjZqVEV/+IiUp\n\tt4wxcAuBKXUnH/Iyqo21ecTYp2SJLQjg61YUdxoc6ZRWDJEgb2CbviZE9iLUZTBSkKCW\n\tLTYGidEk5pMIEf7scZDFkDImTjTD507AsKaGUQNxvFnb58ORTFHAg5k/ML4fJkSF/qqz\n\tppGzOryffVdwrKNMDuuV8goyU42Q8AZ2HEgnyw1lKW/gkkZwvyT6uBhBYt7X02DJPju/\n\t6Pfw==","X-Gm-Message-State":"AJIora8FXP09VUrSQ9FJww8CiM7b+e/8/MQvxuXbEmXACobqblDwa4Hr\n\tR/knz22OR4CKertcXyl6BTAG+/ltecuofVrmjfmWQg==","X-Google-Smtp-Source":"AGRyM1tAHDmemBlBLPgVnUyG+GyPCcs8PWaRHDEG18dRxYDkx5ZbeWAh5nin3jb1tFbDxGsrRWfrHFehzlpqyEGRyVE=","X-Received":"by 2002:a17:906:a383:b0:6f5:132c:1a14 with SMTP id\n\tk3-20020a170906a38300b006f5132c1a14mr17377816ejz.21.1656405310871;\n\tTue, 28 Jun 2022 01:35:10 -0700 (PDT)","MIME-Version":"1.0","References":"<20220624073528.26670-1-naush@raspberrypi.com>\n\t<20220624073528.26670-5-naush@raspberrypi.com>\n\t<165607406117.1149771.550158354264835957@Monstersaurus>","In-Reply-To":"<165607406117.1149771.550158354264835957@Monstersaurus>","Date":"Tue, 28 Jun 2022 09:35:00 +0100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Use\n\tstd::optional in DeviceStatus","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","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23642,"web_url":"https://patchwork.libcamera.org/comment/23642/","msgid":"<165640896906.1958338.4971762925108640085@Monstersaurus>","date":"2022-06-28T09:36:09","subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Use\n\tstd::optional in DeviceStatus","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nQuoting David Plowman (2022-06-28 09:35:00)\n> Hi Naush\n> \n> Thanks for this, I think it's a good improvement!\n> \n> On Fri, 24 Jun 2022 at 13:34, Kieran Bingham via libcamera-devel\n> wrote:\n> >\n> > Quoting Naushir Patuck via libcamera-devel (2022-06-24 08:35:28)\n> > > Switch the aperture, lens_position, and flash_intensity fields in the\n> > > DeviceStatus structure to use std::optional instead of using invalid default\n> > > values.\n> >\n> > \\o/\n> >\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> > >\n> > > Signed-off-by: Naushir Patuck \n> \n> I suppose the only thing I wonder about is that once upon a time we\n> had a convention that .h files could be included in a C program, and\n> .hpp was for C++ only headers. But obviously libcamera doesn't do that\n> so I guess it is indeed time to consign that one to the dustbin of\n> history. Is it a bit weird now that we have both? I don't think I'm\n> bothered. So it's a \\o/ from me too.\n\nOh ... we have .hpp files ? - Oh - we do in controller indeed.\n\nWe should probably make these more consitent, but that can be a patch on\ntop anyway.\n\nAnother comment below too...\n\n> \n> Reviewed-by: David Plowman \n> \n> Thanks!\n> David\n> \n> > > ---\n> > > src/ipa/raspberrypi/controller/device_status.cpp | 14 ++++++++++----\n> > > src/ipa/raspberrypi/controller/device_status.h | 9 ++++-----\n> > > src/ipa/raspberrypi/controller/rpi/lux.cpp | 5 ++---\n> > > 3 files changed, 16 insertions(+), 12 deletions(-)\n> > >\n> > > diff --git a/src/ipa/raspberrypi/controller/device_status.cpp b/src/ipa/raspberrypi/controller/device_status.cpp\n> > > index 05897fc15b50..a389c40dafed 100644\n> > > --- a/src/ipa/raspberrypi/controller/device_status.cpp\n> > > +++ b/src/ipa/raspberrypi/controller/device_status.cpp\n> > > @@ -12,10 +12,16 @@ std::ostream &operator<<(std::ostream &out, const DeviceStatus &d)\n> > > {\n> > > out << \"Exposure: \" << d.shutter_speed\n> > > << \" Frame length: \" << d.frame_length\n> > > - << \" Gain: \" << d.analogue_gain\n> > > - << \" Aperture: \" << d.aperture\n> > > - << \" Lens: \" << d.lens_position\n> > > - << \" Flash: \" << d.flash_intensity;\n> > > + << \" Gain: \" << d.analogue_gain;\n> > > +\n> > > + if (d.aperture)\n> > > + out << \" Aperture: \" << *d.aperture;\n> > > +\n> > > + if (d.lens_position)\n> > > + out << \" Lens: \" << *d.lens_position;\n> > > +\n> > > + if (d.flash_intensity)\n> > > + out << \" Flash: \" << *d.flash_intensity;\n> > >\n> > > if (d.sensor_temperature)\n> > > out << \" Temperature: \" << *d.sensor_temperature;\n> > > diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h\n> > > index eca3bf4b042e..b33f0d093ff3 100644\n> > > --- a/src/ipa/raspberrypi/controller/device_status.h\n> > > +++ b/src/ipa/raspberrypi/controller/device_status.h\n> > > @@ -19,8 +19,7 @@\n> > > struct DeviceStatus {\n> > > DeviceStatus()\n> > > : shutter_speed(std::chrono::seconds(0)), frame_length(0),\n> > > - analogue_gain(0.0), lens_position(0.0), aperture(0.0),\n> > > - flash_intensity(0.0)\n> > > + analogue_gain(0.0)\n> > > {\n> > > }\n> > >\n> > > @@ -32,11 +31,11 @@ struct DeviceStatus {\n> > > uint32_t frame_length;\n> > > double analogue_gain;\n> > > /* 1.0/distance-in-metres, or 0 if unknown */\n> > > - double lens_position;\n> > > + std::optional lens_position;\n> > > /* 1/f so that brightness quadruples when this doubles, or 0 if unknown */\n> > > - double aperture;\n> > > + std::optional aperture;\n> > > /* proportional to brightness with 0 = no flash, 1 = maximum flash */\n> > > - double flash_intensity;\n> > > + std::optional flash_intensity;\n> > > /* Sensor reported temperature value (in degrees) */\n> > > std::optional sensor_temperature;\n> > > };\n> > > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > > index f77e9140ac10..643fb8fbaac6 100644\n> > > --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > > +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > > @@ -63,9 +63,8 @@ void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> > > DeviceStatus device_status;\n> > > if (image_metadata->Get(\"device.status\", device_status) == 0) {\n> > > double current_gain = device_status.analogue_gain;\n> > > - double current_aperture = device_status.aperture;\n> > > - if (current_aperture == 0)\n> > > - current_aperture = current_aperture_;\n> > > + double current_aperture = device_status.aperture ? *device_status.aperture\n> > > + : current_aperture_;\n\nAha, there's actually one small thing here.\n\nWith std::optional, we have value_or(), [0] so this could be:\n\t\tdouble current_aperture = device_status.aperture.value_or(current_aperture_);\n\nthough that comes in at 94 characters long...\nI don't mind either way though, it's only really syntactic sugar.\n\n[0] https://en.cppreference.com/w/cpp/utility/optional/value_or\n\nLet me know what you think/prefer and I'll get these through the matrix\nand merged soon now we have David's review.\n\n--\nKieran\n\n> > > uint64_t sum = 0;\n> > > uint32_t num = 0;\n> > > uint32_t *bin = stats->hist[0].g_hist;\n> > > --\n> > > 2.25.1\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 99571BD808\n\tfor ;\n\tTue, 28 Jun 2022 09:36:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF1F665637;\n\tTue, 28 Jun 2022 11:36:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AF1B36559A\n\tfor ;\n\tTue, 28 Jun 2022 11:36:11 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2B7343D7;\n\tTue, 28 Jun 2022 11:36:11 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656408972;\n\tbh=WkiU18U9WuqCrQG2xGqoOGJ0cvqZfpEegJUwgRttxBk=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=0BIn4gnzhCyVF4Aen2WZYjopUjGcKJA+Fog3nEUJPq0pvdHjRh5iKoFmML/LC6KKU\n\tD7DL8qNWKJaWXHGsPT6eGNi8rUtrwvt4Ly28VnlLlZmaK4yIT8x2x3ylNhdHqLjv+b\n\tyxMi9vEoXha/89LvKR6JtDvkHCQNvEysI2J3lI8mUjQYxQ3ol6yQ5hf5WCrbum5oFc\n\t91BVdjuLVIUkD7JKa8RmAxaAd61cFCZZtQuVWkE4uTVGYRj6ZpK9q9sC7o6jey0lzo\n\tTXZMZQZX7s/69GQ4DgWp3eLJKM/2dfU0eNJ0pX/5TogTP0n5k2W9CUjfk17BiOTKZp\n\tbWSIAjLmGyTvg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656408971;\n\tbh=WkiU18U9WuqCrQG2xGqoOGJ0cvqZfpEegJUwgRttxBk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=IbBDhTUfV7wzOzSRZ6yct4xbPpODjh3++8fvDgebPEO+fiSd9m5/sxaE/9CWoQBrE\n\tEGKyldODCliJfc1JPKxxfreoQCa9ggEZQpKgoHg+K5tw0o+BnjjxOi7+1O0PF1xB3d\n\tLgWI8egSmQIg/46Gjp0ec3L0PQ8UvEAKgtZ/SEAc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"IbBDhTUf\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20220624073528.26670-1-naush@raspberrypi.com>\n\t<20220624073528.26670-5-naush@raspberrypi.com>\n\t<165607406117.1149771.550158354264835957@Monstersaurus>\n\t","To":"David Plowman ","Date":"Tue, 28 Jun 2022 10:36:09 +0100","Message-ID":"<165640896906.1958338.4971762925108640085@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Use\n\tstd::optional in DeviceStatus","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23643,"web_url":"https://patchwork.libcamera.org/comment/23643/","msgid":"","date":"2022-06-28T09:42:14","subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Use\n\tstd::optional in DeviceStatus","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran and David,\n\nOn Tue, 28 Jun 2022 at 10:36, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Quoting David Plowman (2022-06-28 09:35:00)\n> > Hi Naush\n> >\n> > Thanks for this, I think it's a good improvement!\n> >\n> > On Fri, 24 Jun 2022 at 13:34, Kieran Bingham via libcamera-devel\n> > wrote:\n> > >\n> > > Quoting Naushir Patuck via libcamera-devel (2022-06-24 08:35:28)\n> > > > Switch the aperture, lens_position, and flash_intensity fields in the\n> > > > DeviceStatus structure to use std::optional instead of using invalid\n> default\n> > > > values.\n> > >\n> > > \\o/\n> > >\n> > >\n> > > Reviewed-by: Kieran Bingham \n> > >\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> >\n> > I suppose the only thing I wonder about is that once upon a time we\n> > had a convention that .h files could be included in a C program, and\n> > .hpp was for C++ only headers. But obviously libcamera doesn't do that\n> > so I guess it is indeed time to consign that one to the dustbin of\n> > history. Is it a bit weird now that we have both? I don't think I'm\n> > bothered. So it's a \\o/ from me too.\n>\n> Oh ... we have .hpp files ? - Oh - we do in controller indeed.\n>\n> We should probably make these more consitent, but that can be a patch on\n> top anyway.\n>\n\nYes we do. There is a long (long) outstanding task of libcamerifying our\ncontroller\nsource files to match the required style. This includes *.hpp -> *.h and\nconverting\nfrom snake_case to camelCase. As always, it's on my to-do list :-)\n\n\n>\n> Another comment below too...\n>\n> >\n> > Reviewed-by: David Plowman \n> >\n> > Thanks!\n> > David\n> >\n> > > > ---\n> > > > src/ipa/raspberrypi/controller/device_status.cpp | 14 ++++++++++----\n> > > > src/ipa/raspberrypi/controller/device_status.h | 9 ++++-----\n> > > > src/ipa/raspberrypi/controller/rpi/lux.cpp | 5 ++---\n> > > > 3 files changed, 16 insertions(+), 12 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/raspberrypi/controller/device_status.cpp\n> b/src/ipa/raspberrypi/controller/device_status.cpp\n> > > > index 05897fc15b50..a389c40dafed 100644\n> > > > --- a/src/ipa/raspberrypi/controller/device_status.cpp\n> > > > +++ b/src/ipa/raspberrypi/controller/device_status.cpp\n> > > > @@ -12,10 +12,16 @@ std::ostream &operator<<(std::ostream &out,\n> const DeviceStatus &d)\n> > > > {\n> > > > out << \"Exposure: \" << d.shutter_speed\n> > > > << \" Frame length: \" << d.frame_length\n> > > > - << \" Gain: \" << d.analogue_gain\n> > > > - << \" Aperture: \" << d.aperture\n> > > > - << \" Lens: \" << d.lens_position\n> > > > - << \" Flash: \" << d.flash_intensity;\n> > > > + << \" Gain: \" << d.analogue_gain;\n> > > > +\n> > > > + if (d.aperture)\n> > > > + out << \" Aperture: \" << *d.aperture;\n> > > > +\n> > > > + if (d.lens_position)\n> > > > + out << \" Lens: \" << *d.lens_position;\n> > > > +\n> > > > + if (d.flash_intensity)\n> > > > + out << \" Flash: \" << *d.flash_intensity;\n> > > >\n> > > > if (d.sensor_temperature)\n> > > > out << \" Temperature: \" << *d.sensor_temperature;\n> > > > diff --git a/src/ipa/raspberrypi/controller/device_status.h\n> b/src/ipa/raspberrypi/controller/device_status.h\n> > > > index eca3bf4b042e..b33f0d093ff3 100644\n> > > > --- a/src/ipa/raspberrypi/controller/device_status.h\n> > > > +++ b/src/ipa/raspberrypi/controller/device_status.h\n> > > > @@ -19,8 +19,7 @@\n> > > > struct DeviceStatus {\n> > > > DeviceStatus()\n> > > > : shutter_speed(std::chrono::seconds(0)),\n> frame_length(0),\n> > > > - analogue_gain(0.0), lens_position(0.0),\n> aperture(0.0),\n> > > > - flash_intensity(0.0)\n> > > > + analogue_gain(0.0)\n> > > > {\n> > > > }\n> > > >\n> > > > @@ -32,11 +31,11 @@ struct DeviceStatus {\n> > > > uint32_t frame_length;\n> > > > double analogue_gain;\n> > > > /* 1.0/distance-in-metres, or 0 if unknown */\n> > > > - double lens_position;\n> > > > + std::optional lens_position;\n> > > > /* 1/f so that brightness quadruples when this doubles, or 0\n> if unknown */\n> > > > - double aperture;\n> > > > + std::optional aperture;\n> > > > /* proportional to brightness with 0 = no flash, 1 = maximum\n> flash */\n> > > > - double flash_intensity;\n> > > > + std::optional flash_intensity;\n> > > > /* Sensor reported temperature value (in degrees) */\n> > > > std::optional sensor_temperature;\n> > > > };\n> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > > > index f77e9140ac10..643fb8fbaac6 100644\n> > > > --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > > > +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > > > @@ -63,9 +63,8 @@ void Lux::Process(StatisticsPtr &stats, Metadata\n> *image_metadata)\n> > > > DeviceStatus device_status;\n> > > > if (image_metadata->Get(\"device.status\", device_status) ==\n> 0) {\n> > > > double current_gain = device_status.analogue_gain;\n> > > > - double current_aperture = device_status.aperture;\n> > > > - if (current_aperture == 0)\n> > > > - current_aperture = current_aperture_;\n> > > > + double current_aperture = device_status.aperture ?\n> *device_status.aperture\n> > > > + :\n> current_aperture_;\n>\n> Aha, there's actually one small thing here.\n>\n> With std::optional, we have value_or(), [0] so this could be:\n> double current_aperture =\n> device_status.aperture.value_or(current_aperture_);\n>\n> though that comes in at 94 characters long...\n> I don't mind either way though, it's only really syntactic sugar.\n>\n> [0] https://en.cppreference.com/w/cpp/utility/optional/value_or\n>\n> Let me know what you think/prefer and I'll get these through the matrix\n> and merged soon now we have David's review.\n>\n\nHappy to change to value_or() here, it does seem more concise.\nWould you be able to change that while merging, or should I post an update?\n\nRegards,\nNaush\n\n\n\n>\n> --\n> Kieran\n>\n> > > > uint64_t sum = 0;\n> > > > uint32_t num = 0;\n> > > > uint32_t *bin = stats->hist[0].g_hist;\n> > > > --\n> > > > 2.25.1\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 33DE0BE173\n\tfor ;\n\tTue, 28 Jun 2022 09:42:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E08D65635;\n\tTue, 28 Jun 2022 11:42:31 +0200 (CEST)","from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com\n\t[IPv6:2a00:1450:4864:20::22c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 588896559A\n\tfor ;\n\tTue, 28 Jun 2022 11:42:29 +0200 (CEST)","by mail-lj1-x22c.google.com with SMTP id q9so14218665ljp.4\n\tfor ;\n\tTue, 28 Jun 2022 02:42:29 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656409351;\n\tbh=8yU4wvKif/gJ83IWXs8jZ4LazVb4Ax8NGNgASN4OZ18=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=aHSi6sagXgBeanMgnmx44GnwqfEMUMQ+fuPt9NE+usv9IDv8pkt1FHFk4cCZXGkYo\n\tWNE0AvuK9CzU5e0uTPbRVJK98NPplLB81e+Z0W4s9jabMbdhHiOzrsxzwgxJpFYIOY\n\tMeDR9n5Mt9dQamc/NoxNnQ6+1F4Vk27L3DvyD4UUsKGXlhNMlMF9KWE4k25whYqTfg\n\tX77Km8h30JKHPxughFzqdFKdR+3AAkYjrox1qIJGhYm0GV4FxdgK8N1Vc32EaS9udi\n\tnV8I+6A3AYafcXzpoOxv/hToFszMY1++WZA2FbbxaRR8TJnzCDJs87fdv2hWUVCYpj\n\tMLtkF5QJGnytw==","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=OJr0NHdZ5JEugvm+R5ZPc+yiCqPvlFjcoNMCpDSEkvc=;\n\tb=UGYmIbI0leegfj/n8yn4CAol/tsrB44CnKfib+FWt68qNqOTLO/om05qgo+CR/FHlk\n\tqPYdTukQnEkkDcOytwNx+bPl/aqXaqAg8nTdu67i4yD2fSFHirE0/eqq9FxT3UJC4yXG\n\tF95A6Uo+n8a166ha0rHU9cOwbPxBYm9feXv1L6wF0Oi56KioxmgMb90GikDHsrFGB9yX\n\tSLN7Dr2Zg7g5W53Ng9t0dVq0mv/DJ5G3DjWNolabaC79f4kNplx7if8IlC8rcUOCKoD2\n\toQcSeqC+d6Zqzam3Gyx8WGEOug5UNuedwnRbKaWeIgfJrbP/fLHUUDfMqmgvS/zh8Ghp\n\tWPgQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"UGYmIbI0\"; dkim-atps=neutral","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=OJr0NHdZ5JEugvm+R5ZPc+yiCqPvlFjcoNMCpDSEkvc=;\n\tb=4VRKtWcSvjM+AAybiIJDkl/NmT+WvhxOQHXQYImFd2g5HsoMB4xEvLBUNKMrhVyWas\n\txAa7byLlydkiW9+oLu5FC6fR8KXl1td4bmuR7MqrF0o1D5c/CVyI9+u+iPU/2+RwUxxE\n\tYQL+MpSqLSzFRY3MtmT+1g4sYqnqn9zs98nTa4y8DJi5HZAR5Gm38buLvCkRFyI+RvqP\n\tiuHU6LN4Wp3eAthtgtSoocX09WTgiCDDVlgLwp45+WEuAmToKb7dhwLgV2hbOPPZ7u08\n\t9/JCYbR1mfNzwUXA2ardmuNkS/eCEBSsrQGMzA97FoQJBeN++XAgOz8bAvmMUzolC4r2\n\tml5w==","X-Gm-Message-State":"AJIora/l5VeS2TzHZYhIRFbLRdP7aZIgorPWRAaMdJ9nai9IbOmKGVXw\n\tfmx2J54xqFwWcTIPtBo94rfq1g45aro+K4WoAeTQqfxdq4Y7RA==","X-Google-Smtp-Source":"AGRyM1urzdSM0MlnJGaZAJwsEsrAGjmX5mX5FopdTpZQjRjb7xYXDlYfQjwQz8uq0ehFjCG6XPS+CR+fELDWzVh0fKg=","X-Received":"by 2002:a2e:a786:0:b0:25b:c51a:2c0b with SMTP id\n\tc6-20020a2ea786000000b0025bc51a2c0bmr4498553ljf.215.1656409348543;\n\tTue, 28 Jun 2022 02:42:28 -0700 (PDT)","MIME-Version":"1.0","References":"<20220624073528.26670-1-naush@raspberrypi.com>\n\t<20220624073528.26670-5-naush@raspberrypi.com>\n\t<165607406117.1149771.550158354264835957@Monstersaurus>\n\t\n\t<165640896906.1958338.4971762925108640085@Monstersaurus>","In-Reply-To":"<165640896906.1958338.4971762925108640085@Monstersaurus>","Date":"Tue, 28 Jun 2022 10:42:14 +0100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"000000000000d2ad8705e27eda75\"","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Use\n\tstd::optional in DeviceStatus","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]