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

[Naushir Patuck]

Hi,

Here is a revised version of the DelayedControls patch. The only difference between v2 and v1 is in patch 4/5 where the fix was incomplete.  To reiterate, here is a summary of the patches:

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.  This problem is mostly inconsequential.

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.

As a consequence of this change, we must also ensure that controls that have been sent
to the device must have the update flag cleared or there is a change it may be re-used
when going around the ring buffer. 

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().

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            | 79 ++++++++++-----
 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       | 98 +++++++++++++++++++
 7 files changed, 183 insertions(+), 56 deletions(-)
 create mode 100644 utils/raspberrypi/delayedctrls_parse.py

-- 
2.25.1