<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 9 Mar 2021 at 10:33, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@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">Hi Naush,<br>
<br>
On Thu, Mar 04, 2021 at 08:17:21AM +0000, Naushir Patuck wrote:<br>
> Hi,<br>
> <br>
> This version (v4) of the patch series adds two further commits to v3.<br>
> <br>
> Patches 1 to 5 are identical to v3, except that I have added a line to the commit<br>
> messages saying the change would break the unit tests because of a change in the<br>
> logic of the helper.<br>
> <br>
> Patch 6 updates the unit tests with the correct behavior.  Unfortunately, my<br>
> attempts to get vimc working on my platform did not end well, so I made a local<br>
> mod to run the tests with a physical camera device. All tests do pass, but I<br>
> would appreciate if somebody else could run this again with the vimc device to<br>
> verify everything is passing.<br>
> <br>
> Patch 7 simply fixes a typo in the unit test filename.<br>
<br>
Just for my information, I'd like to get a clear understanding of the<br>
issues that this series fixes. As far as I know, it addresses<br>
oscillations in the algorithms for Raspberry Pi. Were those oscillations<br>
introduced by the switch from the StaggeredCtrls implementation to<br>
DelayedControls but only noticed later, or are there other changes that<br>
are involved here ?<br></blockquote><div><br></div><div>This series addresses the AGC oscillations observed on the Raspberry Pi</div><div>using our algorithm.  The key difference between DelayedControls and</div><div>StaggeredCtrls is that the former allows us to queue multiple batches of ctrls,</div><div>whereas the latter would only apply the last queued value.  Hence there</div><div>is a pair of indexes to track in DelayedControls, vs a single index in</div><div>StaggeredCtrl.  Because of this difference, the behavior between the two</div><div>was slightly different (e.g. an additional frame needed to queue a control</div><div>to the device by DelayedControls, causing a delay mismatch).</div><div><br></div><div>This patch aligns the behavior of the two while keeping the new functionality</div><div>of allowing us to queue multiple batches of ctrls.  This effectively fixes our</div><div>AGC oscillations as well.  This change should work equally well for all other</div><div>vendor AGC algorithms, there is nothing specific to Raspberry Pi in these</div><div>commits.</div><div><br></div><div>Hope that makes things clearer.</div><div><br></div><div>Regards,</div><div>Naush</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> Naushir Patuck (7):<br>
>   libcamera: delayed_controls: Add notion of priority write<br>
>   utils: raspberrypi: Add a DelayedControls log parser<br>
>   libcamera: delayed_controls: Remove unneeded write when starting up<br>
>   libcamera: delayed_controls: Remove spurious no-op queued controls<br>
>   libcamera: delayed_controls: Fix off-by-one error in get()<br>
>   test: delayed_controls: Fixup tests after recent DelayedControls<br>
>     changes<br>
>   test: delayed_controls: Rename delayed_contols.cpp to<br>
>     delayed_controls.cpp<br>
> <br>
>  include/libcamera/internal/delayed_controls.h |  13 +-<br>
>  src/libcamera/delayed_controls.cpp            |  74 ++++++++----<br>
>  src/libcamera/pipeline/ipu3/ipu3.cpp          |   8 +-<br>
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  13 +-<br>
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |   8 +-<br>
>  ...layed_contols.cpp => delayed_controls.cpp} |  51 +++++---<br>
>  test/meson.build                              |   2 +-<br>
>  utils/raspberrypi/delayedctrls_parse.py       | 111 ++++++++++++++++++<br>
>  8 files changed, 219 insertions(+), 61 deletions(-)<br>
>  rename test/{delayed_contols.cpp => delayed_controls.cpp} (84%)<br>
>  create mode 100644 utils/raspberrypi/delayedctrls_parse.py<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>