From patchwork Wed Mar 13 12:12: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: 19703 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 9A5F2BD160 for ; Wed, 13 Mar 2024 12:12:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1BECA62C9C; Wed, 13 Mar 2024 13:12:45 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="sA6C716J"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 16A7062C86 for ; Wed, 13 Mar 2024 13:12:44 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 960BE899; Wed, 13 Mar 2024 13:12:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331941; bh=XqL+6G8l6sAh3t0/iCiSp/QB+GBR3E9zSf2oNX9yT4E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sA6C716JccY6Xu2mFk3G0HxcNVstbQKxIHa/9naHGXgPySWbjcEOXNgR85YK72DgB IaHQ8UvIkxqjAjBdH9SdSQAwKQ4HZKb524pUZmq0OkOBd2hNi88ZoTOo8cIwamqbeu 9ZqOXDGjacZAIXw70mDVVBg/lZB4KAJBumuO3524= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 01/12] libcamera: lc-compliance: Add controls param to start() function Date: Wed, 13 Mar 2024 13:12:12 +0100 Message-Id: <20240313121223.138150-2-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug 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 Reviewed-by: Kieran Bingham --- 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 Wed Mar 13 12:12: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: 19704 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 C5BD6BD160 for ; Wed, 13 Mar 2024 12:12:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C62D262C8B; Wed, 13 Mar 2024 13:12:47 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="rgLSV1pm"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 579D162C8B for ; Wed, 13 Mar 2024 13:12:44 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D1F6DA8F; Wed, 13 Mar 2024 13:12:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331941; bh=ej1uK+sfbRUZYyaaAKX1MFmV6CMLr92Fr0BTKMd+7bY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rgLSV1pm8ubg8NZMvsLd0+wCHASUPxsiKqKpxEDtqcTouKJxNPTHNTq7kaR7kufzl Q8zL2Wv97S7VBolwzm4vXljSsSBCV6kKgki6Wq+bp9EIbfurDUg/vsNo8fOzVLo2Jd /ucwEPF5gJnskvuXRR15PeZ+EEppTGRO8pVhDjUc= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 02/12] libcamera: lc-compliance: Add TimeSheet class Date: Wed, 13 Mar 2024 13:12:13 +0100 Message-Id: <20240313121223.138150-3-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug 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 | 145 ++++++++++++++++++++++++++ src/apps/lc-compliance/time_sheet.h | 55 ++++++++++ 3 files changed, 201 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..0ac544d6 --- /dev/null +++ b/src/apps/lc-compliance/time_sheet.cpp @@ -0,0 +1,145 @@ +/* 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; + +double calcPixelMeanNV12(const uint8_t *data) +{ + return (double)*data; +} + +double calcPixelMeanRAW10(const uint8_t *data) +{ + return (double)*((const uint16_t *)data); +} + +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()) { + auto data = in.planes()[0].data(); + auto streamConfig = stream->configuration(); + auto formatInfo = PixelFormatInfo::info(streamConfig.pixelFormat); + + std::function calcPixelMean; + int pixelStride; + + switch (streamConfig.pixelFormat) { + case formats::NV12: + calcPixelMean = calcPixelMeanNV12; + pixelStride = 1; + break; + case formats::SRGGB10: + calcPixelMean = calcPixelMeanRAW10; + 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; +} + +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->getSpotBrightness(); + } + sequence_ = request->sequence(); +} + +void TimeSheetEntry::printInfo() +{ + std::cout << "=== Frame " << sequence_ << std::endl; + if (!controls_.empty()) { + std::cout << "Controls:" << std::endl; + auto idMap = controls_.idMap(); + assert(idMap); + for (const auto &[id, value] : controls_) { + std::cout << " " << idMap->at(id)->name() << " : " << value.toString() << std::endl; + } + } + + if (!metadata_.empty()) { + std::cout << "Metadata:" << std::endl; + auto idMap = metadata_.idMap(); + assert(idMap); + for (const auto &[id, value] : metadata_) { + std::cout << " " << idMap->at(id)->name() << " : " << value.toString() << std::endl; + } + } + + std::cout << "Calculated Brightness: " << spotBrightness_ << std::endl; +} + +TimeSheetEntry &TimeSheet::get(size_t pos) +{ + auto &entry = entries_[pos]; + if (!entry) + entry = std::make_shared(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); +} + +void TimeSheet::printAllInfos() +{ + for (auto entry : entries_) { + if (entry) + entry->printInfo(); + } +} diff --git a/src/apps/lc-compliance/time_sheet.h b/src/apps/lc-compliance/time_sheet.h new file mode 100644 index 00000000..2bb89ab3 --- /dev/null +++ b/src/apps/lc-compliance/time_sheet.h @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Ideas on Board Oy + * + * time_sheet.h + */ + +#pragma once + +#include +#include + +#include + +class TimeSheetEntry +{ +public: + TimeSheetEntry() = delete; + TimeSheetEntry(const libcamera::ControlIdMap &idmap); + TimeSheetEntry(TimeSheetEntry &&other) noexcept = default; + TimeSheetEntry(const TimeSheetEntry &) = delete; + + libcamera::ControlList &controls() { return controls_; }; + libcamera::ControlList &metadata() { return metadata_; }; + void handleCompleteRequest(libcamera::Request *request, const TimeSheetEntry *previous); + void printInfo(); + double getSpotBrightness() const { return spotBrightness_; }; + double getBrightnessChange() const { return brightnessChange_; }; + +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); + void printAllInfos(); + + TimeSheetEntry &operator[](size_t pos) { return get(pos); }; + TimeSheetEntry &get(size_t pos); + size_t size() const { return entries_.size(); }; + +private: + const libcamera::ControlIdMap &idmap_; + std::vector> entries_; +}; From patchwork Wed Mar 13 12:12: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: 19705 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 A97A6C32A3 for ; Wed, 13 Mar 2024 12:12:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 87F3A62C9C; Wed, 13 Mar 2024 13:12:49 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="F1cBlPPD"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 933DF62C92 for ; Wed, 13 Mar 2024 13:12:44 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1CF18899; Wed, 13 Mar 2024 13:12:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331942; bh=MV5YHf3tkBrRjQWbq+rAGFKuDu/i1UsUbjI6bEIQhmg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=F1cBlPPDieKgHlBUpI158eWMSmSbAH6J/DayMCjKmJZLQ0E4HChIQ2fehmGuv4m6n GFJxyEdvtVEtoYMHzRpRTNFix2ENK8h9qmFeDGxK8+WmDv4piCZn2WF9gae89iEhnm QIIorVxrMOLw+1wOXGRUAPsqCd3Nh5bkAZ/FZIeg= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 03/12] libcamera: lc-compliance: Add initial set of per-frame-control tests Date: Wed, 13 Mar 2024 13:12:14 +0100 Message-Id: <20240313121223.138150-4-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" These tests 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. To run only the teste, one can run: lc-compliance -c -f "SingleStream.*" 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. Signed-off-by: Stefan Klug --- src/apps/lc-compliance/capture_test.cpp | 46 +++ src/apps/lc-compliance/meson.build | 1 + src/apps/lc-compliance/per_frame_controls.cpp | 316 ++++++++++++++++++ src/apps/lc-compliance/per_frame_controls.h | 43 +++ 4 files changed, 406 insertions(+) create mode 100644 src/apps/lc-compliance/per_frame_controls.cpp create mode 100644 src/apps/lc-compliance/per_frame_controls.h diff --git a/src/apps/lc-compliance/capture_test.cpp b/src/apps/lc-compliance/capture_test.cpp index 1dcfcf92..b19e8936 100644 --- a/src/apps/lc-compliance/capture_test.cpp +++ b/src/apps/lc-compliance/capture_test.cpp @@ -11,6 +11,7 @@ #include #include "environment.h" +#include "per_frame_controls.h" #include "simple_capture.h" using namespace libcamera; @@ -133,3 +134,48 @@ INSTANTIATE_TEST_SUITE_P(CaptureTests, testing::Combine(testing::ValuesIn(ROLES), testing::ValuesIn(NUMREQUESTS)), SingleStream::nameParameters); + +/* + * Test Per frame controls + */ +TEST_F(SingleStream, testExposureGainChangeOnSameFrame) +{ + PerFrameControls capture(camera_); + capture.configure(StreamRole::VideoRecording); + capture.testExposureGainChangeOnSameFrame(); +} + +TEST_F(SingleStream, testFramePreciseExposureChange) +{ + PerFrameControls capture(camera_); + capture.configure(StreamRole::VideoRecording); + capture.testFramePreciseExposureChange(); +} + +TEST_F(SingleStream, testFramePreciseGainChange) +{ + PerFrameControls capture(camera_); + capture.configure(StreamRole::VideoRecording); + capture.testFramePreciseGainChange(); +} + +TEST_F(SingleStream, testExposureGainIsAppliedOnFirstFrame) +{ + PerFrameControls capture(camera_); + capture.configure(StreamRole::VideoRecording); + capture.testExposureGainIsAppliedOnFirstFrame(); +} + +TEST_F(SingleStream, testExposureGainFromFirstRequestGetsApplied) +{ + PerFrameControls capture(camera_); + capture.configure(StreamRole::VideoRecording); + capture.testExposureGainFromFirstRequestGetsApplied(); +} + +TEST_F(SingleStream, testExposureGainFromFirstAndSecondRequestGetsApplied) +{ + PerFrameControls capture(camera_); + capture.configure(StreamRole::VideoRecording); + capture.testExposureGainFromFirstAndSecondRequestGetsApplied(); +} diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build index eb7b2d71..2a6f52af 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.cpp', 'simple_capture.cpp', 'time_sheet.cpp', ]) diff --git a/src/apps/lc-compliance/per_frame_controls.cpp b/src/apps/lc-compliance/per_frame_controls.cpp new file mode 100644 index 00000000..eb7164e0 --- /dev/null +++ b/src/apps/lc-compliance/per_frame_controls.cpp @@ -0,0 +1,316 @@ +/* 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 "per_frame_controls.h" + +#include + +#include "time_sheet.h" + +using namespace libcamera; + +static const bool doImageTests = true; + +PerFrameControls::PerFrameControls(std::shared_ptr camera) + : SimpleCapture(camera) +{ +} + +std::shared_ptr +PerFrameControls::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 PerFrameControls::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 PerFrameControls::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 PerFrameControls::runCaptureSession() +{ + Stream *stream = config_->at(0).stream(); + const std::vector> &buffers = allocator_->buffers(stream); + + /* Queue the recommended number of reqeuests. */ + 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_; +} + +void PerFrameControls::testExposureGainChangeOnSameFrame() +{ + ControlList startValues; + startValues.set(controls::ExposureTime, 5000); + startValues.set(controls::AnalogueGain, 1.0); + + auto timeSheet = 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); + + runCaptureSession(); + + /* Uncomment this to debug the test */ + /* ts.printAllInfos(); */ + + 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].getBrightnessChange() > 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 mesaured brightness change were not on same frame. " + << "(Wrong control delay?, Start frame event too late?)"; + EXPECT_EQ(exposureChangeIndex, gainChangeIndex) + << "Gain change and mesaured brightness change were not on same frame. " + << "(Wrong control delay?, Start frame event too late?)"; + } +} + +void PerFrameControls::testFramePreciseExposureChange() +{ + auto timeSheet = 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); + + runCaptureSession(); + + /* Uncomment this to debug the test */ + /* ts.printAllInfos(); */ + + 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].getBrightnessChange(), 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].getBrightnessChange(), 1.3) + << "Brightness in frame " << 6 << " did not increase as expected (reference: " + << ts[3].getSpotBrightness() << " current: " << ts[6].getSpotBrightness() << " )" << std::endl; + + /* No increase just after setting exposure */ + EXPECT_NEAR(ts[7].getBrightnessChange(), 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].getBrightnessChange(), 1.0, 0.05) + << "Brightness changed too much 2 frames after the expected time of change (control delay too low?)."; + } +} + +void PerFrameControls::testFramePreciseGainChange() +{ + auto timeSheet = 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); + + runCaptureSession(); + + /* Uncomment this, to debug the test */ + /* ts.printAllInfos(); */ + + 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].getBrightnessChange(), 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].getBrightnessChange(), 1.7) + << "Brightness in frame " << 6 << " did not increase as expected (reference: " + << ts[5].getSpotBrightness() << " current: " << ts[6].getSpotBrightness() << " )" << std::endl; + + /* No increase just after setting gain */ + EXPECT_NEAR(ts[7].getBrightnessChange(), 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].getBrightnessChange(), 1.0, 0.05) + << "Brightness changed too much after the expected time of change (control delay too low?)."; + } +} + +void PerFrameControls::testExposureGainFromFirstRequestGetsApplied() +{ + auto timeSheet = startCaptureWithTimeSheet(5); + auto &ts = *timeSheet; + + ts[0].controls().set(controls::ExposureTime, 10000); + ts[0].controls().set(controls::AnalogueGain, 4.0); + + 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); +} + +void PerFrameControls::testExposureGainFromFirstAndSecondRequestGetsApplied() +{ + auto timeSheet = 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); + + 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); +} + +void PerFrameControls::testExposureGainIsAppliedOnFirstFrame() +{ + ControlList startValues; + startValues.set(controls::ExposureTime, 5000); + startValues.set(controls::AnalogueGain, 1.0); + + auto ts1 = startCaptureWithTimeSheet(3, &startValues); + + 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 = startCaptureWithTimeSheet(3, &startValues); + + 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).getSpotBrightness() / ts1->get(1).getSpotBrightness(); + EXPECT_GT(brightnessChange, 2.0); + } +} diff --git a/src/apps/lc-compliance/per_frame_controls.h b/src/apps/lc-compliance/per_frame_controls.h new file mode 100644 index 00000000..a341c61f --- /dev/null +++ b/src/apps/lc-compliance/per_frame_controls.h @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2024, Ideas on Board Oy + * + * per_frame_controls.h - Tests for per frame controls + */ + +#pragma once + +#include + +#include + +#include "../common/event_loop.h" + +#include "simple_capture.h" +#include "time_sheet.h" + +class PerFrameControls : public SimpleCapture +{ +public: + PerFrameControls(std::shared_ptr camera); + + std::shared_ptr + startCaptureWithTimeSheet(unsigned int framesToCapture, const libcamera::ControlList *controls = nullptr); + void runCaptureSession(); + + void testExposureGainChangeOnSameFrame(); + void testFramePreciseExposureChange(); + void testFramePreciseGainChange(); + void testExposureGainIsAppliedOnFirstFrame(); + void testExposureGainFromFirstRequestGetsApplied(); + void testExposureGainFromFirstAndSecondRequestGetsApplied(); + + int queueRequest(libcamera::Request *request); + void requestComplete(libcamera::Request *request) override; + + unsigned int queueCount_; + unsigned int captureCount_; + unsigned int captureLimit_; + + std::weak_ptr timeSheet_; +}; From patchwork Wed Mar 13 12:12: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: 19706 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 6B6B7BD160 for ; Wed, 13 Mar 2024 12:12:52 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AC7E862CA8; Wed, 13 Mar 2024 13:12:51 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Q/v6zje0"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D261762C86 for ; Wed, 13 Mar 2024 13:12:44 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 59187A8F; Wed, 13 Mar 2024 13:12:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331942; bh=7oapYLMN7cyigWB9FuJ2msVMCOH06w430iE+QjjU9L0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Q/v6zje0Sb1f4w/LjORQklrlrqAIvnbI9qpIRRLSrbMr91WzXMS5QKduBxCvr+SPH pRfM4weyi3z01pL3NwVC5AgCg2o5qqHja0ToTVAJn/NrDjbF3Kl9zikiKL1clAn0KP Re/nP1XfgeaXyxChzaASURxCq9EZUnKZZ8E0klvI= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 04/12] libcamera: delayed_controls: Update unit tests to expected semantics Date: Wed, 13 Mar 2024 13:12:15 +0100 Message-Id: <20240313121223.138150-5-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug 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 | 203 +++++++++++++++++++++++++++++--------- 1 file changed, 156 insertions(+), 47 deletions(-) diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp index a8ce9828..e0634fa1 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,72 @@ 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; @@ -157,7 +224,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 +241,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 +263,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 +296,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 +306,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 +326,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 +336,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 +351,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 +369,36 @@ 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; /* 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 Wed Mar 13 12:12: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: 19707 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 31A81BD160 for ; Wed, 13 Mar 2024 12:12:57 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 05D8C62C9D; Wed, 13 Mar 2024 13:12:56 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="XeSj6bHn"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 202BC62C93 for ; Wed, 13 Mar 2024 13:12:45 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 99923899; Wed, 13 Mar 2024 13:12:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331942; bh=0fdCqsmsRjGgHyqD2+KCaAkGapqqFfm+kwHb3gyTHbo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XeSj6bHn6LQJMavzL9pWP+Z3qlwRNTGlNMRzsre9Azf5R7c1Qlb401CANApEUlhxN MVGbio+AocoMI7sF3+GrNKKPXKMLqf6tZhbOZeBGJ7nKo+CkMPpiZqCD5JsKTDQSSv 0J8kzE3U6Us4p++2wj+Ogjrmexpq1imvJynf13MY= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 05/12] libcamera: delayed_controls: Rename class members Date: Wed, 13 Mar 2024 13:12:16 +0100 Message-Id: <20240313121223.138150-6-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug 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 Wed Mar 13 12:12: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: 19708 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 CACCDC32A3 for ; Wed, 13 Mar 2024 12:12:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3570A62C9B; Wed, 13 Mar 2024 13:12:58 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="f75teChM"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 77E2162C95 for ; Wed, 13 Mar 2024 13:12:45 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D6EFFA8F; Wed, 13 Mar 2024 13:12:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331943; bh=KrTXobubEcNjNHEuv7yp0VY2NjrjW5FghfhtaFzy0pY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=f75teChM2Q9Cn66jNPd0/9CZEZmgt4peSi6n9HGSHZb2beds/fM2KsOlJYzHCEIMZ RwKdAXT37y8e1JSJd7hg7jkRxqiljscyc58AqPWlh2Cb36mE5aYA6+cyCsDPrJjF9p CrMaNAj5MO6EcQBZfPQtJE+YyaFQsBe8o1KJL/kA= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 06/12] libcamera: delayed_controls: Rework delayed controls implementation Date: Wed, 13 Mar 2024 13:12:17 +0100 Message-Id: <20240313121223.138150-7-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Functional changes: - Requests need to be queued for every frame - The startup phase is no longer treated as special case - Requests carry a sequence number, so that 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 it's too late, controls get delayed but they are not lost - Requests attached to frame 0 don't get lost Technical notes: A sourceSequence_ replaces the updated flag to be able to track from which frame the update stems. This is needed for the following use case: Assume e.g. On frame 10 an 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 | 12 +- src/libcamera/delayed_controls.cpp | 207 ++++++++++++++---- 2 files changed, 174 insertions(+), 45 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index 4f8d2424..2738e8bf 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -30,25 +30,29 @@ public: void reset(); bool push(const ControlList &controls); + bool pushForFrame(uint32_t sequence, const ControlList &controls); ControlList get(uint32_t sequence); void applyControls(uint32_t sequence); private: + bool controlsAreQueuedForFrame(unsigned int frame, const ControlList &controls); + class Info : public ControlValue { public: Info() - : updated(false) + : sourceSequence_(0) { } - Info(const ControlValue &v, bool updated_ = true) - : ControlValue(v), updated(updated_) + Info(const ControlValue &v, std::optional sourceSequence = std::nullopt) + : ControlValue(v), sourceSequence_(sourceSequence) { } - bool updated; + /* The sequence id, this info stems from*/ + std::optional 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 86571cd4..59314388 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -115,8 +115,9 @@ DelayedControls::DelayedControls(V4L2Device *device, */ void DelayedControls::reset() { - 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; @@ -133,26 +134,129 @@ void DelayedControls::reset() * 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); } + + /* Copy state from previous frames. */ + for (auto &ctrl : values_) { + for (auto i = queueIndex_; i < writeIndex_; i++) { + Info &info = ctrl.second[i + 1]; + info = ctrl.second[i]; + info.sourceSequence_.reset(); + } + } +} + +/** + * \brief Helper function to check if controls are queued already + * \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::controlsAreQueuedForFrame(unsigned int sequence, const ControlList &controls) +{ + auto idMap = controls.idMap(); + if (!idMap) { + LOG(DelayedControls, Warning) << " idmap is null"; + return false; + } else { + for (const auto &[id, value] : controls) { + if (values_[idMap->at(id)][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 + * \deprecated This function is deprecated in favour of pushForFrame * * Push a set of controls to the control queue. This increases the control queue * depth by one. * - * \returns true if \a controls are accepted, or false otherwise + * \returns See return value of DelayedControls::pushForFrame */ 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; + LOG(DelayedControls, Debug) << "Using deprecated function push(controls): " << queueIndex_; + return pushForFrame(queueIndex_, controls); +} + +/** + * \brief Push a set of controls for a given frame + * \param[in] sequence Sequence to push the controls for + * \param[in] controls List of controls to add to the device queue + * + * Pushes the controls given by \a controls, to be applied at frame \a sequence. + * + * If there are controls already queued for that frame, these 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::pushForFrame(uint32_t sequence, const ControlList &controls) +{ + 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() || controlsAreQueuedForFrame(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 (static_cast(updateIndex) - static_cast(writeIndex_) >= listSize - static_cast(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_; + for (auto i = queueIndex_; i < updateIndex; i++) { + /* Copy state from previous queue. */ + for (auto &ctrl : values_) { + auto &ringBuffer = ctrl.second; + ringBuffer[i] = ringBuffer[i - 1]; + ringBuffer[i].sourceSequence_.reset(); + } + } } /* Update with new controls. */ @@ -167,20 +271,34 @@ 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); + 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_.value_or(0) <= 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: " << queueIndex_ << " it will be active in " << updateIndex; + + if (sequence >= queueIndex_) + queueIndex_ = sequence + 1; return true; } @@ -202,19 +320,17 @@ bool DelayedControls::push(const ControlList &controls) */ ControlList DelayedControls::get(uint32_t sequence) { - unsigned int index = std::max(0, sequence - maxDelay_); - 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; + << " at index " << sequence; } return out; @@ -222,16 +338,21 @@ 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 @@ -241,10 +362,10 @@ 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) { + if (info.sourceSequence_.has_value()) { if (controlParams_[id].priorityWrite) { /* * This control must be written now, it could @@ -262,21 +383,25 @@ void DelayedControls::applyControls(uint32_t sequence) } LOG(DelayedControls, Debug) - << "Setting " << id->name() - << " to " << info.toString() - << " at index " << index; - - /* Done with this update, so mark as completed. */ - info.updated = false; + << "Writing " << id->name() + << " (" << info.toString() << ") " + << " for frame " << index; } } - 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"; + /* Copy state from previous frames without resetting the sourceSequence */ + for (auto &ctrl : values_) { + for (auto i = oldWriteIndex; i < writeIndex_; i++) { + Info &info = ctrl.second[i + 1]; + info = values_[ctrl.first][i]; + } + } } device_->setControls(&out); From patchwork Wed Mar 13 12:12:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19709 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 9DBD7BD160 for ; Wed, 13 Mar 2024 12:13:00 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A0CDF62C9D; Wed, 13 Mar 2024 13:12:59 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="CrVCuWoB"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A45BC62C92 for ; Wed, 13 Mar 2024 13:12:45 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1F798F8B; Wed, 13 Mar 2024 13:12:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331943; bh=eqDcYzYC3IRHoBLNUgYxis11DNDgStZM920gWGOcno8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CrVCuWoBMIo10Rhc8qe16Bhe1oBN1zKJJ9XWzhkWNqZzXq4Bti6eSe6qAZMR++wzR N9wjEPv7RRG9ysD9gX2zbpfcay0HgWf0JozTacyuuJLvO6z28FVUPkPrgBC19aBUv3 eKc7MJFXxs4USuwLv1ZHqV1hF3HtwPLT64ObvjPk= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 07/12] libcamera: delayed_controls: Add some logging Date: Wed, 13 Mar 2024 13:12:18 +0100 Message-Id: <20240313121223.138150-8-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" This replaces changes the log output to a more compact form. There might be different opinions on that one Signed-off-by: Stefan Klug --- src/libcamera/delayed_controls.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 59314388..c46e54d1 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(); } @@ -212,6 +214,14 @@ bool DelayedControls::push(const ControlList &controls) bool DelayedControls::pushForFrame(uint32_t sequence, const ControlList &controls) { + 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; } @@ -320,6 +330,7 @@ bool DelayedControls::pushForFrame(uint32_t sequence, const ControlList &control */ ControlList DelayedControls::get(uint32_t sequence) { + LOG(DelayedControls, Debug) << "get " << sequence << ":"; ControlList out(device_->controls()); for (const auto &ctrl : values_) { const ControlId *id = ctrl.first; @@ -328,9 +339,8 @@ ControlList DelayedControls::get(uint32_t sequence) out.set(id->id(), info); LOG(DelayedControls, Debug) - << "Reading " << id->name() - << " to " << info.toString() - << " at index " << sequence; + << " " << id->name() + << ": " << info.toString(); } return out; From patchwork Wed Mar 13 12:12:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19710 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 8CBDAC32C3 for ; Wed, 13 Mar 2024 12:13:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8342162CA6; Wed, 13 Mar 2024 13:13:01 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="q+UrVthQ"; 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 ED20762C9B for ; Wed, 13 Mar 2024 13:12:45 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5BA92899; Wed, 13 Mar 2024 13:12:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331943; bh=sO0A/gemJARVBZjvuPwdylUf4HYJ/EZt5BDp1fF+NCs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=q+UrVthQG3//1xRgjJc2ssxbuS71ale86Nt94Z2OIW8wBX9b+oK149WKZuUQWw3av QVJDtzFTv+Ep2AYNG3VrHFIrIDYDHtfAgJ/pYxTtPd95BQMZeOfnpk23de1BKSH5CQ 5INR5Q5z7wHKyDGvzwzqzSqrj1bm/kAQ1tgKudYw= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 08/12] libcamera: delayed_controls: Add ctrls list parameter to reset() function Date: Wed, 13 Mar 2024 13:12:19 +0100 Message-Id: <20240313121223.138150-9-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug 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 --- include/libcamera/internal/delayed_controls.h | 2 +- src/libcamera/delayed_controls.cpp | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index 2738e8bf..a3063400 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); bool pushForFrame(uint32_t sequence, const ControlList &controls); diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index c46e54d1..ac38eb64 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -115,7 +115,7 @@ DelayedControls::DelayedControls(V4L2Device *device, * Resets the state machine to a starting position based on control values * retrieved from the device. */ -void DelayedControls::reset() +void DelayedControls::reset(ControlList *ctrls) { queueIndex_ = 0; /* Frames up to maxDelay_ will be based on sensor init values. */ @@ -126,6 +126,18 @@ void DelayedControls::reset() for (auto const ¶m : controlParams_) ids.push_back(param.first->id()); + if (ctrls) { + device_->setControls(ctrls); + + LOG(DelayedControls, Debug) << "reset:"; + auto idMap = ctrls->idMap(); + if (idMap) { + for (const auto &[id, value] : *ctrls) { + LOG(DelayedControls, Debug) << " " << idMap->at(id)->name() << " : " << value.toString(); + } + } + } + ControlList controls = device_->getControls(ids); /* Seed the control queue with the controls reported by the device. */ From patchwork Wed Mar 13 12:12:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19711 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 A876DC32A3 for ; Wed, 13 Mar 2024 12:13:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B075A62C9D; Wed, 13 Mar 2024 13:13:02 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="PYDfwXqb"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3D02A62C9D for ; Wed, 13 Mar 2024 13:12:46 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 982EAA8F; Wed, 13 Mar 2024 13:12:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331943; bh=zUGKUWYuVQ7WOMMNKx2r64dDTVeyWgc35NFIoS2N1sc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=PYDfwXqbxB+iS303v9/G/qyfGGLhZAYS72wqK78NkbNsm8gJM8hcH2u+amh9ulmYe G7P89UW2aD+D0OP0OTWF/McaYpltUqziXhBihicpsbQwCMGQDy9wd1iLv8VK/COik1 dTOIQP58DMF+EeKQGtewsnbizxaOOmlhW38v6e5w= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 09/12] pipeline: rkisp1: Move call to setSensorControls.emit() Date: Wed, 13 Mar 2024 13:12:20 +0100 Message-Id: <20240313121223.138150-10-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug 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 | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 9dc5f53c..7f8d3bc1 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_; @@ -367,7 +367,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 +431,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 +440,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 Wed Mar 13 12:12:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19713 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 44E5FC32C5 for ; Wed, 13 Mar 2024 12:13:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 721C562CA6; Wed, 13 Mar 2024 13:13:05 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="eTg4sVdk"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C4CB862C9A for ; Wed, 13 Mar 2024 13:12:46 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D4343F8B; Wed, 13 Mar 2024 13:12:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331944; bh=uwdYweTAVk/pjADVJ+6SbeTfJDMs2NY3YLaQ6nke5M4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eTg4sVdkyt8VJXJeAfQrY7HjvrCwo3j1ypNWxKdejaVJ0a/yifySbtMMN5QgPsERl T32QuoiSjm39mQQ9Ff06K1RWlHvEdMZlOk2lGgSoz8dK2YvZqcHcLnIGJa88VWiOZk X8X2kD1k4MZcUbURKYeICHX8eZEALGnt36p5FyiA= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 10/12] pipeline: rkisp1: Add more debug logging Date: Wed, 13 Mar 2024 13:12:21 +0100 Message-Id: <20240313121223.138150-11-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug 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 7f8d3bc1..71db2ae7 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -313,6 +313,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()); @@ -353,6 +354,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 Wed Mar 13 12:12:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19712 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 1B013C32C4 for ; Wed, 13 Mar 2024 12:13:05 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8B83F62CAF; Wed, 13 Mar 2024 13:13:04 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="jwfsVsQ5"; 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 9ED9962C96 for ; Wed, 13 Mar 2024 13:12:46 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1D161899; Wed, 13 Mar 2024 13:12:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331944; bh=7OIkJ8llq2WQ664El73vGRKGKvUxgjQIquAjBIulhF4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jwfsVsQ5Y0rRyKAD3SO31uj3GfEFh9TqzmQEiPIfWpqwdS7J500AYtfzdjmtoUGOE GivsZxHqbb/Ff6Ey7Pb7lMIdlE98yyYWO4MeraG2Lqwx/vo9OiN2hxmznaDGeCHe5n yUqdSOoXTZ+/ixtIsb5DaNKal1TRHgi9/QwvdBb0= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 11/12] pipeline: rkisp1: Fix per-frame-controls in manual mode Date: Wed, 13 Mar 2024 13:12:22 +0100 Message-Id: <20240313121223.138150-12-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug 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 71db2ae7..8780abc9 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -321,6 +321,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) @@ -369,8 +375,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 586b46d6..3e061b52 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_->pushForFrame(frame, sensorControls); } void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata) From patchwork Wed Mar 13 12:12:23 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19714 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 43443BD160 for ; Wed, 13 Mar 2024 12:13:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7684E62D27; Wed, 13 Mar 2024 13:13:06 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="TG5jZ4u1"; 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 EC1A162C9F for ; Wed, 13 Mar 2024 13:12:46 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 59942A8F; Wed, 13 Mar 2024 13:12:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331944; bh=K9QJJqMlbyeIAybLkwePUedqoG2A+3ScRBC8rZ4JR0Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TG5jZ4u1WYCeEoNbkTmK986lc90sqfBYTjmcAChNeuSmsziXaEdm91LfWZTwWFkPT +1OVkl3JXlTLpA5BpiMdRg+oxAoMdj+g3lzuZ8YhW/jmpsZCGOA3y4BTJdilGkTvgd C4PuloEbrvvctkWz++VG2MU1OEkgbBRoL29h+Qbw= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 12/12] pipeline: rkisp1: Apply initial controls Date: Wed, 13 Mar 2024 13:12:23 +0100 Message-Id: <20240313121223.138150-13-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-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: , Cc: Stefan Klug 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 | 20 ++++++++++++++++---- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++++-- 3 files changed, 29 insertions(+), 7 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 8780abc9..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,11 +200,19 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, return 0; } -int IPARkISP1::start() +void IPARkISP1::start(const ControlList &controls, StartResult *result) { - setControls(0); + 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() @@ -322,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 3e061b52..7c68073d 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;