[v3,04/16] libcamera: delayed_controls: Update unit tests to expected semantics
diff mbox series

Message ID 20240319120517.362082-5-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
This patches changes the tests in a way DelayedControls is expected to
work. It does not require a interface change in DelayedControls.

The changes are:
- It is expected that for every apply() there was a previous push().
- When a control is pushed for frame 0, that request is not lost, but
  applied as soon as possible

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 test/delayed_controls.cpp | 256 +++++++++++++++++++++++++++++++-------
 1 file changed, 209 insertions(+), 47 deletions(-)

Comments

Jacopo Mondi March 21, 2024, 11:28 a.m. UTC | #1
Hi Stefan

On Tue, Mar 19, 2024 at 01:05:05PM +0100, Stefan Klug wrote:
> This patches changes the tests in a way DelayedControls is expected to
> work. It does not require a interface change in DelayedControls.
>
> The changes are:
> - It is expected that for every apply() there was a previous push().
> - When a control is pushed for frame 0, that request is not lost, but
>   applied as soon as possible
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  test/delayed_controls.cpp | 256 +++++++++++++++++++++++++++++++-------
>  1 file changed, 209 insertions(+), 47 deletions(-)
>
> diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp
> index a8ce9828..fe183de5 100644
> --- a/test/delayed_controls.cpp
> +++ b/test/delayed_controls.cpp
> @@ -84,11 +84,8 @@ protected:
>  		dev_->setControls(&ctrls);
>  		delayed->reset();
>
> -		/* Trigger the first frame start event */
> -		delayed->applyControls(0);
> -
>  		/* Test control without delay are set at once. */
> -		for (unsigned int i = 1; i < 100; i++) {
> +		for (unsigned int i = 0; i < 100; i++) {
>  			int32_t value = 100 + i;
>
>  			ctrls.set(V4L2_CID_BRIGHTNESS, value);
> @@ -121,25 +118,30 @@ protected:
>  		ControlList ctrls;
>
>  		/* Reset control to value that will be first in test. */
> -		int32_t expected = 4;
> -		ctrls.set(V4L2_CID_BRIGHTNESS, expected);
> +		int32_t initial = 4;
> +		ctrls.set(V4L2_CID_BRIGHTNESS, initial);
>  		dev_->setControls(&ctrls);
>  		delayed->reset();
>
> -		/* Trigger the first frame start event */
> -		delayed->applyControls(0);
> +		/* Push a request for frame 0 */
> +		ctrls.set(V4L2_CID_BRIGHTNESS, 10);
> +		delayed->push(ctrls);
>
>  		/* Test single control with delay. */
> -		for (unsigned int i = 1; i < 100; i++) {
> +		for (unsigned int i = 0; i < 100; i++) {
>  			int32_t value = 10 + i;
> +			int32_t expected = i < 1 ? initial : value;

That's a bit weird to read as then to me you expect 'value ==
expected'
>
> -			ctrls.set(V4L2_CID_BRIGHTNESS, value);
> +			/* push the request for frame i+1 */
> +			ctrls.set(V4L2_CID_BRIGHTNESS, value + 1);

While the 'value' you're actually pushing is 'value + 1'

>  			delayed->push(ctrls);
>
>  			delayed->applyControls(i);
>
>  			ControlList result = delayed->get(i);
>  			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
> +			ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS });
> +			int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
>  			if (brightness != expected) {
>  				cerr << "Failed single control with delay"
>  				     << " frame " << i
> @@ -149,7 +151,121 @@ protected:
>  				return TestFail;
>  			}
>
> -			expected = value;
> +			if (i > 0 && brightnessV4L != value + 1) {

This I don't get it.

As I read it, you get the 'current' brightness from the video device
and you expect it to be equal to the value you just pushed, which I
would instead expect to get realized at the -next- frame (frame delay
= 1). What am I missing ?

> +				cerr << "Failed single control with delay"
> +				     << " frame " << i
> +				     << " expected V4L " << value + 1
> +				     << " got " << brightnessV4L
> +				     << endl;
> +				return TestFail;
> +			}
> +		}
> +
> +		return TestPass;
> +	}
> +
> +	/* This fails on the old delayed controls implementation */
> +	int singleControlWithDelayStartUp()

