From patchwork Fri Feb 12 11:33: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: 11254 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 37528BD160 for ; Fri, 12 Feb 2021 11:37:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 072FE6377E; Fri, 12 Feb 2021 12:37:35 +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="rH6rByyi"; dkim-atps=neutral Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DF6A863772 for ; Fri, 12 Feb 2021 12:37:32 +0100 (CET) Received: by mail-wm1-x32e.google.com with SMTP id l17so612811wmq.2 for ; Fri, 12 Feb 2021 03:37:32 -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=z4ZreEGEk3BzO5Jp44WZDMtRVNGW6hoiw0T4y9VtVzw=; b=rH6rByyi45MkgnjdnhmIx5RUi5oNl/66tpytzh4lugFr8xoFb6YO6PNCO7uZ4moES3 gcgduGu6shgffd5STIiRakmzA7RyGrh4QT0Kzhd7lmXQOHzEP7F66uznKTPil/KREXtM QweRHuP0T6X/NOSMd4QKPXU4GVrvRpnkePkeZiUD1qVtIh++vb948xcAGpBTIRgrmz6x ugHDJtJZHbNOYjGHYmrGDMDcRhtDkorddqRmE5Q67l6epSgpNbimFBbgGlaHTH4IFxj8 2IAqmRw7U2sJvrVnVZmNQmvZ51UqoShxeYlHaCtwdV6GvaxE6GLLbYF2imxUQ4MJOe5w sKMg== 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=z4ZreEGEk3BzO5Jp44WZDMtRVNGW6hoiw0T4y9VtVzw=; b=fYbORLowGwIJ7gcwLTgFD9+TuC4hhQzQCwxkHcaW0b9ZimRfPdfh6QtS2V+tth/39p SKUoSoWWFO2i5QHDDvVV/nQYUgSaKEoD3885p2Voot1zXkIjxrIk/4QIXCGVIWOwzbMq ZDLMWJ4K1T/EzWUJYcxN4cjghoty187/xLpvSFjOsvric3B8XGH2Y9wVm0/Lvvb02VZZ nW4Ga11QsWOY1AXurqn/LtHVNV93sn4wgSLZRID8bTPtrLtTlP4T80W+t2yuoXBs+6ZG 8Dy0pg6HI0H1h7wCKTQn9KrT9suuRXhGOB19+qM2aMr6Wiq4E5KDzd6yH6mWDHwKGIAI bkKQ== X-Gm-Message-State: AOAM532reoK9ZoQnDMrYiextMA5+pZl4no89LSP2LIveum/xMqy6DOTn /XGq/U/cQqxqEcYlzOPy8GWYhsjoZ7RUQM3p X-Google-Smtp-Source: ABdhPJxbNswklWwKAKoezp8ulfRN7lhNzFhcnWAXYF5SyjFBP0OtSN07AK0PPHyxy6GL0jjJdB8HjQ== X-Received: by 2002:a1c:107:: with SMTP id 7mr2285689wmb.28.1613129852031; Fri, 12 Feb 2021 03:37:32 -0800 (PST) Received: from naush-laptop.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id v5sm10005020wro.71.2021.02.12.03.37.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Feb 2021 03:37:31 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 12 Feb 2021 11:33:08 +0000 Message-Id: <20210212113312.239076-2-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210212113312.239076-1-naush@raspberrypi.com> References: <20210212113312.239076-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/5] libcamera: delayed_controls: Add notion of priority write 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" If an exposure time change adjusts the vblanking limits, and we write both VBLANK and EXPOSURE controls as a set, the latter may fail if the value is outside of the limits calculated by the old VBLANK value. This is a limitation in V4L2 and cannot be fixed by writing VBLANK before EXPOSURE. The workaround here is to have the DelayedControls object mark the VBLANK control as "priority write", which then write VBLANK separately from (and ahead of) any other controls. This way, the sensor driver will update the EXPOSURE control with new limits before the new values is presented, and will thus be seen as valid. To support this, a new struct DelayedControls::ControlParams is used in the constructor to provide the control delay value as well as the priority write flag. Signed-off-by: Naushir Patuck --- include/libcamera/internal/delayed_controls.h | 9 +++- src/libcamera/delayed_controls.cpp | 54 +++++++++++++------ src/libcamera/pipeline/ipu3/ipu3.cpp | 8 +-- .../pipeline/raspberrypi/raspberrypi.cpp | 13 +++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +-- test/delayed_contols.cpp | 20 +++---- 6 files changed, 68 insertions(+), 44 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index dc447a882514..564d9f2e2440 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -19,8 +19,13 @@ class V4L2Device; class DelayedControls { public: + struct ControlParams { + unsigned int delay; + bool priorityWrite; + }; + DelayedControls(V4L2Device *device, - const std::unordered_map &delays); + const std::unordered_map &controlParams); void reset(); @@ -64,7 +69,7 @@ private: V4L2Device *device_; /* \todo Evaluate if we should index on ControlId * or unsigned int */ - std::unordered_map delays_; + std::unordered_map controlParams_; unsigned int maxDelay_; bool running_; diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index ab1d40057c5f..5317f820b6bd 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -40,15 +40,19 @@ LOG_DEFINE_CATEGORY(DelayedControls) /** * \brief Construct a DelayedControls instance * \param[in] device The V4L2 device the controls have to be applied to - * \param[in] delays Map of the numerical V4L2 control ids to their associated - * delays (in frames) + * \param[in] controlParams Map of the numerical V4L2 control ids to their + * associated control parameters. * - * Only controls specified in \a delays are handled. If it's desired to mix - * delayed controls and controls that take effect immediately the immediate - * controls must be listed in the \a delays map with a delay value of 0. + * The control parameters comprise of delays (in frames) and a priority write + * flag. If this flag is set, the relevant control is written separately from, + * ahead of the reset of the batched controls. + * + * Only controls specified in \a controlParams are handled. If it's desired to + * mix delayed controls and controls that take effect immediately the immediate + * controls must be listed in the \a controlParams map with a delay value of 0. */ DelayedControls::DelayedControls(V4L2Device *device, - const std::unordered_map &delays) + const std::unordered_map &controlParams) : device_(device), maxDelay_(0) { const ControlInfoMap &controls = device_->controls(); @@ -57,12 +61,12 @@ DelayedControls::DelayedControls(V4L2Device *device, * Create a map of control ids to delays for controls exposed by the * device. */ - for (auto const &delay : delays) { - auto it = controls.find(delay.first); + for (auto const ¶m : controlParams) { + auto it = controls.find(param.first); if (it == controls.end()) { LOG(DelayedControls, Error) << "Delay request for control id " - << utils::hex(delay.first) + << utils::hex(param.first) << " but control is not exposed by device " << device_->deviceNode(); continue; @@ -70,13 +74,14 @@ DelayedControls::DelayedControls(V4L2Device *device, const ControlId *id = it->first; - delays_[id] = delay.second; + controlParams_[id] = param.second; LOG(DelayedControls, Debug) - << "Set a delay of " << delays_[id] + << "Set a delay of " << controlParams_[id].delay + << " and priority write flag " << controlParams_[id].priorityWrite << " for " << id->name(); - maxDelay_ = std::max(maxDelay_, delays_[id]); + maxDelay_ = std::max(maxDelay_, controlParams_[id].delay); } reset(); @@ -97,8 +102,8 @@ void DelayedControls::reset() /* Retrieve control as reported by the device. */ std::vector ids; - for (auto const &delay : delays_) - ids.push_back(delay.first->id()); + for (auto const ¶m : controlParams_) + ids.push_back(param.first->id()); ControlList controls = device_->getControls(ids); @@ -140,7 +145,7 @@ bool DelayedControls::push(const ControlList &controls) const ControlId *id = it->second; - if (delays_.find(id) == delays_.end()) + if (controlParams_.find(id) == controlParams_.end()) return false; Info &info = values_[id][queueCount_]; @@ -220,12 +225,27 @@ void DelayedControls::applyControls(uint32_t sequence) ControlList out(device_->controls()); for (const auto &ctrl : values_) { const ControlId *id = ctrl.first; - unsigned int delayDiff = maxDelay_ - delays_[id]; + unsigned int delayDiff = maxDelay_ - controlParams_[id].delay; unsigned int index = std::max(0, writeCount_ - delayDiff); const Info &info = ctrl.second[index]; if (info.updated) { - out.set(id->id(), info); + if (controlParams_[id].priorityWrite) { + /* + * This control must be written now, it could + * affect validity of the other controls. + */ + ControlList priority(device_->controls()); + priority.set(id->id(), info); + device_->setControls(&priority); + } else { + /* + * Batch up the list of controls and write them + * at the end of the function. + */ + out.set(id->id(), info); + } + LOG(DelayedControls, Debug) << "Setting " << id->name() << " to " << info.toString() diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 61f7bf43ea08..eda07d915f0c 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -988,14 +988,14 @@ int PipelineHandlerIPU3::registerCameras() * a sensor database. For now use generic values taken from * the Raspberry Pi and listed as 'generic values'. */ - std::unordered_map delays = { - { V4L2_CID_ANALOGUE_GAIN, 1 }, - { V4L2_CID_EXPOSURE, 2 }, + std::unordered_map params = { + { V4L2_CID_ANALOGUE_GAIN, { 1, false } }, + { V4L2_CID_EXPOSURE, { 2, false } }, }; data->delayedCtrls_ = std::make_unique(cio2->sensor()->device(), - delays); + params); data->cio2_.frameStart().connect(data->delayedCtrls_.get(), &DelayedControls::applyControls); diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index b0bb12ab0f37..bdb439021c46 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1273,16 +1273,15 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) if (result.operation & RPi::IPA_RESULT_SENSOR_PARAMS) { /* * Setup our delayed control writer with the sensor default - * gain and exposure delays. + * gain and exposure delays. Mark VBLANK for priority write. */ - std::unordered_map delays = { - { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] }, - { V4L2_CID_EXPOSURE, result.data[resultIdx++] }, - { V4L2_CID_VBLANK, result.data[resultIdx++] } + std::unordered_map params = { + { V4L2_CID_ANALOGUE_GAIN, { result.data[resultIdx++], false } }, + { V4L2_CID_EXPOSURE, { result.data[resultIdx++], false } }, + { V4L2_CID_VBLANK, { result.data[resultIdx++], true } } }; - delayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), delays); - + delayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), params); sensorMetadata_ = result.data[resultIdx++]; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index e85979a719b7..ea9b1e2b9fcb 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -942,14 +942,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) * a sensor database. For now use generic values taken from * the Raspberry Pi and listed as generic values. */ - std::unordered_map delays = { - { V4L2_CID_ANALOGUE_GAIN, 1 }, - { V4L2_CID_EXPOSURE, 2 }, + std::unordered_map params = { + { V4L2_CID_ANALOGUE_GAIN, { 1, false } }, + { V4L2_CID_EXPOSURE, { 2, false } }, }; data->delayedCtrls_ = std::make_unique(data->sensor_->device(), - delays); + params); isp_->frameStart.connect(data->delayedCtrls_.get(), &DelayedControls::applyControls); diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp index 50169b12e566..3855eb18ecd4 100644 --- a/test/delayed_contols.cpp +++ b/test/delayed_contols.cpp @@ -72,8 +72,8 @@ protected: int singleControlNoDelay() { - std::unordered_map delays = { - { V4L2_CID_BRIGHTNESS, 0 }, + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, { 0, false } }, }; std::unique_ptr delayed = std::make_unique(dev_.get(), delays); @@ -109,8 +109,8 @@ protected: int singleControlWithDelay() { - std::unordered_map delays = { - { V4L2_CID_BRIGHTNESS, 1 }, + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, { 1, false } }, }; std::unique_ptr delayed = std::make_unique(dev_.get(), delays); @@ -150,9 +150,9 @@ protected: int dualControlsWithDelay(uint32_t startOffset) { - std::unordered_map delays = { - { V4L2_CID_BRIGHTNESS, 1 }, - { V4L2_CID_CONTRAST, 2 }, + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, { 1, false } }, + { V4L2_CID_CONTRAST, { 2, false } }, }; std::unique_ptr delayed = std::make_unique(dev_.get(), delays); @@ -197,9 +197,9 @@ protected: int dualControlsMultiQueue() { - std::unordered_map delays = { - { V4L2_CID_BRIGHTNESS, 1 }, - { V4L2_CID_CONTRAST, 2 }, + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, { 1, false } }, + { V4L2_CID_CONTRAST, { 2, false } } }; std::unique_ptr delayed = std::make_unique(dev_.get(), delays); From patchwork Fri Feb 12 11:33: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: 11255 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 D47AEBD160 for ; Fri, 12 Feb 2021 11:37:37 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A619563786; Fri, 12 Feb 2021 12:37:37 +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="gnoGi5a1"; dkim-atps=neutral Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 76DF863778 for ; Fri, 12 Feb 2021 12:37:33 +0100 (CET) Received: by mail-wr1-x435.google.com with SMTP id g10so7513500wrx.1 for ; Fri, 12 Feb 2021 03:37:33 -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=8Js2N/PMpza7AgEjn/VI402ahkH5WnlAn0p4sgmqgn8=; b=gnoGi5a1qxMi3Pwdfs+cgmlUdQHKlZzw3Bb6knyUJr1cQW+dOQ1QrgcZ0slAjhoL4I 3dv+gA9LNDq24H91GRvRbOjYzUPYQgx3LVZDLd2GtUpCeNq6jhEDcn2I0g3Xxf8MQk5V yQR/Dx/GiiF2/2kvIKrVjPeq6WIEKoMUv23pILIUYni1DgSR7gqH1zHlm7N2c1NarGgU huJZjIpzlWbODKxmkghCfVDyQwv2lzoQkR6hl5TtKRedsQl/iVmjghkRNmf52BSEFRT7 Vq398KeuUe4BOECniiHGC5MUc39DB4gwp/YrgZB2KY1rbt1rOg5NqYQy54f0EJUrNU0r BTCg== 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=8Js2N/PMpza7AgEjn/VI402ahkH5WnlAn0p4sgmqgn8=; b=qjNsAVkvn4i/lw+XrobXbiQKiqtIsoa+Q8Mn92YGXNFavxYrUErPLK+VJzRxlmYy1Q VwciDnq0NKGmV67GcNDvi0vf4Xt4/E1p6J8bQkP+4d4ymDdE4KeoHIpXfGEFYZrsS9Em x3N9fafaP+aY9TngL2/QTlc9SkT4N6/Xz+NU4r8+EwPR5KTibVaT5OrNIv0yhXrbT1id 9pdXVNy7NQC6bNt1tbGJdxcs57jbuTcKNl3/iUkXNhrR5CkmNkC96/hT67UeRDqyeO7P Jc07SzjUksB7xiZo9fCJc0z42VS0+tGE4XymiS4NxO3HbQBMLT7Ct93U0Ag8TM6gLuZ5 M9ig== X-Gm-Message-State: AOAM530jMfz4WQnD9Z8qD64i/2npAH1wLuAD93SgaffHaH0zFejX1TsM 0dpqmyjpA8Y9G0SKOqOlV6lCoHFwq7bYmOvI X-Google-Smtp-Source: ABdhPJyOY5l2R72/GVmpwzxIepGg1hNVLG8ajdQRRE5zW4ODK8dtYDBZ1Wpdhh3Oja5mIG2fWekUFg== X-Received: by 2002:adf:82b3:: with SMTP id 48mr2856968wrc.22.1613129852672; Fri, 12 Feb 2021 03:37:32 -0800 (PST) Received: from naush-laptop.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id v5sm10005020wro.71.2021.02.12.03.37.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Feb 2021 03:37:32 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 12 Feb 2021 11:33:09 +0000 Message-Id: <20210212113312.239076-3-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210212113312.239076-1-naush@raspberrypi.com> References: <20210212113312.239076-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/5] utils: raspberrypi: Add a DelayedControls log parser 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" This script will parse log output from the DelayedControls helper, when enabled with: LIBCAMERA_LOG_LEVELS=DelayedControls:0 It tabulates all control queuing/writing/getting per frame and warns about potential issues related to frame delays not being account for, or writes that are lagging behind or missed. Run with the following command: python3 ./delayedctrls_parse.py Signed-off-by: Naushir Patuck --- utils/raspberrypi/delayedctrls_parse.py | 82 +++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 utils/raspberrypi/delayedctrls_parse.py diff --git a/utils/raspberrypi/delayedctrls_parse.py b/utils/raspberrypi/delayedctrls_parse.py new file mode 100644 index 000000000000..2e1c40d8b896 --- /dev/null +++ b/utils/raspberrypi/delayedctrls_parse.py @@ -0,0 +1,82 @@ +import re +import sys +import os + +if len(sys.argv) != 2: + print("Usage: {} ".format(sys.argv[0])) + sys.exit() + +infile = sys.argv[1] +insplit = os.path.splitext(infile) +outfile = insplit[0] + '_parsed' + insplit[1] + +frame_re = re.compile('frame (\d+) started') + +delays = {'Analogue Gain': 1, 'Exposure': 2, 'Vertical Blanking': 2} +ctrl_action = {'Write': {}, 'Get': {}, 'Queue': {}} +ctrl_re = {'Write': re.compile('Setting (.*?) to (\d+) at index (\d+)'), + 'Get': re.compile('Reading (.*?) to (\d+) at index (\d+)'), + 'Queue': re.compile('Queuing (.*?) to (\d+) at index (\d+)')} + +frame_num = -1 + +max_delay = 0 +for k, d in delays.items(): + if max_delay < d: + max_delay = d + +with open(infile) as f: + lines = f.readlines() + +for line in lines: + r = frame_re.search(line) + if r: + frame_num = int(r.group(1)) + + for (key, re) in ctrl_re.items(): + r = re.search(line) + if r: + ctrl_action[key][(frame_num, r.group(1))] = (r.group(2), r.group(3)) + +with open(outfile, 'wt') as f: + f.write('{:<10}{:<15}{:<12}{:<18}{}\n'.format('Frame', 'Action', 'Gain', 'Exposure', 'Vblank')) + for frame in range(0, frame_num + 1): + for (k, a) in ctrl_action.items(): + str = '{:<10}{:<10}'.format(frame, k) + + for c in delays.keys(): + # Tabulate all results + str += '{:>5} {:<10}'.format(a[(frame, c)][0] if (frame, c) in a.keys() else '---', + '[' + (a[(frame, c)][1] if (frame, c) in a.keys() else '-') + ']') + + f.write(str.strip() + '\n') + +for frame in range(0, frame_num + 1): + # Test the write -> get matches the set delay. + if (frame, c) in ctrl_action['Write'].keys(): + set_value = ctrl_action['Write'][(frame, c)][0] + delay_frame = frame + delays[c] + if (delay_frame <= frame_num): + if (delay_frame, c) in ctrl_action['Get']: + get_value = ctrl_action['Get'][(delay_frame, c)][0] + if get_value != set_value: + print('Error: {} written at frame {} to value {} != {} at frame {}' + .format(c, frame, set_value, get_value, delay_frame)) + else: + print('Error: {} written at frame {} to value {} did not get logged on frame {} - dropped frame?' + .format(c, frame, set_value, delay_frame)) + + # Test the queue -> write matches the set delay. + if (frame, c) in ctrl_action['Queue'].keys(): + set_value = ctrl_action['Queue'][(frame, c)][0] + delay_frame = frame + max_delay - delays[c] + 1 + if (delay_frame <= frame_num): + if (delay_frame, c) in ctrl_action['Write']: + write_value = ctrl_action['Write'][(delay_frame, c)][0] + if write_value != set_value: + print('Info: {} queued at frame {} to value {} != {} written at frame {}' + ' - lagging behind or double queue on a single frame!' + .format(c, frame, set_value, write_value, delay_frame)) + else: + print('Error: {} queued at frame {} to value {} did not get logged on frame {} - dropped frame?' + .format(c, frame, set_value, delay_frame)) From patchwork Fri Feb 12 11:33: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: 11256 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 52655BD160 for ; Fri, 12 Feb 2021 11:37:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1ABFA6377C; Fri, 12 Feb 2021 12:37:38 +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="iPjibrta"; dkim-atps=neutral Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F6FB63772 for ; Fri, 12 Feb 2021 12:37:34 +0100 (CET) Received: by mail-wm1-x332.google.com with SMTP id y134so653614wmd.3 for ; Fri, 12 Feb 2021 03:37:34 -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=V+P/b1PkJT480Ym7y0eioeLJj2MS8+A7MRwdCmqsWe0=; b=iPjibrta/LS1jKDF4EHW3KqtJNwbP9LGIoxQrArv4m28EhHKlaWdH1EVD2bdnNSzQr T+7u3wXZt+GsWAZrfZ5JADroTS07oMeoBjsdBrhM9y9XIkue8618yMLtYs/Zqf8RQKpc uem7t0zA3ScbzWEFXM+2hJ4hKNc0QbCvD7e/w8ReK2ZpxX6KJqAf/RYVVqHH03Xsw+Bf eeWwOSJJHxWAzmaOJIMlIa8zbnTplJbluYtCdnYtGiDKJ+HogILdT1FLMtHj9uvGTUiw pwbeK/NYTUOZO00Fbhv+MSEe8Yz5Nu9jjsvlg2JYkdxDV4jyW+U+A8mEwJvUwZHLEc8Z JphA== 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=V+P/b1PkJT480Ym7y0eioeLJj2MS8+A7MRwdCmqsWe0=; b=YTsE2D37jolVlLpq2HqOUZ0WF97F2PghsW2Syw9V2Po1YNL+SdtlZNFhruDl9o3XB/ mY7r9qCj9V1zDq/V7Ub+rLpAcOEsGN2zxCJvL5MYo7zqWIU50APJnCJirWMzdKN9rvpb CmNqZYllmQxtB817spPn2Zy5ggomyGEFPV5aPU0+9GId3aNE1mreUgCu10xUT1+goROW fDevULwNAU8LmhG+ITy8r9wc5Coe3qWP4d1p1Cj35uG3ektRV5Ir2qdzKk2qY+mj6ahm SSpMvAAJqD/itBoty8S9dn3fcI/7j/RZ8n3P7TuxYcSHsngLaT5mEvdAE5Mj4TQbSGqj zRGw== X-Gm-Message-State: AOAM531axW+oDa3N52qYQNpG+qRpCu5ngyEAvJ+clhHkh+yDKG+L1PJO twVFr5uzIDdzld6RZpvqBG96WNwazamH9v9W X-Google-Smtp-Source: ABdhPJzMntizgzKfyFPeB9/V33A4QHSD2O4xg3Y+Cs9NXxB9XdW5a9iydVHTCHJvSFlMPgB8fOn4Dg== X-Received: by 2002:a05:600c:20f:: with SMTP id 15mr2315319wmi.148.1613129854008; Fri, 12 Feb 2021 03:37:34 -0800 (PST) Received: from naush-laptop.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id v5sm10005020wro.71.2021.02.12.03.37.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Feb 2021 03:37:33 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 12 Feb 2021 11:33:10 +0000 Message-Id: <20210212113312.239076-4-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210212113312.239076-1-naush@raspberrypi.com> References: <20210212113312.239076-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/5] libcamera: delayed_controls: Remove unneeded write when starting up 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" On DelayedControls::reset(), the values retrieved from the sensor device were added to the queues with the updated flag set to true. This would cause the helper to write out the value to the device again on the first DelayedControls::applyControls() call. This is unnecessary, as the controls written are identical to what is stored in the device driver. Fix this by explicitly setting the update flag to false in DelayedControls::reset() when adding the controls to the queue. Additionally, use the Info() constructor when adding items to the queue for consistency. Signed-off-by: Naushir Patuck Fixes: 3d4b7b005911 ("libcamera: delayed_controls: Add helper for controls that apply with a delay") --- include/libcamera/internal/delayed_controls.h | 4 ++-- src/libcamera/delayed_controls.cpp | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index 564d9f2e2440..2a6a912bde10 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -43,8 +43,8 @@ private: { } - Info(const ControlValue &v) - : ControlValue(v), updated(true) + Info(const ControlValue &v, bool updated_ = true) + : ControlValue(v), updated(updated_) { } diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 5317f820b6bd..fe2606a0d9aa 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -111,7 +111,11 @@ void DelayedControls::reset() values_.clear(); for (const auto &ctrl : controls) { const ControlId *id = device_->controls().idmap().at(ctrl.first); - values_[id][0] = Info(ctrl.second); + /* + * Do not mark this control value as updated, it does not need + * to be written to to device on startup. + */ + values_[id][0] = Info(ctrl.second, false); } } @@ -126,11 +130,10 @@ void DelayedControls::reset() */ bool DelayedControls::push(const ControlList &controls) { - /* Copy state from previous frame. */ + /* Copy state from previous frame and clear the updated flag */ for (auto &ctrl : values_) { Info &info = ctrl.second[queueCount_]; - info = values_[ctrl.first][queueCount_ - 1]; - info.updated = false; + info = Info(values_[ctrl.first][queueCount_ - 1], false); } /* Update with new controls. */ @@ -150,8 +153,7 @@ bool DelayedControls::push(const ControlList &controls) Info &info = values_[id][queueCount_]; - info = control.second; - info.updated = true; + info = Info(control.second); LOG(DelayedControls, Debug) << "Queuing " << id->name() From patchwork Fri Feb 12 11:33: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: 11257 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 E3400BD160 for ; Fri, 12 Feb 2021 11:37:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A568F63778; Fri, 12 Feb 2021 12:37:38 +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="PyDK2PjH"; 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 7493663773 for ; Fri, 12 Feb 2021 12:37:35 +0100 (CET) Received: by mail-wm1-x329.google.com with SMTP id o15so604784wmq.5 for ; Fri, 12 Feb 2021 03:37:35 -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=5vjj4cYR7d7qVH47wyq/bjJgcWDE/QbUdq7aNrrkbX8=; b=PyDK2PjHBFcyuFWMjuMBkiw5hiuCNMDxoGMCB5/f4mZT6ePPqqyD2oMHYB8lmzws5T AeFrm+2F/m+cBp72J22mQgvwQIJNOz0g/7Uzb1o1onS/uRtlrUaZmSz7hJpLDzH7b1mQ A4v42vtSCVETqG4pQoon5AtSHf9HW93jZj9wA7SuakLyKzE1NSrKpt03HBg1oHon8vhw pwxn5P80I8KXi7XmUoVcJvu24MhkKcKwhoLjyE6gQ/GKegPObwLsDWuYoK3cXClI7FMJ EzcWc2gpC9M6GxQgxEJ4bTuMPsfg3K9ZfnAsLMJ0l0d7ttJ8A4zu/zAPiCKsuBA3tPpj /ESA== 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=5vjj4cYR7d7qVH47wyq/bjJgcWDE/QbUdq7aNrrkbX8=; b=Ox+/qmhw5AxRFU/2xJ906CUY0zilqrW5oN7iaJUONR8DimdHjA+/pG3Yco3hHQE3KX 6k+Z+PTFtJwZPEPhnIk5qIiGY9ErVtojp6h/VpbgpVybTxgVGh1+Ldzz3P4iKIsM+8VD tl0mBj4t2dAnFOwDP7DBwPhhT3jrSDblai1JMdW7ChcRrlF80p9wzj5QiGGBqvxPR2HH uk3HTBI5ptpwnQPov1k4J2mQHJwj4zD2jy3jqqdT2u7hMSDSj1UgO/at5TWvKQfIzZ+E Twh1jlbx/X2xwG2J5QPhZ50F6VLr2mIKLmIdE2OYv4e9s/n7nje2PpKCM+dw4KpQ4hEP h47A== X-Gm-Message-State: AOAM532hI39m3KsfIhHnwoG32WnKwwDBzqAp1cU6Z3yOuq/OqZAQmBdG LCPgaKo+MWf456YcgchnNg2bjkqmll5aDPbU X-Google-Smtp-Source: ABdhPJzGL91ePRhsQRQ1+yUVeIoAw317B2SQ3tVdwiC94nx5xAsPGvP9hRchjMm6rRjYKCGNyN8ewQ== X-Received: by 2002:a05:600c:4242:: with SMTP id r2mr2247020wmm.109.1613129854894; Fri, 12 Feb 2021 03:37:34 -0800 (PST) Received: from naush-laptop.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id v5sm10005020wro.71.2021.02.12.03.37.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Feb 2021 03:37:34 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 12 Feb 2021 11:33:11 +0000 Message-Id: <20210212113312.239076-5-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210212113312.239076-1-naush@raspberrypi.com> References: <20210212113312.239076-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 4/5] libcamera: delayed_controls: Remove spurious no-op queued 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" In DelayedControls::applyControls(), the controls queue would be extended via a no-op control push to fill the intermittent slots with unchanged control values. This is needed so that we read back unchanged control values correctly on every frame. However, there was one additional no-op performed on every frame that is not required, meaning that any controls queued by the pipeline handler would have their write delayed by one further frame. The original StaggeredCtrl did not do this, as it only had one index to manage, whereas DelayedControls uses two. Remove this last no-op push so that the pipeline_handler provided controls would be written on the very next frame if possible. Signed-off-by: Naushir Patuck Reported-by: David Plowman Fixes: 3d4b7b005911 ("libcamera: delayed_controls: Add helper for controls that apply with a delay") --- src/libcamera/delayed_controls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index fe2606a0d9aa..f140b4b562e7 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -257,7 +257,7 @@ void DelayedControls::applyControls(uint32_t sequence) writeCount_++; - while (writeCount_ >= queueCount_) { + while (writeCount_ > queueCount_) { LOG(DelayedControls, Debug) << "Queue is empty, auto queue no-op."; push({}); From patchwork Fri Feb 12 11:33:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11258 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 50C14BD160 for ; Fri, 12 Feb 2021 11:37:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1DB8363784; Fri, 12 Feb 2021 12:37:40 +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="BQl8dwfD"; dkim-atps=neutral Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CD0666376F for ; Fri, 12 Feb 2021 12:37:36 +0100 (CET) Received: by mail-wm1-x330.google.com with SMTP id n10so663744wmq.0 for ; Fri, 12 Feb 2021 03:37:36 -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=Ly/7+R5SplSQlPy0AIFb6Vzt1EE1ot3TspwZ1TuICqY=; b=BQl8dwfDmq2qyTo5td5uEqE94qn4p/CcXVKw0TvXZL+wDMllvwGYrTcnsNUOfCy8kc weacSp1Z8yQMts3t4p6aDtQUbS4DwNOvhD87EldGE/1c33sM4SzLpS6Sy5fjMULGgazJ SKOSfC2smocsnPkfycAmfbdqrlBg2jir841YR0tYHhEYAZOr6ZhogsQSb9KH4PDn5Ljd BRilGZGWm8bIRMB31J9QQzDJLKcgdzYVOCJTJYgINIC8tQPrlvyGgMj+FaWuWdoJYGl1 3NF/AoQwYdNKZHp1JxOUnbCkWkh9TWjf3jWDpE6Ckso7Pqd6V1prih9uFGeO7faWQouP B2Rw== 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=Ly/7+R5SplSQlPy0AIFb6Vzt1EE1ot3TspwZ1TuICqY=; b=tuvTRDYasMHaB6SpV4xMBp/CiNNqTcycRIcsQZnJLl1W//FVi+4snmyMvyvby/TxnK xJtwhESNk/ZvAOqZ+TJU8U6ICGO4WMCiofg9staZX3dmrbqkHomd95lhmESrKoCkdLKq PD4J1ib/Ektjwe/b+yGM60twf7ztrQs1LhSIOEeuGpsFcaQMRbCXVXVc9bkB44XfCY9J n9Ewx8VY6FaqwODmBiFEkV9DhL5Ewfh9m2a8Uw/T2GMGVo7/lS6qZXdaq0L8ZnWxU/nu jzzpsjpOuPLeklHDsbyyu63UmTtf5gPzQtIO8KGb5vYEjI0FT0nvQmirR3G1M5QizN74 oxxw== X-Gm-Message-State: AOAM532gFGZPwVQSRZ03wRE51T0y3wQUElydRZF9LjwW/ztPsvKsL/+X kKnRoOTMEMJTFRgKo0Idrxj+wejaT3z1yQSw X-Google-Smtp-Source: ABdhPJy1ouv9hv+WL+pmdfFV6/EPul2ehMB19K1PkDKJHaedAtBgdOMP1shfHvHDpp+i8HH3SzvZRg== X-Received: by 2002:a7b:cf19:: with SMTP id l25mr2173517wmg.85.1613129856272; Fri, 12 Feb 2021 03:37:36 -0800 (PST) Received: from naush-laptop.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id v5sm10005020wro.71.2021.02.12.03.37.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Feb 2021 03:37:35 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 12 Feb 2021 11:33:12 +0000 Message-Id: <20210212113312.239076-6-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210212113312.239076-1-naush@raspberrypi.com> References: <20210212113312.239076-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 5/5] libcamera: delayed_controls: Fix off-by-one error in get() 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" There was an off-by-one error in DelayedControls::get() when picking controls from the queue to return back to the pipeline handler. This is only noticeable as small oscillations in brightness when closely viewing frame while AGC is running. The old StaggeredCtrl did not show this error as the startup queuing mechanism has changed in DelayedControls. Fix this by indexing to the correct position in the queue. Signed-off-by: Naushir Patuck Reported-by: David Plowman Fixes: 3d4b7b005911 ("libcamera: delayed_controls: Add helper for controls that apply with a delay") --- src/libcamera/delayed_controls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index f140b4b562e7..9d0d17372546 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -183,7 +183,7 @@ bool DelayedControls::push(const ControlList &controls) */ ControlList DelayedControls::get(uint32_t sequence) { - uint32_t adjustedSeq = sequence - firstSequence_ + 1; + uint32_t adjustedSeq = sequence - firstSequence_; unsigned int index = std::max(0, adjustedSeq - maxDelay_); ControlList out(device_->controls());