[PATCH v3 03/16] libcamera: lc-compliance: Add initial set of per-frame-control tests
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Fri Mar 22 15:16:25 CET 2024
Hi Stefan
On Fri, Mar 22, 2024 at 02:15:17PM +0100, Stefan Klug wrote:
> Hi Jacopo,
>
> thanks for looking at the tests.
>
> On Fri, Mar 22, 2024 at 12:25:33PM +0100, Jacopo Mondi wrote:
> > Hi Stefan
> >
> > On Tue, Mar 19, 2024 at 01:05:04PM +0100, Stefan Klug wrote:
> > > Add tests that check if controls (only exposure time and analogue gain at the
> > > moment) get applied on the frame they were requested for. This is tested by
> > > looking at the metadata and the mean brightness of the image center. At the
> > > moment these tests fail. Fixes for the pipelines will be delivered in later
> > > patches (rkisp1 for now).
> > >
> > > To run the pfc tests only:
> > > lc-compliance -c <cam> -f "PerFrameControlTests.*"
> > >
> > > Note that the current implementation is a bit picky on what the camera
> > > actually sees. If it is too dark (or too bright), the tests will fail.
> > > Looking at a white wall in a normally lit office usually works.
> > >
> > > These tests are known to pass using a imx219 (RPi cam v2) with a imx8mp
> > > (debix-som) using the rkisp1 pipeline
> > >
> > > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > > ---
> >
> > [snip]
> >
> > > +
> > > +TEST_F(PerFrameControlTests, testExposureGainChangeOnSameFrame)
> > > +{
> > > + PerFrameControlsCapture capture(camera_);
> > > + capture.configure(StreamRole::VideoRecording);
> > > +
> > > + ControlList startValues;
> > > + startValues.set(controls::ExposureTime, 5000);
> > > + startValues.set(controls::AnalogueGain, 1.0);
> > > +
> > > + auto timeSheet = capture.startCaptureWithTimeSheet(10, &startValues);
> > > + auto &ts = *timeSheet;
> > > +
> > > + /* wait a few frames to settle */
> > > + ts[7].controls().set(controls::ExposureTime, 10000);
> > > + ts[7].controls().set(controls::AnalogueGain, 4.0);
> > > +
> > > + capture.runCaptureSession();
> > > +
> > > + ASSERT_TRUE(ts[5].metadata().contains(controls::ExposureTime.id()))
> > > + << "Required metadata entry is missing";
> > > + ASSERT_TRUE(ts[5].metadata().contains(controls::AnalogueGain.id()))
> > > + << "Required metadata entry is missing";
> > > +
> > > + EXPECT_NEAR(ts[3].metadata().get(controls::ExposureTime).value(), 5000, 20);
> > > + EXPECT_NEAR(ts[3].metadata().get(controls::AnalogueGain).value(), 1.0, 0.05);
> > > +
> > > + /* find the frame with the changes */
> > > + int exposureChangeIndex = 0;
> > > + for (unsigned i = 3; i < ts.size(); i++) {
> > > + if (ts[i].metadata().get(controls::ExposureTime).value() > 7500) {
> > > + exposureChangeIndex = i;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + int gainChangeIndex = 0;
> > > + for (unsigned i = 3; i < ts.size(); i++) {
> > > + if (ts[i].metadata().get(controls::AnalogueGain).value() > 2.0) {
> > > + gainChangeIndex = i;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + EXPECT_NE(exposureChangeIndex, 0) << "Exposure change not found in metadata";
> > > + EXPECT_NE(gainChangeIndex, 0) << "Gain change not found in metadata";
> > > + EXPECT_EQ(exposureChangeIndex, gainChangeIndex)
> > > + << "Metadata contained gain and exposure changes on different frames";
> > > +
> > > + if (doImageTests) {
> > > + int brightnessChangeIndex = 0;
> > > + for (unsigned i = 3; i < ts.size(); i++) {
> > > + if (ts[i].brightnessChange() > 1.3) {
> > > + EXPECT_EQ(brightnessChangeIndex, 0)
> > > + << "Detected multiple frames with brightness increase"
> > > + << " (Wrong control delays?)";
> > > +
> > > + if (!brightnessChangeIndex)
> > > + brightnessChangeIndex = i;
> > > + }
> > > + }
> > > +
> > > + EXPECT_EQ(exposureChangeIndex, brightnessChangeIndex)
> > > + << "Exposure change and measured brightness change were not on same"
> > > + << " frame. (Wrong control delay?, Start frame event too late?)";
> > > + EXPECT_EQ(exposureChangeIndex, gainChangeIndex)
> > > + << "Gain change and measured brightness change were not on same "
> > > + << " frame. (Wrong control delay?, Start frame event too late?)";
> > > + }
> > > +}
> >
> > I think this is a valid test, we want to make sure exposure and gain
> > change on the same frame
> >
> > > +
> > > +TEST_F(PerFrameControlTests, testFramePreciseExposureChange)
> > > +{
> > > + PerFrameControlsCapture capture(camera_);
> > > + capture.configure(StreamRole::VideoRecording);
> > > +
> > > + auto timeSheet = capture.startCaptureWithTimeSheet(10);
> > > + auto &ts = *timeSheet;
> > > +
> > > + ts[3].controls().set(controls::ExposureTime, 5000);
> > > + /* wait a few frames to settle */
> > > + ts[6].controls().set(controls::ExposureTime, 20000);
> > > +
> > > + capture.runCaptureSession();
> > > +
> > > + ASSERT_TRUE(ts[5].metadata().contains(controls::ExposureTime.id()))
> > > + << "Required metadata entry is missing";
> > > +
> > > + EXPECT_NEAR(ts[5].metadata().get(controls::ExposureTime).value(), 5000, 20);
> > > + EXPECT_NEAR(ts[6].metadata().get(controls::ExposureTime).value(), 20000, 20);
> >
> > Why do you need near ? The camera can't get the precise exposure time ?
>
> Some quantization always happens and that is reported back in the
> metadata. That raises the question: Does libcamera make any such promise?
>
> >
> > I think the test is valid, however it uses absolute values (3, 6,
> > 5000, 20000) while in future these should be parametrized (using the
> > pipeline depth and the exposure min, max) but we already agree with
> > this.
> >
> >
> > > +
> > > + if (doImageTests) {
> > > + /* No increase just before setting exposure */
> > > + EXPECT_NEAR(ts[5].brightnessChange(), 1.0, 0.05)
> > > + << "Brightness changed too much before the expected time of change"
> > > + << " (control delay too high?).";
> > > + /*
> > > + * \todo The change is brightness was a bit low
> > > + * (Exposure time increase by 4x resulted in a brightness increase of < 2).
> > > + * This should be investigated.
> > > + */
> > > + EXPECT_GT(ts[6].brightnessChange(), 1.3)
> > > + << "Brightness in frame " << 6 << " did not increase as expected"
> > > + << " (reference: " << ts[3].spotBrightness() << " current: "
> > > + << ts[6].spotBrightness() << " )" << std::endl;
> > > +
> > > + /* No increase just after setting exposure */
> > > + EXPECT_NEAR(ts[7].brightnessChange(), 1.0, 0.05)
> > > + << "Brightness changed too much after the expected time of change"
> > > + << " (control delay too low?).";
> > > +
> > > + /* No increase just after setting exposure */
> > > + EXPECT_NEAR(ts[8].brightnessChange(), 1.0, 0.05)
> > > + << "Brightness changed too much 2 frames after the expected time"
> > > + << " of change (control delay too low?).";
> > > + }
> > > +}
> > > +
> > > +TEST_F(PerFrameControlTests, testFramePreciseGainChange)
> > > +{
> > > + PerFrameControlsCapture capture(camera_);
> > > + capture.configure(StreamRole::VideoRecording);
> > > +
> > > + auto timeSheet = capture.startCaptureWithTimeSheet(10);
> > > + auto &ts = *timeSheet;
> > > +
> > > + ts[3].controls().set(controls::AnalogueGain, 1.0);
> > > + /* wait a few frames to settle */
> > > + ts[6].controls().set(controls::AnalogueGain, 4.0);
> > > +
> > > + capture.runCaptureSession();
> > > +
> > > + ASSERT_TRUE(ts[5].metadata().contains(controls::AnalogueGain.id()))
> > > + << "Required metadata entry is missing";
> > > +
> > > + EXPECT_NEAR(ts[5].metadata().get(controls::AnalogueGain).value(), 1.0, 0.1);
> > > + EXPECT_NEAR(ts[6].metadata().get(controls::AnalogueGain).value(), 4.0, 0.1);
> > > +
> >
> > Same comment as the for the above test.
> >
> > > + if (doImageTests) {
> > > + /* No increase just before setting gain */
> > > + EXPECT_NEAR(ts[5].brightnessChange(), 1.0, 0.05)
> > > + << "Brightness changed too much before the expected time of change"
> > > + << " (control delay too high?).";
> > > + /*
> > > + * \todo I see a brightness change of roughly half the expected one.
> > > + * This is not yet understood and needs investigation
> > > + */
> > > + EXPECT_GT(ts[6].brightnessChange(), 1.7)
> > > + << "Brightness in frame " << 6 << " did not increase as expected"
> > > + << " (reference: " << ts[5].spotBrightness()
> > > + << " current: " << ts[6].spotBrightness() << ")" << std::endl;
> > > +
> > > + /* No increase just after setting gain */
> > > + EXPECT_NEAR(ts[7].brightnessChange(), 1.0, 0.05)
> > > + << "Brightness changed too much after the expected time of change"
> > > + << " (control delay too low?).";
> > > +
> > > + /* No increase just after setting gain */
> > > + EXPECT_NEAR(ts[8].brightnessChange(), 1.0, 0.05)
> > > + << "Brightness changed too much after the expected time of change"
> > > + << " (control delay too low?).";
> > > + }
> > > +}
> > > +
> > > +TEST_F(PerFrameControlTests, testExposureGainFromFirstRequestGetsApplied)
> > > +{
> > > + PerFrameControlsCapture capture(camera_);
> > > + capture.configure(StreamRole::VideoRecording);
> > > +
> > > + auto timeSheet = capture.startCaptureWithTimeSheet(5);
> > > + auto &ts = *timeSheet;
> > > +
> > > + ts[0].controls().set(controls::ExposureTime, 10000);
> > > + ts[0].controls().set(controls::AnalogueGain, 4.0);
> > > +
> > > + capture.runCaptureSession();
> > > +
> > > + ASSERT_TRUE(ts[4].metadata().contains(controls::ExposureTime.id()))
> > > + << "Required metadata entry is missing";
> > > + ASSERT_TRUE(ts[4].metadata().contains(controls::AnalogueGain.id()))
> > > + << "Required metadata entry is missing";
> > > +
> > > + /* We expect it to be applied after 3 frames, the latest*/
> >
> > Should the 'startup' frames be only the sensor delays or the full
> > pipeline depth ?
>
> That depends on the other discussions. My (personal) expectation would be
> that only the sensor delay applies as we are running in manual mode.
>
> >
> > > + EXPECT_NEAR(ts[4].metadata().get(controls::ExposureTime).value(), 10000, 20);
> > > + EXPECT_NEAR(ts[4].metadata().get(controls::AnalogueGain).value(), 4.0, 0.1);
> > > +}
> > > +
> > > +TEST_F(PerFrameControlTests, testExposureGainFromFirstAndSecondRequestGetsApplied)
> > > +{
> > > + PerFrameControlsCapture capture(camera_);
> > > + capture.configure(StreamRole::VideoRecording);
> > > +
> > > + auto timeSheet = capture.startCaptureWithTimeSheet(5);
> > > + auto &ts = *timeSheet;
> > > +
> > > + ts[0].controls().set(controls::ExposureTime, 8000);
> > > + ts[0].controls().set(controls::AnalogueGain, 2.0);
> > > + ts[1].controls().set(controls::ExposureTime, 10000);
> > > + ts[1].controls().set(controls::AnalogueGain, 4.0);
> > > +
> > > + capture.runCaptureSession();
> > > +
> > > + ASSERT_TRUE(ts[4].metadata().contains(controls::ExposureTime.id()))
> > > + << "Required metadata entry is missing";
> > > + ASSERT_TRUE(ts[4].metadata().contains(controls::AnalogueGain.id()))
> > > + << "Required metadata entry is missing";
> > > +
> > > + /* We expect it to be applied after 3 frames, the latest */
> > > + EXPECT_NEAR(ts[4].metadata().get(controls::ExposureTime).value(), 10000, 20);
> > > + EXPECT_NEAR(ts[4].metadata().get(controls::AnalogueGain).value(), 4.0, 0.1);
> >
> > Eh, interesting. How should we expect the startup sequence to be
> > handled ?
> >
> > I presume ts[0] is not applied before streaming is started, right ?
>
> yes.
>
> >
> > Let me graph your test expectations (for ExposureTime only)
> >
> > 0 1 2 3 4 5 6 7 8 9
> > frame +-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+
> >
> > ET 8k 10k
> > sensor [init]
> > [init] [init] 8k
> > [init] 8k 10k
> >
> > Should we define "per frame control" as the expectations that:
> > - given no requests underrun (the application queues requests fast
> > enough to have at least 'depth' requests in queue
> >
> > Controls associated with Request[x] will be applied at Frame[x + depth] ?
>
> Ahh thati's interesting. My expectations are a bit different. (For
> the sake of completeness I added an additional 5k a bit later in the
> pipeline). I would expect the 8k to be lost. I'm not shure if understood
> your lines below sensor correctly, so I squashed them into one line to
> represent the state that is active in the sensor.
>
> David mentioned a god reason why it makes sense to lose the 8k:
> "Actually our implementation takes the 2nd approach even though I prefer
> the first. The reason is that the coupling of requests and controls
> means you end up with a theoretically unbounded delay between them which
> is (theoretically) annoying to handle."
>
>
> So my diagram would be:
> 0 1 2 3 4 5 6 7 8 9
> frame +-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+
>
> ET 8k 10k 5k
> sensor [init] [init] 10k 5k
>
Maybe I should clarify the meaning of the lines first:
EW: time at wich we setControl() on the subdev
sensor: Effective control value
And maybe I was considering a delay of 2 and it's just 1 instead ?
Anyway, even if the delay is one frame, isn't this:
0 1 2 3 4 5 6 7 8 9
frame +-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+
ET 8k 10k 5k
sensor [init] [init] 8k 10k 10k 10k 10k 5k
so in my undertanding, with a delay of 1, if we setControl() while
frame X is being exposed, the setting gets applied to the next frame
(X + 1) and has a delay of 1 to have it realized (X + 2)
What am I missing ?
>
> >
> >
> > > +}
> > > +
> > > +TEST_F(PerFrameControlTests, testExposureGainIsAppliedOnFirstFrame)
> > > +{
> > > + PerFrameControlsCapture capture(camera_);
> > > + capture.configure(StreamRole::VideoRecording);
> > > +
> > > + ControlList startValues;
> > > + startValues.set(controls::ExposureTime, 5000);
> > > + startValues.set(controls::AnalogueGain, 1.0);
> > > +
> > > + auto ts1 = capture.startCaptureWithTimeSheet(3, &startValues);
> > > +
> > > + capture.runCaptureSession();
> > > +
> > > + ASSERT_TRUE((*ts1)[0].metadata().contains(controls::ExposureTime.id()))
> > > + << "Required metadata entry is missing";
> > > + ASSERT_TRUE((*ts1)[0].metadata().contains(controls::AnalogueGain.id()))
> > > + << "Required metadata entry is missing";
> > > +
> > > + EXPECT_NEAR((*ts1)[0].metadata().get(controls::ExposureTime).value(), 5000, 20);
> > > + EXPECT_NEAR((*ts1)[0].metadata().get(controls::AnalogueGain).value(), 1.0, 0.02);
> > > +
> > > + /* Second capture with different values to ensure we don't hit default/old values */
> > > + startValues.set(controls::ExposureTime, 15000);
> > > + startValues.set(controls::AnalogueGain, 4.0);
> > > +
> > > + auto ts2 = capture.startCaptureWithTimeSheet(3, &startValues);
> > > +
> > > + capture.runCaptureSession();
> > > +
> > > + EXPECT_NEAR((*ts2)[0].metadata().get(controls::ExposureTime).value(), 15000, 20);
> > > + EXPECT_NEAR((*ts2)[0].metadata().get(controls::AnalogueGain).value(), 4.0, 0.02);
> > > +
> > > + if (doImageTests) {
> > > + /* With 3x exposure and 4x gain we could expect a brightness increase of 2x */
> > > + double brightnessChange = ts2->get(1).spotBrightness() / ts1->get(1).spotBrightness();
> > > + EXPECT_GT(brightnessChange, 2.0);
> > > + }
> > > +}
> > > --
> > > 2.40.1
> > >
More information about the libcamera-devel
mailing list