[{"id":23303,"web_url":"https://patchwork.libcamera.org/comment/23303/","msgid":"<20220603075953.xsmt6hy44n4v5gud@uno.localdomain>","date":"2022-06-03T07:59:53","subject":"Re: [libcamera-devel] [PATCH v5 4/4] libcamera: controls: Apply\n\texplicit fixed-sized Span type casts","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Christian\n\nOn Thu, Jun 02, 2022 at 12:18:02AM +0100, Christian Rauch via libcamera-devel wrote:\n> The change of types of some Controls from variable- to fixed-sized requires\n> explicit casting of FrameDurationLimits, ColourGains and SensorBlackLevels.\n>\n> Signed-off-by: Christian Rauch \n> ---\n> src/ipa/raspberrypi/raspberrypi.cpp | 19 ++++++++++---------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 4 ++--\n> src/qcam/dng_writer.cpp | 12 ++++++------\n> 3 files changed, 18 insertions(+), 17 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 00600a2e..54a16ac2 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -505,18 +505,19 @@ void IPARPi::reportMetadata()\n>\n> \tAwbStatus *awbStatus = rpiMetadata_.GetLocked(\"awb.status\");\n> \tif (awbStatus) {\n> -\t\tlibcameraMetadata_.set(controls::ColourGains, { static_cast(awbStatus->gain_r),\n> -\t\t\t\t\t\t\t\tstatic_cast(awbStatus->gain_b) });\n> +\t\tlibcameraMetadata_.set(controls::ColourGains,\n> +\t\t\t\t Span({ static_cast(awbStatus->gain_r),\n> +\t\t\t\t\t\t\t static_cast(awbStatus->gain_b) }));\n\n\nThat's a little more verbose, but I guess there are not many ways\naround it :(\n\n> \t\tlibcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperature_K);\n> \t}\n>\n> \tBlackLevelStatus *blackLevelStatus = rpiMetadata_.GetLocked(\"black_level.status\");\n> \tif (blackLevelStatus)\n> \t\tlibcameraMetadata_.set(controls::SensorBlackLevels,\n> -\t\t\t\t { static_cast(blackLevelStatus->black_level_r),\n> -\t\t\t\t\t static_cast(blackLevelStatus->black_level_g),\n> -\t\t\t\t\t static_cast(blackLevelStatus->black_level_g),\n> -\t\t\t\t\t static_cast(blackLevelStatus->black_level_b) });\n> +\t\t\t\t Span({ static_cast(blackLevelStatus->black_level_r),\n> +\t\t\t\t\t\t\t\tstatic_cast(blackLevelStatus->black_level_g),\n> +\t\t\t\t\t\t\t\tstatic_cast(blackLevelStatus->black_level_g),\n> +\t\t\t\t\t\t\t\tstatic_cast(blackLevelStatus->black_level_b) }));\n\nI think you can omit the static_casts, as uint16_t should be\nimplicitly convertible to int32_t ? (my compiler doesn't complain it\nseems).\n\n>\n> \tFocusStatus *focusStatus = rpiMetadata_.GetLocked(\"focus.status\");\n> \tif (focusStatus && focusStatus->num == 12) {\n> @@ -821,7 +822,7 @@ void IPARPi::queueRequest(const ControlList &controls)\n> \t\t\tif (gains[0] != 0.0f && gains[1] != 0.0f)\n> \t\t\t\t/* A gain of 0.0f will switch back to auto mode. */\n> \t\t\t\tlibcameraMetadata_.set(controls::ColourGains,\n> -\t\t\t\t\t\t { gains[0], gains[1] });\n> +\t\t\t\t\t\t Span({ gains[0], gains[1] }));\n> \t\t\tbreak;\n> \t\t}\n>\n> @@ -1105,8 +1106,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur\n>\n> \t/* Return the validated limits via metadata. */\n> \tlibcameraMetadata_.set(controls::FrameDurationLimits,\n> -\t\t\t { static_cast(minFrameDuration_.get()),\n> -\t\t\t\t static_cast(maxFrameDuration_.get()) });\n> +\t\t\t Span({ static_cast(minFrameDuration_.get()),\n> +\t\t\t\t\t\t\tstatic_cast(maxFrameDuration_.get()) }));\n>\n> \t/*\n> \t * Calculate the maximum exposure time possible for the AGC to use.\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 556af882..af7c8927 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1706,9 +1706,9 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\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 =\n> +\t\tlibcamera::Span colourGains =\n> \t\t\tcontrols.get(libcamera::controls::ColourGains).\\\n> -\t\t\t\tvalue_or(libcamera::Span({ 0, 0 }));\n> +\t\t\t\tvalue_or(libcamera::Span({ 0, 0 }));\n\nYou can omit the libcamera:: namespace specified in libcamera::Span<>.\n\nActually it shouldn't have been there from the beginning, or am I\nmissing something ?\n\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{\n> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> index e119ca52..030432e3 100644\n> --- a/src/qcam/dng_writer.cpp\n> +++ b/src/qcam/dng_writer.cpp\n> @@ -438,8 +438,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tconst double eps = 1e-2;\n>\n> \tif (metadata.contains(controls::ColourGains)) {\n> -\t\tSpan const &colourGains =\n> -\t\t\tmetadata.get(controls::ColourGains).value_or(libcamera::Span({ 0, 0 }));\n> +\t\tSpan const &colourGains =\n> +\t\t\tmetadata.get(controls::ColourGains).value_or(libcamera::Span({ 0, 0 }));\n\nAgain I think libcamera:: can be omitted, even if the file seems to be\nusing a mixture of libcamera::Span<> and just Span<>\n\nThanks\n j\n\n> \t\tif (colourGains[0] > eps && colourGains[1] > eps) {\n> \t\t\twbGain = Matrix3d::diag(colourGains[0], 1, colourGains[1]);\n> \t\t\tneutral[0] = 1.0 / colourGains[0]; /* red */\n> @@ -447,8 +447,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \t\t}\n> \t}\n> \tif (metadata.contains(controls::ColourCorrectionMatrix)) {\n> -\t\tSpan const &coeffs =\n> -\t\t\tmetadata.get(controls::ColourCorrectionMatrix).value_or(Span({ 0, 0, 0, 0, 0, 0, 0, 0, 0 }));\n> +\t\tSpan const &coeffs =\n> +\t\t\tmetadata.get(controls::ColourCorrectionMatrix).value_or(Span({ 0, 0, 0, 0, 0, 0, 0, 0, 0 }));\n> \t\tMatrix3d ccmSupplied(coeffs);\n> \t\tif (ccmSupplied.determinant() > eps)\n> \t\t\tccm = ccmSupplied;\n> @@ -517,8 +517,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tuint32_t whiteLevel = (1 << info->bitsPerSample) - 1;\n>\n> \tif (metadata.contains(controls::SensorBlackLevels)) {\n> -\t\tSpan levels =\n> -\t\t\tmetadata.get(controls::SensorBlackLevels).value_or(Span({ 0, 0, 0, 0 }));\n> +\t\tSpan levels =\n> +\t\t\tmetadata.get(controls::SensorBlackLevels).value_or(Span({ 0, 0, 0, 0 }));\n>\n> \t\t/*\n> \t\t * The black levels control is specified in R, Gr, Gb, B order.\n> --\n> 2.34.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 79B2EBD160\n\tfor ;\n\tFri, 3 Jun 2022 07:59:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B860265636;\n\tFri, 3 Jun 2022 09:59:57 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DAA24633A6\n\tfor ;\n\tFri, 3 Jun 2022 09:59:55 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id F33561BF20C;\n\tFri, 3 Jun 2022 07:59:54 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654243197;\n\tbh=0PtCdv0FodwGDw3xEbXyvH6u7FprfonWqp0a57vy6OM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Kq5szWSzrNsfgjaVScp6b2t8mZM6Ik0/IgO5zbO7vyHO7bKVdSbvJ1Af8ffXkpAgu\n\tlXCLNfvTQJMWtSWfi+RmcGsT+Da7WOfUd13Ency+bswbwRhqj62Bcw4PnAZSa81vcB\n\tT2oQCtkflf2fZv40G6jAQtbrMeuccLmsKNngZiL2rfJjcBhpIEKT2PL/xR634ZFkHa\n\tjF0pdg1TG6go0m4RJgA3Fn57YhcvvNeyNK1EVBjiuFmEpatEOIecTzyQ/CpLUpJKic\n\tvI9DDe6FDb0TP3W/hLOj7F3BFkOr012uiCb9Dtu5fPad/i8ChzKfYuPkQlo4qO9pb1\n\tcq5I4/Su1B1YQ==","Date":"Fri, 3 Jun 2022 09:59:53 +0200","To":"Christian Rauch ","Message-ID":"<20220603075953.xsmt6hy44n4v5gud@uno.localdomain>","References":"<20220601231802.16735-1-Rauch.Christian@gmx.de>\n\t<20220601231802.16735-5-Rauch.Christian@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220601231802.16735-5-Rauch.Christian@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH v5 4/4] libcamera: controls: Apply\n\texplicit fixed-sized Span type casts","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]