From patchwork Wed May 5 14:04:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 12197 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5B0CEBDE79 for ; Wed, 5 May 2021 14:04:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0971A6891F; Wed, 5 May 2021 16:04:48 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="hD8Id6jH"; dkim-atps=neutral Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D4BD268901 for ; Wed, 5 May 2021 16:04:45 +0200 (CEST) Received: by mail-wr1-x429.google.com with SMTP id d11so1983155wrw.8 for ; Wed, 05 May 2021 07:04:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=qQF8wzVHpOdV4hJeS5PsdM3L68iFY8KOuYy9yv2LfIQ=; b=hD8Id6jH4TbeNBYEu+WYFKVywHXX4lq38vrTcbLSPivWUPOy5cHqmEOZKljgDw9X3Q BgjitOtCcBPdnJ5ERa3OOmpHYG/fElGX/NGjv42eKvvuerw+Lb+T2HhF6WhJawLltaPQ Vw51eFhwAYp6WHkPUDH/ZArMkHJQihEkaug74dKb8Y3nMrjZi4Fb3zLKNXcSrZVK5lDQ MrzbLNbOlPSBicfGvkFR5KWpdsqlw8EiMcKjS6svHVebqD7sHov3/QtL9DTmllu++PWf x9QA52KcX4ezu6Ym1IdXTsaLtTDRd+WpU1cebn7UrhTV+E/KLEMfNI8TyJf8QER+nx4z VcPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=qQF8wzVHpOdV4hJeS5PsdM3L68iFY8KOuYy9yv2LfIQ=; b=nkEDhw0i/l7zAqyzmxeNKuaMLjbWvwaAH+LDpcskvFQ/xhyExeNut3GXSs6LOnQM5I P/iFW5Th9PEBfaX1HBmWNF7fwCpxaK44EaBfSnbDrXQ4OhtR/z2cjcGCkZIy3MFdCCTH 9H2HFSiPF3wA1YWRhBmOXTQdF78vmy4/wU3iW1kikG6PnA3ezL0o9mMzd4jb2klueRNk dMMHtZIspHt2rFhB99H60Fr6FEScYd7T6t2JH9S9YOMM+cBWHKEbu8ct4SLJ+GmUA6vy tuRZ/mSZTt+QXRdP/DdqAhO1OjUv5u0IvXwl9ItEXmSXO8yi3aW/DALedxscODVjn2rc W/Ug== X-Gm-Message-State: AOAM530JFqFgnPxyYW6X9YlIeLIs7GIFb8OPVmcYENUDH353ctFuNjm0 3dxeZ9w7kss8ntVzdELE+69bZ3rJou6oBQ== X-Google-Smtp-Source: ABdhPJz25vL+QVWIXhoWk0ChG12+AdVujVrM5cFbtJD2SAmJ1vzlGmt8TXqGPry0GXHl8w5YgpUbfQ== X-Received: by 2002:a5d:4fd2:: with SMTP id h18mr38051157wrw.95.1620223485290; Wed, 05 May 2021 07:04:45 -0700 (PDT) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id z6sm5899286wmf.9.2021.05.05.07.04.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 May 2021 07:04:44 -0700 (PDT) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Wed, 5 May 2021 15:04:38 +0100 Message-Id: <20210505140438.4042-2-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210505140438.4042-1-david.plowman@raspberrypi.com> References: <20210505140438.4042-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 1/1] ipa: raspberrypi: Use CamHelpers to generalise sensor embedded data parsing X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" CamHelpers get virtual Prepare() and Process() methods, running just before and just after the ISP, just like Raspberry Pi Algorithms. The Prepare() method is able to parse the register dumps in embedded data buffers, and can be specialised to perform custom processing when necessary. The IPA itself only needs to call the new Prepare() and Process() methods. It must fill in the DeviceStatus from the controls first, in case the Prepare() method does not supply its own values. Signed-off-by: David Plowman Reviewed-by: Naushir Patuck --- src/ipa/raspberrypi/cam_helper.cpp | 51 ++++++++++++++++++ src/ipa/raspberrypi/cam_helper.hpp | 11 +++- src/ipa/raspberrypi/raspberrypi.cpp | 80 ++++++++++------------------- 3 files changed, 87 insertions(+), 55 deletions(-) diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp index 0ae0baa0..5af73c9c 100644 --- a/src/ipa/raspberrypi/cam_helper.cpp +++ b/src/ipa/raspberrypi/cam_helper.cpp @@ -17,6 +17,11 @@ #include "md_parser.hpp" using namespace RPiController; +using namespace libcamera; + +namespace libcamera { +LOG_DECLARE_CATEGORY(IPARPI) +} static std::map cam_helpers; @@ -45,6 +50,17 @@ CamHelper::~CamHelper() delete parser_; } +void CamHelper::Prepare(const Span &buffer, + Metadata &metadata) +{ + parseEmbeddedData(buffer, metadata); +} + +void CamHelper::Process([[maybe_unused]] StatisticsPtr &stats, + [[maybe_unused]] Metadata &metadata) +{ +} + uint32_t CamHelper::ExposureLines(double exposure_us) const { assert(initialized_); @@ -139,6 +155,41 @@ unsigned int CamHelper::MistrustFramesModeSwitch() const return 0; } +void CamHelper::parseEmbeddedData(const Span &buffer, + Metadata &metadata) +{ + if (buffer.size()) { + bool success = false; + uint32_t exposureLines, gainCode; + + parser_->SetBufferSize(buffer.size()); + success = parser_->Parse(buffer.data()) == MdParser::Status::OK && + parser_->GetExposureLines(exposureLines) == MdParser::Status::OK && + parser_->GetGainCode(gainCode) == MdParser::Status::OK; + + /* + * Overwrite the exposure/gain values in the DeviceStatus, as + * we know better. Fetch it first in case any other fields were + * set meaningfully. + */ + struct DeviceStatus deviceStatus; + + if (success && + metadata.Get("device.status", deviceStatus) == 0) { + deviceStatus.shutter_speed = Exposure(exposureLines); + deviceStatus.analogue_gain = Gain(gainCode); + + LOG(IPARPI, Debug) << "Metadata updated - Exposure : " + << deviceStatus.shutter_speed + << " Gain : " + << deviceStatus.analogue_gain; + + metadata.Set("device.status", deviceStatus); + } else + LOG(IPARPI, Error) << "Embedded buffer parsing failed"; + } +} + RegisterCamHelper::RegisterCamHelper(char const *cam_name, CamHelperCreateFunc create_func) { diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp index c3ed5362..6ee5051c 100644 --- a/src/ipa/raspberrypi/cam_helper.hpp +++ b/src/ipa/raspberrypi/cam_helper.hpp @@ -8,7 +8,11 @@ #include +#include + #include "camera_mode.h" +#include "controller/controller.hpp" +#include "controller/metadata.hpp" #include "md_parser.hpp" #include "libcamera/internal/v4l2_videodevice.h" @@ -65,7 +69,9 @@ public: CamHelper(MdParser *parser, unsigned int frameIntegrationDiff); virtual ~CamHelper(); void SetCameraMode(const CameraMode &mode); - MdParser &Parser() const { return *parser_; } + virtual void Prepare(const libcamera::Span &buffer, + Metadata &metadata); + virtual void Process(StatisticsPtr &stats, Metadata &metadata); uint32_t ExposureLines(double exposure_us) const; double Exposure(uint32_t exposure_lines) const; // in us virtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration, @@ -81,6 +87,9 @@ public: virtual unsigned int MistrustFramesModeSwitch() const; protected: + void parseEmbeddedData(const libcamera::Span &buffer, + Metadata &metadata); + MdParser *parser_; CameraMode mode_; diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index dad6395f..b0f61d35 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -101,9 +101,7 @@ private: void returnEmbeddedBuffer(unsigned int bufferId); void prepareISP(const ipa::RPi::ISPConfig &data); void reportMetadata(); - bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus); - void fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode, - struct DeviceStatus &deviceStatus); + void fillDeviceStatus(const ControlList &sensorControls); void processStats(unsigned int bufferId); void applyFrameDurations(double minFrameDuration, double maxFrameDuration); void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls); @@ -894,35 +892,34 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId) void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data) { - struct DeviceStatus deviceStatus = {}; - bool success = false; + Span embeddedBuffer; + + rpiMetadata_.Clear(); + + fillDeviceStatus(data.controls); if (data.embeddedBufferPresent) { /* * Pipeline handler has supplied us with an embedded data buffer, - * so parse it and extract the exposure and gain. + * we must pass it to the CamHelper for parsing. */ - success = parseEmbeddedData(data.embeddedBufferId, deviceStatus); - - /* Done with embedded data now, return to pipeline handler asap. */ - returnEmbeddedBuffer(data.embeddedBufferId); + auto it = buffers_.find(data.embeddedBufferId); + ASSERT(it != buffers_.end()); + embeddedBuffer = it->second.maps()[0]; } - if (!success) { - /* - * Pipeline handler has not supplied an embedded data buffer, - * or embedded data buffer parsing has failed for some reason, - * so pull the exposure and gain values from the control list. - */ - int32_t exposureLines = data.controls.get(V4L2_CID_EXPOSURE).get(); - int32_t gainCode = data.controls.get(V4L2_CID_ANALOGUE_GAIN).get(); - fillDeviceStatus(exposureLines, gainCode, deviceStatus); - } + /* + * This may overwrite the DeviceStatus using values from the sensor + * metadata, and may also do additional custom processing. + */ + helper_->Prepare(embeddedBuffer, rpiMetadata_); + + /* Done with embedded data now, return to pipeline handler asap. */ + if (data.embeddedBufferPresent) + returnEmbeddedBuffer(data.embeddedBufferId); ControlList ctrls(ispCtrls_); - rpiMetadata_.Clear(); - rpiMetadata_.Set("device.status", deviceStatus); controller_.Prepare(&rpiMetadata_); /* Lock the metadata buffer to avoid constant locks/unlocks. */ @@ -972,41 +969,13 @@ void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data) setIspControls.emit(ctrls); } -bool IPARPi::parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus) +void IPARPi::fillDeviceStatus(const ControlList &sensorControls) { - auto it = buffers_.find(bufferId); - if (it == buffers_.end()) { - LOG(IPARPI, Error) << "Could not find embedded buffer!"; - return false; - } - - Span mem = it->second.maps()[0]; - helper_->Parser().SetBufferSize(mem.size()); - RPiController::MdParser::Status status = helper_->Parser().Parse(mem.data()); - if (status != RPiController::MdParser::Status::OK) { - LOG(IPARPI, Error) << "Embedded Buffer parsing failed, error " << status; - return false; - } else { - uint32_t exposureLines, gainCode; - if (helper_->Parser().GetExposureLines(exposureLines) != RPiController::MdParser::Status::OK) { - LOG(IPARPI, Error) << "Exposure time failed"; - return false; - } - - if (helper_->Parser().GetGainCode(gainCode) != RPiController::MdParser::Status::OK) { - LOG(IPARPI, Error) << "Gain failed"; - return false; - } - - fillDeviceStatus(exposureLines, gainCode, deviceStatus); - } + struct DeviceStatus deviceStatus = {}; - return true; -} + int32_t exposureLines = sensorControls.get(V4L2_CID_EXPOSURE).get(); + int32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get(); -void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode, - struct DeviceStatus &deviceStatus) -{ deviceStatus.shutter_speed = helper_->Exposure(exposureLines); deviceStatus.analogue_gain = helper_->Gain(gainCode); @@ -1014,6 +983,8 @@ void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode, << deviceStatus.shutter_speed << " Gain : " << deviceStatus.analogue_gain; + + rpiMetadata_.Set("device.status", deviceStatus); } void IPARPi::processStats(unsigned int bufferId) @@ -1027,6 +998,7 @@ void IPARPi::processStats(unsigned int bufferId) Span mem = it->second.maps()[0]; bcm2835_isp_stats *stats = reinterpret_cast(mem.data()); RPiController::StatisticsPtr statistics = std::make_shared(*stats); + helper_->Process(statistics, rpiMetadata_); controller_.Process(statistics, &rpiMetadata_); struct AgcStatus agcStatus;