[{"id":15946,"web_url":"https://patchwork.libcamera.org/comment/15946/","msgid":"","date":"2021-03-26T13:43:10","subject":"Re: [libcamera-devel] [PATCH v2 1/1] ipa: raspberrypi: Use\n\tCamHelpers to generalise sensor metadata parsing","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\n\nOn Fri, 26 Mar 2021 at 12:43, David Plowman \nwrote:\n\n> CamHelpers get virtual Prepare() and Process() methods, running just\n> before and just after the ISP, just like Raspberry Pi Algorithms.\n>\n> The Prepare() method is able to parse the register dumps in embedded\n> data buffers, and can be specialised to perform custom processing when\n> necessary.\n>\n> The IPA itself only needs to call the new Prepare() and Process()\n> methods. It must fill in the DeviceStatus from the controls first, in\n> case the Prepare() method does not supply its own values.\n>\n> Signed-off-by: David Plowman \n> ---\n> src/ipa/raspberrypi/cam_helper.cpp | 51 ++++++++++++++++++\n> src/ipa/raspberrypi/cam_helper.hpp | 11 +++-\n> src/ipa/raspberrypi/raspberrypi.cpp | 80 ++++++++++-------------------\n> 3 files changed, 87 insertions(+), 55 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp\n> b/src/ipa/raspberrypi/cam_helper.cpp\n> index 0ae0baa0..5af73c9c 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -17,6 +17,11 @@\n> #include \"md_parser.hpp\"\n>\n> using namespace RPiController;\n> +using namespace libcamera;\n> +\n> +namespace libcamera {\n> +LOG_DECLARE_CATEGORY(IPARPI)\n> +}\n>\n> static std::map cam_helpers;\n>\n> @@ -45,6 +50,17 @@ CamHelper::~CamHelper()\n> delete parser_;\n> }\n>\n> +void CamHelper::Prepare(const Span &buffer,\n> + Metadata &metadata)\n> +{\n> + parseEmbeddedData(buffer, metadata);\n> +}\n> +\n> +void CamHelper::Process([[maybe_unused]] StatisticsPtr &stats,\n> + [[maybe_unused]] Metadata &metadata)\n> +{\n> +}\n> +\n> uint32_t CamHelper::ExposureLines(double exposure_us) const\n> {\n> assert(initialized_);\n> @@ -139,6 +155,41 @@ unsigned int CamHelper::MistrustFramesModeSwitch()\n> const\n> return 0;\n> }\n>\n> +void CamHelper::parseEmbeddedData(const Span &buffer,\n> + Metadata &metadata)\n> +{\n> + if (buffer.size()) {\n> + bool success = false;\n> + uint32_t exposureLines, gainCode;\n> +\n> + parser_->SetBufferSize(buffer.size());\n> + success = parser_->Parse(buffer.data()) ==\n> MdParser::Status::OK &&\n> + parser_->GetExposureLines(exposureLines) ==\n> MdParser::Status::OK &&\n> + parser_->GetGainCode(gainCode) ==\n> MdParser::Status::OK;\n> +\n> + /*\n> + * Overwrite the exposure/gain values in the DeviceStatus,\n> as\n> + * we know better. Fetch it first in case any other fields\n> were\n> + * set meaningfully.\n> + */\n> + struct DeviceStatus deviceStatus;\n> +\n> + if (success &&\n> + metadata.Get(\"device.status\", deviceStatus) == 0) {\n>\n\nI'm trying to work out if the second clause is needed? Shouldn't we write\ndevice.status\nunconditionally, whether it is present in the metadata or not? I suppose\nit does not matter,\nit is guaranteed to be there from the IPA.\n\nReviewed-by: Naushir Patuck \n\n\n\n> + deviceStatus.shutter_speed =\n> Exposure(exposureLines);\n> + deviceStatus.analogue_gain = Gain(gainCode);\n> +\n> + LOG(IPARPI, Debug) << \"Metadata updated - Exposure\n> : \"\n> + << deviceStatus.shutter_speed\n> + << \" Gain : \"\n> + << deviceStatus.analogue_gain;\n> +\n> + metadata.Set(\"device.status\", deviceStatus);\n> + } else\n> + LOG(IPARPI, Error) << \"Embedded buffer parsing\n> failed\";\n> + }\n> +}\n> +\n> RegisterCamHelper::RegisterCamHelper(char const *cam_name,\n> CamHelperCreateFunc create_func)\n> {\n> diff --git a/src/ipa/raspberrypi/cam_helper.hpp\n> b/src/ipa/raspberrypi/cam_helper.hpp\n> index 1b2d6eec..930ea39a 100644\n> --- a/src/ipa/raspberrypi/cam_helper.hpp\n> +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> @@ -8,7 +8,11 @@\n>\n> #include \n>\n> +#include \n> +\n> #include \"camera_mode.h\"\n> +#include \"controller/controller.hpp\"\n> +#include \"controller/metadata.hpp\"\n> #include \"md_parser.hpp\"\n>\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> @@ -65,7 +69,9 @@ public:\n> CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);\n> virtual ~CamHelper();\n> void SetCameraMode(const CameraMode &mode);\n> - MdParser &Parser() const { return *parser_; }\n> + virtual void Prepare(const libcamera::Span &buffer,\n> + Metadata &metadata);\n> + virtual void Process(StatisticsPtr &stats, Metadata &metadata);\n> uint32_t ExposureLines(double exposure_us) const;\n> double Exposure(uint32_t exposure_lines) const; // in us\n> virtual uint32_t GetVBlanking(double &exposure_us, double\n> minFrameDuration,\n> @@ -81,6 +87,9 @@ public:\n> virtual unsigned int MistrustFramesModeSwitch() const;\n>\n> protected:\n> + void parseEmbeddedData(const libcamera::Span &buffer,\n> + Metadata &metadata);\n> +\n> MdParser *parser_;\n> CameraMode mode_;\n>\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 1c928b72..32ecbdcd 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -101,9 +101,7 @@ private:\n> void returnEmbeddedBuffer(unsigned int bufferId);\n> void prepareISP(const ipa::RPi::ISPConfig &data);\n> void reportMetadata();\n> - bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus\n> &deviceStatus);\n> - void fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n> - struct DeviceStatus &deviceStatus);\n> + void fillDeviceStatus(const ControlList &sensorControls);\n> void processStats(unsigned int bufferId);\n> void applyFrameDurations(double minFrameDuration, double\n> maxFrameDuration);\n> void applyAGC(const struct AgcStatus *agcStatus, ControlList\n> &ctrls);\n> @@ -895,35 +893,34 @@ void IPARPi::returnEmbeddedBuffer(unsigned int\n> bufferId)\n>\n> void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)\n> {\n> - struct DeviceStatus deviceStatus = {};\n> - bool success = false;\n> + Span embeddedBuffer;\n> +\n> + rpiMetadata_.Clear();\n> +\n> + fillDeviceStatus(data.controls);\n>\n> if (data.embeddedBufferPresent) {\n> /*\n> * Pipeline handler has supplied us with an embedded data\n> buffer,\n> - * so parse it and extract the exposure and gain.\n> + * we must pass it to the CamHelper for parsing.\n> */\n> - success = parseEmbeddedData(data.embeddedBufferId,\n> deviceStatus);\n> -\n> - /* Done with embedded data now, return to pipeline handler\n> asap. */\n> - returnEmbeddedBuffer(data.embeddedBufferId);\n> + auto it = buffers_.find(data.embeddedBufferId);\n> + ASSERT(it != buffers_.end());\n> + embeddedBuffer = it->second.maps()[0];\n> }\n>\n> - if (!success) {\n> - /*\n> - * Pipeline handler has not supplied an embedded data\n> buffer,\n> - * or embedded data buffer parsing has failed for some\n> reason,\n> - * so pull the exposure and gain values from the control\n> list.\n> - */\n> - int32_t exposureLines =\n> data.controls.get(V4L2_CID_EXPOSURE).get();\n> - int32_t gainCode =\n> data.controls.get(V4L2_CID_ANALOGUE_GAIN).get();\n> - fillDeviceStatus(exposureLines, gainCode, deviceStatus);\n> - }\n> + /*\n> + * This may overwrite the DeviceStatus using values from the sensor\n> + * metadata, and may also do additional custom processing.\n> + */\n> + helper_->Prepare(embeddedBuffer, rpiMetadata_);\n> +\n> + /* Done with embedded data now, return to pipeline handler asap. */\n> + if (data.embeddedBufferPresent)\n> + returnEmbeddedBuffer(data.embeddedBufferId);\n>\n> ControlList ctrls(ispCtrls_);\n>\n> - rpiMetadata_.Clear();\n> - rpiMetadata_.Set(\"device.status\", deviceStatus);\n> controller_.Prepare(&rpiMetadata_);\n>\n> /* Lock the metadata buffer to avoid constant locks/unlocks. */\n> @@ -973,41 +970,13 @@ void IPARPi::prepareISP(const ipa::RPi::ISPConfig\n> &data)\n> setIspControls.emit(ctrls);\n> }\n>\n> -bool IPARPi::parseEmbeddedData(unsigned int bufferId, struct DeviceStatus\n> &deviceStatus)\n> +void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n> {\n> - auto it = buffers_.find(bufferId);\n> - if (it == buffers_.end()) {\n> - LOG(IPARPI, Error) << \"Could not find embedded buffer!\";\n> - return false;\n> - }\n> -\n> - Span mem = it->second.maps()[0];\n> - helper_->Parser().SetBufferSize(mem.size());\n> - RPiController::MdParser::Status status =\n> helper_->Parser().Parse(mem.data());\n> - if (status != RPiController::MdParser::Status::OK) {\n> - LOG(IPARPI, Error) << \"Embedded Buffer parsing failed,\n> error \" << status;\n> - return false;\n> - } else {\n> - uint32_t exposureLines, gainCode;\n> - if (helper_->Parser().GetExposureLines(exposureLines) !=\n> RPiController::MdParser::Status::OK) {\n> - LOG(IPARPI, Error) << \"Exposure time failed\";\n> - return false;\n> - }\n> -\n> - if (helper_->Parser().GetGainCode(gainCode) !=\n> RPiController::MdParser::Status::OK) {\n> - LOG(IPARPI, Error) << \"Gain failed\";\n> - return false;\n> - }\n> -\n> - fillDeviceStatus(exposureLines, gainCode, deviceStatus);\n> - }\n> + struct DeviceStatus deviceStatus = {};\n>\n> - return true;\n> -}\n> + int32_t exposureLines =\n> sensorControls.get(V4L2_CID_EXPOSURE).get();\n> + int32_t gainCode =\n> sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get();\n>\n> -void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n> - struct DeviceStatus &deviceStatus)\n> -{\n> deviceStatus.shutter_speed = helper_->Exposure(exposureLines);\n> deviceStatus.analogue_gain = helper_->Gain(gainCode);\n>\n> @@ -1015,6 +984,8 @@ void IPARPi::fillDeviceStatus(uint32_t exposureLines,\n> uint32_t gainCode,\n> << deviceStatus.shutter_speed\n> << \" Gain : \"\n> << deviceStatus.analogue_gain;\n> +\n> + rpiMetadata_.Set(\"device.status\", deviceStatus);\n> }\n>\n> void IPARPi::processStats(unsigned int bufferId)\n> @@ -1028,6 +999,7 @@ void IPARPi::processStats(unsigned int bufferId)\n> Span mem = it->second.maps()[0];\n> bcm2835_isp_stats *stats = reinterpret_cast *>(mem.data());\n> RPiController::StatisticsPtr statistics =\n> std::make_shared(*stats);\n> + helper_->Process(statistics, rpiMetadata_);\n> controller_.Process(statistics, &rpiMetadata_);\n>\n> struct AgcStatus agcStatus;\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 2A7C5C32EA\n\tfor ;\n\tFri, 26 Mar 2021 13:43:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6472D68D6D;\n\tFri, 26 Mar 2021 14:43:30 +0100 (CET)","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 24F9C602D7\n\tfor ;\n\tFri, 26 Mar 2021 14:43:28 +0100 (CET)","by mail-lj1-x22c.google.com with SMTP id u9so7388152ljd.11\n\tfor ;\n\tFri, 26 Mar 2021 06:43:28 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"ZdD9x/aW\"; 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=mf/kXbmtfWftrzH6CGLhrkhYhOcl6DL7I0P3Lol65uA=;\n\tb=ZdD9x/aWkeHf0JAviDf7uLaa/zKZIzRh5vQnEd0WaSH+afr/FtAr5pG4QWgMmjsCFt\n\t/k0f++MXSqZWB7Ww5PitIwPV9RbAYRYC067xpCyM3AO7+IJooCHgdvmOlRG9+wsbTGh6\n\tWZnLe5OI9u3mahe30BNzkR1fmPgcyeBlcSufaFJ0HlkEM9djfZFTS9n99q194qI6/HmT\n\tKnQu/x0Sb/uxIc9JZ5XQQVKkcIN8AFIaa0BxN34JFnPBIIc/HM5RmZVYa7/1MXkM17co\n\tWcyg86CA9Hat2SGoaMGTP5wGYpHkBuh3rxB/BQVA/h/Rbs78czAN2R1auoYZEPRU+EzV\n\tDehA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=mf/kXbmtfWftrzH6CGLhrkhYhOcl6DL7I0P3Lol65uA=;\n\tb=Oz13xxj6hhd31kkvYRKkWMFMxGU7SazfHWvcevJ+mI0JSUvPBks04laMoVT54xkf4A\n\tsAb2dutEZs+IsZrxJXQFUqHhuXm6kC0dVOqTRSU5RK7th04f9PJ4MM976z4mDvFPfNxj\n\tIAV3a5ByEdipRIJe+TIS0KJWMv4JULRqQBxGUfFdiMH9O6ZGbYoxIxjjefddxoZGELx5\n\t+HEp4CLW8auxMUDAbbHOJtHW1+IhVPt4axBQXC+rTv7lQnwelTqZ9w2GYkdVGB4UlBuH\n\tOoqCMVSzuhQh97sBvZR/JCoPTBnbDcsfNr6tvYEyhWxO6gTRWRQapiQgyG3rfBiG/WCb\n\tCobQ==","X-Gm-Message-State":"AOAM531rcsXG2L7WIqhtOm1WM2UhnURZD6o+xVMOJMaC/afjXRHtRN8G\n\twg4ZNmtaFGqa4xT1KqSlIgWproo6oh1cCEutMaLzRg==","X-Google-Smtp-Source":"ABdhPJw8pFpwVAwnp36BKCkSv4eIDSwTxM3tFWdVCQ8UkY5Un5q5V6vzouvNZB6nHIa4QTpgt2PBYal39FU++pWOepM=","X-Received":"by 2002:a2e:864d:: with SMTP id i13mr8863949ljj.48.1616766207307;\n\tFri, 26 Mar 2021 06:43:27 -0700 (PDT)","MIME-Version":"1.0","References":"<20210326124321.28617-1-david.plowman@raspberrypi.com>\n\t<20210326124321.28617-2-david.plowman@raspberrypi.com>","In-Reply-To":"<20210326124321.28617-2-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Fri, 26 Mar 2021 13:43:10 +0000","Message-ID":"","To":"David Plowman ","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] ipa: raspberrypi: Use\n\tCamHelpers to generalise sensor metadata parsing","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============0903852847953432992==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16435,"web_url":"https://patchwork.libcamera.org/comment/16435/","msgid":"","date":"2021-04-21T08:59:32","subject":"Re: [libcamera-devel] [PATCH v2 1/1] ipa: raspberrypi: Use\n\tCamHelpers to generalise sensor metadata parsing","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi everyone\n\nI was wondering if I could apply a little nudge to this one too as\nthere are some other patches in the system relying on it. Does this\nneed more thought, do we think?\n\nThanks!\nDavid\n\nOn Fri, 26 Mar 2021 at 13:43, Naushir Patuck wrote:\n>\n> Hi David,\n>\n>\n> On Fri, 26 Mar 2021 at 12:43, David Plowman wrote:\n>>\n>> CamHelpers get virtual Prepare() and Process() methods, running just\n>> before and just after the ISP, just like Raspberry Pi Algorithms.\n>>\n>> The Prepare() method is able to parse the register dumps in embedded\n>> data buffers, and can be specialised to perform custom processing when\n>> necessary.\n>>\n>> The IPA itself only needs to call the new Prepare() and Process()\n>> methods. It must fill in the DeviceStatus from the controls first, in\n>> case the Prepare() method does not supply its own values.\n>>\n>> Signed-off-by: David Plowman \n>> ---\n>> src/ipa/raspberrypi/cam_helper.cpp | 51 ++++++++++++++++++\n>> src/ipa/raspberrypi/cam_helper.hpp | 11 +++-\n>> src/ipa/raspberrypi/raspberrypi.cpp | 80 ++++++++++-------------------\n>> 3 files changed, 87 insertions(+), 55 deletions(-)\n>>\n>> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\n>> index 0ae0baa0..5af73c9c 100644\n>> --- a/src/ipa/raspberrypi/cam_helper.cpp\n>> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n>> @@ -17,6 +17,11 @@\n>> #include \"md_parser.hpp\"\n>>\n>> using namespace RPiController;\n>> +using namespace libcamera;\n>> +\n>> +namespace libcamera {\n>> +LOG_DECLARE_CATEGORY(IPARPI)\n>> +}\n>>\n>> static std::map cam_helpers;\n>>\n>> @@ -45,6 +50,17 @@ CamHelper::~CamHelper()\n>> delete parser_;\n>> }\n>>\n>> +void CamHelper::Prepare(const Span &buffer,\n>> + Metadata &metadata)\n>> +{\n>> + parseEmbeddedData(buffer, metadata);\n>> +}\n>> +\n>> +void CamHelper::Process([[maybe_unused]] StatisticsPtr &stats,\n>> + [[maybe_unused]] Metadata &metadata)\n>> +{\n>> +}\n>> +\n>> uint32_t CamHelper::ExposureLines(double exposure_us) const\n>> {\n>> assert(initialized_);\n>> @@ -139,6 +155,41 @@ unsigned int CamHelper::MistrustFramesModeSwitch() const\n>> return 0;\n>> }\n>>\n>> +void CamHelper::parseEmbeddedData(const Span &buffer,\n>> + Metadata &metadata)\n>> +{\n>> + if (buffer.size()) {\n>> + bool success = false;\n>> + uint32_t exposureLines, gainCode;\n>> +\n>> + parser_->SetBufferSize(buffer.size());\n>> + success = parser_->Parse(buffer.data()) == MdParser::Status::OK &&\n>> + parser_->GetExposureLines(exposureLines) == MdParser::Status::OK &&\n>> + parser_->GetGainCode(gainCode) == MdParser::Status::OK;\n>> +\n>> + /*\n>> + * Overwrite the exposure/gain values in the DeviceStatus, as\n>> + * we know better. Fetch it first in case any other fields were\n>> + * set meaningfully.\n>> + */\n>> + struct DeviceStatus deviceStatus;\n>> +\n>> + if (success &&\n>> + metadata.Get(\"device.status\", deviceStatus) == 0) {\n>\n>\n> I'm trying to work out if the second clause is needed? Shouldn't we write device.status\n> unconditionally, whether it is present in the metadata or not? I suppose it does not matter,\n> it is guaranteed to be there from the IPA.\n>\n> Reviewed-by: Naushir Patuck \n>\n>\n>>\n>> + deviceStatus.shutter_speed = Exposure(exposureLines);\n>> + deviceStatus.analogue_gain = Gain(gainCode);\n>> +\n>> + LOG(IPARPI, Debug) << \"Metadata updated - Exposure : \"\n>> + << deviceStatus.shutter_speed\n>> + << \" Gain : \"\n>> + << deviceStatus.analogue_gain;\n>> +\n>> + metadata.Set(\"device.status\", deviceStatus);\n>> + } else\n>> + LOG(IPARPI, Error) << \"Embedded buffer parsing failed\";\n>> + }\n>> +}\n>> +\n>> RegisterCamHelper::RegisterCamHelper(char const *cam_name,\n>> CamHelperCreateFunc create_func)\n>> {\n>> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp\n>> index 1b2d6eec..930ea39a 100644\n>> --- a/src/ipa/raspberrypi/cam_helper.hpp\n>> +++ b/src/ipa/raspberrypi/cam_helper.hpp\n>> @@ -8,7 +8,11 @@\n>>\n>> #include \n>>\n>> +#include \n>> +\n>> #include \"camera_mode.h\"\n>> +#include \"controller/controller.hpp\"\n>> +#include \"controller/metadata.hpp\"\n>> #include \"md_parser.hpp\"\n>>\n>> #include \"libcamera/internal/v4l2_videodevice.h\"\n>> @@ -65,7 +69,9 @@ public:\n>> CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);\n>> virtual ~CamHelper();\n>> void SetCameraMode(const CameraMode &mode);\n>> - MdParser &Parser() const { return *parser_; }\n>> + virtual void Prepare(const libcamera::Span &buffer,\n>> + Metadata &metadata);\n>> + virtual void Process(StatisticsPtr &stats, Metadata &metadata);\n>> uint32_t ExposureLines(double exposure_us) const;\n>> double Exposure(uint32_t exposure_lines) const; // in us\n>> virtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration,\n>> @@ -81,6 +87,9 @@ public:\n>> virtual unsigned int MistrustFramesModeSwitch() const;\n>>\n>> protected:\n>> + void parseEmbeddedData(const libcamera::Span &buffer,\n>> + Metadata &metadata);\n>> +\n>> MdParser *parser_;\n>> CameraMode mode_;\n>>\n>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n>> index 1c928b72..32ecbdcd 100644\n>> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>> @@ -101,9 +101,7 @@ private:\n>> void returnEmbeddedBuffer(unsigned int bufferId);\n>> void prepareISP(const ipa::RPi::ISPConfig &data);\n>> void reportMetadata();\n>> - bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus);\n>> - void fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n>> - struct DeviceStatus &deviceStatus);\n>> + void fillDeviceStatus(const ControlList &sensorControls);\n>> void processStats(unsigned int bufferId);\n>> void applyFrameDurations(double minFrameDuration, double maxFrameDuration);\n>> void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n>> @@ -895,35 +893,34 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)\n>>\n>> void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)\n>> {\n>> - struct DeviceStatus deviceStatus = {};\n>> - bool success = false;\n>> + Span embeddedBuffer;\n>> +\n>> + rpiMetadata_.Clear();\n>> +\n>> + fillDeviceStatus(data.controls);\n>>\n>> if (data.embeddedBufferPresent) {\n>> /*\n>> * Pipeline handler has supplied us with an embedded data buffer,\n>> - * so parse it and extract the exposure and gain.\n>> + * we must pass it to the CamHelper for parsing.\n>> */\n>> - success = parseEmbeddedData(data.embeddedBufferId, deviceStatus);\n>> -\n>> - /* Done with embedded data now, return to pipeline handler asap. */\n>> - returnEmbeddedBuffer(data.embeddedBufferId);\n>> + auto it = buffers_.find(data.embeddedBufferId);\n>> + ASSERT(it != buffers_.end());\n>> + embeddedBuffer = it->second.maps()[0];\n>> }\n>>\n>> - if (!success) {\n>> - /*\n>> - * Pipeline handler has not supplied an embedded data buffer,\n>> - * or embedded data buffer parsing has failed for some reason,\n>> - * so pull the exposure and gain values from the control list.\n>> - */\n>> - int32_t exposureLines = data.controls.get(V4L2_CID_EXPOSURE).get();\n>> - int32_t gainCode = data.controls.get(V4L2_CID_ANALOGUE_GAIN).get();\n>> - fillDeviceStatus(exposureLines, gainCode, deviceStatus);\n>> - }\n>> + /*\n>> + * This may overwrite the DeviceStatus using values from the sensor\n>> + * metadata, and may also do additional custom processing.\n>> + */\n>> + helper_->Prepare(embeddedBuffer, rpiMetadata_);\n>> +\n>> + /* Done with embedded data now, return to pipeline handler asap. */\n>> + if (data.embeddedBufferPresent)\n>> + returnEmbeddedBuffer(data.embeddedBufferId);\n>>\n>> ControlList ctrls(ispCtrls_);\n>>\n>> - rpiMetadata_.Clear();\n>> - rpiMetadata_.Set(\"device.status\", deviceStatus);\n>> controller_.Prepare(&rpiMetadata_);\n>>\n>> /* Lock the metadata buffer to avoid constant locks/unlocks. */\n>> @@ -973,41 +970,13 @@ void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)\n>> setIspControls.emit(ctrls);\n>> }\n>>\n>> -bool IPARPi::parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus)\n>> +void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n>> {\n>> - auto it = buffers_.find(bufferId);\n>> - if (it == buffers_.end()) {\n>> - LOG(IPARPI, Error) << \"Could not find embedded buffer!\";\n>> - return false;\n>> - }\n>> -\n>> - Span mem = it->second.maps()[0];\n>> - helper_->Parser().SetBufferSize(mem.size());\n>> - RPiController::MdParser::Status status = helper_->Parser().Parse(mem.data());\n>> - if (status != RPiController::MdParser::Status::OK) {\n>> - LOG(IPARPI, Error) << \"Embedded Buffer parsing failed, error \" << status;\n>> - return false;\n>> - } else {\n>> - uint32_t exposureLines, gainCode;\n>> - if (helper_->Parser().GetExposureLines(exposureLines) != RPiController::MdParser::Status::OK) {\n>> - LOG(IPARPI, Error) << \"Exposure time failed\";\n>> - return false;\n>> - }\n>> -\n>> - if (helper_->Parser().GetGainCode(gainCode) != RPiController::MdParser::Status::OK) {\n>> - LOG(IPARPI, Error) << \"Gain failed\";\n>> - return false;\n>> - }\n>> -\n>> - fillDeviceStatus(exposureLines, gainCode, deviceStatus);\n>> - }\n>> + struct DeviceStatus deviceStatus = {};\n>>\n>> - return true;\n>> -}\n>> + int32_t exposureLines = sensorControls.get(V4L2_CID_EXPOSURE).get();\n>> + int32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get();\n>>\n>> -void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n>> - struct DeviceStatus &deviceStatus)\n>> -{\n>> deviceStatus.shutter_speed = helper_->Exposure(exposureLines);\n>> deviceStatus.analogue_gain = helper_->Gain(gainCode);\n>>\n>> @@ -1015,6 +984,8 @@ void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n>> << deviceStatus.shutter_speed\n>> << \" Gain : \"\n>> << deviceStatus.analogue_gain;\n>> +\n>> + rpiMetadata_.Set(\"device.status\", deviceStatus);\n>> }\n>>\n>> void IPARPi::processStats(unsigned int bufferId)\n>> @@ -1028,6 +999,7 @@ void IPARPi::processStats(unsigned int bufferId)\n>> Span mem = it->second.maps()[0];\n>> bcm2835_isp_stats *stats = reinterpret_cast(mem.data());\n>> RPiController::StatisticsPtr statistics = std::make_shared(*stats);\n>> + helper_->Process(statistics, rpiMetadata_);\n>> controller_.Process(statistics, &rpiMetadata_);\n>>\n>> struct AgcStatus agcStatus;\n>> --\n>> 2.20.1\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 0113BBDB15\n\tfor ;\n\tWed, 21 Apr 2021 08:59:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1C2668847;\n\tWed, 21 Apr 2021 10:59:46 +0200 (CEST)","from mail-oi1-x230.google.com (mail-oi1-x230.google.com\n\t[IPv6:2607:f8b0:4864:20::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 98CF8602C8\n\tfor ;\n\tWed, 21 Apr 2021 10:59:45 +0200 (CEST)","by mail-oi1-x230.google.com with SMTP id e25so11790726oii.2\n\tfor ;\n\tWed, 21 Apr 2021 01:59:45 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"CiZpk8mt\"; 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=6MAKUqJd4vonY8M3SBCZv++EepO278ZbRWJRI9S7FVk=;\n\tb=CiZpk8mtNiERAf5sk9/6DZJey9T9AfKSIBdGenK2c4U0g4Njbvkq7C/22akgwvhpa7\n\tc86l5d9nyrr6gOyvTnvOv/SEmdfYcZ5crWIWYd5Q8k4DYjBIu80AxeATGpS/my6hE0HO\n\tAmbCDzFpI4Jna/0empsvOOnQD6eFBnsbe6tXqkhKhqRIpWPxK8VgbmTFYVF1aF9BN6rS\n\tfg5042R/zkJrq7BGgk8yNYmLKqsVjtpTQCsg1It8IxHr+NtEApM+WpKaVOC/lWpDbyDa\n\t6Y/YoybUfmAtf+DNE73EWbMOz4TSKRazhRq1w+Zdw/8pyuarAhEnqEL4H8P1DV2M/wk/\n\tVRvg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=6MAKUqJd4vonY8M3SBCZv++EepO278ZbRWJRI9S7FVk=;\n\tb=eYMj7swB0hzyQHh1haGEn/RQC3cymD20xiLhqIELZQ4N7lHffPZSGBkmwbawh3yDfH\n\tTAxkObM3NXqAd4ORFapEz0VSnFRkJzQanHP16FrofEhg+hUFaB9QYvhGkc9UBdEA8C0p\n\tdzyVNLINpdMcUG5BWxVHZnRCdWfCCCXP1g3Ab2VVnzbtSqcrvuUvq60CNPJ5/1pQGv6z\n\t8pw3HA1X4cNQrc9QP2cNtkXPP/eRCKas8olIQLYwPdL/rchO9A/6CA7v6kbugADrTEF5\n\tQHsZ2sYMNk9zY7z2YYt8tqwsD9Hrt+Lk+ivCvlvANKUquOvL+lazj2OBPkMev/yta2Hl\n\tiVGA==","X-Gm-Message-State":"AOAM532BZJk6AFojcWqEdRpO1hfWh81dNAg6vetqMi+QFUDlB4TP1ppK\n\txgjq9F5OjV03y3h8F6lvHaV7AMQ3tSX+/RKXdkQc2w==","X-Google-Smtp-Source":"ABdhPJzZlZFWubrcvnzuph063/A/vvD2ogTuaEMSbZ2cGYSybwvS4EWh5D65BcQzRcZI1XWJDcwLJ8BLp4qXES/+95Q=","X-Received":"by 2002:aca:4bc1:: with SMTP id\n\ty184mr6190572oia.55.1618995583138; \n\tWed, 21 Apr 2021 01:59:43 -0700 (PDT)","MIME-Version":"1.0","References":"<20210326124321.28617-1-david.plowman@raspberrypi.com>\n\t<20210326124321.28617-2-david.plowman@raspberrypi.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Wed, 21 Apr 2021 09:59:32 +0100","Message-ID":"","To":"Naushir Patuck ","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] ipa: raspberrypi: Use\n\tCamHelpers to generalise sensor metadata parsing","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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16754,"web_url":"https://patchwork.libcamera.org/comment/16754/","msgid":"","date":"2021-05-04T08:20:11","subject":"Re: [libcamera-devel] [PATCH v2 1/1] ipa: raspberrypi: Use\n\tCamHelpers to generalise sensor metadata parsing","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi again everyone\n\nCan I give this one another little prod too, please?\n\nThanks!\nDavid\n\nOn Wed, 21 Apr 2021 at 09:59, David Plowman\n wrote:\n>\n> Hi everyone\n>\n> I was wondering if I could apply a little nudge to this one too as\n> there are some other patches in the system relying on it. Does this\n> need more thought, do we think?\n>\n> Thanks!\n> David\n>\n> On Fri, 26 Mar 2021 at 13:43, Naushir Patuck wrote:\n> >\n> > Hi David,\n> >\n> >\n> > On Fri, 26 Mar 2021 at 12:43, David Plowman wrote:\n> >>\n> >> CamHelpers get virtual Prepare() and Process() methods, running just\n> >> before and just after the ISP, just like Raspberry Pi Algorithms.\n> >>\n> >> The Prepare() method is able to parse the register dumps in embedded\n> >> data buffers, and can be specialised to perform custom processing when\n> >> necessary.\n> >>\n> >> The IPA itself only needs to call the new Prepare() and Process()\n> >> methods. It must fill in the DeviceStatus from the controls first, in\n> >> case the Prepare() method does not supply its own values.\n> >>\n> >> Signed-off-by: David Plowman \n> >> ---\n> >> src/ipa/raspberrypi/cam_helper.cpp | 51 ++++++++++++++++++\n> >> src/ipa/raspberrypi/cam_helper.hpp | 11 +++-\n> >> src/ipa/raspberrypi/raspberrypi.cpp | 80 ++++++++++-------------------\n> >> 3 files changed, 87 insertions(+), 55 deletions(-)\n> >>\n> >> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\n> >> index 0ae0baa0..5af73c9c 100644\n> >> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> >> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> >> @@ -17,6 +17,11 @@\n> >> #include \"md_parser.hpp\"\n> >>\n> >> using namespace RPiController;\n> >> +using namespace libcamera;\n> >> +\n> >> +namespace libcamera {\n> >> +LOG_DECLARE_CATEGORY(IPARPI)\n> >> +}\n> >>\n> >> static std::map cam_helpers;\n> >>\n> >> @@ -45,6 +50,17 @@ CamHelper::~CamHelper()\n> >> delete parser_;\n> >> }\n> >>\n> >> +void CamHelper::Prepare(const Span &buffer,\n> >> + Metadata &metadata)\n> >> +{\n> >> + parseEmbeddedData(buffer, metadata);\n> >> +}\n> >> +\n> >> +void CamHelper::Process([[maybe_unused]] StatisticsPtr &stats,\n> >> + [[maybe_unused]] Metadata &metadata)\n> >> +{\n> >> +}\n> >> +\n> >> uint32_t CamHelper::ExposureLines(double exposure_us) const\n> >> {\n> >> assert(initialized_);\n> >> @@ -139,6 +155,41 @@ unsigned int CamHelper::MistrustFramesModeSwitch() const\n> >> return 0;\n> >> }\n> >>\n> >> +void CamHelper::parseEmbeddedData(const Span &buffer,\n> >> + Metadata &metadata)\n> >> +{\n> >> + if (buffer.size()) {\n> >> + bool success = false;\n> >> + uint32_t exposureLines, gainCode;\n> >> +\n> >> + parser_->SetBufferSize(buffer.size());\n> >> + success = parser_->Parse(buffer.data()) == MdParser::Status::OK &&\n> >> + parser_->GetExposureLines(exposureLines) == MdParser::Status::OK &&\n> >> + parser_->GetGainCode(gainCode) == MdParser::Status::OK;\n> >> +\n> >> + /*\n> >> + * Overwrite the exposure/gain values in the DeviceStatus, as\n> >> + * we know better. Fetch it first in case any other fields were\n> >> + * set meaningfully.\n> >> + */\n> >> + struct DeviceStatus deviceStatus;\n> >> +\n> >> + if (success &&\n> >> + metadata.Get(\"device.status\", deviceStatus) == 0) {\n> >\n> >\n> > I'm trying to work out if the second clause is needed? Shouldn't we write device.status\n> > unconditionally, whether it is present in the metadata or not? I suppose it does not matter,\n> > it is guaranteed to be there from the IPA.\n> >\n> > Reviewed-by: Naushir Patuck \n> >\n> >\n> >>\n> >> + deviceStatus.shutter_speed = Exposure(exposureLines);\n> >> + deviceStatus.analogue_gain = Gain(gainCode);\n> >> +\n> >> + LOG(IPARPI, Debug) << \"Metadata updated - Exposure : \"\n> >> + << deviceStatus.shutter_speed\n> >> + << \" Gain : \"\n> >> + << deviceStatus.analogue_gain;\n> >> +\n> >> + metadata.Set(\"device.status\", deviceStatus);\n> >> + } else\n> >> + LOG(IPARPI, Error) << \"Embedded buffer parsing failed\";\n> >> + }\n> >> +}\n> >> +\n> >> RegisterCamHelper::RegisterCamHelper(char const *cam_name,\n> >> CamHelperCreateFunc create_func)\n> >> {\n> >> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp\n> >> index 1b2d6eec..930ea39a 100644\n> >> --- a/src/ipa/raspberrypi/cam_helper.hpp\n> >> +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> >> @@ -8,7 +8,11 @@\n> >>\n> >> #include \n> >>\n> >> +#include \n> >> +\n> >> #include \"camera_mode.h\"\n> >> +#include \"controller/controller.hpp\"\n> >> +#include \"controller/metadata.hpp\"\n> >> #include \"md_parser.hpp\"\n> >>\n> >> #include \"libcamera/internal/v4l2_videodevice.h\"\n> >> @@ -65,7 +69,9 @@ public:\n> >> CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);\n> >> virtual ~CamHelper();\n> >> void SetCameraMode(const CameraMode &mode);\n> >> - MdParser &Parser() const { return *parser_; }\n> >> + virtual void Prepare(const libcamera::Span &buffer,\n> >> + Metadata &metadata);\n> >> + virtual void Process(StatisticsPtr &stats, Metadata &metadata);\n> >> uint32_t ExposureLines(double exposure_us) const;\n> >> double Exposure(uint32_t exposure_lines) const; // in us\n> >> virtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration,\n> >> @@ -81,6 +87,9 @@ public:\n> >> virtual unsigned int MistrustFramesModeSwitch() const;\n> >>\n> >> protected:\n> >> + void parseEmbeddedData(const libcamera::Span &buffer,\n> >> + Metadata &metadata);\n> >> +\n> >> MdParser *parser_;\n> >> CameraMode mode_;\n> >>\n> >> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> >> index 1c928b72..32ecbdcd 100644\n> >> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> >> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> >> @@ -101,9 +101,7 @@ private:\n> >> void returnEmbeddedBuffer(unsigned int bufferId);\n> >> void prepareISP(const ipa::RPi::ISPConfig &data);\n> >> void reportMetadata();\n> >> - bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus);\n> >> - void fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n> >> - struct DeviceStatus &deviceStatus);\n> >> + void fillDeviceStatus(const ControlList &sensorControls);\n> >> void processStats(unsigned int bufferId);\n> >> void applyFrameDurations(double minFrameDuration, double maxFrameDuration);\n> >> void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n> >> @@ -895,35 +893,34 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)\n> >>\n> >> void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)\n> >> {\n> >> - struct DeviceStatus deviceStatus = {};\n> >> - bool success = false;\n> >> + Span embeddedBuffer;\n> >> +\n> >> + rpiMetadata_.Clear();\n> >> +\n> >> + fillDeviceStatus(data.controls);\n> >>\n> >> if (data.embeddedBufferPresent) {\n> >> /*\n> >> * Pipeline handler has supplied us with an embedded data buffer,\n> >> - * so parse it and extract the exposure and gain.\n> >> + * we must pass it to the CamHelper for parsing.\n> >> */\n> >> - success = parseEmbeddedData(data.embeddedBufferId, deviceStatus);\n> >> -\n> >> - /* Done with embedded data now, return to pipeline handler asap. */\n> >> - returnEmbeddedBuffer(data.embeddedBufferId);\n> >> + auto it = buffers_.find(data.embeddedBufferId);\n> >> + ASSERT(it != buffers_.end());\n> >> + embeddedBuffer = it->second.maps()[0];\n> >> }\n> >>\n> >> - if (!success) {\n> >> - /*\n> >> - * Pipeline handler has not supplied an embedded data buffer,\n> >> - * or embedded data buffer parsing has failed for some reason,\n> >> - * so pull the exposure and gain values from the control list.\n> >> - */\n> >> - int32_t exposureLines = data.controls.get(V4L2_CID_EXPOSURE).get();\n> >> - int32_t gainCode = data.controls.get(V4L2_CID_ANALOGUE_GAIN).get();\n> >> - fillDeviceStatus(exposureLines, gainCode, deviceStatus);\n> >> - }\n> >> + /*\n> >> + * This may overwrite the DeviceStatus using values from the sensor\n> >> + * metadata, and may also do additional custom processing.\n> >> + */\n> >> + helper_->Prepare(embeddedBuffer, rpiMetadata_);\n> >> +\n> >> + /* Done with embedded data now, return to pipeline handler asap. */\n> >> + if (data.embeddedBufferPresent)\n> >> + returnEmbeddedBuffer(data.embeddedBufferId);\n> >>\n> >> ControlList ctrls(ispCtrls_);\n> >>\n> >> - rpiMetadata_.Clear();\n> >> - rpiMetadata_.Set(\"device.status\", deviceStatus);\n> >> controller_.Prepare(&rpiMetadata_);\n> >>\n> >> /* Lock the metadata buffer to avoid constant locks/unlocks. */\n> >> @@ -973,41 +970,13 @@ void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)\n> >> setIspControls.emit(ctrls);\n> >> }\n> >>\n> >> -bool IPARPi::parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus)\n> >> +void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n> >> {\n> >> - auto it = buffers_.find(bufferId);\n> >> - if (it == buffers_.end()) {\n> >> - LOG(IPARPI, Error) << \"Could not find embedded buffer!\";\n> >> - return false;\n> >> - }\n> >> -\n> >> - Span mem = it->second.maps()[0];\n> >> - helper_->Parser().SetBufferSize(mem.size());\n> >> - RPiController::MdParser::Status status = helper_->Parser().Parse(mem.data());\n> >> - if (status != RPiController::MdParser::Status::OK) {\n> >> - LOG(IPARPI, Error) << \"Embedded Buffer parsing failed, error \" << status;\n> >> - return false;\n> >> - } else {\n> >> - uint32_t exposureLines, gainCode;\n> >> - if (helper_->Parser().GetExposureLines(exposureLines) != RPiController::MdParser::Status::OK) {\n> >> - LOG(IPARPI, Error) << \"Exposure time failed\";\n> >> - return false;\n> >> - }\n> >> -\n> >> - if (helper_->Parser().GetGainCode(gainCode) != RPiController::MdParser::Status::OK) {\n> >> - LOG(IPARPI, Error) << \"Gain failed\";\n> >> - return false;\n> >> - }\n> >> -\n> >> - fillDeviceStatus(exposureLines, gainCode, deviceStatus);\n> >> - }\n> >> + struct DeviceStatus deviceStatus = {};\n> >>\n> >> - return true;\n> >> -}\n> >> + int32_t exposureLines = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> >> + int32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get();\n> >>\n> >> -void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n> >> - struct DeviceStatus &deviceStatus)\n> >> -{\n> >> deviceStatus.shutter_speed = helper_->Exposure(exposureLines);\n> >> deviceStatus.analogue_gain = helper_->Gain(gainCode);\n> >>\n> >> @@ -1015,6 +984,8 @@ void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n> >> << deviceStatus.shutter_speed\n> >> << \" Gain : \"\n> >> << deviceStatus.analogue_gain;\n> >> +\n> >> + rpiMetadata_.Set(\"device.status\", deviceStatus);\n> >> }\n> >>\n> >> void IPARPi::processStats(unsigned int bufferId)\n> >> @@ -1028,6 +999,7 @@ void IPARPi::processStats(unsigned int bufferId)\n> >> Span mem = it->second.maps()[0];\n> >> bcm2835_isp_stats *stats = reinterpret_cast(mem.data());\n> >> RPiController::StatisticsPtr statistics = std::make_shared(*stats);\n> >> + helper_->Process(statistics, rpiMetadata_);\n> >> controller_.Process(statistics, &rpiMetadata_);\n> >>\n> >> struct AgcStatus agcStatus;\n> >> --\n> >> 2.20.1\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 117EBBDE78\n\tfor ;\n\tTue, 4 May 2021 08:20:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C9B3468919;\n\tTue, 4 May 2021 10:20:25 +0200 (CEST)","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 0F39B6890C\n\tfor ;\n\tTue, 4 May 2021 10:20:24 +0200 (CEST)","by mail-wm1-x32b.google.com with SMTP id\n\ts5-20020a7bc0c50000b0290147d0c21c51so757756wmh.4\n\tfor ;\n\tTue, 04 May 2021 01:20:24 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"IZqwxYgt\"; 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=2ix/B4lAspKtN8ZJzdQyDi6FFuXXI7gFXgF+hVI3I5U=;\n\tb=IZqwxYgtdzpYEwhxCwN/1gj2k/Tssxjj2buP5j1ycFrV5ARGywdRLH8QP5nb1N60tM\n\t5Y4V2SYRCGr55uToqVmuRMByeBMxHopjTeDPrCYMSllcRSRbAjLxXZ24yArufzlY/nqa\n\toRgaXEtBOvks1i73m/lzs09JfzUsSuc35J+1VB1KitRElM1Pt3R/+2iDSyfzOgiINb6o\n\tQ3/VuJ9HZBZN8pQgP+w4VyPRJQ0aQ0v3Xa2xUvZrLHRbpitx1yBHsfdXZVqFEMLCzvny\n\tR85N7casKHNVrKgFOZycpaEGJVz4UyUrw6qWL4CqQX6Uw8yYR/XWIBjGsNVdDH8oNGR9\n\tdx3Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=2ix/B4lAspKtN8ZJzdQyDi6FFuXXI7gFXgF+hVI3I5U=;\n\tb=UCeyIJJYUGZUwHKP1s0K1TkVZ1L2Hd5hw6sqitNWnQEQlUgZ6w03DYaDa2xBTygptS\n\tVQLml8m9OUDpT9A9BqBbVMBb8Au7uBCcm9PAETh0wyNhsMfOK9anXnY532i2dS7ew+A5\n\tT59l9iGR3aOdIHunXbk8XD1KYbr8tzRXqSSyhWzBmUu2mFyOsGd7Iuox/z+5eXPuLAkf\n\tlRddV4cy55s3qShmw5dd8W3HKX/Fr/+f8xSHQfLkoLKVHL9F1IYz43h8D5ocKxgpf+pa\n\tBH+i5wBkriuzPamc5DkXMVl2DYIyg3Ugxz6+x7xdnSFUTJkeC9xjrK9JiQDYRUmTfb1A\n\tU5bA==","X-Gm-Message-State":"AOAM5317aCnGNcGgKCKi8M0hLGmovsNIngruRVkrSW7eh5d0SyM6MU2+\n\tWMEcX6gWo2fpXnQcRgJhUYmEDP8sZU+GAkQFJZPEXeFETEk=","X-Google-Smtp-Source":"ABdhPJzmj6jzyDo+WXThPwVGhTT1KmvKK+Ii++5COzFjbIpzxomoseQAppMmyWLyvXN5410AMsMbl/fCgxAs+XYz5G4=","X-Received":"by 2002:a1c:a507:: with SMTP id o7mr2607705wme.130.1620116423608;\n\tTue, 04 May 2021 01:20:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20210326124321.28617-1-david.plowman@raspberrypi.com>\n\t<20210326124321.28617-2-david.plowman@raspberrypi.com>\n\t\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Tue, 4 May 2021 09:20:11 +0100","Message-ID":"","To":"Naushir Patuck ","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] ipa: raspberrypi: Use\n\tCamHelpers to generalise sensor metadata parsing","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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]