[PATCH v3 03/16] libcamera: lc-compliance: Add initial set of per-frame-control tests
Stefan Klug
stefan.klug at ideasonboard.com
Fri Mar 22 14:15:17 CET 2024
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
>
>
> > +}
> > +
> > +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