[v3,03/16] libcamera: lc-compliance: Add initial set of per-frame-control tests
diff mbox series

Message ID 20240319120517.362082-4-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Preparation for per-frame-controls and initial tests
Related show

Commit Message

Stefan Klug March 19, 2024, 12:05 p.m. UTC
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@ideasonboard.com>
---
 src/apps/lc-compliance/meson.build            |   1 +
 .../lc-compliance/per_frame_controls_test.cpp | 398 ++++++++++++++++++
 2 files changed, 399 insertions(+)
 create mode 100644 src/apps/lc-compliance/per_frame_controls_test.cpp

Comments

Jacopo Mondi March 22, 2024, 11:25 a.m. UTC | #1
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@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 ?

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 ?

> +	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 ?

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] ?


> +}
> +
> +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
>
Stefan Klug March 22, 2024, 1:15 p.m. UTC | #2
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@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
> >
Jacopo Mondi March 22, 2024, 2:16 p.m. UTC | #3
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@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
> > >
Stefan Klug March 22, 2024, 2:59 p.m. UTC | #4
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@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
> > > >

Patch
diff mbox series

diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build
index eb7b2d71..8f4eec55 100644
--- a/src/apps/lc-compliance/meson.build
+++ b/src/apps/lc-compliance/meson.build
@@ -15,6 +15,7 @@  lc_compliance_sources = files([
     'capture_test.cpp',
     'environment.cpp',
     'main.cpp',
+    'per_frame_controls_test.cpp',
     'simple_capture.cpp',
     'time_sheet.cpp',
 ])
diff --git a/src/apps/lc-compliance/per_frame_controls_test.cpp b/src/apps/lc-compliance/per_frame_controls_test.cpp
new file mode 100644
index 00000000..017e8d60
--- /dev/null
+++ b/src/apps/lc-compliance/per_frame_controls_test.cpp
@@ -0,0 +1,398 @@ 
+/* 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 <gtest/gtest.h>
+
+#include "environment.h"
+#include "simple_capture.h"
+#include "time_sheet.h"
+
+using namespace libcamera;
+
+class PerFrameControlTests : public testing::Test
+{
+protected:
+	void SetUp() override;
+	void TearDown() override;
+
+	std::shared_ptr<Camera> camera_;
+};
+
+/*
+ * We use gtest's SetUp() and TearDown() instead of constructor and destructor
+ * in order to be able to assert on them.
+ */
+void PerFrameControlTests::SetUp()
+{
+	Environment *env = Environment::get();
+
+	camera_ = env->cm()->get(env->cameraId());
+
+	ASSERT_EQ(camera_->acquire(), 0);
+}
+
+void PerFrameControlTests::TearDown()
+{
+	if (!camera_)
+		return;
+
+	camera_->release();
+	camera_.reset();
+}
+
+class PerFrameControlsCapture : public SimpleCapture
+{
+public:
+	PerFrameControlsCapture(std::shared_ptr<libcamera::Camera> camera);
+
+	std::shared_ptr<TimeSheet>
+	startCaptureWithTimeSheet(unsigned int framesToCapture,
+				  const libcamera::ControlList *controls = nullptr);
+
+	void runCaptureSession();
+	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_;
+};
+
+static const bool doImageTests = true;
+
+PerFrameControlsCapture::PerFrameControlsCapture(std::shared_ptr<Camera> camera)
+	: SimpleCapture(camera)
+{
+}
+
+std::shared_ptr<TimeSheet>
+PerFrameControlsCapture::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, ControlList::MergePolicy::OverwriteExisting);
+
+	start(&ctrls);
+
+	queueCount_ = 0;
+	captureCount_ = 0;
+	captureLimit_ = framesToCapture;
+
+	auto timeSheet = std::make_shared<TimeSheet>(captureLimit_,
+						     camera_->controls().idmap());
+	timeSheet_ = timeSheet;
+	return timeSheet;
+}
+
+int PerFrameControlsCapture::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 PerFrameControlsCapture::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 PerFrameControlsCapture::runCaptureSession()
+{
+	Stream *stream = config_->at(0).stream();
+	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
+
+	/* Queue the recommended number of requests. */
+	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_;
+}
+
+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?)";
+	}
+}
+
+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);
+
+	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);
+
+	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*/
+	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);
+}
+
+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);
+	}
+}