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