[RFC] libcamera: lc-compliance: Add initial set of per frame control tests

Kieran Bingham kieran.bingham at ideasonboard.com
Mon Mar 4 12:58:34 CET 2024


Quoting David Plowman (2024-03-04 11:35:41)
> Hi Stefan
> 
> On Fri, 1 Mar 2024 at 13:22, Stefan Klug <stefan.klug at ideasonboard.com> wrote:
> >
> > Hi David,
> >
> > Am 01.03.24 um 13:04 schrieb David Plowman:
> > > Hi Stefan
> > >
> > > On Fri, 1 Mar 2024 at 11:22, Stefan Klug <stefan.klug at ideasonboard.com> wrote:
> > >>
> > >> Hi David,
> > >>
> > >> thanks for your comment. This is the same message I sent before, but with proper
> > >> linewrapping - sorry about that.
> > >>
> > >> Am 01.03.24 um 08:56 schrieb David Plowman:
> > >>> Hi Stefan
> > >>>
> > >>> Thanks for posting this and re-starting the discussion of per-frame controls.
> > >>>
> > >>> Could you perhaps just summarise exactly what you mean by per-frame
> > >>> controls, that is to say, what is being tested here?
> > >>>
> > >>> Eye-balling the code, I think I understood that:
> > >>>
> > >>> * You are setting controls in a specific request.
> > >>> * When that request completes, you are expecting the controls to have
> > >>> taken effect in the images that were returned with that request (but
> > >>> not earlier).
> > >>>
> > >>> But I wasn't totally sure - have I understood that correctly?
> > >>>
> > >>> Also, do we know if any other pipeline handlers implement this
> > >>> behaviour? Do folks think that all pipeline handlers should implement
> > >>> this behaviour?
> > >>>
> > >>> Thanks again!
> > >>> David
> > >>>
> > >> You are completely right. This needs a bit more context.
> > >>
> > >> It all started on my side with implementing metadata support in SimplePipeline
> > >> handler. I soon hit some corner cases where I expected things to behave
> > >> differently in cooperation with DelayedControls. So either I hit a bug in
> > >> DelayedControls or I didn't fully understand the concept behind it.
> > >>
> > >> I ended up changing several things in DelayedControls which I believe where
> > >> correct. The question then was how to prove correctness as all current users of
> > >> DelayedControls where ISP implementations. There are some unittests, but the
> > >> order of calls in these tests also felt counterintuitive (might well be, that
> > >> it's just missing knowledge on my side).
> > >>
> > >> In that area there were also no tests in lc-compliance which tested the
> > >> behaviour of an actual sensor/isp combination. So I started to write these tests
> > >> with my personal expectation of how I believe things should work. These tests
> > >> pass on my SimplePipeline. I also tried to massage the rkisp pipeline to pass
> > >> these tests and hit some corners where work would be required.
> > >>
> > >> Before digging deeper into that I believe it makes sense to see if my
> > >> expectations are correct and if a broader audience agrees to them. So here we
> > >> are, that's the reason for the RFC.
> > >>
> > >> Now on to the technical details:
> > >> Yes my expectation on per-frame-controls would be: If I queue something for
> > >> frame x, I expect that the system tries to fullfill exacty that request (and
> > >> never earlier). I'm well aware that this will not be possible in every case when
> > >> an ISP is involved, so the ISP should do the best it can, but the metadata
> > >> should reflect what was actually achieved. All the tests are currently for
> > >> manual mode, so no regulation is involved (at least for the params I test).
> > >>
> > >> Do you agree with these assumptions? Looking forward to your opinion.
> > >
> > > Thanks for the clarification, and glad that I've understood!
> > >
> > > A bit of context from our side... We looked at and implemented a
> > > per-frame controls solution for the Raspberry Pi back in summer 2022.
> > > I would say there were a couple of differences:
> > >
> > > 1. We included a mechanism for reporting failure cases. The only real
> > > failure case we have is failing to update camera exposure/gain
> > > settings in time because there is quite a short window in which to
> > > service the camera frame interrupts (especially when the system is
> > > busy and the framerate is fast).
> >
> > That's interesting. Could you explain the mechanics you used for that
> > feedback or point me to a patchset? Would be interesting to see which
> > failure cases you modelled. How did you test these?
> 
> The last time we did any work on this was last summer for a libcamera
> F2F in Prague. I think this
> https://github.com/naushir/libcamera/tree/pfc_update was the code we
> were running at that time. Obviously things have moved on since then,
> not least the existence of Pi 5, which will need handling as well.
> 
> The way the feedback worked was that you got a sequence number every
> time you submitted a request with controls in it (distinct from the
> request sequence number), which we called the "submit id". Every
> request that completed contained one of these sequence numbers
> indicating which the most recent controls were that had been applied,
> and this was known as the "sync id". So to ask "does this request
> implement the controls that I set in it when I queued it?" you would
> check "requset->syncId == request->submitId".
> 
> > >
> > > The mechanism allowed the application to check whether controls had
> > > been applied as expected, or whether they'd been delayed to the next
> > > frame. It told you whether they'd got "merged" with the next set of
> > > controls, or whether all the controls are now running "a frame late".
> > >
> > > 2. We implemented our scheme in the same way as you've described,
> > > where the request where you set the controls has the first images that
> > > fulfill the request. We called this "Android mode" though tbh I'm not
> > > sure whether or not this really is what Android does!!!
> > >
> > > Although in the end we decided we didn't like this behaviour so much
> > > because controls get delayed to the back of all the requests that have
> > > been queued (and we normally queue quite a lot so as to avoid the risk
> >
> > Why was it necessary to move them to the back of all request? I guess in
> > a typical usecase most requests would not contain controls (at least not
> > from user side). So there would be a way to apply them as early in the
> > queue as possible. I guess I'm missing somethimng here.
> 
> Sorry, I'm not being super clear! We never "moved" controls to the
> back of the requests, the problem is that when you queue a request
> with some controls in it, those controls are necessarily behind all
> the other requests that you queued previously. So you have potentially
> many extra frames of latency.
> 
> But you're exactly right, most requests don't contain any controls, so
> we used to move the controls up the request queue so that they would
> happen earlier.
> 
> >
> > > of frame drops). So we also added "Raspberry Pi mode" where controls
> > > are applied earlier.
> > >
> > > In principle I would be happy to let the Pi run in either mode
> > > ("Android" or "Pi"), so long as we can choose "Pi mode" for our
> > > applications.  In both cases the same reporting mechanism told the
> > > application exactly when the controls had been applied, so that
> > > applications could be "mode" agnostic.
> > >
> > > Since Summer 2022, per-frame controls have not really progressed so
> > > far as I know. We are intending to resurrect our per-frame controls
> > > implementation again once we've got over the Pi 5 libcamera
> > > integration, so I'm very happy to be resuming this discussion.
> >
> > Sure. Would be great if we could come up with a set of tests that wee
> > all agree on.
> 
> I agree! Though the last 18 months suggests to me that we're not there yet.

Indeed, the whole topic hasn't had enough attention or development, I
hope that will get more attention soon.

> There's currently work going on that splits buffers out of requests.
> To me, the problem with the control lists suggests that possibly
> controls should have their own queues and be removed from requests as
> well. At which point this all becomes an even bigger discussion.
> 
> So I'm kind of sorry to be listing all the ways in which per-frame
> controls are an unresolved problem, but not really knowing where it
> goes from here. Nonetheless, we are wanting to move forward once the
> Pi 5 integration is out of the way.

Likewise!.

I actually envisaged this work here being more about validating that
DelayedControls applies controls correctly at the right frames, and that
the delays given for a specific sensor were correct. But it's quickly
becoming more involved and the 'per-frame' topic grows ;-)

--
Kieran


> 
> David
> 
> >
> > Cheers,
> > Stefan
> >
> > >
> > > Thanks!
> > > David
> > >
> > >>
> > >> Best regards,
> > >> Stefan
> > >>
> > >>>
> > >>> On Thu, 29 Feb 2024 at 17:01, Stefan Klug <stefan.klug at ideasonboard.com> wrote:
> > >>>>
> > >>>> 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_;
> > >>>> +};
> > >>>> --
> > >>>> 2.40.1
> > >>>>
> > >>
> > >> --
> > >> Regards,
> > >>
> > >> Stefan Klug
> > >>
> >
> > --
> > Regards,
> >
> > Stefan Klug
> >


More information about the libcamera-devel mailing list