From patchwork Wed Mar 13 10:56:33 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19689 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 54F3FBD1F1 for ; Wed, 13 Mar 2024 10:57:01 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1EABB62C92; Wed, 13 Mar 2024 11:56:56 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="AHD3ERE6"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CB6AA62973 for ; Wed, 13 Mar 2024 11:56:52 +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 44AABA8F; Wed, 13 Mar 2024 11:56:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327390; bh=XqL+6G8l6sAh3t0/iCiSp/QB+GBR3E9zSf2oNX9yT4E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=AHD3ERE6U4oqRW0HaZNgvI9GcXE/FfQ3bx4NuOo/MCRcWEPezIbwlYyobu6kTmore 3s1op3dF7kqrIu/8oko4KnhYLE3Fo7x2xnOVxbv7CaPvbK/quXzw2Cp+ok0XInnHJ4 VQJahULy5JQSuYxTr72/V9tm6QlRcwKq3aNLoGss= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 01/12] libcamera: lc-compliance: Add controls param to start() function Date: Wed, 13 Mar 2024 11:56:33 +0100 Message-Id: <20240313105645.120317-2-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 --- 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 10:56:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19690 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 9A9E1BD1F1 for ; Wed, 13 Mar 2024 10:57:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BB1F262C9A; Wed, 13 Mar 2024 11:56:57 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="idtXUAoM"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E77EC62C80 for ; Wed, 13 Mar 2024 11:56:52 +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 7CF87F8B; Wed, 13 Mar 2024 11:56:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327390; bh=ej1uK+sfbRUZYyaaAKX1MFmV6CMLr92Fr0BTKMd+7bY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=idtXUAoMO3+XFusyH1ZqBvntYw87gz0H7ODmnewcCwZio7EseFbr4GxYYOq7Q8WAM FGoIDcWkVyHVxRXciVSvOUzkfue/yQvsI0lEoW7RKtu9rI8nEZTTpDzWSTazv2XPDE GxJlKb05vZrekQiAOFk/W/48yo8d5X34OvIKHEJ4= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 02/12] libcamera: lc-compliance: Add TimeSheet class Date: Wed, 13 Mar 2024 11:56:34 +0100 Message-Id: <20240313105645.120317-3-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 | 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 10:56:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19691 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 66111BD1F1 for ; Wed, 13 Mar 2024 10:57:05 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 10DE762C96; Wed, 13 Mar 2024 11:56:59 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="FBf94LHd"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 347E662C85 for ; Wed, 13 Mar 2024 11:56:53 +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 B8D43899; Wed, 13 Mar 2024 11:56:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327390; bh=MV5YHf3tkBrRjQWbq+rAGFKuDu/i1UsUbjI6bEIQhmg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FBf94LHdjjm2nI5lXKinnPz3kOekcjr1LQmP5WQN4l9sF+p3ScF/wJqmwPbXT1aXm m91PWSZdPw8q+2Z8w6j3DMZsO93b+8gGnpbbyoyORfo4CFn/JghdeWoPORQH+AZIx+ VQEd+aF8TzoUujHSVnoK8kUMRCz7QjHWkPI9llsU= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 03/12] libcamera: lc-compliance: Add initial set of per-frame-control tests Date: Wed, 13 Mar 2024 11:56:35 +0100 Message-Id: <20240313105645.120317-4-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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" 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 10:56:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19692 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 E44D8C32A3 for ; Wed, 13 Mar 2024 10:57:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1181B62C8C; Wed, 13 Mar 2024 11:57:01 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="S5tSoRva"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 69DAA62C87 for ; Wed, 13 Mar 2024 11:56:53 +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 F33FBA8F; Wed, 13 Mar 2024 11:56:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327391; bh=7oapYLMN7cyigWB9FuJ2msVMCOH06w430iE+QjjU9L0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=S5tSoRva1KXH5Dojug4RUYR8uz8UDR1iv/KD6Cm3YE5+coRLsS+wtr0ZjVSpXImqs RVTeCSnWC2UmKpWwuwK926zfSQHNBOZmGiCkzR5SabwaNqf3J0BaF42AKBAPNrvMjr BQzXH1HJILne+KBLojvMiYrlUy4pgWqDnHOzRY5g= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 04/12] libcamera: delayed_controls: Update unit tests to expected semantics Date: Wed, 13 Mar 2024 11:56:36 +0100 Message-Id: <20240313105645.120317-5-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 | 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 10:56:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19694 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 0F1E3C32A3 for ; Wed, 13 Mar 2024 10:57:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4772162CA5; Wed, 13 Mar 2024 11:57:03 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="tVwHY1hv"; 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 A182D62C8B for ; Wed, 13 Mar 2024 11:56:53 +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 38B74899; Wed, 13 Mar 2024 11:56:31 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327391; bh=0fdCqsmsRjGgHyqD2+KCaAkGapqqFfm+kwHb3gyTHbo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tVwHY1hv8WTgJoLPL9veER7FHSlLMqE4DA+VL9RWTNHiqNpy2j54WU6cQ8NAkL1Hl C5iWzIGdqPl460rc7UlJVw1uTOFU3eJGcx9tXLyEJ/8WLBn1fWQpE3bN+WlLD/y3rq RGnhxm+OpLtCClyQUFQKmPrhvRpnpRolInC4bLzE= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 05/12] libcamera: delayed_controls: Rename class members Date: Wed, 13 Mar 2024 11:56:37 +0100 Message-Id: <20240313105645.120317-6-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 Wed Mar 13 10:56:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19693 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 8E275BD1F1 for ; Wed, 13 Mar 2024 10:57:08 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C4A3262C8B; Wed, 13 Mar 2024 11:57:01 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="oT9Po/0s"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DE60062C8C for ; Wed, 13 Mar 2024 11:56:53 +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 749FEA8F; Wed, 13 Mar 2024 11:56:31 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327391; bh=KrTXobubEcNjNHEuv7yp0VY2NjrjW5FghfhtaFzy0pY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=oT9Po/0sKHwyBrJlunmvMbkr0JUPPLAY0Fv5p/lZfMNAR7YmYzRTZA2wugLXWDth/ 99HpQcq7wSdMTESFfo4z9wcQyvIcJCpXgz7OZGNsWWn3eu2VMUKUyOfMg9uqxNfeqI ex/63vAGiT+aVkNUC2cc+xcar9WQIzVsdI0ZfBzg= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 06/12] libcamera: delayed_controls: Rework delayed controls implementation Date: Wed, 13 Mar 2024 11:56:38 +0100 Message-Id: <20240313105645.120317-7-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 - 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 10:56:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19695 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 09B18C32C3 for ; Wed, 13 Mar 2024 10:57:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1E8E462C80; Wed, 13 Mar 2024 11:57:04 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="heWHkB3M"; 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 28BD36294A for ; Wed, 13 Mar 2024 11:56:54 +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 AEC98899; Wed, 13 Mar 2024 11:56:31 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327391; bh=eqDcYzYC3IRHoBLNUgYxis11DNDgStZM920gWGOcno8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=heWHkB3Mf0YWQZgT+KXqKdJ/EjZdTSoycc7G5aPMlqcwIiWB6np9JdE7OkCvU1y5a qlGzNTcXoCvaQL9p/h3pfmBTbCRkMI5r4+dg8MXPmDjebyiq/MEH9wvE2CZLSCh9C+ cfTFjqZTxsZ0Ta/hjQnmA/vbQ+1sK9uxhK1liJKs= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 07/12] libcamera: delayed_controls: Add some logging Date: Wed, 13 Mar 2024 11:56:39 +0100 Message-Id: <20240313105645.120317-8-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 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 10:56:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19696 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 F2101BD1F1 for ; Wed, 13 Mar 2024 10:57:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9DE0962C9A; Wed, 13 Mar 2024 11:57:05 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="pJUn/1/x"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E2D262C8F for ; Wed, 13 Mar 2024 11:56:54 +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 E8CA0A8F; Wed, 13 Mar 2024 11:56:31 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327392; bh=sO0A/gemJARVBZjvuPwdylUf4HYJ/EZt5BDp1fF+NCs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=pJUn/1/xqvb6YsdxGXCkkt211+QHSU7lH/Zl6/a41JNa+CwZxkjUrw23RySvwoO8y nH0Lwxszim00JKR02RFjjyyhucHkgCkVsxWUsIXaLF8+wVel0t9gZ6pOEudXUiP/na z4rL8RWyu/C7wUhX9O0PLRmeJjO9ojPJXZ5sGYh0= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 08/12] libcamera: delayed_controls: Add ctrls list parameter to reset() function Date: Wed, 13 Mar 2024 11:56:40 +0100 Message-Id: <20240313105645.120317-9-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 --- 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 10:56:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19697 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 2718AC32C4 for ; Wed, 13 Mar 2024 10:57:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D1FD162CA8; Wed, 13 Mar 2024 11:57:06 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="O6DrcvbA"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D9EF562C93 for ; Wed, 13 Mar 2024 11:56:54 +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 6956AA8F; Wed, 13 Mar 2024 11:56:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327392; bh=zUGKUWYuVQ7WOMMNKx2r64dDTVeyWgc35NFIoS2N1sc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=O6DrcvbAC6YgVANs+ylK8Y1MCcTPvB3LGSoInGS3aWNvvn6RhhHcp6Z0o20SeL9jq EMjQl3LtGYBB+fL7VnVMQjW8/jdofzSzjzWgQhj097s1Mi5uw1l0zZkTdkHrZelLWJ rkLjcRvs1xSQO7BFDOHJvLWsXOPXnxOXyhVUc3Wc= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 09/12] pipeline: rkisp1: Move call to setSensorControls.emit() Date: Wed, 13 Mar 2024 11:56:42 +0100 Message-Id: <20240313105645.120317-11-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 | 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 10:56:43 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19700 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 3385CC32C6 for ; Wed, 13 Mar 2024 10:57:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5722C62CAB; Wed, 13 Mar 2024 11:57:11 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="KyQTjJv9"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F9CB62C94 for ; Wed, 13 Mar 2024 11:56:55 +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 A4422899; Wed, 13 Mar 2024 11:56:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327392; bh=uwdYweTAVk/pjADVJ+6SbeTfJDMs2NY3YLaQ6nke5M4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KyQTjJv9ngEUdM/snTKCpkyJvs3lJ8v2I5xo7fJiOeiUGhTOviJoZRbFCyhhUnkiE ubnH4UI7jck75EgtGvt+D6rj6b3qa/eVS9AEstXzn25DiZbwwMUMWOH3oTxyP+iFW7 YZAbouNU3RBtcCMmUj4KDTjANcJOKalhDzfHD/N0= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 10/12] pipeline: rkisp1: Add more debug logging Date: Wed, 13 Mar 2024 11:56:43 +0100 Message-Id: <20240313105645.120317-12-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 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 10:56:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19699 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 6F867C32A3 for ; Wed, 13 Mar 2024 10:57:14 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0248662C96; Wed, 13 Mar 2024 11:57:10 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="CyNfWp8v"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 652FF62C90 for ; Wed, 13 Mar 2024 11:56:55 +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 DCCEBA8F; Wed, 13 Mar 2024 11:56:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327393; bh=7OIkJ8llq2WQ664El73vGRKGKvUxgjQIquAjBIulhF4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CyNfWp8v0h+iqXP5I3NOu7Bm8xIsAxhwn0kzBAWXv2sbBaQxHbO2vyQNGtTiBdIUP 8k/KCWcAA5uFGkCB7nQG53iA6jQhoYYdKztAE0fkysXK0cYnsHkxOHxfKAN6expak4 +PozwzgAzwyiAAZ66+qFoOPLizNQe8LluSXRfRCg= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 11/12] pipeline: rkisp1: Fix per-frame-controls in manual mode Date: Wed, 13 Mar 2024 11:56:44 +0100 Message-Id: <20240313105645.120317-13-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 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 10:56:45 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19701 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 B7D16C32C7 for ; Wed, 13 Mar 2024 10:57:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C101762CAC; Wed, 13 Mar 2024 11:57:12 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="jKPZgN4U"; 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 9F2D462C98 for ; Wed, 13 Mar 2024 11:56:55 +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 22D5CF8B; Wed, 13 Mar 2024 11:56:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710327393; bh=K9QJJqMlbyeIAybLkwePUedqoG2A+3ScRBC8rZ4JR0Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jKPZgN4Ugonw4fCPLw24/V/JDbR/BC4KbZhhljNfxlSef9P4ZuxrUlJinzXzceyXW cek8MaZjPVyjZQUMMz2G+mZhM1WbQu2azNjcRF8t6PNOJzizKJeaF2et5GhSuFRukB XX5HRVkoZmtn6e+Kidbxv8EUXP46NHKl0IY/yi+Q= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH 12/12] pipeline: rkisp1: Apply initial controls Date: Wed, 13 Mar 2024 11:56:45 +0100 Message-Id: <20240313105645.120317-14-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313105645.120317-1-stefan.klug@ideasonboard.com> References: <20240313105645.120317-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 | 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;