I really must have missed something, as this seems identical to the
previous function

This is the diff I get between the two

-------------------------------------------------------------------------------
2c2,3
< 	int singleControlWithDelay()
---
> 	/* This fails on the old delayed controls implementation */
> 	int singleControlWithDelayStartUp()
17c18
< 		/* Push a request for frame 0 */
---
> 		/* push a request for frame 0 */
35a37
>
37c39
< 				cerr << "Failed single control with delay"
---
> 				cerr << "Failed single control with delay start up"
46c48
< 				cerr << "Failed single control with delay"
---
> 				cerr << "Failed single control with delay start up"
-------------------------------------------------------------------------------

> +	{
> +		std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
> +			{ V4L2_CID_BRIGHTNESS, { 1, false } },
> +		};
> +		std::unique_ptr<DelayedControls> delayed =
> +			std::make_unique<DelayedControls>(dev_.get(), delays);
> +		ControlList ctrls;
> +
> +		/* Reset control to value that will be first in test. */
> +		int32_t initial = 4;
> +		ctrls.set(V4L2_CID_BRIGHTNESS, initial);
> +		dev_->setControls(&ctrls);
> +		delayed->reset();
> +
> +		/* push a request for frame 0 */
> +		ctrls.set(V4L2_CID_BRIGHTNESS, 10);
> +		delayed->push(ctrls);
> +
> +		/* Test single control with delay. */
> +		for (unsigned int i = 0; i < 100; i++) {
> +			int32_t value = 10 + i;
> +			int32_t expected = i < 1 ? initial : value;
> +
> +			/* push the request for frame i+1 */
> +			ctrls.set(V4L2_CID_BRIGHTNESS, value + 1);
> +			delayed->push(ctrls);
> +
> +			delayed->applyControls(i);
> +
> +			ControlList result = delayed->get(i);
> +			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
> +			ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS });
> +			int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
> +
> +			if (brightness != expected) {
> +				cerr << "Failed single control with delay start up"
> +				     << " frame " << i
> +				     << " expected " << expected
> +				     << " got " << brightness
> +				     << endl;
> +				return TestFail;
> +			}
> +
> +			if (i > 0 && brightnessV4L != value + 1) {
> +				cerr << "Failed single control with delay start up"
> +				     << " frame " << i
> +				     << " expected V4L " << value + 1
> +				     << " got " << brightnessV4L
> +				     << endl;
> +				return TestFail;
> +			}
> +		}
> +
> +		return TestPass;
> +	}
> +
> +	/* This fails on the old delayed controls implementation */
> +	int doNotLoseFirstRequest()
> +	{
> +		/* no delay at all */
> +		std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
> +			{ V4L2_CID_BRIGHTNESS, { 0, false } },
> +		};
> +		std::unique_ptr<DelayedControls> delayed =
> +			std::make_unique<DelayedControls>(dev_.get(), delays);
> +		ControlList ctrls;
> +
> +		/* Reset control to value that will be first in test. */
> +		int32_t initial = 4;
> +		ctrls.set(V4L2_CID_BRIGHTNESS, initial);
> +		dev_->setControls(&ctrls);
> +		delayed->reset();
> +
> +		/* push a request for frame 0 */
> +		int32_t expected = 10;
> +		ctrls.set(V4L2_CID_BRIGHTNESS, expected);
> +		delayed->push(ctrls);
> +		delayed->applyControls(0);
> +
> +		ControlList result = delayed->get(0);
> +		int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
> +		ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS });
> +		int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
> +
> +		if (brightness != expected) {


Is this testing only frame #0, right ?

So you set a control value with 0 delay, apply it, and make sure it
gets applied and overwites the initial value from the video device.

Considering that controls handled with delayed controls will always
(?) have a delay, I'm not sure this situation will ever be possible in
a real use case ? Or will there be cases where controls won't have a
delay ?

> +			cerr << "Failed doNotLoseFirstRequest"
> +			     << " frame " << 0
> +			     << " expected " << expected
> +			     << " got " << brightness
> +			     << endl;
> +			return TestFail;
> +		}
> +
> +		if (brightnessV4L != expected) {
> +			cerr << "Failed doNotLoseFirstRequest"
> +			     << " frame " << 0
> +			     << " expected V4L " << expected
> +			     << " got " << brightnessV4L
> +			     << endl;
> +			return TestFail;
>  		}
>
>  		return TestPass;
> @@ -157,7 +273,7 @@ protected:
>
>  	int dualControlsWithDelay()
>  	{
> -		static const unsigned int maxDelay = 2;
> +		static const int maxDelay = 2;
>
>  		std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
>  			{ V4L2_CID_BRIGHTNESS, { 1, false } },
> @@ -174,15 +290,21 @@ protected:
>  		dev_->setControls(&ctrls);
>  		delayed->reset();
>
> -		/* Trigger the first frame start event */
> -		delayed->applyControls(0);
> +		/* push two requests into the queue */
> +		ctrls.set(V4L2_CID_BRIGHTNESS, 10);
> +		ctrls.set(V4L2_CID_CONTRAST, 10);
> +		delayed->push(ctrls);
> +		ctrls.set(V4L2_CID_BRIGHTNESS, 11);
> +		ctrls.set(V4L2_CID_CONTRAST, 11);
> +		delayed->push(ctrls);
>
>  		/* Test dual control with delay. */
> -		for (unsigned int i = 1; i < 100; i++) {
> +		for (unsigned int i = 0; i < 100; i++) {
>  			int32_t value = 10 + i;
>
> -			ctrls.set(V4L2_CID_BRIGHTNESS, value);
> -			ctrls.set(V4L2_CID_CONTRAST, value + 1);
> +			/* we are pushing 2 frames ahead */
> +			ctrls.set(V4L2_CID_BRIGHTNESS, value + 2);
> +			ctrls.set(V4L2_CID_CONTRAST, value + 2);
>  			delayed->push(ctrls);
>
>  			delayed->applyControls(i);
> @@ -190,17 +312,32 @@ protected:
>  			ControlList result = delayed->get(i);
>  			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
>  			int32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();
> -			if (brightness != expected || contrast != expected + 1) {
> -				cerr << "Failed dual controls"
> -				     << " frame " << i
> -				     << " brightness " << brightness
> -				     << " contrast " << contrast
> -				     << " expected " << expected
> -				     << endl;
> -				return TestFail;
> -			}
>
> -			expected = i < maxDelay ? expected : value - 1;
> +			ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS, V4L2_CID_CONTRAST });
> +			int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
> +			int32_t contrastV4L = ctrlsV4L.get(V4L2_CID_CONTRAST).get<int32_t>();
> +
> +			if (i > maxDelay) {
> +				if (brightness != value || contrast != value) {
> +					cerr << "Failed dual controls"
> +					     << " frame " << i
> +					     << " brightness " << brightness
> +					     << " contrast " << contrast
> +					     << " expected " << value
> +					     << endl;
> +					return TestFail;
> +				}
> +				if (brightnessV4L != value + 1 || contrastV4L != value + maxDelay) {
> +					cerr << "Failed dual controls"
> +					     << " frame " << i
> +					     << " brightnessV4L " << brightnessV4L
> +					     << " expected " << value + 1
> +					     << " contrastV4L " << contrastV4L
> +					     << " expected " << value + maxDelay
> +					     << endl;
> +					return TestFail;
> +				}
> +			}

I might have missed the behavioral change you tried to capture here
between your forthcoming delay controls changes and the current
version. If I'm not mistaken the test ignores the first two frames,
you don't see any change related to the startup condition ?

>  		}
>
>  		return TestPass;
> @@ -208,7 +345,7 @@ protected:
>
>  	int dualControlsMultiQueue()
>  	{
> -		static const unsigned int maxDelay = 2;
> +		static const int maxDelay = 2;
>
>  		std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
>  			{ V4L2_CID_BRIGHTNESS, { 1, false } },
> @@ -218,22 +355,19 @@ protected:
>  			std::make_unique<DelayedControls>(dev_.get(), delays);
>  		ControlList ctrls;
>
> -		/* Reset control to value that will be first two frames in test. */
> -		int32_t expected = 100;
> -		ctrls.set(V4L2_CID_BRIGHTNESS, expected);
> -		ctrls.set(V4L2_CID_CONTRAST, expected);
> +		/* Reset control to a value that will be first two frames in test. */
> +		int32_t initial = 100;
> +		ctrls.set(V4L2_CID_BRIGHTNESS, initial);
> +		ctrls.set(V4L2_CID_CONTRAST, initial);
>  		dev_->setControls(&ctrls);
>  		delayed->reset();
>
> -		/* Trigger the first frame start event */
> -		delayed->applyControls(0);
> -
>  		/*
> -		 * Queue all controls before any fake frame start. Note we
> +		 * Queue all controls before applyControls(). Note we
>  		 * can't queue up more then the delayed controls history size
> -		 * which is 16. Where one spot is used by the reset control.
> +		 * which is 16.
>  		 */
> -		for (unsigned int i = 0; i < 15; i++) {
> +		for (unsigned int i = 0; i < 14; i++) {
>  			int32_t value = 10 + i;
>
>  			ctrls.set(V4L2_CID_BRIGHTNESS, value);
> @@ -241,9 +375,9 @@ protected:
>  			delayed->push(ctrls);
>  		}
>
> -		/* Process all queued controls. */
> -		for (unsigned int i = 1; i < 16; i++) {
> -			int32_t value = 10 + i - 1;
> +		/* Process 2 frames less than queued, so that the V4L controls are correct */
> +		for (unsigned int i = 0; i < 12; i++) {
> +			int32_t expected = i < maxDelay ? initial : 10 + i;
>
>  			delayed->applyControls(i);
>
> @@ -251,6 +385,11 @@ protected:
>
>  			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
>  			int32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();
> +
> +			ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS, V4L2_CID_CONTRAST });
> +			int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
> +			int32_t contrastV4L = ctrlsV4L.get(V4L2_CID_CONTRAST).get<int32_t>();
> +
>  			if (brightness != expected || contrast != expected) {
>  				cerr << "Failed multi queue"
>  				     << " frame " << i
> @@ -261,7 +400,17 @@ protected:
>  				return TestFail;
>  			}
>
> -			expected = i < maxDelay ? expected : value - 1;
> +			/* Check v4l values after they've settled */
> +			if (i >= maxDelay && (brightnessV4L != expected + 1 || contrastV4L != expected + maxDelay)) {
> +				cerr << "Failed multi queue"
> +				     << " frame " << i
> +				     << " brightnessV4L " << brightnessV4L
> +				     << " expected " << expected + 1
> +				     << " contrastV4L " << contrastV4L
> +				     << " expected " << expected + maxDelay
> +				     << endl;
> +				return TestFail;
> +			}
>  		}
>
>  		return TestPass;
> @@ -269,27 +418,40 @@ protected:
>
>  	int run() override
>  	{
> -		int ret;
> +		int ret = 0;
> +		bool failed = false;
>
>  		/* Test single control without delay. */
>  		ret = singleControlNoDelay();
>  		if (ret)
> -			return ret;
> +			failed = true;
>
>  		/* Test single control with delay. */
>  		ret = singleControlWithDelay();
>  		if (ret)
> -			return ret;
> +			failed = true;
> +
> +		/* Test single control with delay. */
> +		ret = singleControlWithDelayStartUp();
> +		if (ret)
> +			failed = true;
> +
> +		ret = doNotLoseFirstRequest();
> +		if (ret)
> +			failed = true;
>
>  		/* Test dual controls with different delays. */
>  		ret = dualControlsWithDelay();
>  		if (ret)
> -			return ret;
> +			failed = true;
>
>  		/* Test control values produced faster than consumed. */
>  		ret = dualControlsMultiQueue();
>  		if (ret)
> -			return ret;
> +			failed = true;
> +
> +		if (failed)
> +			return TestFail;
>
>  		return TestPass;
>  	}
> --
> 2.40.1
>

Patch
diff mbox series

diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp
index a8ce9828..fe183de5 100644
--- a/test/delayed_controls.cpp
+++ b/test/delayed_controls.cpp
@@ -84,11 +84,8 @@  protected:
 		dev_->setControls(&ctrls);
 		delayed->reset();
 
-		/* Trigger the first frame start event */
-		delayed->applyControls(0);
-
 		/* Test control without delay are set at once. */
-		for (unsigned int i = 1; i < 100; i++) {
+		for (unsigned int i = 0; i < 100; i++) {
 			int32_t value = 100 + i;
 
 			ctrls.set(V4L2_CID_BRIGHTNESS, value);
@@ -121,25 +118,30 @@  protected:
 		ControlList ctrls;
 
 		/* Reset control to value that will be first in test. */
-		int32_t expected = 4;
-		ctrls.set(V4L2_CID_BRIGHTNESS, expected);
+		int32_t initial = 4;
+		ctrls.set(V4L2_CID_BRIGHTNESS, initial);
 		dev_->setControls(&ctrls);
 		delayed->reset();
 
-		/* Trigger the first frame start event */
-		delayed->applyControls(0);
+		/* Push a request for frame 0 */
+		ctrls.set(V4L2_CID_BRIGHTNESS, 10);
+		delayed->push(ctrls);
 
 		/* Test single control with delay. */
-		for (unsigned int i = 1; i < 100; i++) {
+		for (unsigned int i = 0; i < 100; i++) {
 			int32_t value = 10 + i;
+			int32_t expected = i < 1 ? initial : value;
 
-			ctrls.set(V4L2_CID_BRIGHTNESS, value);
+			/* push the request for frame i+1 */
+			ctrls.set(V4L2_CID_BRIGHTNESS, value + 1);
 			delayed->push(ctrls);
 
 			delayed->applyControls(i);
 
 			ControlList result = delayed->get(i);
 			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
+			ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS });
+			int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
 			if (brightness != expected) {
 				cerr << "Failed single control with delay"
 				     << " frame " << i
@@ -149,7 +151,121 @@  protected:
 				return TestFail;
 			}
 
-			expected = value;
+			if (i > 0 && brightnessV4L != value + 1) {
+				cerr << "Failed single control with delay"
+				     << " frame " << i
+				     << " expected V4L " << value + 1
+				     << " got " << brightnessV4L
+				     << endl;
+				return TestFail;
+			}
+		}
+
+		return TestPass;
+	}
+
+	/* This fails on the old delayed controls implementation */
+	int singleControlWithDelayStartUp()
+	{
+		std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
+			{ V4L2_CID_BRIGHTNESS, { 1, false } },
+		};
+		std::unique_ptr<DelayedControls> delayed =
+			std::make_unique<DelayedControls>(dev_.get(), delays);
+		ControlList ctrls;
+
+		/* Reset control to value that will be first in test. */
+		int32_t initial = 4;
+		ctrls.set(V4L2_CID_BRIGHTNESS, initial);
+		dev_->setControls(&ctrls);
+		delayed->reset();
+
+		/* push a request for frame 0 */
+		ctrls.set(V4L2_CID_BRIGHTNESS, 10);
+		delayed->push(ctrls);
+
+		/* Test single control with delay. */
+		for (unsigned int i = 0; i < 100; i++) {
+			int32_t value = 10 + i;
+			int32_t expected = i < 1 ? initial : value;
+
+			/* push the request for frame i+1 */
+			ctrls.set(V4L2_CID_BRIGHTNESS, value + 1);
+			delayed->push(ctrls);
+
+			delayed->applyControls(i);
+
+			ControlList result = delayed->get(i);
+			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
+			ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS });
+			int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
+
+			if (brightness != expected) {
+				cerr << "Failed single control with delay start up"
+				     << " frame " << i
+				     << " expected " << expected
+				     << " got " << brightness
+				     << endl;
+				return TestFail;
+			}
+
+			if (i > 0 && brightnessV4L != value + 1) {
+				cerr << "Failed single control with delay start up"
+				     << " frame " << i
+				     << " expected V4L " << value + 1
+				     << " got " << brightnessV4L
+				     << endl;
+				return TestFail;
+			}
+		}
+
+		return TestPass;
+	}
+
+	/* This fails on the old delayed controls implementation */
+	int doNotLoseFirstRequest()
+	{
+		/* no delay at all */
+		std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
+			{ V4L2_CID_BRIGHTNESS, { 0, false } },
+		};
+		std::unique_ptr<DelayedControls> delayed =
+			std::make_unique<DelayedControls>(dev_.get(), delays);
+		ControlList ctrls;
+
+		/* Reset control to value that will be first in test. */
+		int32_t initial = 4;
+		ctrls.set(V4L2_CID_BRIGHTNESS, initial);
+		dev_->setControls(&ctrls);
+		delayed->reset();
+
+		/* push a request for frame 0 */
+		int32_t expected = 10;
+		ctrls.set(V4L2_CID_BRIGHTNESS, expected);
+		delayed->push(ctrls);
+		delayed->applyControls(0);
+
+		ControlList result = delayed->get(0);
+		int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
+		ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS });
+		int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
+
+		if (brightness != expected) {
+			cerr << "Failed doNotLoseFirstRequest"
+			     << " frame " << 0
+			     << " expected " << expected
+			     << " got " << brightness
+			     << endl;
+			return TestFail;
+		}
+
+		if (brightnessV4L != expected) {
+			cerr << "Failed doNotLoseFirstRequest"
+			     << " frame " << 0
+			     << " expected V4L " << expected
+			     << " got " << brightnessV4L
+			     << endl;
+			return TestFail;
 		}
 
 		return TestPass;
