From patchwork Tue Mar 2 15:11:08 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11462 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 65B6BBD1F1 for ; Tue, 2 Mar 2021 15:11:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2EED568A9C; Tue, 2 Mar 2021 16:11:24 +0100 (CET) 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="qqO133hJ"; dkim-atps=neutral Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3CA7F68A96 for ; Tue, 2 Mar 2021 16:11:23 +0100 (CET) Received: by mail-wr1-x436.google.com with SMTP id u14so20248844wri.3 for ; Tue, 02 Mar 2021 07:11:23 -0800 (PST) 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=tP+Eg5evXQwdP7aCk1A2z3hjC9sT4abiIfKfHyocIx8=; b=qqO133hJKBYG0uOg4j027Ew9ci+Lpx06JxEDhjUSxXOK4FsyPCBKOtJhKnNSyBDK2d QHr+PYIoPpi2adQAXeJG2lunB5Hh1enH/ttzmrM4r3olFab4h9rSUMHSxh71OO5hLuyi IStwwyVHElzPHNVmggfz7gBkQV2bzEoiWNKPRm7+glTeVSE82+vLCJCqEKGBecbBALQC 4Hkp6FxNTug1DBergWVRFBmfjyIxisxeEYOvxKQr41fnyMhKIMEU6w0xfSvlkeqjHr+/ V6S1wjMA9xfpmp9UJemhfW8I7jBddnHwLKUUtmsEdWF1JTGEn1c0xbfrneG9dosojG60 m2CQ== 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=tP+Eg5evXQwdP7aCk1A2z3hjC9sT4abiIfKfHyocIx8=; b=V8+ccfnzVc6m8ryOrLy1oAyLgOGqXmI6bXa+QN8CznUSBR8MdIPduWbO8OY1q8nv+w r1fHznl57pnTlHez4F/+aqGpmGym6GOFmvEpnEcilhP4tbvaJNJvnXKeM19WjkL2zxvP 4K5sYC4tob3mThNbL4TLn0g/Jj+kAWF4/gRzBQPJejcvC4HiGNpsMIkDRZNSmYgQHH6X RSIh5K6e0UxlEQKWHQKpBl6oTRPQvut/wNv3pbiY0HZOM+k2az15GquILgmgOKZATBET JONMYBJxa/HWhF+BBk4nfO5JmUxIJuPSp97wPC9hWwYSlB5XL9lF0m4GFWgf/nKyz5xK u7Eg== X-Gm-Message-State: AOAM530NuV6FirQPbOZdwX7yuZvn0e3OAKR6tfv5ZjaKHPoxMXtXQ7UJ cT6ikUbu2/xRPPfNm27Vubi3lyR1a3OB5rZu X-Google-Smtp-Source: ABdhPJxr7/wHl/PaoE43SajHJrC1/jS5mt59YuNZN4Hxv7teyYJ+qoPi2HyjLN5VymHmGpZRbQr42A== X-Received: by 2002:a05:6000:2cf:: with SMTP id o15mr22168465wry.184.1614697882411; Tue, 02 Mar 2021 07:11:22 -0800 (PST) Received: from naush-laptop.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id w6sm7561062wrl.49.2021.03.02.07.11.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Mar 2021 07:11:21 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Tue, 2 Mar 2021 15:11:08 +0000 Message-Id: <20210302151111.212591-2-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210302151111.212591-1-naush@raspberrypi.com> References: <20210302151111.212591-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 1/4] pipeline: ipa: raspberrypi: Pass exposure/gain values to IPA though controls 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" When running with sensors that had no embedded data, the pipeline handler would fill a dummy embedded data buffer with gain/exposure values, and pass this buffer to the IPA together with the bayer buffer. The IPA would extract these values for use in the controller algorithms. Rework this logic entirely by having a new RPiCameraData::BayerFrame queue to replace the existing bayer queue. In addition to storing the FrameBuffer pointer, this also stores all the controls tracked by DelayedControls for that frame in a ControlList. This includes include exposure and gain values. On signalling RPi::IPA_EVENT_SIGNAL_ISP_PREPARE IPA event, the pipeline handler now passes this ControlList from the RPiCameraData::BayerFrame queue. The IPA now extracts the gain and exposure values from the ControlList instead of using RPiController::MdParserRPi to parse the embedded data buffer. Signed-off-by: Naushir Patuck Tested-by: David Plowman Reviewed-by: Laurent Pinchart --- include/libcamera/ipa/raspberrypi.mojom | 2 + src/ipa/raspberrypi/raspberrypi.cpp | 132 +++++++++++------- .../pipeline/raspberrypi/raspberrypi.cpp | 62 ++++---- 3 files changed, 111 insertions(+), 85 deletions(-) diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom index 5a27b1e4fc2d..f733a2cd2a40 100644 --- a/include/libcamera/ipa/raspberrypi.mojom +++ b/include/libcamera/ipa/raspberrypi.mojom @@ -29,6 +29,8 @@ struct SensorConfig { struct ISPConfig { uint32 embeddedBufferId; uint32 bayerBufferId; + bool embeddedBufferPresent; + ControlList controls; }; struct ConfigInput { diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 1226ea514521..f9ab6417866e 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: BSD-2-Clause */ /* - * Copyright (C) 2019-2020, Raspberry Pi (Trading) Ltd. + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Ltd. * * rpi.cpp - Raspberry Pi Image Processing Algorithms */ @@ -101,9 +101,11 @@ private: bool validateIspControls(); void queueRequest(const ControlList &controls); void returnEmbeddedBuffer(unsigned int bufferId); - void prepareISP(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 processStats(unsigned int bufferId); void applyFrameDurations(double minFrameDuration, double maxFrameDuration); void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls); @@ -447,7 +449,7 @@ void IPARPi::signalIspPrepare(const ipa::RPi::ISPConfig &data) * avoid running the control algos for a few frames in case * they are "unreliable". */ - prepareISP(data.embeddedBufferId); + prepareISP(data); frameCount_++; /* Ready to push the input buffer into the ISP. */ @@ -909,67 +911,84 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId) embeddedComplete.emit(bufferId & ipa::RPi::MaskID); } -void IPARPi::prepareISP(unsigned int bufferId) +void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data) { struct DeviceStatus deviceStatus = {}; - bool success = parseEmbeddedData(bufferId, deviceStatus); + bool success = false; - /* Done with embedded data now, return to pipeline handler asap. */ - returnEmbeddedBuffer(bufferId); + if (data.embeddedBufferPresent) { + /* + * Pipeline handler has supplied us with an embedded data buffer, + * so parse it and extract the exposure and gain. + */ + success = parseEmbeddedData(data.embeddedBufferId, deviceStatus); - if (success) { - ControlList ctrls(ispCtrls_); + /* Done with embedded data now, return to pipeline handler asap. */ + returnEmbeddedBuffer(data.embeddedBufferId); + } - rpiMetadata_.Clear(); - rpiMetadata_.Set("device.status", deviceStatus); - controller_.Prepare(&rpiMetadata_); + 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); + } - /* Lock the metadata buffer to avoid constant locks/unlocks. */ - std::unique_lock lock(rpiMetadata_); + ControlList ctrls(ispCtrls_); - AwbStatus *awbStatus = rpiMetadata_.GetLocked("awb.status"); - if (awbStatus) - applyAWB(awbStatus, ctrls); + rpiMetadata_.Clear(); + rpiMetadata_.Set("device.status", deviceStatus); + controller_.Prepare(&rpiMetadata_); - CcmStatus *ccmStatus = rpiMetadata_.GetLocked("ccm.status"); - if (ccmStatus) - applyCCM(ccmStatus, ctrls); + /* Lock the metadata buffer to avoid constant locks/unlocks. */ + std::unique_lock lock(rpiMetadata_); - AgcStatus *dgStatus = rpiMetadata_.GetLocked("agc.status"); - if (dgStatus) - applyDG(dgStatus, ctrls); + AwbStatus *awbStatus = rpiMetadata_.GetLocked("awb.status"); + if (awbStatus) + applyAWB(awbStatus, ctrls); - AlscStatus *lsStatus = rpiMetadata_.GetLocked("alsc.status"); - if (lsStatus) - applyLS(lsStatus, ctrls); + CcmStatus *ccmStatus = rpiMetadata_.GetLocked("ccm.status"); + if (ccmStatus) + applyCCM(ccmStatus, ctrls); - ContrastStatus *contrastStatus = rpiMetadata_.GetLocked("contrast.status"); - if (contrastStatus) - applyGamma(contrastStatus, ctrls); + AgcStatus *dgStatus = rpiMetadata_.GetLocked("agc.status"); + if (dgStatus) + applyDG(dgStatus, ctrls); - BlackLevelStatus *blackLevelStatus = rpiMetadata_.GetLocked("black_level.status"); - if (blackLevelStatus) - applyBlackLevel(blackLevelStatus, ctrls); + AlscStatus *lsStatus = rpiMetadata_.GetLocked("alsc.status"); + if (lsStatus) + applyLS(lsStatus, ctrls); - GeqStatus *geqStatus = rpiMetadata_.GetLocked("geq.status"); - if (geqStatus) - applyGEQ(geqStatus, ctrls); + ContrastStatus *contrastStatus = rpiMetadata_.GetLocked("contrast.status"); + if (contrastStatus) + applyGamma(contrastStatus, ctrls); - DenoiseStatus *denoiseStatus = rpiMetadata_.GetLocked("denoise.status"); - if (denoiseStatus) - applyDenoise(denoiseStatus, ctrls); + BlackLevelStatus *blackLevelStatus = rpiMetadata_.GetLocked("black_level.status"); + if (blackLevelStatus) + applyBlackLevel(blackLevelStatus, ctrls); - SharpenStatus *sharpenStatus = rpiMetadata_.GetLocked("sharpen.status"); - if (sharpenStatus) - applySharpen(sharpenStatus, ctrls); + GeqStatus *geqStatus = rpiMetadata_.GetLocked("geq.status"); + if (geqStatus) + applyGEQ(geqStatus, ctrls); - DpcStatus *dpcStatus = rpiMetadata_.GetLocked("dpc.status"); - if (dpcStatus) - applyDPC(dpcStatus, ctrls); + DenoiseStatus *denoiseStatus = rpiMetadata_.GetLocked("denoise.status"); + if (denoiseStatus) + applyDenoise(denoiseStatus, ctrls); - if (!ctrls.empty()) - setIspControls.emit(ctrls); - } + SharpenStatus *sharpenStatus = rpiMetadata_.GetLocked("sharpen.status"); + if (sharpenStatus) + applySharpen(sharpenStatus, ctrls); + + DpcStatus *dpcStatus = rpiMetadata_.GetLocked("dpc.status"); + if (dpcStatus) + applyDPC(dpcStatus, ctrls); + + if (!ctrls.empty()) + setIspControls.emit(ctrls); } bool IPARPi::parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus) @@ -985,6 +1004,7 @@ bool IPARPi::parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &devic 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) { @@ -992,21 +1012,29 @@ bool IPARPi::parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &devic return false; } - deviceStatus.shutter_speed = helper_->Exposure(exposureLines); if (helper_->Parser().GetGainCode(gainCode) != RPiController::MdParser::Status::OK) { LOG(IPARPI, Error) << "Gain failed"; return false; } - deviceStatus.analogue_gain = helper_->Gain(gainCode); - LOG(IPARPI, Debug) << "Metadata - Exposure : " - << deviceStatus.shutter_speed << " Gain : " - << deviceStatus.analogue_gain; + fillDeviceStatus(exposureLines, gainCode, deviceStatus); } return true; } +void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode, + struct DeviceStatus &deviceStatus) +{ + deviceStatus.shutter_speed = helper_->Exposure(exposureLines); + deviceStatus.analogue_gain = helper_->Gain(gainCode); + + LOG(IPARPI, Debug) << "Metadata - Exposure : " + << deviceStatus.shutter_speed + << " Gain : " + << deviceStatus.analogue_gain; +} + void IPARPi::processStats(unsigned int bufferId) { auto it = buffers_.find(bufferId); diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index ba74ace183db..d057241b9c76 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ /* - * Copyright (C) 2019-2020, Raspberry Pi (Trading) Ltd. + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Ltd. * * raspberrypi.cpp - Pipeline handler for Raspberry Pi devices */ @@ -197,7 +197,13 @@ public: */ enum class State { Stopped, Idle, Busy, IpaComplete }; State state_; - std::queue bayerQueue_; + + struct BayerFrame { + FrameBuffer *buffer; + ControlList controls; + }; + + std::queue bayerQueue_; std::queue embeddedQueue_; std::deque requestQueue_; @@ -222,7 +228,7 @@ public: private: void checkRequestCompleted(); void tryRunPipeline(); - bool findMatchingBuffers(FrameBuffer *&bayerBuffer, FrameBuffer *&embeddedBuffer); + bool findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&embeddedBuffer); unsigned int ispOutputCount_; }; @@ -1355,7 +1361,7 @@ void RPiCameraData::setIspControls(const ControlList &controls) void RPiCameraData::setDelayedControls(const ControlList &controls) { if (!delayedCtrls_->push(controls)) - LOG(RPI, Error) << "V4L2 staggered set failed"; + LOG(RPI, Error) << "V4L2 DelayedControl set failed"; handleState(); } @@ -1383,29 +1389,14 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) << ", timestamp: " << buffer->metadata().timestamp; if (stream == &unicam_[Unicam::Image]) { - bayerQueue_.push(buffer); - } else { - embeddedQueue_.push(buffer); - - ControlList ctrl = delayedCtrls_->get(buffer->metadata().sequence); - /* - * Sensor metadata is unavailable, so put the expected ctrl - * values (accounting for the staggered delays) into the empty - * metadata buffer. + * Lookup the sensor controls used for this frame sequence from + * DelayedControl and queue them along with the frame buffer. */ - if (!sensorMetadata_) { - unsigned int bufferId = unicam_[Unicam::Embedded].getBufferId(buffer); - auto it = mappedEmbeddedBuffers_.find(bufferId); - if (it != mappedEmbeddedBuffers_.end()) { - uint32_t *mem = reinterpret_cast(it->second.maps()[0].data()); - mem[0] = ctrl.get(V4L2_CID_EXPOSURE).get(); - mem[1] = ctrl.get(V4L2_CID_ANALOGUE_GAIN).get(); - } else { - LOG(RPI, Warning) << "Failed to find embedded buffer " - << bufferId; - } - } + ControlList ctrl = delayedCtrls_->get(buffer->metadata().sequence); + bayerQueue_.push({ buffer, std::move(ctrl) }); + } else { + embeddedQueue_.push(buffer); } handleState(); @@ -1656,14 +1647,15 @@ void RPiCameraData::applyScalerCrop(const ControlList &controls) void RPiCameraData::tryRunPipeline() { - FrameBuffer *bayerBuffer, *embeddedBuffer; + FrameBuffer *embeddedBuffer; + BayerFrame bayerFrame; /* If any of our request or buffer queues are empty, we cannot proceed. */ if (state_ != State::Idle || requestQueue_.empty() || bayerQueue_.empty() || embeddedQueue_.empty()) return; - if (!findMatchingBuffers(bayerBuffer, embeddedBuffer)) + if (!findMatchingBuffers(bayerFrame, embeddedBuffer)) return; /* Take the first request from the queue and action the IPA. */ @@ -1682,7 +1674,7 @@ void RPiCameraData::tryRunPipeline() /* Set our state to say the pipeline is active. */ state_ = State::Busy; - unsigned int bayerId = unicam_[Unicam::Image].getBufferId(bayerBuffer); + unsigned int bayerId = unicam_[Unicam::Image].getBufferId(bayerFrame.buffer); unsigned int embeddedId = unicam_[Unicam::Embedded].getBufferId(embeddedBuffer); LOG(RPI, Debug) << "Signalling signalIspPrepare:" @@ -1692,17 +1684,19 @@ void RPiCameraData::tryRunPipeline() ipa::RPi::ISPConfig ispPrepare; ispPrepare.embeddedBufferId = ipa::RPi::MaskEmbeddedData | embeddedId; ispPrepare.bayerBufferId = ipa::RPi::MaskBayerData | bayerId; + ispPrepare.embeddedBufferPresent = sensorMetadata_; + ispPrepare.controls = std::move(bayerFrame.controls); ipa_->signalIspPrepare(ispPrepare); } -bool RPiCameraData::findMatchingBuffers(FrameBuffer *&bayerBuffer, FrameBuffer *&embeddedBuffer) +bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&embeddedBuffer) { unsigned int embeddedRequeueCount = 0, bayerRequeueCount = 0; /* Loop until we find a matching bayer and embedded data buffer. */ while (!bayerQueue_.empty()) { /* Start with the front of the bayer queue. */ - bayerBuffer = bayerQueue_.front(); + FrameBuffer *bayerBuffer = bayerQueue_.front().buffer; /* * Find the embedded data buffer with a matching timestamp to pass to @@ -1739,7 +1733,7 @@ bool RPiCameraData::findMatchingBuffers(FrameBuffer *&bayerBuffer, FrameBuffer * * the front of the queue. This buffer is now orphaned, so requeue * it back to the device. */ - unicam_[Unicam::Image].queueBuffer(bayerQueue_.front()); + unicam_[Unicam::Image].queueBuffer(bayerQueue_.front().buffer); bayerQueue_.pop(); bayerRequeueCount++; LOG(RPI, Warning) << "Dropping unmatched input frame in stream " @@ -1757,7 +1751,7 @@ bool RPiCameraData::findMatchingBuffers(FrameBuffer *&bayerBuffer, FrameBuffer * LOG(RPI, Warning) << "Flushing bayer stream!"; while (!bayerQueue_.empty()) { - unicam_[Unicam::Image].queueBuffer(bayerQueue_.front()); + unicam_[Unicam::Image].queueBuffer(bayerQueue_.front().buffer); bayerQueue_.pop(); } flushedBuffers = true; @@ -1790,8 +1784,10 @@ bool RPiCameraData::findMatchingBuffers(FrameBuffer *&bayerBuffer, FrameBuffer * } else { /* * We have found a matching bayer and embedded data buffer, so - * nothing more to do apart from popping the buffers from the queue. + * nothing more to do apart from assigning the bayer frame and + * popping the buffers from the queue. */ + bayerFrame = std::move(bayerQueue_.front()); bayerQueue_.pop(); embeddedQueue_.pop(); return true; From patchwork Tue Mar 2 15:11:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11463 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 BC69EBD1F1 for ; Tue, 2 Mar 2021 15:11:26 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8717B68AA3; Tue, 2 Mar 2021 16:11:26 +0100 (CET) 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="ERNZJvRf"; dkim-atps=neutral Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C78A68AA1 for ; Tue, 2 Mar 2021 16:11:24 +0100 (CET) Received: by mail-wm1-x329.google.com with SMTP id n22so2508943wmc.2 for ; Tue, 02 Mar 2021 07:11:24 -0800 (PST) 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=rJK70tFYES6lmId9jEuXnVH3QNFhvyi2Se4PWzZ3Lm8=; b=ERNZJvRfEhN7a2wEE2slVdNArEyNGTE96aeugT0FvvDoIgQbg8noYhyEGTwwgL3DFN 8FH81OxUuFXv+zpKGUblcEuZVK3U5AV+qE+2b4sGkbJQaiYUM0LmqMYOP6FGox6vU+BM tB7ju0dbWFepMlC99XRGbSOOBeg//wMJGIqKyvJaaekmCE1yeeX6cqz17rV/rtosQo2g FvGZMSDSivpqovnXw9Gi8xcBftTKKK2TfI8jtaOqMf8bgG/uHiI6pxwEF0ClMxjMs9iU y4y6z3EMFRNVi9oC+yv+nbKKOFW7h7UdGWqF9y9sTkRIc5G6pu9Lxcy8H1vPzUVD9LPn pQWQ== 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=rJK70tFYES6lmId9jEuXnVH3QNFhvyi2Se4PWzZ3Lm8=; b=iyGXeJdUzepxmwMY1JMitPII/Zy2W/7u3iHjTDXKswc4WBjGTdO37z8zyo4PEJCViS o2VmJvFVM9nrvEIU41M7u4aJOsySywVxDdN8kN6XDE3ynI5+piLtFF69D6QuRcq/n6ce h3STsNJxQATvb6b0rIrqYOipLE2HQIf8sY2WmoYBUWmLL+ZGvUAZSoGUhuUMvDbLlf4f WHoiM1yevcL/xX8N6+oZ1SHLMCsstvOMx1PLRDSHKkeg88u+7jhV5pYrHRuKOv3SwDnC 3snacV6hXxS9b/grQeZHFTYn78I9rrNRCkMywNmB5red29qAfvHPBhbJw5uKj9o2+lXB j0NQ== X-Gm-Message-State: AOAM532FA4I9tDemy8h558l4WgGwQWF8bxoScqT2byeIP9kIS/OEtM4v suXDXlFhV1plVwuD7uRwgRTYSJOG7c1PlIKV X-Google-Smtp-Source: ABdhPJwpQvxz2u3GFGgS3he9m98x+N3GMwH7LPEWSrm0FaHEzpO1Da2uXBe9wSK2Bbcf7YoaYK6aXw== X-Received: by 2002:a1c:7e85:: with SMTP id z127mr4544168wmc.131.1614697883767; Tue, 02 Mar 2021 07:11:23 -0800 (PST) Received: from naush-laptop.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id w6sm7561062wrl.49.2021.03.02.07.11.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Mar 2021 07:11:23 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Tue, 2 Mar 2021 15:11:09 +0000 Message-Id: <20210302151111.212591-3-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210302151111.212591-1-naush@raspberrypi.com> References: <20210302151111.212591-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 2/4] ipa: raspberrypi: Remove MdParserRPi 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" With the recent change to pass a ControlList to the IPA with exposure and gain values used for a frame, RPiController::MdParserRPi is not needed any more. Remove all traces of it. The derived CamHelper objects now pass nullptr values for the parser to the base CamHelper class when sensors do not use metadata. Signed-off-by: Naushir Patuck Tested-by: David Plowman Reviewed-by: Laurent Pinchart --- src/ipa/raspberrypi/cam_helper.cpp | 6 ++-- src/ipa/raspberrypi/cam_helper_imx219.cpp | 4 +-- src/ipa/raspberrypi/cam_helper_ov5647.cpp | 3 +- src/ipa/raspberrypi/md_parser_rpi.cpp | 37 ----------------------- src/ipa/raspberrypi/md_parser_rpi.hpp | 32 -------------------- src/ipa/raspberrypi/meson.build | 1 - 6 files changed, 6 insertions(+), 77 deletions(-) delete mode 100644 src/ipa/raspberrypi/md_parser_rpi.cpp delete mode 100644 src/ipa/raspberrypi/md_parser_rpi.hpp diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp index 93d1b7b0296a..2837fcce6de5 100644 --- a/src/ipa/raspberrypi/cam_helper.cpp +++ b/src/ipa/raspberrypi/cam_helper.cpp @@ -88,8 +88,10 @@ uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration, void CamHelper::SetCameraMode(const CameraMode &mode) { mode_ = mode; - parser_->SetBitsPerPixel(mode.bitdepth); - parser_->SetLineLengthBytes(0); /* We use SetBufferSize. */ + if (parser_) { + parser_->SetBitsPerPixel(mode.bitdepth); + parser_->SetLineLengthBytes(0); /* We use SetBufferSize. */ + } initialized_ = true; } diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp index 95b8e698fe3b..0e454d0de2dc 100644 --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp @@ -19,8 +19,6 @@ #include "cam_helper.hpp" #if ENABLE_EMBEDDED_DATA #include "md_parser.hpp" -#else -#include "md_parser_rpi.hpp" #endif using namespace RPiController; @@ -62,7 +60,7 @@ CamHelperImx219::CamHelperImx219() #if ENABLE_EMBEDDED_DATA : CamHelper(new MdParserImx219(), frameIntegrationDiff) #else - : CamHelper(new MdParserRPi(), frameIntegrationDiff) + : CamHelper(nullptr, frameIntegrationDiff) #endif { } diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp index a7f417324048..75486e900d31 100644 --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp @@ -8,7 +8,6 @@ #include #include "cam_helper.hpp" -#include "md_parser_rpi.hpp" using namespace RPiController; @@ -38,7 +37,7 @@ private: */ CamHelperOv5647::CamHelperOv5647() - : CamHelper(new MdParserRPi(), frameIntegrationDiff) + : CamHelper(nullptr, frameIntegrationDiff) { } diff --git a/src/ipa/raspberrypi/md_parser_rpi.cpp b/src/ipa/raspberrypi/md_parser_rpi.cpp deleted file mode 100644 index 2b0bcfc5f034..000000000000 --- a/src/ipa/raspberrypi/md_parser_rpi.cpp +++ /dev/null @@ -1,37 +0,0 @@ -/* SPDX-License-Identifier: BSD-2-Clause */ -/* - * Copyright (C) 2020, Raspberry Pi (Trading) Limited - * - * md_parser_rpi.cpp - Metadata parser for generic Raspberry Pi metadata - */ - -#include - -#include "md_parser_rpi.hpp" - -using namespace RPiController; - -MdParserRPi::MdParserRPi() -{ -} - -MdParser::Status MdParserRPi::Parse(void *data) -{ - if (buffer_size_bytes_ < sizeof(rpiMetadata)) - return ERROR; - - memcpy(&metadata, data, sizeof(rpiMetadata)); - return OK; -} - -MdParser::Status MdParserRPi::GetExposureLines(unsigned int &lines) -{ - lines = metadata.exposure; - return OK; -} - -MdParser::Status MdParserRPi::GetGainCode(unsigned int &gain_code) -{ - gain_code = metadata.gain; - return OK; -} diff --git a/src/ipa/raspberrypi/md_parser_rpi.hpp b/src/ipa/raspberrypi/md_parser_rpi.hpp deleted file mode 100644 index 52f54f008056..000000000000 --- a/src/ipa/raspberrypi/md_parser_rpi.hpp +++ /dev/null @@ -1,32 +0,0 @@ -/* SPDX-License-Identifier: BSD-2-Clause */ -/* - * Copyright (C) 2019, Raspberry Pi (Trading) Limited - * - * md_parser_rpi.hpp - Raspberry Pi metadata parser interface - */ -#pragma once - -#include "md_parser.hpp" - -namespace RPiController { - -class MdParserRPi : public MdParser -{ -public: - MdParserRPi(); - Status Parse(void *data) override; - Status GetExposureLines(unsigned int &lines) override; - Status GetGainCode(unsigned int &gain_code) override; - -private: - // This must be the same struct that is filled into the metadata buffer - // in the pipeline handler. - struct rpiMetadata - { - uint32_t exposure; - uint32_t gain; - }; - rpiMetadata metadata; -}; - -} diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build index 9af755259747..59e49686bc09 100644 --- a/src/ipa/raspberrypi/meson.build +++ b/src/ipa/raspberrypi/meson.build @@ -17,7 +17,6 @@ rpi_ipa_includes = [ rpi_ipa_sources = files([ 'raspberrypi.cpp', 'md_parser.cpp', - 'md_parser_rpi.cpp', 'cam_helper.cpp', 'cam_helper_ov5647.cpp', 'cam_helper_imx219.cpp', From patchwork Tue Mar 2 15:11:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11464 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 48498BD1F1 for ; Tue, 2 Mar 2021 15:11:28 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1491768A99; Tue, 2 Mar 2021 16:11:28 +0100 (CET) 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="thIKZiJd"; dkim-atps=neutral Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0842F68A7E for ; Tue, 2 Mar 2021 16:11:26 +0100 (CET) Received: by mail-wm1-x331.google.com with SMTP id i9so2520080wml.0 for ; Tue, 02 Mar 2021 07:11:26 -0800 (PST) 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=Us06vAcUvmULLy6W6CRlr9gSJQeT9lvyMp5Qa4ZH2eo=; b=thIKZiJdZ44smq6/fQocwOP2sgsKA8ma/xXn9D8CNonC6QZobVuZXxjmoObGNlMnoO TmGQ/EVfU9VUx0YiFgwC6PEKwnvmBy8RR/yZ3pxHTPrYmemSr+jAa24syu/0ZJJbkz7D 5pegPPuQyGOYb0T6Y3+ZtYUmSX85EXoeQGzlY8auO8sTvXBUPFjYxJGpnIXAEbjwalmw gwOVyz5qGsdX412MDB09Y10WYpIZaoxeSlcJISMAMArnfzKMFXp+usoyzTOpN25qnHXj O+6gJKuYKn9IesM6cVKD2EkvwM56Uo7DyvjFlA5SpCBPzYjs7g8XY1TZTpVlCxiI8XB6 GPxw== 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=Us06vAcUvmULLy6W6CRlr9gSJQeT9lvyMp5Qa4ZH2eo=; b=osSK5eXkI1V/DooOMOYoOxsS1hoDAYpvWCUaf0V3/cS/X3+kvqka2SCY3rRfFzS1Tr scYeuMawvcjrQuC2d3FiWJ5OjazEZkfM47Eb+M1Oxle9YuRTfHOcuwjrISQ7F855Uhss ftg7lm5ecB5KgJx6oU89NE8YzQ/B4V02jHIh3KxDHxs1NoklCGc2h3OsP0hmvjl1TctO ZiFO6ONnntVzaY++afDOOAI3INiBxd0T7OO/d2XEGAmvg1aHbMQUnLwrzwbLFsnGvIzO dt8HuBhT5rl1lPxVnpgWFO0OeMln6Upr7E/lT0+7UVQOQHSpv04I1iEmxNJqjocls3D7 rpZw== X-Gm-Message-State: AOAM533S1AxSgshpI43I6F3fgpb63/QrVycvsy5d3gd/4wL3RkVUwnHT dk2lfuIHl5LStiLKf3sDFlL8xvqLyN9ElvUi X-Google-Smtp-Source: ABdhPJy43eNVmscQg1bBrvZ0zwCHo7KMtOU1TWhStusgp+IBpYgv5OgK5JoSa6TyZG17977g2rdCJQ== X-Received: by 2002:a1c:4b15:: with SMTP id y21mr4622866wma.94.1614697885342; Tue, 02 Mar 2021 07:11:25 -0800 (PST) Received: from naush-laptop.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id w6sm7561062wrl.49.2021.03.02.07.11.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Mar 2021 07:11:24 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Tue, 2 Mar 2021 15:11:10 +0000 Message-Id: <20210302151111.212591-4-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210302151111.212591-1-naush@raspberrypi.com> References: <20210302151111.212591-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/4] pipeline: raspberrypi: Only enable embedded stream when available 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" The pipeline handler would enable and use the Unicam embedded data stream even if the sensor did not support it. This was to allow a means to pass exposure and gain values for the frame to the IPA in a synchronised way. The recent changes to get the pipeline handler to pass a ControlList with exposure and gain values means this is no longer required. Disable the use of the embedded data stream when a sensor does not support it. This change also removes the mappedEmbeddedBuffers_ map as it is no longer used. Signed-off-by: Naushir Patuck Tested-by: David Plowman Reviewed-by: Laurent Pinchart --- .../pipeline/raspberrypi/raspberrypi.cpp | 114 +++++++++++------- 1 file changed, 72 insertions(+), 42 deletions(-) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index d057241b9c76..5ae2551957f8 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -177,12 +177,6 @@ public: /* Stores the ids of the buffers mapped in the IPA. */ std::unordered_set ipaBuffers_; - /* - * Map of (internal) mmaped embedded data buffers, to avoid having to - * map/unmap on every frame. - */ - std::map mappedEmbeddedBuffers_; - /* DMAHEAP allocation helper. */ RPi::DmaHeap dmaHeap_; FileDescriptor lsTable_; @@ -636,14 +630,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) if (isRaw(cfg.pixelFormat)) { cfg.setStream(&data->unicam_[Unicam::Image]); - /* - * We must set both Unicam streams as external, even - * though the application may only request RAW frames. - * This is because we match timestamps on both streams - * to synchronise buffers. - */ data->unicam_[Unicam::Image].setExternal(true); - data->unicam_[Unicam::Embedded].setExternal(true); continue; } @@ -715,17 +702,6 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) return ret; } - /* Unicam embedded data output format. */ - format = {}; - format.fourcc = V4L2PixelFormat(V4L2_META_FMT_SENSOR_DATA); - LOG(RPI, Debug) << "Setting embedded data format."; - ret = data->unicam_[Unicam::Embedded].dev()->setFormat(&format); - if (ret) { - LOG(RPI, Error) << "Failed to set format on Unicam embedded: " - << format.toString(); - return ret; - } - /* Figure out the smallest selection the ISP will allow. */ Rectangle testCrop(0, 0, 1, 1); data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &testCrop); @@ -742,6 +718,41 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) if (ret) LOG(RPI, Error) << "Failed to configure the IPA: " << ret; + /* + * The IPA will set data->sensorMetadata_ to true if embedded data is + * supported on this sensor. If so, open the Unicam embedded data + * node and configure the output format. + */ + if (data->sensorMetadata_) { + format = {}; + format.fourcc = V4L2PixelFormat(V4L2_META_FMT_SENSOR_DATA); + LOG(RPI, Debug) << "Setting embedded data format."; + data->unicam_[Unicam::Embedded].dev()->open(); + ret = data->unicam_[Unicam::Embedded].dev()->setFormat(&format); + if (ret) { + LOG(RPI, Error) << "Failed to set format on Unicam embedded: " + << format.toString(); + return ret; + } + + /* + * If a RAW/Bayer stream has been requested by the application, + * we must set both Unicam streams as external, even though the + * application may only request RAW frames. This is because we + * match timestamps on both streams to synchronise buffers. + */ + if (rawStream) + data->unicam_[Unicam::Embedded].setExternal(true); + } else { + /* + * No embedded data present, so we do not want to iterate over + * the embedded data stream when starting and stopping. + */ + data->streams_.erase(std::remove(data->streams_.begin(), data->streams_.end(), + &data->unicam_[Unicam::Embedded]), + data->streams_.end()); + } + /* * Update the ScalerCropMaximum to the correct value for this camera mode. * For us, it's the same as the "analogue crop". @@ -949,10 +960,16 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) for (auto &stream : data->isp_) data->streams_.push_back(&stream); - /* Open all Unicam and ISP streams. */ + /* + * Open all Unicam and ISP streams. The exception is the embedded data + * stream, which only gets opened if the IPA reports that the sensor + * supports embedded data. This happens in RPiCameraData::configureIPA(). + */ for (auto const stream : data->streams_) { - if (stream->dev()->open()) - return false; + if (stream != &data->unicam_[Unicam::Embedded]) { + if (stream->dev()->open()) + return false; + } } /* Wire up all the buffer connections. */ @@ -1109,19 +1126,14 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) return ret; } - if (!data->sensorMetadata_) { - for (auto const &it : data->unicam_[Unicam::Embedded].getBuffers()) { - MappedFrameBuffer fb(it.second, PROT_READ | PROT_WRITE); - data->mappedEmbeddedBuffers_.emplace(it.first, std::move(fb)); - } - } - /* * Pass the stats and embedded data buffers to the IPA. No other * buffers need to be passed. */ mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(), ipa::RPi::MaskStats); - mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(), ipa::RPi::MaskEmbeddedData); + if (data->sensorMetadata_) + mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(), + ipa::RPi::MaskEmbeddedData); return 0; } @@ -1154,7 +1166,6 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera) std::vector ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end()); data->ipa_->unmapBuffers(ipaBuffers); data->ipaBuffers_.clear(); - data->mappedEmbeddedBuffers_.clear(); for (auto const stream : data->streams_) stream->releaseBuffers(); @@ -1652,7 +1663,7 @@ void RPiCameraData::tryRunPipeline() /* If any of our request or buffer queues are empty, we cannot proceed. */ if (state_ != State::Idle || requestQueue_.empty() || - bayerQueue_.empty() || embeddedQueue_.empty()) + bayerQueue_.empty() || (embeddedQueue_.empty() && sensorMetadata_)) return; if (!findMatchingBuffers(bayerFrame, embeddedBuffer)) @@ -1675,17 +1686,24 @@ void RPiCameraData::tryRunPipeline() state_ = State::Busy; unsigned int bayerId = unicam_[Unicam::Image].getBufferId(bayerFrame.buffer); - unsigned int embeddedId = unicam_[Unicam::Embedded].getBufferId(embeddedBuffer); LOG(RPI, Debug) << "Signalling signalIspPrepare:" - << " Bayer buffer id: " << bayerId - << " Embedded buffer id: " << embeddedId; + << " Bayer buffer id: " << bayerId; ipa::RPi::ISPConfig ispPrepare; - ispPrepare.embeddedBufferId = ipa::RPi::MaskEmbeddedData | embeddedId; ispPrepare.bayerBufferId = ipa::RPi::MaskBayerData | bayerId; - ispPrepare.embeddedBufferPresent = sensorMetadata_; ispPrepare.controls = std::move(bayerFrame.controls); + + if (embeddedBuffer) { + unsigned int embeddedId = unicam_[Unicam::Embedded].getBufferId(embeddedBuffer); + + ispPrepare.embeddedBufferId = ipa::RPi::MaskEmbeddedData | embeddedId; + ispPrepare.embeddedBufferPresent = true; + + LOG(RPI, Debug) << "Signalling signalIspPrepare:" + << " Bayer buffer id: " << embeddedId; + } + ipa_->signalIspPrepare(ispPrepare); } @@ -1727,6 +1745,18 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em LOG(RPI, Debug) << "Could not find matching embedded buffer"; + if (!sensorMetadata_) { + /* + * If there is no sensor metadata, simply return the + * first bayer frame in the queue. + */ + LOG(RPI, Debug) << "Returning bayer frame without a match"; + bayerFrame = std::move(bayerQueue_.front()); + bayerQueue_.pop(); + embeddedBuffer = nullptr; + return true; + } + if (!embeddedQueue_.empty()) { /* * Not found a matching embedded buffer for the bayer buffer in From patchwork Tue Mar 2 15:11:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11465 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 CF03DBD1F1 for ; Tue, 2 Mar 2021 15:11:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 98C9B68A9A; Tue, 2 Mar 2021 16:11:29 +0100 (CET) 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="Q1H2WZdy"; dkim-atps=neutral Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4D61668AA0 for ; Tue, 2 Mar 2021 16:11:28 +0100 (CET) Received: by mail-wm1-x335.google.com with SMTP id m1so3107227wml.2 for ; Tue, 02 Mar 2021 07:11:28 -0800 (PST) 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=VM2nJZYIxUbh0H6j3sg6aGAGAOyrxfuBY7kCfr0KnlE=; b=Q1H2WZdyUvSQCowTXEGmEeUSbHbcSU/QTewABRTdr7P97RHTlIh6r9JA9x01IN5aRd 2S7QU2Xhyv7UX+Eaijj8ZiqQFwGzwFNCt1eDu5f3+sfx8QJotMyqe0URmq+UYou6g+AA bOSSmsoqSUHtIUZTuMehM407609J3yLDyjJMFTnpUn2/9RWpVYSSnpbHXEZcj9RIgVcd nXKbKGofVNASKBp0c3U5T4Cd9pAExnHf5xZCdze9tAdC7CVxqV0MPpS+jMwLGH6mxDu9 T/RM5tCDmSPjSOHwFOj4OqlZ/uqJScdeRdbZzellMnX1d1VhCYHIjGntyfQrUjqPaFhJ wucg== 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=VM2nJZYIxUbh0H6j3sg6aGAGAOyrxfuBY7kCfr0KnlE=; b=ommN8YpG92iuF79cSW2i/Zh9wYdR7guOiXf23ZkgvTwV6PB0TF1AZRycmIIUqYHaq9 j8s6bvsTF4i87tKMlCuH6EeTGDrU4UfduDjaCyb6LBBN+g12X1CmLCI6zFm3FrjtYALO bLe/9gjVV7WdnNIwVAfB9oXqVCGN/ZLfEz2dKYrniMUuXwUFB9fWLXe9yY8UWaXfPMEn HFdBWWp7JhzeSoO3c46slh65aNuYbnAJNADl4Jfdqk6C5r34OdNYP88PmsNod6DTJmn2 S3+Qhl/JhYjlyI7WL7F67JsB++eauIchhmFUWvAmGMsXHZLb13e4woKvHrxCbCOqNVKG 7csA== X-Gm-Message-State: AOAM531CzZounBHyQXcCX1fONIRHkOn8AZ/mmIu2luHGaEMAusVWeRTA UbD/zjZ+BV97W1dwusmqtnzNWgv4NEbRJdZs X-Google-Smtp-Source: ABdhPJzlwBJENnZAvxGNQ730GdwxXZaO5gNRxSJr7gLJEnsbYLSGM/1OLcln+RqUVGzAGuNjpmeHew== X-Received: by 2002:a05:600c:2dda:: with SMTP id e26mr4574260wmh.155.1614697887695; Tue, 02 Mar 2021 07:11:27 -0800 (PST) Received: from naush-laptop.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id w6sm7561062wrl.49.2021.03.02.07.11.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Mar 2021 07:11:26 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Tue, 2 Mar 2021 15:11:11 +0000 Message-Id: <20210302151111.212591-5-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210302151111.212591-1-naush@raspberrypi.com> References: <20210302151111.212591-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 4/4] pipeline: raspberrypi: Allow either strict or non-strict buffer matching 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" A new flag, StrictBufferMatching, is used to control the behavior of the embedded and bayer buffer matching routine. If set to true (default), we reject and drop all bayer frames that do not have a matching embedded data buffer and vice-versa. This guarantees the IPA will always have the correct frame exposure and gain values to use. If set to false, we use bayer frames that do not have a matching embedded data buffer. In this case, IPA will use use the ControlList passed to it for gain and exposure values. Additionally, allow external stream buffers to behave as if StrictBufferMatching = false since we are not allowed to drop them. Signed-off-by: Naushir Patuck Tested-by: David Plowman --- .../pipeline/raspberrypi/raspberrypi.cpp | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 5ae2551957f8..549d6147cd60 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -46,6 +46,22 @@ namespace libcamera { LOG_DEFINE_CATEGORY(RPI) +/* + * Set this to true to reject and drop all bayer frames that do not have a + * matching embedded data buffer and vice-versa. This guarantees the IPA will + * always have the correct frame exposure and gain values to use. + * + * Set this to false to use bayer frames that do not have a matching embedded + * data buffer. In this case, IPA will use use our local history for gain and + * exposure values, occasional frame drops may cause these number to be out of + * sync for a short period. + * + * \todo Ideally, this property should be set by the application, but we don't + * have any mechanism to pass generic properties into a pipeline handler at + * present. + */ +static const bool StrictBufferMatching = true; + namespace { bool isRaw(PixelFormat &pixFmt) @@ -1725,13 +1741,13 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em embeddedBuffer = nullptr; while (!embeddedQueue_.empty()) { FrameBuffer *b = embeddedQueue_.front(); - if (!unicam_[Unicam::Embedded].isExternal() && b->metadata().timestamp < ts) { + if (b->metadata().timestamp < ts) { embeddedQueue_.pop(); unicam_[Unicam::Embedded].queueBuffer(b); embeddedRequeueCount++; LOG(RPI, Warning) << "Dropping unmatched input frame in stream " << unicam_[Unicam::Embedded].name(); - } else if (unicam_[Unicam::Embedded].isExternal() || b->metadata().timestamp == ts) { + } else if (b->metadata().timestamp == ts) { /* We pop the item from the queue lower down. */ embeddedBuffer = b; break; @@ -1745,10 +1761,15 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em LOG(RPI, Debug) << "Could not find matching embedded buffer"; - if (!sensorMetadata_) { + if (unicam_[Unicam::Embedded].isExternal() || + !StrictBufferMatching || !sensorMetadata_) { /* - * If there is no sensor metadata, simply return the - * first bayer frame in the queue. + * If any of the following is true: + * - This is an external stream buffer (so cannot be dropped). + * - We do not care about strict buffer matching. + * - There is no sensor metadata present. + * we can simply return the first bayer frame in the queue + * without a matching embedded buffer. */ LOG(RPI, Debug) << "Returning bayer frame without a match"; bayerFrame = std::move(bayerQueue_.front());