[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 15:59:32 CET 2024
Hi Jacopo,
On Fri, Mar 22, 2024 at 03:16:25PM +0100, Jacopo Mondi wrote:
> 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 ?
Ahh I think I got it. I misinterpreted the ET line as the ExposureTime
on the corresponsing request.
I assume that roughly at start of exposure of frame 0 all requests are
queued in. So the system can look ahead in the requests. I further
assume that the EW value for column 0 is the one written in response to
the SoF signal for frame 0 (at that point in time, all/enough requests
are queued). I added a R line to show the requests:
0 1 2 3 4 5 6 7 8 9
frame +-----+-----+-----+-----+-----+-----+-----+-----+-----+-----+
R 8k 10k 5k
ET 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