@@ -157,7 +273,7 @@  protected:
 
 	int dualControlsWithDelay()
 	{
-		static const unsigned int maxDelay = 2;
+		static const int maxDelay = 2;
 
 		std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
 			{ V4L2_CID_BRIGHTNESS, { 1, false } },
@@ -174,15 +290,21 @@  protected:
 		dev_->setControls(&ctrls);
 		delayed->reset();
 
-		/* Trigger the first frame start event */
-		delayed->applyControls(0);
+		/* push two requests into the queue */
+		ctrls.set(V4L2_CID_BRIGHTNESS, 10);
+		ctrls.set(V4L2_CID_CONTRAST, 10);
+		delayed->push(ctrls);
+		ctrls.set(V4L2_CID_BRIGHTNESS, 11);
+		ctrls.set(V4L2_CID_CONTRAST, 11);
+		delayed->push(ctrls);
 
 		/* Test dual control with delay. */
-		for (unsigned int i = 1; i < 100; i++) {
+		for (unsigned int i = 0; i < 100; i++) {
 			int32_t value = 10 + i;
 
-			ctrls.set(V4L2_CID_BRIGHTNESS, value);
-			ctrls.set(V4L2_CID_CONTRAST, value + 1);
+			/* we are pushing 2 frames ahead */
+			ctrls.set(V4L2_CID_BRIGHTNESS, value + 2);
+			ctrls.set(V4L2_CID_CONTRAST, value + 2);
 			delayed->push(ctrls);
 
 			delayed->applyControls(i);
@@ -190,17 +312,32 @@  protected:
 			ControlList result = delayed->get(i);
 			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
 			int32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();
-			if (brightness != expected || contrast != expected + 1) {
-				cerr << "Failed dual controls"
-				     << " frame " << i
-				     << " brightness " << brightness
-				     << " contrast " << contrast
-				     << " expected " << expected
-				     << endl;
-				return TestFail;
-			}
 
-			expected = i < maxDelay ? expected : value - 1;
+			ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS, V4L2_CID_CONTRAST });
+			int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
+			int32_t contrastV4L = ctrlsV4L.get(V4L2_CID_CONTRAST).get<int32_t>();
+
+			if (i > maxDelay) {
+				if (brightness != value || contrast != value) {
+					cerr << "Failed dual controls"
+					     << " frame " << i
+					     << " brightness " << brightness
+					     << " contrast " << contrast
+					     << " expected " << value
+					     << endl;
+					return TestFail;
+				}
+				if (brightnessV4L != value + 1 || contrastV4L != value + maxDelay) {
+					cerr << "Failed dual controls"
+					     << " frame " << i
+					     << " brightnessV4L " << brightnessV4L
+					     << " expected " << value + 1
+					     << " contrastV4L " << contrastV4L
+					     << " expected " << value + maxDelay
+					     << endl;
+					return TestFail;
+				}
+			}
 		}
 
 		return TestPass;
