[{"id":25741,"web_url":"https://patchwork.libcamera.org/comment/25741/","msgid":"<166792923454.896787.2264542798095099254@Monstersaurus>","date":"2022-11-08T17:40:34","subject":"Re: [libcamera-devel] [PATCH v5 6/7] pipeline: ipa: raspberrypi:\n\tUse IPA cookies","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-10-31 11:45:21)\n> Pass an IPA cookie from the pipeline handler to the IPA and eventually back to\n> the pipeline handler through the setDelayedControls signal. This cookie is used\n> to index the RPiController::Metadata object to be used for the frame.\n> \n> The IPA cookie is then returned from DelayedControls when the frame with the\n> applied controls has been returned from the sensor, and eventually passed back\n> to the IPA from the signalIspPrepare signal.\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> Tested-by: David Plowman \n\nReviewed-by: Kieran Bingham \n\n> ---\n> include/libcamera/ipa/raspberrypi.mojom | 6 ++-\n> src/ipa/raspberrypi/raspberrypi.cpp | 42 ++++++++++---------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 16 ++++---\n> 3 files changed, 37 insertions(+), 27 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index 40f78d9e3b3f..325d2d855bc0 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -37,6 +37,8 @@ struct ISPConfig {\n> uint32 bayerBufferId;\n> bool embeddedBufferPresent;\n> libcamera.ControlList controls;\n> + uint32 ipaContext;\n> + uint32 delayContext;\n> };\n> \n> struct IPAConfig {\n> @@ -127,7 +129,7 @@ interface IPARPiInterface {\n> */\n> unmapBuffers(array ids);\n> \n> - [async] signalStatReady(uint32 bufferId);\n> + [async] signalStatReady(uint32 bufferId, uint32 ipaContext);\n> [async] signalQueueRequest(libcamera.ControlList controls);\n> [async] signalIspPrepare(ISPConfig data);\n> };\n> @@ -137,5 +139,5 @@ interface IPARPiEventInterface {\n> runIsp(uint32 bufferId);\n> embeddedComplete(uint32 bufferId);\n> setIspControls(libcamera.ControlList controls);\n> - setDelayedControls(libcamera.ControlList controls);\n> + setDelayedControls(libcamera.ControlList controls, uint32 delayContext);\n> };\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 799a4fe70000..194171a8bc96 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -126,7 +126,7 @@ public:\n> ControlList *controls, IPAConfigResult *result) override;\n> void mapBuffers(const std::vector &buffers) override;\n> void unmapBuffers(const std::vector &ids) override;\n> - void signalStatReady(const uint32_t bufferId) override;\n> + void signalStatReady(const uint32_t bufferId, uint32_t ipaContext) override;\n> void signalQueueRequest(const ControlList &controls) override;\n> void signalIspPrepare(const ISPConfig &data) override;\n> \n> @@ -137,9 +137,9 @@ private:\n> void queueRequest(const ControlList &controls);\n> void returnEmbeddedBuffer(unsigned int bufferId);\n> void prepareISP(const ISPConfig &data);\n> - void reportMetadata();\n> - void fillDeviceStatus(const ControlList &sensorControls);\n> - void processStats(unsigned int bufferId);\n> + void reportMetadata(unsigned int ipaContext);\n> + void fillDeviceStatus(const ControlList &sensorControls, unsigned int ipaContext);\n> + void processStats(unsigned int bufferId, unsigned int ipaContext);\n> void applyFrameDurations(Duration minFrameDuration, Duration maxFrameDuration);\n> void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n> void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);\n> @@ -509,14 +509,16 @@ void IPARPi::unmapBuffers(const std::vector &ids)\n> }\n> }\n> \n> -void IPARPi::signalStatReady(uint32_t bufferId)\n> +void IPARPi::signalStatReady(uint32_t bufferId, uint32_t ipaContext)\n> {\n> + unsigned int context = ipaContext % rpiMetadata_.size();\n> +\n> if (++checkCount_ != frameCount_) /* assert here? */\n> LOG(IPARPI, Error) << \"WARNING: Prepare/Process mismatch!!!\";\n> if (processPending_ && frameCount_ > mistrustCount_)\n> - processStats(bufferId);\n> + processStats(bufferId, context);\n> \n> - reportMetadata();\n> + reportMetadata(context);\n> \n> statsMetadataComplete.emit(bufferId & MaskID, libcameraMetadata_);\n> }\n> @@ -540,9 +542,9 @@ void IPARPi::signalIspPrepare(const ISPConfig &data)\n> runIsp.emit(data.bayerBufferId & MaskID);\n> }\n> \n> -void IPARPi::reportMetadata()\n> +void IPARPi::reportMetadata(unsigned int ipaContext)\n> {\n> - RPiController::Metadata &rpiMetadata = rpiMetadata_[0];\n> + RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext];\n> std::unique_lock lock(rpiMetadata);\n> \n> /*\n> @@ -1009,12 +1011,12 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)\n> void IPARPi::prepareISP(const ISPConfig &data)\n> {\n> int64_t frameTimestamp = data.controls.get(controls::SensorTimestamp).value_or(0);\n> - RPiController::Metadata lastMetadata;\n> - RPiController::Metadata &rpiMetadata = rpiMetadata_[0];\n> + unsigned int ipaContext = data.ipaContext % rpiMetadata_.size();\n> + RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext];\n> Span embeddedBuffer;\n> \n> - lastMetadata = std::move(rpiMetadata);\n> - fillDeviceStatus(data.controls);\n> + rpiMetadata.clear();\n> + fillDeviceStatus(data.controls, ipaContext);\n> \n> if (data.embeddedBufferPresent) {\n> /*\n> @@ -1046,7 +1048,9 @@ void IPARPi::prepareISP(const ISPConfig &data)\n> * current frame, or any other bits of metadata that were added\n> * in helper_->Prepare().\n> */\n> - rpiMetadata.merge(lastMetadata);\n> + RPiController::Metadata &lastMetadata =\n> + rpiMetadata_[ipaContext ? ipaContext : rpiMetadata_.size()];\n> + rpiMetadata.mergeCopy(lastMetadata);\n> processPending_ = false;\n> return;\n> }\n> @@ -1105,7 +1109,7 @@ void IPARPi::prepareISP(const ISPConfig &data)\n> setIspControls.emit(ctrls);\n> }\n> \n> -void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n> +void IPARPi::fillDeviceStatus(const ControlList &sensorControls, unsigned int ipaContext)\n> {\n> DeviceStatus deviceStatus = {};\n> \n> @@ -1121,12 +1125,12 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n> \n> LOG(IPARPI, Debug) << \"Metadata - \" << deviceStatus;\n> \n> - rpiMetadata_[0].set(\"device.status\", deviceStatus);\n> + rpiMetadata_[ipaContext].set(\"device.status\", deviceStatus);\n> }\n> \n> -void IPARPi::processStats(unsigned int bufferId)\n> +void IPARPi::processStats(unsigned int bufferId, unsigned int ipaContext)\n> {\n> - RPiController::Metadata &rpiMetadata = rpiMetadata_[0];\n> + RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext];\n> \n> auto it = buffers_.find(bufferId);\n> if (it == buffers_.end()) {\n> @@ -1145,7 +1149,7 @@ void IPARPi::processStats(unsigned int bufferId)\n> ControlList ctrls(sensorCtrls_);\n> applyAGC(&agcStatus, ctrls);\n> \n> - setDelayedControls.emit(ctrls);\n> + setDelayedControls.emit(ctrls, ipaContext);\n> }\n> }\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 29626c0ef14e..cae1f7b9aea3 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -206,7 +206,7 @@ public:\n> void runIsp(uint32_t bufferId);\n> void embeddedComplete(uint32_t bufferId);\n> void setIspControls(const ControlList &controls);\n> - void setDelayedControls(const ControlList &controls);\n> + void setDelayedControls(const ControlList &controls, uint32_t delayContext);\n> void setSensorControls(ControlList &controls);\n> void unicamTimeout();\n> \n> @@ -262,6 +262,7 @@ public:\n> struct BayerFrame {\n> FrameBuffer *buffer;\n> ControlList controls;\n> + unsigned int delayContext;\n> };\n> \n> std::queue bayerQueue_;\n> @@ -1792,9 +1793,9 @@ void RPiCameraData::setIspControls(const ControlList &controls)\n> handleState();\n> }\n> \n> -void RPiCameraData::setDelayedControls(const ControlList &controls)\n> +void RPiCameraData::setDelayedControls(const ControlList &controls, uint32_t delayContext)\n> {\n> - if (!delayedCtrls_->push(controls, 0))\n> + if (!delayedCtrls_->push(controls, delayContext))\n> LOG(RPI, Error) << \"V4L2 DelayedControl set failed\";\n> handleState();\n> }\n> @@ -1867,13 +1868,13 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> * Lookup the sensor controls used for this frame sequence from\n> * DelayedControl and queue them along with the frame buffer.\n> */\n> - auto [ctrl, cookie] = delayedCtrls_->get(buffer->metadata().sequence);\n> + auto [ctrl, delayContext] = delayedCtrls_->get(buffer->metadata().sequence);\n> /*\n> * Add the frame timestamp to the ControlList for the IPA to use\n> * as it does not receive the FrameBuffer object.\n> */\n> ctrl.set(controls::SensorTimestamp, buffer->metadata().timestamp);\n> - bayerQueue_.push({ buffer, std::move(ctrl) });\n> + bayerQueue_.push({ buffer, std::move(ctrl), delayContext });\n> } else {\n> embeddedQueue_.push(buffer);\n> }\n> @@ -1923,7 +1924,8 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> * application until after the IPA signals so.\n> */\n> if (stream == &isp_[Isp::Stats]) {\n> - ipa_->signalStatReady(ipa::RPi::MaskStats | static_cast(index));\n> + ipa_->signalStatReady(ipa::RPi::MaskStats | static_cast(index),\n> + requestQueue_.front()->sequence());\n> } else {\n> /* Any other ISP output can be handed back to the application now. */\n> handleStreamBuffer(buffer, stream);\n> @@ -2168,6 +2170,8 @@ void RPiCameraData::tryRunPipeline()\n> ipa::RPi::ISPConfig ispPrepare;\n> ispPrepare.bayerBufferId = ipa::RPi::MaskBayerData | bayerId;\n> ispPrepare.controls = std::move(bayerFrame.controls);\n> + ispPrepare.ipaContext = request->sequence();\n> + ispPrepare.delayContext = bayerFrame.delayContext;\n> \n> if (embeddedBuffer) {\n> unsigned int embeddedId = unicam_[Unicam::Embedded].getBufferId(embeddedBuffer);\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 839FDBD16B\n\tfor ;\n\tTue, 8 Nov 2022 17:40:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E9F0163083;\n\tTue, 8 Nov 2022 18:40:37 +0100 (CET)","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 65FBF6303D\n\tfor ;\n\tTue, 8 Nov 2022 18:40:37 +0100 (CET)","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 EB6A44DA;\n\tTue, 8 Nov 2022 18:40:36 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1667929237;\n\tbh=yC9rAELtje86bJqpD+/z+mgRpDn7MTgXJykAwqzRU1U=;\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=J1L2LZB4jiGvrTCulhxv2g/G5GFg20Y6KhS12WZl3w8O65lLFOJGcQP7AeSGc3RfN\n\tb+i6nj0g+c6vsyyxyEHts+3jzOVgGzYEJm2Is6lznuZ9l+zcrcEuqnZcltMsJdSXBK\n\tTea1TE2O4QcfSO4ZHlKYdZ+D1XPkMzVDP12w7vmXazheOfmPJkVG1ZYMTSO9tQxkDk\n\txmQ1E6Aw8x9vgOW3YOmv7CftpNmkDHoR7NGxaHg8k5fNwUrIxQo8SziAhOZ3lJPusP\n\tBNNGeJtFNlHGdvH8o3/ZUP92qTEUzB0llZbNs1aJ1G2aGeAat75qNwFI9FMYlR1skb\n\tMywGfKEDEpSgA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1667929237;\n\tbh=yC9rAELtje86bJqpD+/z+mgRpDn7MTgXJykAwqzRU1U=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=GBLydR0jM69G+a2RxlPepS2IDTD8thQzaiSiu1eYxniyngAoyOCZFWBwhR38O0V6z\n\tql4fBmIcYH8BMnWBGmjMJwKL4TqEUrWOdaq8R8uFcn8dTWLBZ4w1hytnBO/NHSYxw2\n\tb77cGNBo+IixAT4Z852o7Ee53Y9q4tieCL9uSFYk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"GBLydR0j\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20221031114522.14215-7-naush@raspberrypi.com>","References":"<20221031114522.14215-1-naush@raspberrypi.com>\n\t<20221031114522.14215-7-naush@raspberrypi.com>","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 08 Nov 2022 17:40:34 +0000","Message-ID":"<166792923454.896787.2264542798095099254@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v5 6/7] pipeline: ipa: raspberrypi:\n\tUse IPA cookies","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\" "}}]