<div dir="ltr"><div dir="ltr">Hi Kieran,<div><br></div><div>Thanks for your review.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 26 Oct 2022 at 17:42, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Naushir Patuck via libcamera-devel (2022-10-19 10:01:03)<br>
> Add a test for passing and returning cookie values in DelayedControls.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> ---<br>
>  test/delayed_controls.cpp | 44 +++++++++++++++++++++++++++++++++++++++<br>
>  1 file changed, 44 insertions(+)<br>
> <br>
> diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp<br>
> index 322c545998b2..5cc7d3aed4fd 100644<br>
> --- a/test/delayed_controls.cpp<br>
> +++ b/test/delayed_controls.cpp<br>
> @@ -267,6 +267,45 @@ protected:<br>
>                 return TestPass;<br>
>         }<br>
>  <br>
> +       int cookieValue()<br>
> +       {<br>
> +               std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {<br>
> +                       { V4L2_CID_BRIGHTNESS, { 1, false } },<br>
> +               };<br>
> +               std::unique_ptr<DelayedControls> delayed =<br>
> +                       std::make_unique<DelayedControls>(dev_.get(), delays);<br>
> +               ControlList ctrls;<br>
> +<br>
> +               /* Set a cookie to the reset value. */<br>
> +               const unsigned int startCookie = 0x1234;<br>
> +               ctrls.set(V4L2_CID_BRIGHTNESS, 1);<br>
> +               dev_->setControls(&ctrls);<br>
> +               delayed->reset(startCookie);<br>
> +<br>
> +               /* Trigger the first frame start event */<br>
> +               delayed->applyControls(0);<br>
> +<br>
> +               for (unsigned int i = 1; i < 100; i++) {<br>
> +                       ctrls.set(V4L2_CID_BRIGHTNESS, 1);<br>
> +                       delayed->push(ctrls, startCookie + i);<br>
> +<br>
> +                       delayed->applyControls(i);<br>
<br>
The interesting part might be what happens in the event that the pushs<br>
and the applyControls() get out of sync. I think there's something that<br>
internally propogates to push another set of controls on or such ?<br>
<br>
Does that just propogate the cookie forwards from the previous one too?<br></blockquote><div><br></div><div>Yes it does!  The advantage of using DelayedControls to track everything means<br>we effectively get this sync handling for free.<br><br>I'll add another test to this patch that exercises exactly this behavior.<br></div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
<br>
> +<br>
> +                       auto [result, cookie] = delayed->get(i);<br>
> +                       unsigned int expected = startCookie + i - 1;<br>
> +                       if (cookie != expected) {<br>
> +                               cerr << "Failed cookie value"<br>
> +                                    << " frame " << i<br>
> +                                    << " expected cookie " << expected<br>
> +                                    << " got cookie " << cookie<br>
> +                                    << endl;<br>
> +                               return TestFail;<br>
> +                       }<br>
> +               }<br>
> +<br>
> +               return TestPass;<br>
> +       }<br>
> +<br>
>         int run() override<br>
>         {<br>
>                 int ret;<br>
> @@ -291,6 +330,11 @@ protected:<br>
>                 if (ret)<br>
>                         return ret;<br>
>  <br>
> +               /* Test cookie values. */<br>
> +               ret = cookieValue();<br>
> +               if (ret)<br>
> +                       return ret;<br>
> +<br>
>                 return TestPass;<br>
>         }<br>
>  <br>
> -- <br>
> 2.25.1<br>
><br>
</blockquote></div></div>