[libcamera-devel,v4,3/7] tests: delayed_controls: Add cookie value test

17628 diff mbox series

Add a test for passing and returning cookie values in DelayedControls.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
---
 test/delayed_controls.cpp | 44 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

Quoting Naushir Patuck via libcamera-devel (2022-10-19 10:01:03)
> Add a test for passing and returning cookie values in DelayedControls.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  test/delayed_controls.cpp | 44 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp
> index 322c545998b2..5cc7d3aed4fd 100644
> --- a/test/delayed_controls.cpp
> +++ b/test/delayed_controls.cpp
> @@ -267,6 +267,45 @@ protected:
>                 return TestPass;
>         }
>  
> +       int cookieValue()
> +       {
> +               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;
> +
> +               /* Set a cookie to the reset value. */
> +               const unsigned int startCookie = 0x1234;
> +               ctrls.set(V4L2_CID_BRIGHTNESS, 1);
> +               dev_->setControls(&ctrls);
> +               delayed->reset(startCookie);
> +
> +               /* Trigger the first frame start event */
> +               delayed->applyControls(0);
> +
> +               for (unsigned int i = 1; i < 100; i++) {
> +                       ctrls.set(V4L2_CID_BRIGHTNESS, 1);
> +                       delayed->push(ctrls, startCookie + i);
> +
> +                       delayed->applyControls(i);

The interesting part might be what happens in the event that the pushs
and the applyControls() get out of sync. I think there's something that
internally propogates to push another set of controls on or such ?

Does that just propogate the cookie forwards from the previous one too?



> +
> +                       auto [result, cookie] = delayed->get(i);
> +                       unsigned int expected = startCookie + i - 1;
> +                       if (cookie != expected) {
> +                               cerr << "Failed cookie value"
> +                                    << " frame " << i
> +                                    << " expected cookie " << expected
> +                                    << " got cookie " << cookie
> +                                    << endl;
> +                               return TestFail;
> +                       }
> +               }
> +
> +               return TestPass;
> +       }
> +
>         int run() override
>         {
>                 int ret;
> @@ -291,6 +330,11 @@ protected:
>                 if (ret)
>                         return ret;
>  
> +               /* Test cookie values. */
> +               ret = cookieValue();
> +               if (ret)
> +                       return ret;
> +
>                 return TestPass;
>         }
>  
> -- 
> 2.25.1
>

Hi Kieran,

Thanks for your review.

On Wed, 26 Oct 2022 at 17:42, Kieran Bingham <
kieran.bingham@ideasonboard.com> wrote:

> Quoting Naushir Patuck via libcamera-devel (2022-10-19 10:01:03)
> > Add a test for passing and returning cookie values in DelayedControls.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  test/delayed_controls.cpp | 44 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >
> > diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp
> > index 322c545998b2..5cc7d3aed4fd 100644
> > --- a/test/delayed_controls.cpp
> > +++ b/test/delayed_controls.cpp
> > @@ -267,6 +267,45 @@ protected:
> >                 return TestPass;
> >         }
> >
> > +       int cookieValue()
> > +       {
> > +               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;
> > +
> > +               /* Set a cookie to the reset value. */
> > +               const unsigned int startCookie = 0x1234;
> > +               ctrls.set(V4L2_CID_BRIGHTNESS, 1);
> > +               dev_->setControls(&ctrls);
> > +               delayed->reset(startCookie);
> > +
> > +               /* Trigger the first frame start event */
> > +               delayed->applyControls(0);
> > +
> > +               for (unsigned int i = 1; i < 100; i++) {
> > +                       ctrls.set(V4L2_CID_BRIGHTNESS, 1);
> > +                       delayed->push(ctrls, startCookie + i);
> > +
> > +                       delayed->applyControls(i);
>
> The interesting part might be what happens in the event that the pushs
> and the applyControls() get out of sync. I think there's something that
> internally propogates to push another set of controls on or such ?
>
> Does that just propogate the cookie forwards from the previous one too?
>

Yes it does!  The advantage of using DelayedControls to track everything
means
we effectively get this sync handling for free.

I'll add another test to this patch that exercises exactly this behavior.

Regards,
Naush


>
>
>
> > +
> > +                       auto [result, cookie] = delayed->get(i);
> > +                       unsigned int expected = startCookie + i - 1;
> > +                       if (cookie != expected) {
> > +                               cerr << "Failed cookie value"
> > +                                    << " frame " << i
> > +                                    << " expected cookie " << expected
> > +                                    << " got cookie " << cookie
> > +                                    << endl;
> > +                               return TestFail;
> > +                       }
> > +               }
> > +
> > +               return TestPass;
> > +       }
> > +
> >         int run() override
> >         {
> >                 int ret;
> > @@ -291,6 +330,11 @@ protected:
> >                 if (ret)
> >                         return ret;
> >
> > +               /* Test cookie values. */
> > +               ret = cookieValue();
> > +               if (ret)
> > +                       return ret;
> > +
> >                 return TestPass;
> >         }
> >
> > --
> > 2.25.1
> >
>