From patchwork Tue Mar 19 12:05:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19741 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 7FB24BD160 for ; Tue, 19 Mar 2024 12:05:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AD90F62CAE; Tue, 19 Mar 2024 13:05:30 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="N+FfNjSJ"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C9E4A62CA4 for ; Tue, 19 Mar 2024 13:05:27 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E62FDF02; Tue, 19 Mar 2024 13:05:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849901; bh=XqL+6G8l6sAh3t0/iCiSp/QB+GBR3E9zSf2oNX9yT4E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=N+FfNjSJ6O0NZf01NpATeZ0+p70S28HBk9eygqxx+j8x8MxzzmgdJ3s6km8fGTSNt PjBepDrT5nOtL/pVtOhz6ZSCFbMoyi/tY1mZEgyatLbUt04EVWErM6RK3KvEBtcYGO CYFeIVrPJXmAsIuYTwL03aVYzoYc6lrg/TTn/JP8= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 01/16] libcamera: lc-compliance: Add controls param to start() function Date: Tue, 19 Mar 2024 13:05:02 +0100 Message-Id: <20240319120517.362082-2-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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 is needed for tests that need to pass controls to Camera::start(). Signed-off-by: Stefan Klug Reviewed-by: Jacopo Mondi --- src/apps/lc-compliance/simple_capture.cpp | 4 ++-- src/apps/lc-compliance/simple_capture.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/apps/lc-compliance/simple_capture.cpp b/src/apps/lc-compliance/simple_capture.cpp index cf4d7cf3..56680a83 100644 --- a/src/apps/lc-compliance/simple_capture.cpp +++ b/src/apps/lc-compliance/simple_capture.cpp @@ -42,7 +42,7 @@ void SimpleCapture::configure(StreamRole role) } } -void SimpleCapture::start() +void SimpleCapture::start(const ControlList *controls) { Stream *stream = config_->at(0).stream(); int count = allocator_->allocate(stream); @@ -52,7 +52,7 @@ void SimpleCapture::start() camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete); - ASSERT_EQ(camera_->start(), 0) << "Failed to start camera"; + ASSERT_EQ(camera_->start(controls), 0) << "Failed to start camera"; } void SimpleCapture::stop() diff --git a/src/apps/lc-compliance/simple_capture.h b/src/apps/lc-compliance/simple_capture.h index 2911d601..54b1d54b 100644 --- a/src/apps/lc-compliance/simple_capture.h +++ b/src/apps/lc-compliance/simple_capture.h @@ -22,7 +22,7 @@ protected: SimpleCapture(std::shared_ptr camera); virtual ~SimpleCapture(); - void start(); + void start(const libcamera::ControlList *controls = nullptr); void stop(); virtual void requestComplete(libcamera::Request *request) = 0; From patchwork Tue Mar 19 12:05:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19742 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 7BA72C3274 for ; Tue, 19 Mar 2024 12:05:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 50F1862D35; Tue, 19 Mar 2024 13:05:31 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="OzjbpHBf"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E399362CA7 for ; Tue, 19 Mar 2024 13:05:27 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3201716F9; Tue, 19 Mar 2024 13:05:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849901; bh=sum0LNiyBWcI02Cm1czo0s5fVuD5BqSZAwIRpFzcYm8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OzjbpHBfB1TDaV0FOBeuw1qT8thNb25Mrs8D8Cr++mnEX743epAs/TWMY5Cn7Zy9d Ul+RR4oidHlN7Bd03dSOlyNnEHLgcms9hP/voOwjXVqwWaL6yhJ3pDiFMrfu6IIBnl FSh2LCdDEJDQsQO4x4Arp7nOZAltGFV1H1AHnDWc= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 02/16] libcamera: lc-compliance: Add TimeSheet class Date: Tue, 19 Mar 2024 13:05:03 +0100 Message-Id: <20240319120517.362082-3-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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 class allows us to prepare a time sheet with controls for the frames to capture (a bit like the capture script in cam). During capture the metadata is recorded. Additionally a mean brightness value of a 20x20 pixel spot in the middle of the frame is calculated. This allows easy analysis after running the capture without complicated state handling due to the asynchronous nature of the capturing process. Signed-off-by: Stefan Klug --- src/apps/lc-compliance/meson.build | 1 + src/apps/lc-compliance/time_sheet.cpp | 148 ++++++++++++++++++++++++++ src/apps/lc-compliance/time_sheet.h | 62 +++++++++++ 3 files changed, 211 insertions(+) create mode 100644 src/apps/lc-compliance/time_sheet.cpp create mode 100644 src/apps/lc-compliance/time_sheet.h diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build index c792f072..eb7b2d71 100644 --- a/src/apps/lc-compliance/meson.build +++ b/src/apps/lc-compliance/meson.build @@ -16,6 +16,7 @@ lc_compliance_sources = files([ 'environment.cpp', 'main.cpp', 'simple_capture.cpp', + 'time_sheet.cpp', ]) lc_compliance = executable('lc-compliance', lc_compliance_sources, diff --git a/src/apps/lc-compliance/time_sheet.cpp b/src/apps/lc-compliance/time_sheet.cpp new file mode 100644 index 00000000..8048cf30 --- /dev/null +++ b/src/apps/lc-compliance/time_sheet.cpp @@ -0,0 +1,148 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Ideas on Board Oy + * + * time_sheet.cpp + */ + +#include "time_sheet.h" + +#include +#include + +#include "libcamera/internal/formats.h" +#include "libcamera/internal/mapped_framebuffer.h" + +using namespace libcamera; + +namespace { + +double calculateMeanBrightnessFromCenterSpot(libcamera::Request *request) +{ + const Request::BufferMap &buffers = request->buffers(); + for (const auto &[stream, buffer] : buffers) { + MappedFrameBuffer in(buffer, MappedFrameBuffer::MapFlag::Read); + if (!in.isValid()) + continue; + + const uint8_t *data = in.planes()[0].data(); + const auto &streamConfig = stream->configuration(); + const auto &formatInfo = PixelFormatInfo::info(streamConfig.pixelFormat); + double (*calcPixelMean)(const uint8_t *); + int pixelStride; + + switch (streamConfig.pixelFormat) { + case formats::NV12: + calcPixelMean = [](const uint8_t *d) -> double { + return static_cast(*d); + }; + pixelStride = 1; + break; + case formats::SRGGB10: + calcPixelMean = [](const uint8_t *d) -> double { + return static_cast(*reinterpret_cast(d)); + }; + pixelStride = 2; + break; + default: + std::stringstream s; + s << "Unsupported Pixelformat " << formatInfo.name; + throw std::invalid_argument(s.str()); + } + + double sum = 0; + int w = 20; + int xs = streamConfig.size.width / 2 - w / 2; + int ys = streamConfig.size.height / 2 - w / 2; + + for (auto y = ys; y < ys + w; y++) { + auto line = data + y * streamConfig.stride; + for (auto x = xs; x < xs + w; x++) + sum += calcPixelMean(line + x * pixelStride); + } + sum = sum / (w * w); + return sum; + } + + return 0; +} + +} /* namespace */ + +TimeSheetEntry::TimeSheetEntry(const ControlIdMap &idmap) + : controls_(idmap) +{ +} + +void TimeSheetEntry::handleCompleteRequest(libcamera::Request *request, + const TimeSheetEntry *previous) +{ + metadata_ = request->metadata(); + + spotBrightness_ = calculateMeanBrightnessFromCenterSpot(request); + if (previous) + brightnessChange_ = spotBrightness_ / previous->spotBrightness(); + + sequence_ = request->sequence(); +} + +std::ostream &operator<<(std::ostream &os, const TimeSheetEntry &te) +{ + os << "=== Frame " << te.sequence_ << std::endl; + if (!te.controls_.empty()) { + os << "Controls:" << std::endl; + const auto &idMap = te.controls_.idMap(); + assert(idMap); + for (const auto &[id, value] : te.controls_) + os << " " << idMap->at(id)->name() << " : " + << value.toString() << std::endl; + } + + if (!te.metadata_.empty()) { + os << "Metadata:" << std::endl; + const auto &idMap = te.metadata_.idMap(); + assert(idMap); + for (const auto &[id, value] : te.metadata_) + os << " " << idMap->at(id)->name() << " : " + << value.toString() << std::endl; + } + + os << "Calculated Brightness: " << te.spotBrightness() << std::endl; + return os; +} + +TimeSheetEntry &TimeSheet::get(size_t pos) +{ + entries_.reserve(pos + 1); + auto &entry = entries_[pos]; + if (!entry) + entry = std::make_unique(idmap_); + + return *entry; +} + +void TimeSheet::prepareForQueue(libcamera::Request *request, uint32_t sequence) +{ + request->controls() = get(sequence).controls(); +} + +void TimeSheet::handleCompleteRequest(libcamera::Request *request) +{ + uint32_t sequence = request->sequence(); + auto &entry = get(sequence); + TimeSheetEntry *previous = nullptr; + if (sequence >= 1) + previous = entries_[sequence - 1].get(); + + entry.handleCompleteRequest(request, previous); +} + +std::ostream &operator<<(std::ostream &os, const TimeSheet &ts) +{ + for (const auto &entry : ts.entries_) { + if (entry) + os << *entry; + } + + return os; +} diff --git a/src/apps/lc-compliance/time_sheet.h b/src/apps/lc-compliance/time_sheet.h new file mode 100644 index 00000000..417277aa --- /dev/null +++ b/src/apps/lc-compliance/time_sheet.h @@ -0,0 +1,62 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Ideas on Board Oy + * + * time_sheet.h + */ + +#pragma once + +#include +#include +#include + +#include + +class TimeSheetEntry +{ +public: + TimeSheetEntry() = delete; + TimeSheetEntry(const libcamera::ControlIdMap &idmap); + TimeSheetEntry(TimeSheetEntry &&other) = default; + TimeSheetEntry(const TimeSheetEntry &) = delete; + ~TimeSheetEntry() = default; + + libcamera::ControlList &controls() { return controls_; } + const libcamera::ControlList &metadata() const { return metadata_; } + void handleCompleteRequest(libcamera::Request *request, + const TimeSheetEntry *previous); + double spotBrightness() const { return spotBrightness_; } + double brightnessChange() const { return brightnessChange_; } + + friend std::ostream &operator<<(std::ostream &os, const TimeSheetEntry &te); + +private: + double spotBrightness_ = 0.0; + double brightnessChange_ = 0.0; + libcamera::ControlList controls_; + libcamera::ControlList metadata_; + uint32_t sequence_ = 0; +}; + +class TimeSheet +{ +public: + TimeSheet(int count, const libcamera::ControlIdMap &idmap) + : idmap_(idmap), entries_(count) + { + } + + void prepareForQueue(libcamera::Request *request, uint32_t sequence); + void handleCompleteRequest(libcamera::Request *request); + + TimeSheetEntry &operator[](size_t pos) { return get(pos); } + TimeSheetEntry &get(size_t pos); + size_t size() const { return entries_.size(); } + + friend std::ostream &operator<<(std::ostream &os, const TimeSheet &ts); + +private: + const libcamera::ControlIdMap &idmap_; + std::vector> entries_; +}; From patchwork Tue Mar 19 12:05:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19743 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 C9A62BD160 for ; Tue, 19 Mar 2024 12:05:37 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EC06662CAB; Tue, 19 Mar 2024 13:05:31 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="k4Xvmih+"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 20A6962973 for ; Tue, 19 Mar 2024 13:05:28 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6F44B480; Tue, 19 Mar 2024 13:05:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849901; bh=YWcx60Xes0BHpk2TegmXEa7+lGhzQhQJfS3TcDAV8iI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=k4Xvmih+w1jxLZBhr7oQwpNK1S3UfkuD236IJMWPnuBQrerhUebHDnpKfVMyCAjIK 5ZBrfZXxDXu7qkrtMaXNw67tNY0qirT5gRv4f314Uq0m47A08rKSUhm0tDRQ6e6VPj /GJNhURw5PneCB/1ptk+ZSXGpmm+iNCEZva/X2Mg= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 03/16] libcamera: lc-compliance: Add initial set of per-frame-control tests Date: Tue, 19 Mar 2024 13:05:04 +0100 Message-Id: <20240319120517.362082-4-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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" Add tests that check if controls (only exposure time and analogue gain at the moment) get applied on the frame they were requested for. This is tested by looking at the metadata and the mean brightness of the image center. At the moment these tests fail. Fixes for the pipelines will be delivered in later patches (rkisp1 for now). To run the pfc tests only: lc-compliance -c -f "PerFrameControlTests.*" Note that the current implementation is a bit picky on what the camera actually sees. If it is too dark (or too bright), the tests will fail. Looking at a white wall in a normally lit office usually works. These tests are known to pass using a imx219 (RPi cam v2) with a imx8mp (debix-som) using the rkisp1 pipeline Signed-off-by: Stefan Klug --- src/apps/lc-compliance/meson.build | 1 + .../lc-compliance/per_frame_controls_test.cpp | 398 ++++++++++++++++++ 2 files changed, 399 insertions(+) create mode 100644 src/apps/lc-compliance/per_frame_controls_test.cpp diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build index eb7b2d71..8f4eec55 100644 --- a/src/apps/lc-compliance/meson.build +++ b/src/apps/lc-compliance/meson.build @@ -15,6 +15,7 @@ lc_compliance_sources = files([ 'capture_test.cpp', 'environment.cpp', 'main.cpp', + 'per_frame_controls_test.cpp', 'simple_capture.cpp', 'time_sheet.cpp', ]) diff --git a/src/apps/lc-compliance/per_frame_controls_test.cpp b/src/apps/lc-compliance/per_frame_controls_test.cpp new file mode 100644 index 00000000..017e8d60 --- /dev/null +++ b/src/apps/lc-compliance/per_frame_controls_test.cpp @@ -0,0 +1,398 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2024, Ideas on Board Oy + * + * per_frame_controls.cpp - Tests for per frame controls + */ +#include + +#include "environment.h" +#include "simple_capture.h" +#include "time_sheet.h" + +using namespace libcamera; + +class PerFrameControlTests : public testing::Test +{ +protected: + void SetUp() override; + void TearDown() override; + + std::shared_ptr camera_; +}; + +/* + * We use gtest's SetUp() and TearDown() instead of constructor and destructor + * in order to be able to assert on them. + */ +void PerFrameControlTests::SetUp() +{ + Environment *env = Environment::get(); + + camera_ = env->cm()->get(env->cameraId()); + + ASSERT_EQ(camera_->acquire(), 0); +} + +void PerFrameControlTests::TearDown() +{ + if (!camera_) + return; + + camera_->release(); + camera_.reset(); +} + +class PerFrameControlsCapture : public SimpleCapture +{ +public: + PerFrameControlsCapture(std::shared_ptr camera); + + std::shared_ptr + startCaptureWithTimeSheet(unsigned int framesToCapture, + const libcamera::ControlList *controls = nullptr); + + void runCaptureSession(); + int queueRequest(libcamera::Request *request); + void requestComplete(libcamera::Request *request) override; + + unsigned int queueCount_; + unsigned int captureCount_; + unsigned int captureLimit_; + + std::weak_ptr timeSheet_; +}; + +static const bool doImageTests = true; + +PerFrameControlsCapture::PerFrameControlsCapture(std::shared_ptr camera) + : SimpleCapture(camera) +{ +} + +std::shared_ptr +PerFrameControlsCapture::startCaptureWithTimeSheet(unsigned int framesToCapture, + const ControlList *controls) +{ + ControlList ctrls(camera_->controls().idmap()); + + /* Ensure defined default values */ + ctrls.set(controls::AeEnable, false); + ctrls.set(controls::AeExposureMode, controls::ExposureCustom); + ctrls.set(controls::ExposureTime, 10000); + ctrls.set(controls::AnalogueGain, 1.0); + + if (controls) + ctrls.merge(*controls, ControlList::MergePolicy::OverwriteExisting); + + start(&ctrls); + + queueCount_ = 0; + captureCount_ = 0; + captureLimit_ = framesToCapture; + + auto timeSheet = std::make_shared(captureLimit_, + camera_->controls().idmap()); + timeSheet_ = timeSheet; + return timeSheet; +} + +int PerFrameControlsCapture::queueRequest(Request *request) +{ + queueCount_++; + if (queueCount_ > captureLimit_) + return 0; + + auto ts = timeSheet_.lock(); + if (ts) + ts->prepareForQueue(request, queueCount_ - 1); + + return camera_->queueRequest(request); +} + +void PerFrameControlsCapture::requestComplete(Request *request) +{ + auto ts = timeSheet_.lock(); + if (ts) + ts->handleCompleteRequest(request); + + captureCount_++; + if (captureCount_ >= captureLimit_) { + loop_->exit(0); + return; + } + + request->reuse(Request::ReuseBuffers); + if (queueRequest(request)) + loop_->exit(-EINVAL); +} + +void PerFrameControlsCapture::runCaptureSession() +{ + Stream *stream = config_->at(0).stream(); + const std::vector> &buffers = allocator_->buffers(stream); + + /* Queue the recommended number of requests. */ + for (const std::unique_ptr &buffer : buffers) { + std::unique_ptr request = camera_->createRequest(); + request->addBuffer(stream, buffer.get()); + queueRequest(request.get()); + requests_.push_back(std::move(request)); + } + + /* Run capture session. */ + loop_ = new EventLoop(); + loop_->exec(); + stop(); + delete loop_; +} + +TEST_F(PerFrameControlTests, testExposureGainChangeOnSameFrame) +{ + PerFrameControlsCapture capture(camera_); + capture.configure(StreamRole::VideoRecording); + + ControlList startValues; + startValues.set(controls::ExposureTime, 5000); + startValues.set(controls::AnalogueGain, 1.0); + + auto timeSheet = capture.startCaptureWithTimeSheet(10, &startValues); + auto &ts = *timeSheet; + + /* wait a few frames to settle */ + ts[7].controls().set(controls::ExposureTime, 10000); + ts[7].controls().set(controls::AnalogueGain, 4.0); + + capture.runCaptureSession(); + + ASSERT_TRUE(ts[5].metadata().contains(controls::ExposureTime.id())) + << "Required metadata entry is missing"; + ASSERT_TRUE(ts[5].metadata().contains(controls::AnalogueGain.id())) + << "Required metadata entry is missing"; + + EXPECT_NEAR(ts[3].metadata().get(controls::ExposureTime).value(), 5000, 20); + EXPECT_NEAR(ts[3].metadata().get(controls::AnalogueGain).value(), 1.0, 0.05); + + /* find the frame with the changes */ + int exposureChangeIndex = 0; + for (unsigned i = 3; i < ts.size(); i++) { + if (ts[i].metadata().get(controls::ExposureTime).value() > 7500) { + exposureChangeIndex = i; + break; + } + } + + int gainChangeIndex = 0; + for (unsigned i = 3; i < ts.size(); i++) { + if (ts[i].metadata().get(controls::AnalogueGain).value() > 2.0) { + gainChangeIndex = i; + break; + } + } + + EXPECT_NE(exposureChangeIndex, 0) << "Exposure change not found in metadata"; + EXPECT_NE(gainChangeIndex, 0) << "Gain change not found in metadata"; + EXPECT_EQ(exposureChangeIndex, gainChangeIndex) + << "Metadata contained gain and exposure changes on different frames"; + + if (doImageTests) { + int brightnessChangeIndex = 0; + for (unsigned i = 3; i < ts.size(); i++) { + if (ts[i].brightnessChange() > 1.3) { + EXPECT_EQ(brightnessChangeIndex, 0) + << "Detected multiple frames with brightness increase" + << " (Wrong control delays?)"; + + if (!brightnessChangeIndex) + brightnessChangeIndex = i; + } + } + + EXPECT_EQ(exposureChangeIndex, brightnessChangeIndex) + << "Exposure change and measured brightness change were not on same" + << " frame. (Wrong control delay?, Start frame event too late?)"; + EXPECT_EQ(exposureChangeIndex, gainChangeIndex) + << "Gain change and measured brightness change were not on same " + << " frame. (Wrong control delay?, Start frame event too late?)"; + } +} + +TEST_F(PerFrameControlTests, testFramePreciseExposureChange) +{ + PerFrameControlsCapture capture(camera_); + capture.configure(StreamRole::VideoRecording); + + auto timeSheet = capture.startCaptureWithTimeSheet(10); + auto &ts = *timeSheet; + + ts[3].controls().set(controls::ExposureTime, 5000); + /* wait a few frames to settle */ + ts[6].controls().set(controls::ExposureTime, 20000); + + capture.runCaptureSession(); + + ASSERT_TRUE(ts[5].metadata().contains(controls::ExposureTime.id())) + << "Required metadata entry is missing"; + + EXPECT_NEAR(ts[5].metadata().get(controls::ExposureTime).value(), 5000, 20); + EXPECT_NEAR(ts[6].metadata().get(controls::ExposureTime).value(), 20000, 20); + + if (doImageTests) { + /* No increase just before setting exposure */ + EXPECT_NEAR(ts[5].brightnessChange(), 1.0, 0.05) + << "Brightness changed too much before the expected time of change" + << " (control delay too high?)."; + /* + * \todo The change is brightness was a bit low + * (Exposure time increase by 4x resulted in a brightness increase of < 2). + * This should be investigated. + */ + EXPECT_GT(ts[6].brightnessChange(), 1.3) + << "Brightness in frame " << 6 << " did not increase as expected" + << " (reference: " << ts[3].spotBrightness() << " current: " + << ts[6].spotBrightness() << " )" << std::endl; + + /* No increase just after setting exposure */ + EXPECT_NEAR(ts[7].brightnessChange(), 1.0, 0.05) + << "Brightness changed too much after the expected time of change" + << " (control delay too low?)."; + + /* No increase just after setting exposure */ + EXPECT_NEAR(ts[8].brightnessChange(), 1.0, 0.05) + << "Brightness changed too much 2 frames after the expected time" + << " of change (control delay too low?)."; + } +} + +TEST_F(PerFrameControlTests, testFramePreciseGainChange) +{ + PerFrameControlsCapture capture(camera_); + capture.configure(StreamRole::VideoRecording); + + auto timeSheet = capture.startCaptureWithTimeSheet(10); + auto &ts = *timeSheet; + + ts[3].controls().set(controls::AnalogueGain, 1.0); + /* wait a few frames to settle */ + ts[6].controls().set(controls::AnalogueGain, 4.0); + + capture.runCaptureSession(); + + ASSERT_TRUE(ts[5].metadata().contains(controls::AnalogueGain.id())) + << "Required metadata entry is missing"; + + EXPECT_NEAR(ts[5].metadata().get(controls::AnalogueGain).value(), 1.0, 0.1); + EXPECT_NEAR(ts[6].metadata().get(controls::AnalogueGain).value(), 4.0, 0.1); + + if (doImageTests) { + /* No increase just before setting gain */ + EXPECT_NEAR(ts[5].brightnessChange(), 1.0, 0.05) + << "Brightness changed too much before the expected time of change" + << " (control delay too high?)."; + /* + * \todo I see a brightness change of roughly half the expected one. + * This is not yet understood and needs investigation + */ + EXPECT_GT(ts[6].brightnessChange(), 1.7) + << "Brightness in frame " << 6 << " did not increase as expected" + << " (reference: " << ts[5].spotBrightness() + << " current: " << ts[6].spotBrightness() << ")" << std::endl; + + /* No increase just after setting gain */ + EXPECT_NEAR(ts[7].brightnessChange(), 1.0, 0.05) + << "Brightness changed too much after the expected time of change" + << " (control delay too low?)."; + + /* No increase just after setting gain */ + EXPECT_NEAR(ts[8].brightnessChange(), 1.0, 0.05) + << "Brightness changed too much after the expected time of change" + << " (control delay too low?)."; + } +} + +TEST_F(PerFrameControlTests, testExposureGainFromFirstRequestGetsApplied) +{ + PerFrameControlsCapture capture(camera_); + capture.configure(StreamRole::VideoRecording); + + auto timeSheet = capture.startCaptureWithTimeSheet(5); + auto &ts = *timeSheet; + + ts[0].controls().set(controls::ExposureTime, 10000); + ts[0].controls().set(controls::AnalogueGain, 4.0); + + capture.runCaptureSession(); + + ASSERT_TRUE(ts[4].metadata().contains(controls::ExposureTime.id())) + << "Required metadata entry is missing"; + ASSERT_TRUE(ts[4].metadata().contains(controls::AnalogueGain.id())) + << "Required metadata entry is missing"; + + /* We expect it to be applied after 3 frames, the latest*/ + EXPECT_NEAR(ts[4].metadata().get(controls::ExposureTime).value(), 10000, 20); + EXPECT_NEAR(ts[4].metadata().get(controls::AnalogueGain).value(), 4.0, 0.1); +} + +TEST_F(PerFrameControlTests, testExposureGainFromFirstAndSecondRequestGetsApplied) +{ + PerFrameControlsCapture capture(camera_); + capture.configure(StreamRole::VideoRecording); + + auto timeSheet = capture.startCaptureWithTimeSheet(5); + auto &ts = *timeSheet; + + ts[0].controls().set(controls::ExposureTime, 8000); + ts[0].controls().set(controls::AnalogueGain, 2.0); + ts[1].controls().set(controls::ExposureTime, 10000); + ts[1].controls().set(controls::AnalogueGain, 4.0); + + capture.runCaptureSession(); + + ASSERT_TRUE(ts[4].metadata().contains(controls::ExposureTime.id())) + << "Required metadata entry is missing"; + ASSERT_TRUE(ts[4].metadata().contains(controls::AnalogueGain.id())) + << "Required metadata entry is missing"; + + /* We expect it to be applied after 3 frames, the latest */ + EXPECT_NEAR(ts[4].metadata().get(controls::ExposureTime).value(), 10000, 20); + EXPECT_NEAR(ts[4].metadata().get(controls::AnalogueGain).value(), 4.0, 0.1); +} + +TEST_F(PerFrameControlTests, testExposureGainIsAppliedOnFirstFrame) +{ + PerFrameControlsCapture capture(camera_); + capture.configure(StreamRole::VideoRecording); + + ControlList startValues; + startValues.set(controls::ExposureTime, 5000); + startValues.set(controls::AnalogueGain, 1.0); + + auto ts1 = capture.startCaptureWithTimeSheet(3, &startValues); + + capture.runCaptureSession(); + + ASSERT_TRUE((*ts1)[0].metadata().contains(controls::ExposureTime.id())) + << "Required metadata entry is missing"; + ASSERT_TRUE((*ts1)[0].metadata().contains(controls::AnalogueGain.id())) + << "Required metadata entry is missing"; + + EXPECT_NEAR((*ts1)[0].metadata().get(controls::ExposureTime).value(), 5000, 20); + EXPECT_NEAR((*ts1)[0].metadata().get(controls::AnalogueGain).value(), 1.0, 0.02); + + /* Second capture with different values to ensure we don't hit default/old values */ + startValues.set(controls::ExposureTime, 15000); + startValues.set(controls::AnalogueGain, 4.0); + + auto ts2 = capture.startCaptureWithTimeSheet(3, &startValues); + + capture.runCaptureSession(); + + EXPECT_NEAR((*ts2)[0].metadata().get(controls::ExposureTime).value(), 15000, 20); + EXPECT_NEAR((*ts2)[0].metadata().get(controls::AnalogueGain).value(), 4.0, 0.02); + + if (doImageTests) { + /* With 3x exposure and 4x gain we could expect a brightness increase of 2x */ + double brightnessChange = ts2->get(1).spotBrightness() / ts1->get(1).spotBrightness(); + EXPECT_GT(brightnessChange, 2.0); + } +} From patchwork Tue Mar 19 12:05:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19744 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 BBFE7C32C3 for ; Tue, 19 Mar 2024 12:05:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 896FF62CAD; Tue, 19 Mar 2024 13:05:32 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="LR+zJ+Jm"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C12962CAA for ; Tue, 19 Mar 2024 13:05:28 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AEF61F02; Tue, 19 Mar 2024 13:05:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849901; bh=3VNlfSy6nh7wgteyGedv5lEr64KZZ2UQ7/gy8dme1p4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LR+zJ+JmCQzDfWj8P2u/HEh3qNG4CJfye8DECGkye62fbUYknqmCPBaAwlzZ/p+Kk GyZzeKTDG6sJNmTNFXNWIcJpygXq1tWSojDkF2ouA3BdPD0UUHP/FncytTEZ4D1uSe UdhHKGk75Lq2dbha5WcULOLKplmDsCBp6VAcDvIg= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 04/16] libcamera: delayed_controls: Update unit tests to expected semantics Date: Tue, 19 Mar 2024 13:05:05 +0100 Message-Id: <20240319120517.362082-5-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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 patches changes the tests in a way DelayedControls is expected to work. It does not require a interface change in DelayedControls. The changes are: - It is expected that for every apply() there was a previous push(). - When a control is pushed for frame 0, that request is not lost, but applied as soon as possible Signed-off-by: Stefan Klug --- test/delayed_controls.cpp | 256 +++++++++++++++++++++++++++++++------- 1 file changed, 209 insertions(+), 47 deletions(-) diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp index a8ce9828..fe183de5 100644 --- a/test/delayed_controls.cpp +++ b/test/delayed_controls.cpp @@ -84,11 +84,8 @@ protected: dev_->setControls(&ctrls); delayed->reset(); - /* Trigger the first frame start event */ - delayed->applyControls(0); - /* Test control without delay are set at once. */ - for (unsigned int i = 1; i < 100; i++) { + for (unsigned int i = 0; i < 100; i++) { int32_t value = 100 + i; ctrls.set(V4L2_CID_BRIGHTNESS, value); @@ -121,25 +118,30 @@ protected: ControlList ctrls; /* Reset control to value that will be first in test. */ - int32_t expected = 4; - ctrls.set(V4L2_CID_BRIGHTNESS, expected); + int32_t initial = 4; + ctrls.set(V4L2_CID_BRIGHTNESS, initial); dev_->setControls(&ctrls); delayed->reset(); - /* Trigger the first frame start event */ - delayed->applyControls(0); + /* Push a request for frame 0 */ + ctrls.set(V4L2_CID_BRIGHTNESS, 10); + delayed->push(ctrls); /* Test single control with delay. */ - for (unsigned int i = 1; i < 100; i++) { + for (unsigned int i = 0; i < 100; i++) { int32_t value = 10 + i; + int32_t expected = i < 1 ? initial : value; - ctrls.set(V4L2_CID_BRIGHTNESS, value); + /* push the request for frame i+1 */ + ctrls.set(V4L2_CID_BRIGHTNESS, value + 1); delayed->push(ctrls); delayed->applyControls(i); ControlList result = delayed->get(i); int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); + ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS }); + int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get(); if (brightness != expected) { cerr << "Failed single control with delay" << " frame " << i @@ -149,7 +151,121 @@ protected: return TestFail; } - expected = value; + if (i > 0 && brightnessV4L != value + 1) { + cerr << "Failed single control with delay" + << " frame " << i + << " expected V4L " << value + 1 + << " got " << brightnessV4L + << endl; + return TestFail; + } + } + + return TestPass; + } + + /* This fails on the old delayed controls implementation */ + int singleControlWithDelayStartUp() + { + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, { 1, false } }, + }; + std::unique_ptr delayed = + std::make_unique(dev_.get(), delays); + ControlList ctrls; + + /* Reset control to value that will be first in test. */ + int32_t initial = 4; + ctrls.set(V4L2_CID_BRIGHTNESS, initial); + dev_->setControls(&ctrls); + delayed->reset(); + + /* push a request for frame 0 */ + ctrls.set(V4L2_CID_BRIGHTNESS, 10); + delayed->push(ctrls); + + /* Test single control with delay. */ + for (unsigned int i = 0; i < 100; i++) { + int32_t value = 10 + i; + int32_t expected = i < 1 ? initial : value; + + /* push the request for frame i+1 */ + ctrls.set(V4L2_CID_BRIGHTNESS, value + 1); + delayed->push(ctrls); + + delayed->applyControls(i); + + ControlList result = delayed->get(i); + int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); + ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS }); + int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get(); + + if (brightness != expected) { + cerr << "Failed single control with delay start up" + << " frame " << i + << " expected " << expected + << " got " << brightness + << endl; + return TestFail; + } + + if (i > 0 && brightnessV4L != value + 1) { + cerr << "Failed single control with delay start up" + << " frame " << i + << " expected V4L " << value + 1 + << " got " << brightnessV4L + << endl; + return TestFail; + } + } + + return TestPass; + } + + /* This fails on the old delayed controls implementation */ + int doNotLoseFirstRequest() + { + /* no delay at all */ + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, { 0, false } }, + }; + std::unique_ptr delayed = + std::make_unique(dev_.get(), delays); + ControlList ctrls; + + /* Reset control to value that will be first in test. */ + int32_t initial = 4; + ctrls.set(V4L2_CID_BRIGHTNESS, initial); + dev_->setControls(&ctrls); + delayed->reset(); + + /* push a request for frame 0 */ + int32_t expected = 10; + ctrls.set(V4L2_CID_BRIGHTNESS, expected); + delayed->push(ctrls); + delayed->applyControls(0); + + ControlList result = delayed->get(0); + int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); + ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS }); + int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get(); + + if (brightness != expected) { + cerr << "Failed doNotLoseFirstRequest" + << " frame " << 0 + << " expected " << expected + << " got " << brightness + << endl; + return TestFail; + } + + if (brightnessV4L != expected) { + cerr << "Failed doNotLoseFirstRequest" + << " frame " << 0 + << " expected V4L " << expected + << " got " << brightnessV4L + << endl; + return TestFail; } return TestPass; @@ -157,7 +273,7 @@ protected: int dualControlsWithDelay() { - static const unsigned int maxDelay = 2; + static const int maxDelay = 2; std::unordered_map delays = { { V4L2_CID_BRIGHTNESS, { 1, false } }, @@ -174,15 +290,21 @@ protected: dev_->setControls(&ctrls); delayed->reset(); - /* Trigger the first frame start event */ - delayed->applyControls(0); + /* push two requests into the queue */ + ctrls.set(V4L2_CID_BRIGHTNESS, 10); + ctrls.set(V4L2_CID_CONTRAST, 10); + delayed->push(ctrls); + ctrls.set(V4L2_CID_BRIGHTNESS, 11); + ctrls.set(V4L2_CID_CONTRAST, 11); + delayed->push(ctrls); /* Test dual control with delay. */ - for (unsigned int i = 1; i < 100; i++) { + for (unsigned int i = 0; i < 100; i++) { int32_t value = 10 + i; - ctrls.set(V4L2_CID_BRIGHTNESS, value); - ctrls.set(V4L2_CID_CONTRAST, value + 1); + /* we are pushing 2 frames ahead */ + ctrls.set(V4L2_CID_BRIGHTNESS, value + 2); + ctrls.set(V4L2_CID_CONTRAST, value + 2); delayed->push(ctrls); delayed->applyControls(i); @@ -190,17 +312,32 @@ protected: ControlList result = delayed->get(i); int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); int32_t contrast = result.get(V4L2_CID_CONTRAST).get(); - if (brightness != expected || contrast != expected + 1) { - cerr << "Failed dual controls" - << " frame " << i - << " brightness " << brightness - << " contrast " << contrast - << " expected " << expected - << endl; - return TestFail; - } - expected = i < maxDelay ? expected : value - 1; + ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS, V4L2_CID_CONTRAST }); + int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get(); + int32_t contrastV4L = ctrlsV4L.get(V4L2_CID_CONTRAST).get(); + + if (i > maxDelay) { + if (brightness != value || contrast != value) { + cerr << "Failed dual controls" + << " frame " << i + << " brightness " << brightness + << " contrast " << contrast + << " expected " << value + << endl; + return TestFail; + } + if (brightnessV4L != value + 1 || contrastV4L != value + maxDelay) { + cerr << "Failed dual controls" + << " frame " << i + << " brightnessV4L " << brightnessV4L + << " expected " << value + 1 + << " contrastV4L " << contrastV4L + << " expected " << value + maxDelay + << endl; + return TestFail; + } + } } return TestPass; @@ -208,7 +345,7 @@ protected: int dualControlsMultiQueue() { - static const unsigned int maxDelay = 2; + static const int maxDelay = 2; std::unordered_map delays = { { V4L2_CID_BRIGHTNESS, { 1, false } }, @@ -218,22 +355,19 @@ protected: std::make_unique(dev_.get(), delays); ControlList ctrls; - /* Reset control to value that will be first two frames in test. */ - int32_t expected = 100; - ctrls.set(V4L2_CID_BRIGHTNESS, expected); - ctrls.set(V4L2_CID_CONTRAST, expected); + /* Reset control to a value that will be first two frames in test. */ + int32_t initial = 100; + ctrls.set(V4L2_CID_BRIGHTNESS, initial); + ctrls.set(V4L2_CID_CONTRAST, initial); dev_->setControls(&ctrls); delayed->reset(); - /* Trigger the first frame start event */ - delayed->applyControls(0); - /* - * Queue all controls before any fake frame start. Note we + * Queue all controls before applyControls(). Note we * can't queue up more then the delayed controls history size - * which is 16. Where one spot is used by the reset control. + * which is 16. */ - for (unsigned int i = 0; i < 15; i++) { + for (unsigned int i = 0; i < 14; i++) { int32_t value = 10 + i; ctrls.set(V4L2_CID_BRIGHTNESS, value); @@ -241,9 +375,9 @@ protected: delayed->push(ctrls); } - /* Process all queued controls. */ - for (unsigned int i = 1; i < 16; i++) { - int32_t value = 10 + i - 1; + /* Process 2 frames less than queued, so that the V4L controls are correct */ + for (unsigned int i = 0; i < 12; i++) { + int32_t expected = i < maxDelay ? initial : 10 + i; delayed->applyControls(i); @@ -251,6 +385,11 @@ protected: int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); int32_t contrast = result.get(V4L2_CID_CONTRAST).get(); + + ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS, V4L2_CID_CONTRAST }); + int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get(); + int32_t contrastV4L = ctrlsV4L.get(V4L2_CID_CONTRAST).get(); + if (brightness != expected || contrast != expected) { cerr << "Failed multi queue" << " frame " << i @@ -261,7 +400,17 @@ protected: return TestFail; } - expected = i < maxDelay ? expected : value - 1; + /* Check v4l values after they've settled */ + if (i >= maxDelay && (brightnessV4L != expected + 1 || contrastV4L != expected + maxDelay)) { + cerr << "Failed multi queue" + << " frame " << i + << " brightnessV4L " << brightnessV4L + << " expected " << expected + 1 + << " contrastV4L " << contrastV4L + << " expected " << expected + maxDelay + << endl; + return TestFail; + } } return TestPass; @@ -269,27 +418,40 @@ protected: int run() override { - int ret; + int ret = 0; + bool failed = false; /* Test single control without delay. */ ret = singleControlNoDelay(); if (ret) - return ret; + failed = true; /* Test single control with delay. */ ret = singleControlWithDelay(); if (ret) - return ret; + failed = true; + + /* Test single control with delay. */ + ret = singleControlWithDelayStartUp(); + if (ret) + failed = true; + + ret = doNotLoseFirstRequest(); + if (ret) + failed = true; /* Test dual controls with different delays. */ ret = dualControlsWithDelay(); if (ret) - return ret; + failed = true; /* Test control values produced faster than consumed. */ ret = dualControlsMultiQueue(); if (ret) - return ret; + failed = true; + + if (failed) + return TestFail; return TestPass; } From patchwork Tue Mar 19 12:05:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19745 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 ECF1AC3274 for ; Tue, 19 Mar 2024 12:05:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C608162D2F; Tue, 19 Mar 2024 13:05:35 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="soxGyqYm"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9DF4162CAD for ; Tue, 19 Mar 2024 13:05:28 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EBA5F480; Tue, 19 Mar 2024 13:05:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849902; bh=0fdCqsmsRjGgHyqD2+KCaAkGapqqFfm+kwHb3gyTHbo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=soxGyqYmMaTIVE3GtClGxonkRle2RCUDArDCuGPYWjEH6QAdeHPPW9ZV0Lk7jyuB0 6+/4SsZXNiKCO1Sgt+vDjEHSzxkhPio6Zf6TpGYc2omJ5xYfHGX7MlUkWyUCdXTfGO Bjkccn9tm/IXE4TOovzNb9Df3pG9jutkXJc+zNQQ= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 05/16] libcamera: delayed_controls: Rename class members Date: Tue, 19 Mar 2024 13:05:06 +0100 Message-Id: <20240319120517.362082-6-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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 preperation for the following patch, the class members are renamed to better express their intent. This might be a little picky, but my head is just more used to thinking of an index than a count. Signed-off-by: Stefan Klug --- include/libcamera/internal/delayed_controls.h | 6 ++++-- src/libcamera/delayed_controls.cpp | 20 +++++++++---------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index aef37077..4f8d2424 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -72,8 +72,10 @@ private: std::unordered_map controlParams_; unsigned int maxDelay_; - uint32_t queueCount_; - uint32_t writeCount_; + /* Index of the next request to queue */ + uint32_t queueIndex_; + /* Index of the next request that gets written and is guaranteed to be fully applied */ + uint32_t writeIndex_; /* \todo Evaluate if we should index on ControlId * or unsigned int */ std::unordered_map values_; }; diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 777441e8..86571cd4 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -115,8 +115,8 @@ DelayedControls::DelayedControls(V4L2Device *device, */ void DelayedControls::reset() { - queueCount_ = 1; - writeCount_ = 0; + queueIndex_ = 1; + writeIndex_ = 0; /* Retrieve control as reported by the device. */ std::vector ids; @@ -150,8 +150,8 @@ bool DelayedControls::push(const ControlList &controls) { /* Copy state from previous frame. */ for (auto &ctrl : values_) { - Info &info = ctrl.second[queueCount_]; - info = values_[ctrl.first][queueCount_ - 1]; + Info &info = ctrl.second[queueIndex_]; + info = values_[ctrl.first][queueIndex_ - 1]; info.updated = false; } @@ -170,17 +170,17 @@ bool DelayedControls::push(const ControlList &controls) if (controlParams_.find(id) == controlParams_.end()) return false; - Info &info = values_[id][queueCount_]; + Info &info = values_[id][queueIndex_]; info = Info(control.second); LOG(DelayedControls, Debug) << "Queuing " << id->name() << " to " << info.toString() - << " at index " << queueCount_; + << " at index " << queueIndex_; } - queueCount_++; + queueIndex_++; return true; } @@ -241,7 +241,7 @@ void DelayedControls::applyControls(uint32_t sequence) for (auto &ctrl : values_) { const ControlId *id = ctrl.first; unsigned int delayDiff = maxDelay_ - controlParams_[id].delay; - unsigned int index = std::max(0, writeCount_ - delayDiff); + unsigned int index = std::max(0, writeIndex_ - delayDiff); Info &info = ctrl.second[index]; if (info.updated) { @@ -271,9 +271,9 @@ void DelayedControls::applyControls(uint32_t sequence) } } - writeCount_ = sequence + 1; + writeIndex_ = sequence + 1; - while (writeCount_ > queueCount_) { + while (writeIndex_ > queueIndex_) { LOG(DelayedControls, Debug) << "Queue is empty, auto queue no-op."; push({}); From patchwork Tue Mar 19 12:05:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19746 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 D9E5EC32C4 for ; Tue, 19 Mar 2024 12:05:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3D3FE62CAE; Tue, 19 Mar 2024 13:05:37 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="jtES7uPO"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E4D1A62CAC for ; Tue, 19 Mar 2024 13:05:28 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 35D7BF02; Tue, 19 Mar 2024 13:05:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849902; bh=ogedsUNacptSc0oIPMfSH3q1GS42oyNh/snuUSKDJSg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jtES7uPOy8o9BAvpCvFVO01KqqNzDdFQlXx8Ilbe/JmW4VUMEo7jrPQs37iwIpuCF j7dk/fQA7XTkya33tibsuZHoS9qUdVUFxeFvjG161mTYGs9OPmj/bNj2Luj86KIaM6 wnVYN5ysSkQPsqaT4qUlgJSBivPTdboHYrCCwKXc= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 06/16] libcamera: delayed_controls: Make listSize unsigned Date: Tue, 19 Mar 2024 13:05:07 +0100 Message-Id: <20240319120517.362082-7-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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" Make listSize unsigned, so that later calculations with other unsigned ints don't require casts. Signed-off-by: Stefan Klug --- include/libcamera/internal/delayed_controls.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index 4f8d2424..0e28106a 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -52,7 +52,7 @@ private: }; /* \todo Make the listSize configurable at instance creation time. */ - static constexpr int listSize = 16; + static constexpr unsigned int listSize = 16; class ControlRingBuffer : public std::array { public: From patchwork Tue Mar 19 12:05:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19747 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 9BF7DC32C5 for ; Tue, 19 Mar 2024 12:05:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3C84A62CA7; Tue, 19 Mar 2024 13:05:38 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="TkK93nuG"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D21862973 for ; Tue, 19 Mar 2024 13:05:29 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D640480; Tue, 19 Mar 2024 13:05:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849902; bh=CY+7rwNcPmr9tbggrYi5pB0eqv6maFoX762dCfO5dT4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TkK93nuG8AJ8wb1Sh3KZ9hoKK2eQJgqRisE+hjt2hRXNB5aoXAF/+pJJu4P2JvA2T GDbxxQ1twQHLaqQZ86iGjLZB5n0gcaSDm63h/UU9iWFUfSHHIaNjwz/Pt9nRFGqQUw OCg4DltFH5/1dJnm/wPvzpj0lX9kcKLNw/KiWLLE= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 07/16] libcamera: delayed_controls: Add controlsAreQueued() helper Date: Tue, 19 Mar 2024 13:05:08 +0100 Message-Id: <20240319120517.362082-8-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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 gets used in the upcoming patches to decide if controls need to be queued for a later frame, or if the controls can be skipped because they are already queued. Signed-off-by: Stefan Klug --- include/libcamera/internal/delayed_controls.h | 2 ++ src/libcamera/delayed_controls.cpp | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index 0e28106a..ccbe7239 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -67,6 +67,8 @@ private: } }; + bool controlsAreQueued(unsigned int frame, const ControlList &controls); + V4L2Device *device_; /* \todo Evaluate if we should index on ControlId * or unsigned int */ std::unordered_map controlParams_; diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 86571cd4..6c766ede 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -137,6 +137,40 @@ void DelayedControls::reset() } } +/** + * \brief Helper function to check if controls are already queued + * \param[in] sequence Sequence number to check + * \param[in] controls List of controls to compare against + * + * This function checks if the controls queued for frame \a sequence + * are equal to \a controls. This is helpful in cases where a control algorithm + * unconditionally queues controls for every frame, but always too late. + * In that case this can be used to check if incoming controls are already + * queued or need to be queued for a later frame. + * + * \returns true if \a controls are queued for the given sequence + */ +bool DelayedControls::controlsAreQueued(unsigned int sequence, + const ControlList &controls) +{ + const ControlIdMap &idmap = device_->controls().idmap(); + for (const auto &[id, value] : controls) { + const auto &it = idmap.find(id); + if (it == idmap.end()) { + LOG(DelayedControls, Warning) + << "Unknown control " << id; + return false; + } + + const ControlId *ctrlId = it->second; + + if (values_[ctrlId][sequence] != value) + return false; + } + + return true; +} + /** * \brief Push a set of controls on the queue * \param[in] controls List of controls to add to the device queue From patchwork Tue Mar 19 12:05:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19748 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 86653BD160 for ; Tue, 19 Mar 2024 12:05:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 12CDB62D3C; Tue, 19 Mar 2024 13:05:39 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="wkrwpne/"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 75D0262D27 for ; Tue, 19 Mar 2024 13:05:29 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BBACAF02; Tue, 19 Mar 2024 13:05:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849902; bh=kyRJ45pWGO4NCh/fbdEU9zKRDng0jLzHN++IvImihv8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=wkrwpne/dFq1nmvWoek1KSwQWRW7nNfwnZUaHE8/eXR9Om4TYQ9ODLOrlOzsBh+o4 POJ81MskCLbXWTROuduTaaM/zBf+j2T85oSs3h07RDYsp9aZdiKLXE9ySgPrUsHjDB PPj50bMFDiBr8xXXnrLc14drLgaIcb7wEYQaCG68= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 08/16] libcamera: delayed_controls: Add ctrls list parameter to reset() function Date: Tue, 19 Mar 2024 13:05:09 +0100 Message-Id: <20240319120517.362082-9-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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 makes it easier for the caller. There is often this pattern sensor_->setControls(controls); delayedControls_->reset(); which can then be reduced to delayedControls_->reset(controls); Signed-off-by: Stefan Klug Reviewed-by: Jacopo Mondi --- include/libcamera/internal/delayed_controls.h | 2 +- src/libcamera/delayed_controls.cpp | 22 +++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index ccbe7239..224c1f7e 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -27,7 +27,7 @@ public: DelayedControls(V4L2Device *device, const std::unordered_map &controlParams); - void reset(); + void reset(ControlList *controls = nullptr); bool push(const ControlList &controls); ControlList get(uint32_t sequence); diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 6c766ede..6f06950e 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -109,11 +109,13 @@ DelayedControls::DelayedControls(V4L2Device *device, /** * \brief Reset state machine + * \param[in,out] controls The controls to apply to the device * * Resets the state machine to a starting position based on control values - * retrieved from the device. + * retrieved from the device. If \a controls is given, these controls are set + * on the device before retrieving the reset values. */ -void DelayedControls::reset() +void DelayedControls::reset(ControlList *controls) { queueIndex_ = 1; writeIndex_ = 0; @@ -123,11 +125,23 @@ void DelayedControls::reset() for (auto const ¶m : controlParams_) ids.push_back(param.first->id()); - ControlList controls = device_->getControls(ids); + if (controls) { + device_->setControls(controls); + + LOG(DelayedControls, Debug) << "reset:"; + auto idMap = controls->idMap(); + if (idMap) { + for (const auto &[id, value] : *controls) + LOG(DelayedControls, Debug) << " " << idMap->at(id)->name() + << " : " << value.toString(); + } + } + + ControlList ctrls = device_->getControls(ids); /* Seed the control queue with the controls reported by the device. */ values_.clear(); - for (const auto &ctrl : controls) { + for (const auto &ctrl : ctrls) { const ControlId *id = device_->controls().idmap().at(ctrl.first); /* * Do not mark this control value as updated, it does not need From patchwork Tue Mar 19 12:05:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19749 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 3A0D8C32C6 for ; Tue, 19 Mar 2024 12:05:43 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AE83362D3B; Tue, 19 Mar 2024 13:05:40 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="t1oPXfi9"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AC24B62D2B for ; Tue, 19 Mar 2024 13:05:29 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0A161480; Tue, 19 Mar 2024 13:05:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849903; bh=dPKmjSnAJz5mm9u/EZMLMt1WGbC78RUn68Vf+zv7Jx0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=t1oPXfi9OgEQvt/pBRh3gAa702elI+U3DMjX9Iycy8jFlMhFqyYsjKuCc8nv3+xa5 8RatR/J1tLj6gErVwWUMmhfvwP0zdsBXFd8GQBi1xtewJTzKhvllCl20gcXfWPa28J 5Jk2pf2gOhJzs+nkCcGlw1weG6krOm/Ehfll2zUs= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 09/16] libcamera: delayed_controls: Add sourceSequence property Date: Tue, 19 Mar 2024 13:05:10 +0100 Message-Id: <20240319120517.362082-10-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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 is necessary for the upcoming rework. It allows to track the sequence number from which a request stems. The use case where this is needed is as follows: Assume for frame 10 a manual ExposureTime=42 is queued because the user wants manual exposure. Now the agc gets the stats for frame 8 (where auto regulation is still active) and pushes a new ExposureTime for frame 9. Frame 9 was already sent out, so it gets delayed to frame 11 (assuming a delay of 2 on ExposureTime). This would revert the request from frame 10. Taking the sourceSequence into account, the delayed request from frame 9 will be discarded, which is correct. Signed-off-by: Stefan Klug --- include/libcamera/internal/delayed_controls.h | 8 +++++--- src/libcamera/delayed_controls.cpp | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index 224c1f7e..5a0428e0 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -39,16 +39,18 @@ private: { public: Info() - : updated(false) + : updated(false), sourceSequence(0) { } - Info(const ControlValue &v, bool updated_ = true) - : ControlValue(v), updated(updated_) + Info(const ControlValue &v, uint32_t sourceSeq, bool updated_ = true) + : ControlValue(v), updated(updated_), sourceSequence(sourceSeq) { } bool updated; + /* The sequence id for which this value was requested */ + uint32_t sourceSequence; }; /* \todo Make the listSize configurable at instance creation time. */ diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 6f06950e..8cfa6bbb 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -147,7 +147,7 @@ void DelayedControls::reset(ControlList *controls) * 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); + values_[id][0] = Info(ctrl.second, 0, false); } } @@ -220,7 +220,7 @@ bool DelayedControls::push(const ControlList &controls) Info &info = values_[id][queueIndex_]; - info = Info(control.second); + info = Info(control.second, 0); LOG(DelayedControls, Debug) << "Queuing " << id->name() From patchwork Tue Mar 19 12:05:11 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19750 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 347C2C32C7 for ; Tue, 19 Mar 2024 12:05:44 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6775D62D32; Tue, 19 Mar 2024 13:05:42 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="wde0jtBV"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F41E962CA8 for ; Tue, 19 Mar 2024 13:05:29 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 48304F02; Tue, 19 Mar 2024 13:05:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849903; bh=7jyWwJWbWc+w2r/bxH6jSapXRfGGacSKdC6GGq/QzNo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=wde0jtBVq+Nk423Satk+gDeFhMWrWivGARUt6ViPJXFt3XomtUnpQt3TmOX/ZxPJQ qoaqkD7oCEh9146qoKs+Ls79agPhIwkMa8ipaU41POJciyfl+faEBt1Cu+VgZYTJmQ jCnBLeA1hf0Qk2pISDRKD2WoC0ccs3saQSTRAL0Q= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 10/16] libcamera: delayed_controls: Add fillValues() helper Date: Tue, 19 Mar 2024 13:05:11 +0100 Message-Id: <20240319120517.362082-11-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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 the upcoming patches the function to initialize the value in the ringbuffer is needed at multiple places. So it makes sense to factor it out. Signed-off-by: Stefan Klug --- include/libcamera/internal/delayed_controls.h | 1 + src/libcamera/delayed_controls.cpp | 28 +++++++++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index 5a0428e0..db5c29e9 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -70,6 +70,7 @@ private: }; bool controlsAreQueued(unsigned int frame, const ControlList &controls); + void fillValues(unsigned int fromIndex, unsigned int toIndex); V4L2Device *device_; /* \todo Evaluate if we should index on ControlId * or unsigned int */ diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 8cfa6bbb..e325b6b2 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -185,6 +185,27 @@ bool DelayedControls::controlsAreQueued(unsigned int sequence, return true; } +/** + * @brief Helper function to fill the values with the data from previous entries + * + * @param fromIndex The index to start the copy from + * @param toIndex The last index that gets filled + * + * The updated member of the control values is reset to false. + */ +void DelayedControls::fillValues(unsigned int fromIndex, unsigned int toIndex) +{ + /* Copy state from previous queue. */ + for (auto &ctrl : values_) { + auto &ringBuffer = ctrl.second; + for (auto i = fromIndex; i < toIndex; i++) { + Info &info = ringBuffer[i + 1]; + info = ringBuffer[i]; + info.updated = false; + } + } +} + /** * \brief Push a set of controls on the queue * \param[in] controls List of controls to add to the device queue @@ -196,12 +217,7 @@ bool DelayedControls::controlsAreQueued(unsigned int sequence, */ bool DelayedControls::push(const ControlList &controls) { - /* Copy state from previous frame. */ - for (auto &ctrl : values_) { - Info &info = ctrl.second[queueIndex_]; - info = values_[ctrl.first][queueIndex_ - 1]; - info.updated = false; - } + fillValues(queueIndex_ - 1, queueIndex_); /* Update with new controls. */ const ControlIdMap &idmap = device_->controls().idmap(); From patchwork Tue Mar 19 12:05:12 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19751 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 95F68C32C3 for ; Tue, 19 Mar 2024 12:05:44 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 010F762D8E; Tue, 19 Mar 2024 13:05:44 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="jVnGyAVL"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 39B1062CEC for ; Tue, 19 Mar 2024 13:05:30 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 86C87480; Tue, 19 Mar 2024 13:05:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849903; bh=Lu4hJ17sxhpzAJDwsSY3jzJ+WHfKBnj/pqZDaUS7apc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jVnGyAVLZWdSCX8A8yITTso5C8TyB/nlG4flvDeLW3Wni7FxStmZQxWsqZxZ1V2dE h1snP4DBx2SbqhBoXuVn9tMLM49tv0U69TUT/EkRnjBJgu+I9msdkLwsGJE8LPeynv 8KcJv9Dy/8TImOtn6a0Uuz+rOhtKrngCr5o10gqI= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 11/16] libcamera: delayed_controls: Rework delayed controls implementation Date: Tue, 19 Mar 2024 13:05:12 +0100 Message-Id: <20240319120517.362082-12-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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" Functional changes: - Requests need to be queued for every frame. That was already true in the old implementation. Still the unit tests did a applyControls before the first push which seems counterintuitive - The startup phase is no longer treated as special case - Controls are now pushed together with the sequence number where they should be active. This way we can detect when the system gets out of sync - Controls for a given sequence number can be updated multiple times as long as they are not yet sent out - If a request is too late, controls get delayed but they are not lost - Requests attached to frame 0 don't get lost (they get delayed by maxDelay frames) Signed-off-by: Stefan Klug --- include/libcamera/internal/delayed_controls.h | 2 +- src/libcamera/delayed_controls.cpp | 163 ++++++++++++++---- test/delayed_controls.cpp | 67 +++++++ 3 files changed, 194 insertions(+), 38 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index db5c29e9..e2decbcc 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -29,7 +29,7 @@ public: void reset(ControlList *controls = nullptr); - bool push(const ControlList &controls); + bool push(const ControlList &controls, std::optional sequence = std::nullopt); ControlList get(uint32_t sequence); void applyControls(uint32_t sequence); diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index e325b6b2..3fd5a0f7 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -104,6 +104,8 @@ DelayedControls::DelayedControls(V4L2Device *device, maxDelay_ = std::max(maxDelay_, controlParams_[id].delay); } + LOG(DelayedControls, Debug) << "Maximum delay: " << maxDelay_; + reset(); } @@ -117,8 +119,9 @@ DelayedControls::DelayedControls(V4L2Device *device, */ void DelayedControls::reset(ControlList *controls) { - queueIndex_ = 1; - writeIndex_ = 0; + queueIndex_ = 0; + /* Frames up to maxDelay_ will be based on sensor init values. */ + writeIndex_ = maxDelay_; /* Retrieve control as reported by the device. */ std::vector ids; @@ -149,6 +152,9 @@ void DelayedControls::reset(ControlList *controls) */ values_[id][0] = Info(ctrl.second, 0, false); } + + /* Propagate initial state */ + fillValues(0, writeIndex_); } /** @@ -207,17 +213,78 @@ void DelayedControls::fillValues(unsigned int fromIndex, unsigned int toIndex) } /** - * \brief Push a set of controls on the queue + * \brief Push a set of controls for a given frame * \param[in] controls List of controls to add to the device queue + * \param[in] sequence_ Sequence to push the \a controls for + + * + * Pushes the controls given by \a controls, to be applied at frame \a sequence. * - * Push a set of controls to the control queue. This increases the control queue - * depth by one. + * If there are controls already queued for that frame, they get updated. + * + * If it's too late for frame \a sequence (controls are already sent to the + * sensor), the system checks if the controls that where written out for frame + * \a sequence are the same as the requested ones. In this case, nothing is + * done. If they differ, the controls get queued for the earliest frame possible + * if no other controls with a higher sequence number are queued for that frame + * already. * * \returns true if \a controls are accepted, or false otherwise */ -bool DelayedControls::push(const ControlList &controls) +bool DelayedControls::push(const ControlList &controls, std::optional sequence_) { - fillValues(queueIndex_ - 1, queueIndex_); + uint32_t sequence = sequence_.value_or(queueIndex_); + + LOG(DelayedControls, Debug) << "push: " << sequence; + auto idMap = controls.idMap(); + if (idMap) { + for (const auto &[id, value] : controls) + LOG(DelayedControls, Debug) << " " << idMap->at(id)->name() + << " : " << value.toString(); + } + + if (sequence < queueIndex_) + LOG(DelayedControls, Debug) << "Got updated data for frame:" << sequence; + + if (sequence > queueIndex_) + LOG(DelayedControls, Warning) << "Hole in queue sequence." + << " This should not happen. Expected: " + << queueIndex_ << " got " << sequence; + + uint32_t updateIndex = sequence; + /* check if its too late for the request */ + if (sequence < writeIndex_) { + /* Check if we can safely ignore the request */ + if (controls.empty() || controlsAreQueued(sequence, controls)) { + if (sequence >= queueIndex_) { + queueIndex_ = sequence + 1; + return true; + } + } else { + LOG(DelayedControls, Debug) << "Controls for frame " << sequence + << " are already in flight. Will be queued for frame " + << writeIndex_; + updateIndex = writeIndex_; + } + } + + if (updateIndex - writeIndex_ >= listSize - maxDelay_) + /* + * The system is in an undefined state now. This will heal itself, + * as soon as all controls where rewritten + */ + LOG(DelayedControls, Error) << "Queue length exceeded." + << " The system is out of sync. Index to update:" + << updateIndex << " Next Index to apply: " << writeIndex_; + + /* + * Prepare the ringbuffer entries with previous data. + * Data up to [writeIndex_] gets prepared in applyControls. + */ + if (updateIndex > writeIndex_ && updateIndex >= queueIndex_) { + LOG(DelayedControls, Debug) << "Copy from previous " << queueIndex_; + fillValues(queueIndex_ - 1, updateIndex); + } /* Update with new controls. */ const ControlIdMap &idmap = device_->controls().idmap(); @@ -231,20 +298,36 @@ bool DelayedControls::push(const ControlList &controls) const ControlId *id = it->second; - if (controlParams_.find(id) == controlParams_.end()) - return false; - - Info &info = values_[id][queueIndex_]; + if (controlParams_.find(id) == controlParams_.end()) { + LOG(DelayedControls, Error) << "Could not find params for control " + << id << " ignored"; + continue; + } - info = Info(control.second, 0); + Info &info = values_[id][updateIndex]; + /* + * Update the control only if the already existing value stems from a + * request with a sequence number smaller or equal to the current one + */ + if (info.sourceSequence <= sequence) { + info = Info(control.second, sequence); - LOG(DelayedControls, Debug) - << "Queuing " << id->name() - << " to " << info.toString() - << " at index " << queueIndex_; + LOG(DelayedControls, Debug) + << "Queuing " << id->name() + << " to " << info.toString() + << " at index " << updateIndex; + } else { + LOG(DelayedControls, Warning) + << "Skipped update " << id->name() + << " at index " << updateIndex; + } } - queueIndex_++; + LOG(DelayedControls, Debug) << "Queued frame: " << sequence + << " it will be active in " << updateIndex; + + if (sequence >= queueIndex_) + queueIndex_ = sequence + 1; return true; } @@ -266,19 +349,17 @@ bool DelayedControls::push(const ControlList &controls) */ ControlList DelayedControls::get(uint32_t sequence) { - unsigned int index = std::max(0, sequence - maxDelay_); - + LOG(DelayedControls, Debug) << "get " << sequence << ":"; ControlList out(device_->controls()); for (const auto &ctrl : values_) { const ControlId *id = ctrl.first; - const Info &info = ctrl.second[index]; + const Info &info = ctrl.second[sequence]; out.set(id->id(), info); LOG(DelayedControls, Debug) - << "Reading " << id->name() - << " to " << info.toString() - << " at index " << index; + << " " << id->name() + << ": " << info.toString(); } return out; @@ -286,16 +367,22 @@ ControlList DelayedControls::get(uint32_t sequence) /** * \brief Inform DelayedControls of the start of a new frame - * \param[in] sequence Sequence number of the frame that started + * \param[in] sequence Sequence number of the frame that started (0-based) * - * Inform the state machine that a new frame has started and of its sequence - * number. Any user of these helpers is responsible to inform the helper about - * the start of any frame. This can be connected with ease to the start of a - * exposure (SOE) V4L2 event. + * Inform the state machine that a new frame has started to arrive at the receiver + * (e.g. the sensor started to clock out pixels) and of its sequence + * number. This is usually the earliest point in time to update registers in the + * sensor for upcoming frames. Any user of these helpers is responsible to inform + * the helper about the start of any frame. This can be connected with ease to + * the start of a exposure (SOE) V4L2 event. */ void DelayedControls::applyControls(uint32_t sequence) { - LOG(DelayedControls, Debug) << "frame " << sequence << " started"; + LOG(DelayedControls, Debug) << "Apply controls " << sequence; + if (sequence != writeIndex_ - maxDelay_) + LOG(DelayedControls, Warning) << "Sequence and writeIndex are out of sync." + << " Expected seq: " << writeIndex_ - maxDelay_ + << " got " << sequence; /* * Create control list peeking ahead in the value queue to ensure @@ -305,7 +392,7 @@ void DelayedControls::applyControls(uint32_t sequence) for (auto &ctrl : values_) { const ControlId *id = ctrl.first; unsigned int delayDiff = maxDelay_ - controlParams_[id].delay; - unsigned int index = std::max(0, writeIndex_ - delayDiff); + unsigned int index = writeIndex_ - delayDiff; Info &info = ctrl.second[index]; if (info.updated) { @@ -326,21 +413,23 @@ void DelayedControls::applyControls(uint32_t sequence) } LOG(DelayedControls, Debug) - << "Setting " << id->name() - << " to " << info.toString() - << " at index " << index; + << "Writing " << id->name() + << " (" << info.toString() << ") " + << " for frame " << index; /* Done with this update, so mark as completed. */ info.updated = false; } } - writeIndex_ = sequence + 1; + auto oldWriteIndex = writeIndex_; + writeIndex_ = sequence + maxDelay_ + 1; - while (writeIndex_ > queueIndex_) { + if (writeIndex_ >= queueIndex_ && writeIndex_ > oldWriteIndex) { LOG(DelayedControls, Debug) - << "Queue is empty, auto queue no-op."; - push({}); + << "Index " << writeIndex_ + << " is not yet queued. Prepare with old state"; + fillValues(oldWriteIndex, writeIndex_); } device_->setControls(&out); diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp index fe183de5..481334e7 100644 --- a/test/delayed_controls.cpp +++ b/test/delayed_controls.cpp @@ -271,6 +271,69 @@ protected: return TestPass; } + int updateTooLateGetsDelayed() + { + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, { 2, false } }, + }; + std::unique_ptr delayed = + std::make_unique(dev_.get(), delays); + ControlList ctrls; + + /* Reset control to value that will be first in test. */ + int32_t initial = 4; + ctrls.set(V4L2_CID_BRIGHTNESS, initial); + dev_->setControls(&ctrls); + delayed->reset(); + + int32_t expected = 10; + + delayed->push({}, 0); + delayed->push({}, 1); + /* push a request for frame 2 */ + ctrls.set(V4L2_CID_BRIGHTNESS, 40); + delayed->push(ctrls, 2); + + delayed->applyControls(0); + delayed->applyControls(1); + /* + * update frame 2 to the correct value. But it is too late, delayed + * controls will delay and the value shall be available on frame 4 + */ + ctrls.set(V4L2_CID_BRIGHTNESS, expected); + delayed->push(ctrls, 2); + delayed->applyControls(2); + delayed->applyControls(3); + delayed->applyControls(4); + + int frame = 4; + + ControlList result = delayed->get(frame); + int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); + ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS }); + int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get(); + + if (brightness != expected) { + cerr << "Failed " << __func__ + << " frame " << frame + << " expected " << expected + << " got " << brightness + << endl; + return TestFail; + } + + if (brightnessV4L != expected) { + cerr << "Failed " << __func__ + << " frame " << frame + << " expected V4L " << expected + << " got " << brightnessV4L + << endl; + return TestFail; + } + + return TestPass; + } + int dualControlsWithDelay() { static const int maxDelay = 2; @@ -440,6 +503,10 @@ protected: if (ret) failed = true; + ret = updateTooLateGetsDelayed(); + if (ret) + failed = true; + /* Test dual controls with different delays. */ ret = dualControlsWithDelay(); if (ret) From patchwork Tue Mar 19 12:05:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19752 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 7EF4EC32C8 for ; Tue, 19 Mar 2024 12:05:45 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2C54162D6B; Tue, 19 Mar 2024 13:05:45 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="hLF9ZMOk"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B75262D2F for ; Tue, 19 Mar 2024 13:05:30 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C997715EE; Tue, 19 Mar 2024 13:05:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849903; bh=e65H+BR9HKWcZ4SGv9EqEiTMDJzzWQA55nciN3BBRyQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hLF9ZMOk7NV69QuCM1cTurHVtwFF4Ep8LU8GhYAKvz69cBil9vLNH/vcDe9bONMWk x2NjdcxDQHU9Vn43XTUR8neqttp4mQ7zH7Zh7oE04H+Rxc6rVYiW0qTjP7i46gS78v OOXd5ByPFZgo7YFj0vsMB3hDw6WP075G01GMSlow= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 12/16] libcamera: delayed_controls: Ignore delayed request, if there is a newer one Date: Tue, 19 Mar 2024 13:05:13 +0100 Message-Id: <20240319120517.362082-13-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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" Assume for frame 10 a ExposureTime=42 is queued. We have a delay of 2. After receiving stats for frame 8, the isp tries to queue for frame 9. As it's too lae for that frame, delayed controls schedules the update for frame 11. But as frame 10 was already queued, the request should be discarded. --- include/libcamera/internal/delayed_controls.h | 2 + src/libcamera/delayed_controls.cpp | 19 +++++- test/delayed_controls.cpp | 66 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index e2decbcc..91c9415a 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -67,6 +67,8 @@ private: { return std::array::operator[](index % listSize); } + + unsigned int largestValidIndex; }; bool controlsAreQueued(unsigned int frame, const ControlList &controls); diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 3fd5a0f7..7f2bb479 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -151,6 +151,7 @@ void DelayedControls::reset(ControlList *controls) * to be written to to device on startup. */ values_[id][0] = Info(ctrl.second, 0, false); + values_[id].largestValidIndex = 0; } /* Propagate initial state */ @@ -304,18 +305,34 @@ bool DelayedControls::push(const ControlList &controls, std::optional continue; } - Info &info = values_[id][updateIndex]; + ControlRingBuffer &ring = values_[id]; + Info &info = ring[updateIndex]; /* * Update the control only if the already existing value stems from a * request with a sequence number smaller or equal to the current one */ if (info.sourceSequence <= sequence) { info = Info(control.second, sequence); + if (updateIndex > ring.largestValidIndex) + ring.largestValidIndex = updateIndex; LOG(DelayedControls, Debug) << "Queuing " << id->name() << " to " << info.toString() << " at index " << updateIndex; + + /* fill up the next indices with the new information */ + unsigned int i = updateIndex + 1; + while (i <= ring.largestValidIndex) { + LOG(DelayedControls, Error) << "update " << i; + Info &next = ring[i]; + if (next.sourceSequence <= sequence) + next = info; + else + break; + + i++; + } } else { LOG(DelayedControls, Warning) << "Skipped update " << id->name() diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp index 481334e7..7d671a0e 100644 --- a/test/delayed_controls.cpp +++ b/test/delayed_controls.cpp @@ -271,6 +271,68 @@ protected: return TestPass; } + int updateTooLateMustSometimesBeIgnored() + { + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, { 2, false } }, + }; + std::unique_ptr delayed = + std::make_unique(dev_.get(), delays); + ControlList ctrls; + + /* Reset control to value that will be first in test. */ + int32_t initial = 4; + ctrls.set(V4L2_CID_BRIGHTNESS, initial); + dev_->setControls(&ctrls); + delayed->reset(); + + int32_t expected = 10; + + delayed->push({}, 0); + delayed->push({}, 1); + ctrls.set(V4L2_CID_BRIGHTNESS, expected); + delayed->push(ctrls, 2); + delayed->applyControls(0); /* puts 10 on the bus */ + + /* + * Post an update for frame 1. It's too late to fulfill that request, + * delayed controls will therefore try to delay it to frame 3. But as + * frame 2 is already queued, the update must be dropped. + */ + ctrls.set(V4L2_CID_BRIGHTNESS, 20); + delayed->push(ctrls, 1); + delayed->applyControls(1); + delayed->applyControls(2); + delayed->applyControls(3); + + int frame = 3; + + ControlList result = delayed->get(frame); + int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); + ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS }); + int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get(); + + if (brightness != expected) { + cerr << "Failed " << __func__ + << " frame " << frame + << " expected " << expected + << " got " << brightness + << endl; + return TestFail; + } + + if (brightnessV4L != expected) { + cerr << "Failed " << __func__ + << " frame " << frame + << " expected V4L " << expected + << " got " << brightnessV4L + << endl; + return TestFail; + } + + return TestPass; + } + int updateTooLateGetsDelayed() { std::unordered_map delays = { @@ -503,6 +565,10 @@ protected: if (ret) failed = true; + ret = updateTooLateMustSometimesBeIgnored(); + if (ret) + failed = true; + ret = updateTooLateGetsDelayed(); if (ret) failed = true; From patchwork Tue Mar 19 12:05:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19753 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 57B21C32C9 for ; Tue, 19 Mar 2024 12:05:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EE3E762E75; Tue, 19 Mar 2024 13:05:45 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Dq4+NEh8"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BAE7D62D31 for ; Tue, 19 Mar 2024 13:05:30 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 13EBC480; Tue, 19 Mar 2024 13:05:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849904; bh=FTsVWm29oC2K18aJ3DAR3rYpJmCwuCj03DIg7SYbji0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Dq4+NEh88bPnTntWURYDSbKC+knGijwu1ttyQ2ZtLtYjKRufHHKz7QsWNBPyNTGL6 uC+umDPdMHmxGDQ3FTfZl/JmewmhAODTNHt/5edIOf1trv/B1FU5MzabQTgCLB+N+g jxUiq43YGGbx9SaeuiCpZFHog5SODkoZbRg9zhtQ= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 13/16] pipeline: rkisp1: Move call to setSensorControls.emit() Date: Tue, 19 Mar 2024 13:05:14 +0100 Message-Id: <20240319120517.362082-14-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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" into processStateBuffer(). This is a non-functional change and in preparation for the next commit. Signed-off-by: Stefan Klug --- src/ipa/rkisp1/rkisp1.cpp | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 9dc5f53c..44d03eb8 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -74,7 +74,7 @@ private: void updateControls(const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls); - void setControls(unsigned int frame); + ControlList getControls(unsigned int frame); std::map buffers_; std::map mappedBuffers_; @@ -201,7 +201,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, int IPARkISP1::start() { - setControls(0); + ControlList ctrls = getControls(0); + setSensorControls.emit(0, ctrls); return 0; } @@ -367,7 +368,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId algo->process(context_, frame, frameContext, stats, metadata); } - setControls(frame); + ControlList ctrls = getControls(frame); + setSensorControls.emit(frame, ctrls); metadataReady.emit(frame, metadata); } @@ -430,13 +432,8 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo, *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls); } -void IPARkISP1::setControls(unsigned int frame) +ControlList IPARkISP1::getControls(unsigned int frame) { - /* - * \todo The frame number is most likely wrong here, we need to take - * internal sensor delays and other timing parameters into account. - */ - IPAFrameContext &frameContext = context_.frameContexts.get(frame); uint32_t exposure = frameContext.agc.exposure; uint32_t gain = camHelper_->gainCode(frameContext.agc.gain); @@ -444,8 +441,7 @@ void IPARkISP1::setControls(unsigned int frame) ControlList ctrls(sensorControls_); ctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure)); ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain)); - - setSensorControls.emit(frame, ctrls); + return ctrls; } } /* namespace ipa::rkisp1 */ From patchwork Tue Mar 19 12:05:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19755 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 AE95DC32CA for ; Tue, 19 Mar 2024 12:05:47 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6E5F263055; Tue, 19 Mar 2024 13:05:47 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="XvbGxiUG"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 02AA962D33 for ; Tue, 19 Mar 2024 13:05:31 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5160DF02; Tue, 19 Mar 2024 13:05:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849904; bh=TN69nrXh+FZ9LrbBYsdiBRy2H8uwrQH5cjkM8gmFPA8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XvbGxiUGv5r6Qm4wlifW/heLrL1DWEAitRGeiyt7BjkU/sXRolgVG/3A2+AcFwhi6 IcJl1vIB0vPCf5EnG9bkiFUgte5U8JSgNC5Qf6VtgrRJeE+4b2qzdhbWLjIMp0yiHI pUeoefEHpHkoNGHouO5RieM9ql8q7dz2Dk/yiQ6Y= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 14/16] pipeline: rkisp1: Add more debug logging Date: Tue, 19 Mar 2024 13:05:15 +0100 Message-Id: <20240319120517.362082-15-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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" Signed-off-by: Stefan Klug --- src/ipa/rkisp1/algorithms/agc.cpp | 4 ++-- src/ipa/rkisp1/rkisp1.cpp | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 47a6f7b2..f737ba92 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -125,14 +125,14 @@ void Agc::queueRequest(IPAContext &context, / context.configuration.sensor.lineDuration; LOG(RkISP1Agc, Debug) - << "Set exposure to " << agc.manual.exposure; + << "Set manual exposure to " << agc.manual.exposure; } const auto &gain = controls.get(controls::AnalogueGain); if (gain && !agc.autoEnabled) { agc.manual.gain = *gain; - LOG(RkISP1Agc, Debug) << "Set gain to " << agc.manual.gain; + LOG(RkISP1Agc, Debug) << "Set manual gain to " << agc.manual.gain; } frameContext.agc.autoEnabled = agc.autoEnabled; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 44d03eb8..6bc09198 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -314,6 +314,7 @@ void IPARkISP1::unmapBuffers(const std::vector &ids) void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls) { IPAFrameContext &frameContext = context_.frameContexts.alloc(frame); + LOG(IPARkISP1, Debug) << "queueRequest: " << frame; for (auto const &a : algorithms()) { Algorithm *algo = static_cast(a.get()); @@ -354,6 +355,7 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId stats = reinterpret_cast( mappedBuffers_.at(bufferId).planes()[0].data()); + LOG(IPARkISP1, Debug) << "processStatsBuffer for frame " << frame; frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get(); frameContext.sensor.gain = From patchwork Tue Mar 19 12:05:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19754 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 E1060C3274 for ; Tue, 19 Mar 2024 12:05:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8FFC362CA7; Tue, 19 Mar 2024 13:05:46 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="flobxmU8"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B54E62D36 for ; Tue, 19 Mar 2024 13:05:31 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 90BA2480; Tue, 19 Mar 2024 13:05:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849904; bh=d5uXxDEA4RWOM0Eb7ankorQ6javh8XT2aBCxCfLX1jg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=flobxmU8Vw4lk1poyKBZzWIqifJmDSWo9mAA1xyH4ht3egjm6vJNGTuAmgQZtoErX mtqOz67u4ay2ozadIoXkOu07+Pup3H42/M+eWwPxi3P1Hqu1Eflo4Bftqq8OofeBv7 3Cg9RWsAiQnkEHcqTwwyaNgB3EIFDKcihkjKvww8= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 15/16] pipeline: rkisp1: Fix per-frame-controls in manual mode Date: Tue, 19 Mar 2024 13:05:16 +0100 Message-Id: <20240319120517.362082-16-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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 the isp is not regulating, it can forward the request directly to the pipeline handler which will then be able to feed the delayed controls early enough. This solutions is quite simplistic and will need more thoughts as soon as other algorithms get implemented. Signed-off-by: Stefan Klug --- src/ipa/rkisp1/rkisp1.cpp | 16 ++++++++++++++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 6bc09198..240e11e6 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -322,6 +322,12 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls) continue; algo->queueRequest(context_, frame, frameContext, controls); } + + /* Fast path, if we are not regulating */ + if (!frameContext.agc.autoEnabled) { + ControlList ctrls = getControls(frame); + setSensorControls.emit(frame, ctrls); + } } void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) @@ -370,8 +376,14 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId algo->process(context_, frame, frameContext, stats, metadata); } - ControlList ctrls = getControls(frame); - setSensorControls.emit(frame, ctrls); + /* + * Set controls only when we are actually regulation. Otherwise, the controls + * where already set in queueRequest + */ + if (frameContext.agc.autoEnabled) { + ControlList ctrls = getControls(frame); + setSensorControls.emit(frame, ctrls); + } metadataReady.emit(frame, metadata); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index abb21968..ab452d8f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -401,7 +401,7 @@ void RkISP1CameraData::paramFilled(unsigned int frame) void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame, const ControlList &sensorControls) { - delayedCtrls_->push(sensorControls); + delayedCtrls_->push(sensorControls, frame); } void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata) From patchwork Tue Mar 19 12:05:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19756 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 90D0FC32CB for ; Tue, 19 Mar 2024 12:05:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3041E63075; Tue, 19 Mar 2024 13:05:48 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="XkLeYQyy"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9CBE462CA7 for ; Tue, 19 Mar 2024 13:05:31 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:1478:344b:8fcb:baf5]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id ED148F02; Tue, 19 Mar 2024 13:05:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710849905; bh=j0qVsA7Ec5j4cYznYL9ApPVX34uJ15f5sSF/tB0rpSw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XkLeYQyyIJPVtLtIxhBYuusqkoKU57B19AaqzDMz9x6x9iV3lYN1BTk9qR60ijiYB pOAOg+Ik3FzQm6+v1LWaQQVb6Akv4zPYK0EM2RdDkUEgdGjzrbsJHq20xlg1Be7WFG 1+CFOB7rdqKb6haFKXXD33Qs+Vs88GQIl8SfuMcM= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 16/16] pipeline: rkisp1: Apply initial controls Date: Tue, 19 Mar 2024 13:05:17 +0100 Message-Id: <20240319120517.362082-17-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240319120517.362082-1-stefan.klug@ideasonboard.com> References: <20240319120517.362082-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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" Controls passed to Camera::start() are now correctly applied. This is needed to pass the per-frame-controls test. The code uses the return value of ipa->start() to return the initial controls and pass them to the sensor instead of the setSensorControls signal because setSensorControls is asynchronous and would reach the sensor too late. Signed-off-by: Stefan Klug --- include/libcamera/ipa/rkisp1.mojom | 7 ++++++- src/ipa/rkisp1/rkisp1.cpp | 21 ++++++++++++++++----- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++++-- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom index 1009e970..828308c6 100644 --- a/include/libcamera/ipa/rkisp1.mojom +++ b/include/libcamera/ipa/rkisp1.mojom @@ -13,13 +13,18 @@ struct IPAConfigInfo { libcamera.ControlInfoMap sensorControls; }; +struct StartResult { + libcamera.ControlList controls; + int32 code; +}; + interface IPARkISP1Interface { init(libcamera.IPASettings settings, uint32 hwRevision, libcamera.IPACameraSensorInfo sensorInfo, libcamera.ControlInfoMap sensorControls) => (int32 ret, libcamera.ControlInfoMap ipaControls); - start() => (int32 ret); + start(libcamera.ControlList controls) => (StartResult result); stop(); configure(IPAConfigInfo configInfo, diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 240e11e6..6cde23cb 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -53,7 +53,7 @@ public: const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls) override; - int start() override; + void start(const ControlList &controls, StartResult *result) override; void stop() override; int configure(const IPAConfigInfo &ipaConfig, @@ -86,6 +86,7 @@ private: /* Local parameter storage */ struct IPAContext context_; + bool initialControlsDone_; }; namespace { @@ -199,12 +200,19 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, return 0; } -int IPARkISP1::start() +void IPARkISP1::start(const ControlList &controls, StartResult *result) { - ControlList ctrls = getControls(0); - setSensorControls.emit(0, ctrls); + initialControlsDone_ = false; + /* + * Todo: This is really shady. In order to apply the initial controls we already + * queue a request for frame 0 and return the results. + * We need to discuss the official recommendation to handle start controls + */ + queueRequest(0, controls); - return 0; + result->controls = getControls(0); + result->code = 0; + initialControlsDone_ = true; } void IPARkISP1::stop() @@ -323,6 +331,9 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls) algo->queueRequest(context_, frame, frameContext, controls); } + if (!initialControlsDone_) + return; + /* Fast path, if we are not regulating */ if (!frameContext.agc.autoEnabled) { ControlList ctrls = getControls(frame); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index ab452d8f..fec3c58b 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -927,13 +927,18 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL if (ret) return ret; - ret = data->ipa_->start(); - if (ret) { + ControlList ctrls; + if (!!controls) + ctrls = *controls; + ipa::rkisp1::StartResult res; + data->ipa_->start(ctrls, &res); + if (res.code) { freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start IPA " << camera->id(); return ret; } + data->delayedCtrls_->reset(&res.controls); data->frame_ = 0;