[libcamera-devel] [PATCH] test: delayed_controls: Remove sequenceOffset

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Jul 05, 2022 at 08:43:50PM +0200, Jacopo Mondi via libcamera-devel wrote:
> Commit 6f539a6d2fa9 ("delayed_controls: Remove reduandant firstSequence_")
> removed support for frame number start offset from the DelayedControls
> class, as it is now guaranteed that the first sequence number as it comes
> from the V4L2VideoDevice will always be 0.
> 
> However the delayed_controls.cpp unit still has two tests that passes
> a non-zero first sequence number to the DelayedControl class, causing
> the test to spin forever and consequentially fail.
> 
> Remove the two tests from the unit to fix this.
> 
> The first removed test was testing the class against frame start
> sequence numbers greater than zero and can safely be removed.
> 
> The second test was instead validating the class against sequence number
> overflow, which is now not possible to test anymore as the DelayedControls
> class now assumes 0 as first frame sequence number.
> 
> Fixes: 6f539a6d2fa9 ("delayed_controls: Remove reduandant firstSequence_")
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  test/delayed_controls.cpp | 23 ++++++-----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp
> index c6f195b7bc7f..a8ce9828d73d 100644
> --- a/test/delayed_controls.cpp
> +++ b/test/delayed_controls.cpp
> @@ -155,7 +155,7 @@ protected:
>  		return TestPass;
>  	}
>  
> -	int dualControlsWithDelay(uint32_t startOffset)
> +	int dualControlsWithDelay()
>  	{
>  		static const unsigned int maxDelay = 2;
>  
> @@ -175,25 +175,24 @@ protected:
>  		delayed->reset();
>  
>  		/* Trigger the first frame start event */
> -		delayed->applyControls(startOffset);
> +		delayed->applyControls(0);
>  
>  		/* Test dual control with delay. */
>  		for (unsigned int i = 1; i < 100; i++) {
> -			uint32_t frame = startOffset + i;
>  			int32_t value = 10 + i;
>  
>  			ctrls.set(V4L2_CID_BRIGHTNESS, value);
>  			ctrls.set(V4L2_CID_CONTRAST, value + 1);
>  			delayed->push(ctrls);
>  
> -			delayed->applyControls(frame);
> +			delayed->applyControls(i);
>  
> -			ControlList result = delayed->get(frame);
> +			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 " << frame
> +				     << " frame " << i
>  				     << " brightness " << brightness
>  				     << " contrast " << contrast
>  				     << " expected " << expected
> @@ -283,17 +282,7 @@ protected:
>  			return ret;
>  
>  		/* Test dual controls with different delays. */
> -		ret = dualControlsWithDelay(0);
> -		if (ret)
> -			return ret;
> -
> -		/* Test dual controls with non-zero sequence start. */
> -		ret = dualControlsWithDelay(10000);
> -		if (ret)
> -			return ret;
> -
> -		/* Test dual controls with sequence number wraparound. */
> -		ret = dualControlsWithDelay(UINT32_MAX - 50);
> +		ret = dualControlsWithDelay();
>  		if (ret)
>  			return ret;
>  

-- 
Regards,

Laurent Pinchart