@@ -208,7 +345,7 @@  protected:
 
 	int dualControlsMultiQueue()
 	{
-		static const unsigned int maxDelay = 2;
+		static const int maxDelay = 2;
 
 		std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
 			{ V4L2_CID_BRIGHTNESS, { 1, false } },
@@ -218,22 +355,19 @@  protected:
 			std::make_unique<DelayedControls>(dev_.get(), delays);
 		ControlList ctrls;
 
-		/* Reset control to value that will be first two frames in test. */
-		int32_t expected = 100;
-		ctrls.set(V4L2_CID_BRIGHTNESS, expected);
-		ctrls.set(V4L2_CID_CONTRAST, expected);
+		/* Reset control to a value that will be first two frames in test. */
+		int32_t initial = 100;
+		ctrls.set(V4L2_CID_BRIGHTNESS, initial);
+		ctrls.set(V4L2_CID_CONTRAST, initial);
 		dev_->setControls(&ctrls);
 		delayed->reset();
 
-		/* Trigger the first frame start event */
-		delayed->applyControls(0);
-
 		/*
-		 * Queue all controls before any fake frame start. Note we
+		 * Queue all controls before applyControls(). Note we
 		 * can't queue up more then the delayed controls history size
-		 * which is 16. Where one spot is used by the reset control.
+		 * which is 16.
 		 */
-		for (unsigned int i = 0; i < 15; i++) {
+		for (unsigned int i = 0; i < 14; i++) {
 			int32_t value = 10 + i;
 
 			ctrls.set(V4L2_CID_BRIGHTNESS, value);
@@ -241,9 +375,9 @@  protected:
 			delayed->push(ctrls);
 		}
 
-		/* Process all queued controls. */
-		for (unsigned int i = 1; i < 16; i++) {
-			int32_t value = 10 + i - 1;
+		/* Process 2 frames less than queued, so that the V4L controls are correct */
+		for (unsigned int i = 0; i < 12; i++) {
+			int32_t expected = i < maxDelay ? initial : 10 + i;
 
 			delayed->applyControls(i);
 
@@ -251,6 +385,11 @@  protected:
 
 			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
 			int32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();
+
+			ControlList ctrlsV4L = dev_->getControls({ V4L2_CID_BRIGHTNESS, V4L2_CID_CONTRAST });
+			int32_t brightnessV4L = ctrlsV4L.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
+			int32_t contrastV4L = ctrlsV4L.get(V4L2_CID_CONTRAST).get<int32_t>();
+
 			if (brightness != expected || contrast != expected) {
 				cerr << "Failed multi queue"
 				     << " frame " << i
@@ -261,7 +400,17 @@  protected:
 				return TestFail;
 			}
 
-			expected = i < maxDelay ? expected : value - 1;
+			/* Check v4l values after they've settled */
+			if (i >= maxDelay && (brightnessV4L != expected + 1 || contrastV4L != expected + maxDelay)) {
+				cerr << "Failed multi queue"
+				     << " frame " << i
+				     << " brightnessV4L " << brightnessV4L
+				     << " expected " << expected + 1
+				     << " contrastV4L " << contrastV4L
+				     << " expected " << expected + maxDelay
+				     << endl;
+				return TestFail;
+			}
 		}
 
 		return TestPass;
@@ -269,27 +418,40 @@  protected:
 
 	int run() override
 	{
-		int ret;
+		int ret = 0;
+		bool failed = false;
 
 		/* Test single control without delay. */
 		ret = singleControlNoDelay();
 		if (ret)
-			return ret;
+			failed = true;
 
 		/* Test single control with delay. */
 		ret = singleControlWithDelay();
 		if (ret)
-			return ret;
+			failed = true;
+
+		/* Test single control with delay. */
+		ret = singleControlWithDelayStartUp();
+		if (ret)
+			failed = true;
+
+		ret = doNotLoseFirstRequest();
+		if (ret)
+			failed = true;
 
 		/* Test dual controls with different delays. */
 		ret = dualControlsWithDelay();
 		if (ret)
-			return ret;
+			failed = true;
 
 		/* Test control values produced faster than consumed. */
 		ret = dualControlsMultiQueue();
 		if (ret)
-			return ret;
+			failed = true;
+
+		if (failed)
+			return TestFail;
 
 		return TestPass;
 	}