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

[Laurent Pinchart]

Hi Kieran,

On Tue, Jul 05, 2022 at 08:13:40PM +0100, Kieran Bingham via libcamera-devel wrote:
> 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!

Are there unit tests that normally fail for you ?

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

-- 
Regards,

Laurent Pinchart