[libcamera-devel] [PATCH 0/5] DelayedControls updates and fixes

[Naushir Patuck]

Hi all,

We think there may be still further underlying issues causing some agc
oscillations. As such please do not review this patch set until I send an
update.

Regards,
Naush

On Fri, 12 Feb 2021, 11:37 am Naushir Patuck, <naush at raspberrypi.com> wrote:

> Hi all,
>
> This patchset mainly addresses some minor issues we have encountered with
> DelayedControls
> when running on the Raspberry Pi platform.  Apologies for the slightly
> long cover letter,
> but I wanted to explain the problems we are seeing in a bit of detail :-)
>
> Patch 1/5
> This adds the notion of priority write to fixup the known issue of
> settings VBLANK and
> EXPOSURE in a single batch.  It is simply a port of the fix applied to
> StaggeredCtrl
> that had been reviewed and subsequently deprecated, so hopefully nothing
> too controversial.
>
> Patch 2/5
> This is simply a python script that I am using to debug the small problems
> (more details
> below) that we have encountered.  It parses the DelayedControls logs and
> nicely tabulates
> the results to show what controls and values have been set/queued/get on
> each frame.
> This has helped me tremendously in identifying problems and fixing them.
> However, this
> may not be useful to others, so I am happy to not have this merged if
> folks do not think
> it is the right place to put it.
>
> Patch 3/5
> Fixes a spurious write to the device on startup.  The following is an
> extract from using
> the script to parse the logs:
>
> Frame     Action         Gain        Exposure          Vblank
> 0         Write         0 [0]          52 [0]         531 [0] <<<<<
> 0         Get           0 [0]          52 [0]         531 [0]
> 0         Queue       --- [-]         --- [-]         --- [-]
> 1         Write         0 [0]         --- [-]         --- [-]
> 1         Get           0 [0]          52 [0]         531 [0]
> 1         Queue       --- [-]         --- [-]         --- [-]
> 2         Write       --- [-]         --- [-]         --- [-]
> 2         Get           0 [1]          52 [1]         531 [1]
> 2         Queue       192 [4]        1664 [4]         531 [4]
>
> You can see above, on frame 0 we are writing controls to the sensor, but
> this is unneeded.
> This spurious write should really not happen as there is no controls
> queued by the
> pipeline_handler at this point.  It is, however, mostly inconsequential at
> runtime.
>
> Patch 4/5
> This fixes an issue where controls queued by the pipeline handler are
> delayed by and
> additional frame when writing.  You can see better in the parsed log:
>
> Frame     Action         Gain        Exposure          Vblank
> 2         Write       --- [-]         --- [-]         --- [-]
> 2         Get           0 [1]          52 [1]         531 [1]
> 2         Queue       192 [4]        1664 [4]         531 [4] <<<<<
> 3         Write       --- [-]         --- [-]         --- [-] <<<<<
> 3         Get           0 [2]          52 [2]         531 [2]
> 3         Queue       192 [5]        1664 [5]         531 [5]
> 4         Write       --- [-]        1664 [4]         531 [4] <<<<<
> 4         Get           0 [3]          52 [3]         531 [3]
> 4         Queue       192 [6]        1664 [6]         531 [6]
>
> On frame 2, we queue controls from the pipeline handler.  Exposure and
> Vblank must be
> written one frame before gain, so you would expect them to be written on
> frame 3 as
> nothing else is in the queue.  However, they only get written on frame 4,
> one frame
> later than expected.
>
> This is because of how DelayedControls handles "no-op" queue items, i.e.
> frames where
> we do not provide the helper with controls to use.  It was adding one more
> no-op than
> needed, and causing an extra frame delay when setting the control on the
> device.
>
> Patch 5/5
> We had an off-by-one error when reading back values from the queues.  See
> the parsed
> logs below:
>
> Frame     Action         Gain        Exposure          Vblank
> 7         Write       192 [6]        1664 [7]         531 [7] <<<<<
> 7         Get         192 [6]        1664 [6]         531 [6]
> 7         Queue       210 [9]        3174 [9]        1946 [9]
> 8         Write       192 [7]        3526 [8]        2298 [8]
> 8         Get         192 [7]        1664 [7]         531 [7] <<<<<
> 8         Queue       210 [10]       3174 [10]       1946 [10]
> 9         Write       213 [8]        3174 [9]        1946 [9]
> 9         Get         213 [8]        3526 [8]        2298 [8] <<<<<
> 9         Queue       213 [12]       3526 [12]       2298 [12]
>
> On frame 7, we write exposure and vblank with values 1664 and 531
> respectively.  These
> values take 2 frames to consume, so should be retuned to the pipeline
> handler by the
> DelayedControls::get() on frame 9.  However, it returns on frame 8 instead.
>
> This only causes slight (but visible) oscillations in brightness in the
> AGC loop as
> the values used by the sensor are not in lock-step to what is reported by
> DelayedControls::get().
>
> Hope that is all reasonably well explained :-)  Any questions, please do
> shout.
>
> Regards,
> Naush
>
> Naushir Patuck (5):
>   libcamera: delayed_controls: Add notion of priority write
>   utils: raspberrypi: Add a DelayedControls log parser
>   libcamera: delayed_controls: Remove unneeded write when starting up
>   libcamera: delayed_controls: Remove spurious no-op queued controls
>   libcamera: delayed_controls: Fix off-by-one error in get()
>
>  include/libcamera/internal/delayed_controls.h | 13 ++-
>  src/libcamera/delayed_controls.cpp            | 72 ++++++++++------
>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  8 +-
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 13 ++-
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  8 +-
>  test/delayed_contols.cpp                      | 20 ++---
>  utils/raspberrypi/delayedctrls_parse.py       | 82 +++++++++++++++++++
>  7 files changed, 162 insertions(+), 54 deletions(-)
>  create mode 100644 utils/raspberrypi/delayedctrls_parse.py
>
> --
> 2.25.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210215/d7ea35a7/attachment-0001.htm>