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

[Kieran Bingham]

Hi Jacopo,

Quoting Jacopo Mondi via libcamera-devel (2022-07-05 19:43:50)
> 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.
> 

Ayeee, I can't believe I missed this. I must have glanced too casually
at the test results and placed more attention on the fact that CTS had
succeed!

> 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.

Hrm ... that's a bit of a shame, but I don't think it's too troublesome
just yet.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> Fixes: 6f539a6d2fa9 ("delayed_controls: Remove reduandant firstSequence_")
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  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;
>  
> -- 
> 2.36.1
>