{"id":12230,"url":"https://patchwork.libcamera.org/api/1.1/patches/12230/?format=json","web_url":"https://patchwork.libcamera.org/patch/12230/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210507113728.14037-3-david.plowman@raspberrypi.com>","date":"2021-05-07T11:37:28","name":"[libcamera-devel,v4,2/2] ipa: raspberrypi: Use CamHelpers to generalise sensor embedded data parsing","commit_ref":"5fda81e1f441670796b81095595b98af7719779c","pull_url":null,"state":"accepted","archived":false,"hash":"e263e73bf4251a7610fe8a491125d8d238326111","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/1.1/people/42/?format=json","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/12230/mbox/","series":[{"id":2012,"url":"https://patchwork.libcamera.org/api/1.1/series/2012/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2012","date":"2021-05-07T11:37:26","name":"Raspberry Pi generalised sensor metadata parsing","version":4,"mbox":"https://patchwork.libcamera.org/series/2012/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/12230/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/12230/checks/","tags":{},"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 C4146BF831\n\tfor ;\n\tFri, 7 May 2021 11:37:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B8EC868918;\n\tFri, 7 May 2021 13:37:35 +0200 (CEST)","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 5FE52602BB\n\tfor ;\n\tFri, 7 May 2021 13:37:33 +0200 (CEST)","by mail-wr1-x434.google.com with SMTP id d11so8878810wrw.8\n\tfor ;\n\tFri, 07 May 2021 04:37:33 -0700 (PDT)","from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72])\n\tby smtp.gmail.com with ESMTPSA id\n\tw22sm13739009wmc.13.2021.05.07.04.37.32\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 07 May 2021 04:37:32 -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=\"OOHInNgZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=H5tXhgNnZvpT7uH55DnBMeopzIT6bN1cjg1nDK+SYTc=;\n\tb=OOHInNgZKXQGLZERT847qUZamUyv5x1LyqLfR8Glo3zF+SitENJfeKpxx7M0U7i3i7\n\tDL/I/JILrcOTW9iyme/3c86cwNbLoq9Mf7R2ZlbXx1JiZlekhaScVOGPoxpKCmn4FOsv\n\tJpTkgk/Kx/p+2DUw4eDxof1/rduk9LjzcfEWg29zoMwho3PJkv4SWL43dnOMcFAUQpG8\n\ttTKCPF68MlIbwqEJH9UB7W2lvnBKwUE1cREFtwKYTesWA+b3P7PpxKI+QYSA/iIZhDwf\n\tGBmOuschYlMqJBUj376A0ZZ4swlM6e4hvKn9zyXLafGMDQ2U6lK8M8yScfoaJPdnUOA/\n\tU7Ig==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=H5tXhgNnZvpT7uH55DnBMeopzIT6bN1cjg1nDK+SYTc=;\n\tb=qvAcSSWMAo6uh1vnw/jPBMe+/H7U9JcQx6/KDscPYr4HsCnTZvLAq1/Gzduxjz8NAq\n\tdmeObcnNtFutgJE7PCMQn3FHd0WUfRBxnRJucA1gcWTiGNDD2+35WroCvqPg154N6CgJ\n\tBU+4+0iIe2ozwlI72eE8j5lgMyL9XviqWZC532XkmsTb7mqCqfNY8j+dmvsVNqEt9QH8\n\tAABUTDlePGR8V8baVil+xf+2o3/wuqcE04ltEnuhRNo69iGMi9fGJJ93GeKTZyC5Gzfo\n\tzcDzKqKXmoFn/Vytb7fuUfa9pmA7BiaUNw+FU1nh5e4DCm1Sfdasd8TiwgHEmeFd28MA\n\tEPKA==","X-Gm-Message-State":"AOAM532y2qANO11Nn9Bpjq+7+lnP55Du+EyYqPqWtPQi07zV5bOV8Iz7\n\tGyt/rc9O/oCmxpQtoBPEZGTAfU16RghksA==","X-Google-Smtp-Source":"ABdhPJxx1Xb9BNa7pP4bDOP7aLHOjapr3XCJdEwt4UinouZ6bylCrcME0d0C0ycBtpfmkFLmrzsFxA==","X-Received":"by 2002:a05:6000:108f:: with SMTP id\n\ty15mr12168538wrw.115.1620387452809; \n\tFri, 07 May 2021 04:37:32 -0700 (PDT)","From":"David Plowman ","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 7 May 2021 12:37:28 +0100","Message-Id":"<20210507113728.14037-3-david.plowman@raspberrypi.com>","X-Mailer":"git-send-email 2.20.1","In-Reply-To":"<20210507113728.14037-1-david.plowman@raspberrypi.com>","References":"<20210507113728.14037-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v4 2/2] ipa: raspberrypi: Use CamHelpers\n\tto generalise sensor embedded data 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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"CamHelpers get virtual Prepare() and Process() methods, running just\nbefore and just after the ISP, just like Raspberry Pi Algorithms.\n\nThe Prepare() method is able to parse the register dumps in embedded\ndata buffers, and can be specialised to perform custom processing when\nnecessary.\n\nThe IPA itself only needs to call the new Prepare() and Process()\nmethods. It must fill in the DeviceStatus from the controls first, in\ncase the Prepare() method does not supply its own values.\n\nSigned-off-by: David Plowman \nReviewed-by: Naushir Patuck \n---\n src/ipa/raspberrypi/cam_helper.cpp | 54 +++++++++++++++++++\n src/ipa/raspberrypi/cam_helper.hpp | 11 +++-\n src/ipa/raspberrypi/raspberrypi.cpp | 80 ++++++++++-------------------\n 3 files changed, 90 insertions(+), 55 deletions(-)","diff":"diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\nindex 0ae0baa0..09917f3c 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 \tdelete parser_;\n }\n \n+void CamHelper::Prepare(Span buffer,\n+\t\t\tMetadata &metadata)\n+{\n+\tparseEmbeddedData(buffer, metadata);\n+}\n+\n+void CamHelper::Process([[maybe_unused]] StatisticsPtr &stats,\n+\t\t\t[[maybe_unused]] Metadata &metadata)\n+{\n+}\n+\n uint32_t CamHelper::ExposureLines(double exposure_us) const\n {\n \tassert(initialized_);\n@@ -139,6 +155,44 @@ unsigned int CamHelper::MistrustFramesModeSwitch() const\n \treturn 0;\n }\n \n+void CamHelper::parseEmbeddedData(Span buffer,\n+\t\t\t\t Metadata &metadata)\n+{\n+\tif (buffer.empty())\n+\t\treturn;\n+\n+\tuint32_t exposureLines, gainCode;\n+\n+\tif (parser_->Parse(buffer) != MdParser::Status::OK ||\n+\t parser_->GetExposureLines(exposureLines) != MdParser::Status::OK ||\n+\t parser_->GetGainCode(gainCode) != MdParser::Status::OK) {\n+\t\tLOG(IPARPI, Error) << \"Embedded data buffer parsing failed\";\n+\t\treturn;\n+\t}\n+\n+\t/*\n+\t * Overwrite the exposure/gain values in the DeviceStatus, as\n+\t * we know better. Fetch it first in case any other fields were\n+\t * set meaningfully.\n+\t */\n+\tDeviceStatus deviceStatus;\n+\n+\tif (metadata.Get(\"device.status\", deviceStatus) != 0) {\n+\t\tLOG(IPARPI, Error) << \"DeviceStatus not found\";\n+\t\treturn;\n+\t}\n+\n+\tdeviceStatus.shutter_speed = Exposure(exposureLines);\n+\tdeviceStatus.analogue_gain = Gain(gainCode);\n+\n+\tLOG(IPARPI, Debug) << \"Metadata updated - Exposure : \"\n+\t\t\t << deviceStatus.shutter_speed\n+\t\t\t << \" Gain : \"\n+\t\t\t << deviceStatus.analogue_gain;\n+\n+\tmetadata.Set(\"device.status\", deviceStatus);\n+}\n+\n RegisterCamHelper::RegisterCamHelper(char const *cam_name,\n \t\t\t\t CamHelperCreateFunc create_func)\n {\ndiff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp\nindex c3ed5362..a52f3f0b 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 \tCamHelper(MdParser *parser, unsigned int frameIntegrationDiff);\n \tvirtual ~CamHelper();\n \tvoid SetCameraMode(const CameraMode &mode);\n-\tMdParser &Parser() const { return *parser_; }\n+\tvirtual void Prepare(libcamera::Span buffer,\n+\t\t\t Metadata &metadata);\n+\tvirtual void Process(StatisticsPtr &stats, Metadata &metadata);\n \tuint32_t ExposureLines(double exposure_us) const;\n \tdouble Exposure(uint32_t exposure_lines) const; // in us\n \tvirtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration,\n@@ -81,6 +87,9 @@ public:\n \tvirtual unsigned int MistrustFramesModeSwitch() const;\n \n protected:\n+\tvoid parseEmbeddedData(libcamera::Span buffer,\n+\t\t\t Metadata &metadata);\n+\n \tMdParser *parser_;\n \tCameraMode mode_;\n \ndiff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex dad6395f..bb55f931 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -101,9 +101,7 @@ private:\n \tvoid returnEmbeddedBuffer(unsigned int bufferId);\n \tvoid prepareISP(const ipa::RPi::ISPConfig &data);\n \tvoid reportMetadata();\n-\tbool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus);\n-\tvoid fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n-\t\t\t struct DeviceStatus &deviceStatus);\n+\tvoid fillDeviceStatus(const ControlList &sensorControls);\n \tvoid processStats(unsigned int bufferId);\n \tvoid applyFrameDurations(double minFrameDuration, double maxFrameDuration);\n \tvoid applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n@@ -894,35 +892,34 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)\n \n void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)\n {\n-\tstruct DeviceStatus deviceStatus = {};\n-\tbool success = false;\n+\tSpan embeddedBuffer;\n+\n+\trpiMetadata_.Clear();\n+\n+\tfillDeviceStatus(data.controls);\n \n \tif (data.embeddedBufferPresent) {\n \t\t/*\n \t\t * Pipeline handler has supplied us with an embedded data buffer,\n-\t\t * so parse it and extract the exposure and gain.\n+\t\t * we must pass it to the CamHelper for parsing.\n \t\t */\n-\t\tsuccess = parseEmbeddedData(data.embeddedBufferId, deviceStatus);\n-\n-\t\t/* Done with embedded data now, return to pipeline handler asap. */\n-\t\treturnEmbeddedBuffer(data.embeddedBufferId);\n+\t\tauto it = buffers_.find(data.embeddedBufferId);\n+\t\tASSERT(it != buffers_.end());\n+\t\tembeddedBuffer = it->second.maps()[0];\n \t}\n \n-\tif (!success) {\n-\t\t/*\n-\t\t * Pipeline handler has not supplied an embedded data buffer,\n-\t\t * or embedded data buffer parsing has failed for some reason,\n-\t\t * so pull the exposure and gain values from the control list.\n-\t\t */\n-\t\tint32_t exposureLines = data.controls.get(V4L2_CID_EXPOSURE).get();\n-\t\tint32_t gainCode = data.controls.get(V4L2_CID_ANALOGUE_GAIN).get();\n-\t\tfillDeviceStatus(exposureLines, gainCode, deviceStatus);\n-\t}\n+\t/*\n+\t * This may overwrite the DeviceStatus using values from the sensor\n+\t * metadata, and may also do additional custom processing.\n+\t */\n+\thelper_->Prepare(embeddedBuffer, rpiMetadata_);\n+\n+\t/* Done with embedded data now, return to pipeline handler asap. */\n+\tif (data.embeddedBufferPresent)\n+\t\treturnEmbeddedBuffer(data.embeddedBufferId);\n \n \tControlList ctrls(ispCtrls_);\n \n-\trpiMetadata_.Clear();\n-\trpiMetadata_.Set(\"device.status\", deviceStatus);\n \tcontroller_.Prepare(&rpiMetadata_);\n \n \t/* Lock the metadata buffer to avoid constant locks/unlocks. */\n@@ -972,41 +969,13 @@ void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)\n \t\tsetIspControls.emit(ctrls);\n }\n \n-bool IPARPi::parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus)\n+void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n {\n-\tauto it = buffers_.find(bufferId);\n-\tif (it == buffers_.end()) {\n-\t\tLOG(IPARPI, Error) << \"Could not find embedded buffer!\";\n-\t\treturn false;\n-\t}\n-\n-\tSpan mem = it->second.maps()[0];\n-\thelper_->Parser().SetBufferSize(mem.size());\n-\tRPiController::MdParser::Status status = helper_->Parser().Parse(mem.data());\n-\tif (status != RPiController::MdParser::Status::OK) {\n-\t\tLOG(IPARPI, Error) << \"Embedded Buffer parsing failed, error \" << status;\n-\t\treturn false;\n-\t} else {\n-\t\tuint32_t exposureLines, gainCode;\n-\t\tif (helper_->Parser().GetExposureLines(exposureLines) != RPiController::MdParser::Status::OK) {\n-\t\t\tLOG(IPARPI, Error) << \"Exposure time failed\";\n-\t\t\treturn false;\n-\t\t}\n-\n-\t\tif (helper_->Parser().GetGainCode(gainCode) != RPiController::MdParser::Status::OK) {\n-\t\t\tLOG(IPARPI, Error) << \"Gain failed\";\n-\t\t\treturn false;\n-\t\t}\n-\n-\t\tfillDeviceStatus(exposureLines, gainCode, deviceStatus);\n-\t}\n+\tDeviceStatus deviceStatus = {};\n \n-\treturn true;\n-}\n+\tint32_t exposureLines = sensorControls.get(V4L2_CID_EXPOSURE).get();\n+\tint32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get();\n \n-void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n-\t\t\t struct DeviceStatus &deviceStatus)\n-{\n \tdeviceStatus.shutter_speed = helper_->Exposure(exposureLines);\n \tdeviceStatus.analogue_gain = helper_->Gain(gainCode);\n \n@@ -1014,6 +983,8 @@ void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,\n \t\t\t << deviceStatus.shutter_speed\n \t\t\t << \" Gain : \"\n \t\t\t << deviceStatus.analogue_gain;\n+\n+\trpiMetadata_.Set(\"device.status\", deviceStatus);\n }\n \n void IPARPi::processStats(unsigned int bufferId)\n@@ -1027,6 +998,7 @@ void IPARPi::processStats(unsigned int bufferId)\n \tSpan mem = it->second.maps()[0];\n \tbcm2835_isp_stats *stats = reinterpret_cast(mem.data());\n \tRPiController::StatisticsPtr statistics = std::make_shared(*stats);\n+\thelper_->Process(statistics, rpiMetadata_);\n \tcontroller_.Process(statistics, &rpiMetadata_);\n \n \tstruct AgcStatus agcStatus;\n","prefixes":["libcamera-devel","v4","2/2"]}