[RFC] libcamera: lc-compliance: Add initial set of per frame control tests
Stefan Klug
stefan.klug at ideasonboard.com
Thu Feb 29 18:08:52 CET 2024
Just to clarify. This patch fails to run on master. I wanted to collect
some feedback on the overall idea and if my assumptions in the tests are
correct.
Cheers,
Stefan
Am 29.02.24 um 18:01 schrieb Stefan Klug:
> 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 additionally by
> calculating a mean brightness on a centered rect of 20x20 pixels.
>
> Until today, these tests where only run on a project specific branch
> with a modified simple pipeline. In theory they should pass on a
> current master :-)
>
> Current test setup: imx219 with simple pipeline on an imx8mp.
> Modifications of either the exposure delay or the gain delay in
> the camera_sensor class resulted in test failures.
> Which is exactly what this test shall proove.
>
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
> src/apps/lc-compliance/capture_test.cpp | 39 ++++
> src/apps/lc-compliance/meson.build | 2 +
> src/apps/lc-compliance/per_frame_controls.cpp | 214 ++++++++++++++++++
> src/apps/lc-compliance/per_frame_controls.h | 41 ++++
> src/apps/lc-compliance/simple_capture.cpp | 4 +-
> src/apps/lc-compliance/simple_capture.h | 2 +-
> src/apps/lc-compliance/time_sheet.cpp | 135 +++++++++++
> src/apps/lc-compliance/time_sheet.h | 53 +++++
> 8 files changed, 487 insertions(+), 3 deletions(-)
> create mode 100644 src/apps/lc-compliance/per_frame_controls.cpp
> create mode 100644 src/apps/lc-compliance/per_frame_controls.h
> 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/capture_test.cpp b/src/apps/lc-compliance/capture_test.cpp
> index 1dcfcf92..43fe59f3 100644
> --- a/src/apps/lc-compliance/capture_test.cpp
> +++ b/src/apps/lc-compliance/capture_test.cpp
> @@ -11,6 +11,7 @@
> #include <gtest/gtest.h>
>
> #include "environment.h"
> +#include "per_frame_controls.h"
> #include "simple_capture.h"
>
> using namespace libcamera;
> @@ -133,3 +134,41 @@ INSTANTIATE_TEST_SUITE_P(CaptureTests,
> testing::Combine(testing::ValuesIn(ROLES),
> testing::ValuesIn(NUMREQUESTS)),
> SingleStream::nameParameters);
> +
> +/*
> + * Test Per frame controls
> + */
> +TEST_F(SingleStream, testFramePreciseExposureChange)
> +{
> + PerFrameControls capture(camera_);
> + capture.configure(StreamRole::Viewfinder);
> + capture.testFramePreciseExposureChange();
> +}
> +
> +TEST_F(SingleStream, testFramePreciseGainChange)
> +{
> + PerFrameControls capture(camera_);
> + capture.configure(StreamRole::Viewfinder);
> + capture.testFramePreciseGainChange();
> +}
> +
> +TEST_F(SingleStream, testExposureGainIsAppliedOnFirstFrame)
> +{
> + PerFrameControls capture(camera_);
> + capture.configure(StreamRole::Viewfinder);
> + capture.testExposureGainIsAppliedOnFirstFrame();
> +}
> +
> +TEST_F(SingleStream, testExposureGainFromFirstRequestGetsApplied)
> +{
> + PerFrameControls capture(camera_);
> + capture.configure(StreamRole::Viewfinder);
> + capture.testExposureGainFromFirstRequestGetsApplied();
> +}
> +
> +TEST_F(SingleStream, testExposureGainFromFirstAndSecondRequestGetsApplied)
> +{
> + PerFrameControls capture(camera_);
> + capture.configure(StreamRole::Viewfinder);
> + capture.testExposureGainFromFirstAndSecondRequestGetsApplied();
> +}
> diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build
> index c792f072..2a6f52af 100644
> --- a/src/apps/lc-compliance/meson.build
> +++ b/src/apps/lc-compliance/meson.build
> @@ -15,7 +15,9 @@ lc_compliance_sources = files([
> 'capture_test.cpp',
> 'environment.cpp',
> 'main.cpp',
> + 'per_frame_controls.cpp',
> 'simple_capture.cpp',
> + 'time_sheet.cpp',
> ])
>
> lc_compliance = executable('lc-compliance', lc_compliance_sources,
> 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..70fc44ac
> --- /dev/null
> +++ b/src/apps/lc-compliance/per_frame_controls.cpp
> @@ -0,0 +1,214 @@
> +/* 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 <gtest/gtest.h>
> +
> +#include "time_sheet.h"
> +
> +using namespace libcamera;
> +
> +PerFrameControls::PerFrameControls(std::shared_ptr<Camera> camera)
> + : SimpleCapture(camera)
> +{
> +}
> +
> +std::shared_ptr<TimeSheet> 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, true);
> + }
> +
> + start(&ctrls);
> +
> + queueCount_ = 0;
> + captureCount_ = 0;
> + captureLimit_ = framesToCapture;
> +
> + auto timeSheet = std::make_shared<TimeSheet>(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<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
> +
> + /* Queue the recommended number of reqeuests. */
> + for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> + std::unique_ptr<Request> 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::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);
> + ts.printAllInfos();
> +
> + runCaptureSession();
> +
> + EXPECT_NEAR(ts[5].metadata().get(controls::ExposureTime).value(), 5000, 20);
> + EXPECT_NEAR(ts[6].metadata().get(controls::ExposureTime).value(), 20000, 20);
> +
> + /* 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 much2 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();
> +
> + 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);
> +
> + /* 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?).";
> +}
> +
> +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();
> +
> + /* 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();
> +
> + /* 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();
> +
> + EXPECT_NEAR((*ts1)[0].metadata().get(controls::ExposureTime).value(), 5000, 20);
> + EXPECT_NEAR((*ts1)[0].metadata().get(controls::AnalogueGain).value(), 1.0, 0.01);
> +
> + /* 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.01);
> +
> + /* 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..e783f024
> --- /dev/null
> +++ b/src/apps/lc-compliance/per_frame_controls.h
> @@ -0,0 +1,41 @@
> +/* 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 <memory>
> +
> +#include <libcamera/libcamera.h>
> +
> +#include "../common/event_loop.h"
> +
> +#include "simple_capture.h"
> +#include "time_sheet.h"
> +
> +class PerFrameControls : public SimpleCapture
> +{
> +public:
> + PerFrameControls(std::shared_ptr<libcamera::Camera> camera);
> +
> + std::shared_ptr<TimeSheet> startCaptureWithTimeSheet(unsigned int framesToCapture, const libcamera::ControlList *controls = nullptr);
> + void runCaptureSession();
> +
> + 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> timeSheet_;
> +};
> 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<libcamera::Camera> camera);
> virtual ~SimpleCapture();
>
> - void start();
> + void start(const libcamera::ControlList *controls = nullptr);
> void stop();
>
> virtual void requestComplete(libcamera::Request *request) = 0;
> diff --git a/src/apps/lc-compliance/time_sheet.cpp b/src/apps/lc-compliance/time_sheet.cpp
> new file mode 100644
> index 00000000..9a0e6544
> --- /dev/null
> +++ b/src/apps/lc-compliance/time_sheet.cpp
> @@ -0,0 +1,135 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas on Board Oy
> + *
> + * time_sheet.cpp
> + */
> +#include "time_sheet.h"
> +
> +#include <sstream>
> +#include <libcamera/libcamera.h>
> +
> +#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<double(const uint8_t *data)> 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;
> + std::cout << "Brightness: " << spotBrightness_ << 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;
> + }
> + }
> +}
> +
> +TimeSheetEntry &TimeSheet::get(size_t pos)
> +{
> + auto &entry = entries_[pos];
> + if (!entry)
> + entry = std::make_shared<TimeSheetEntry>(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..c155763c
> --- /dev/null
> +++ b/src/apps/lc-compliance/time_sheet.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas on Board Oy
> + *
> + * time_sheet.h
> + */
> +
> +#pragma once
> +
> +#include <future>
> +#include <vector>
> +
> +#include <libcamera/libcamera.h>
> +
> +class TimeSheetEntry
> +{
> +public:
> + 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);
> +
> +private:
> + const libcamera::ControlIdMap &idmap_;
> + std::vector<std::shared_ptr<TimeSheetEntry>> entries_;
> +};
--
Regards,
Stefan Klug
More information about the libcamera-devel
mailing list