[PATCH RFC 0/3] Pass sensor delays from rkisp1 IPA to the pipeline handler

[Dan Scally]

Hi Mikhail - thanks for the RFC

On 28/10/2024 17:36, Mikhail Rudenko wrote:
> Hi,
>
> At present, rkisp1 uses hardcoded sensor control delays. In the case
> when they do not match real sensor delays, the AGC algorithm operates
> in a suboptimal mode, leading to gain/exposure oscillations on capture
> start and slower convergence.
>
> This series does 3 things to fix this situation. First, it adds an IPC
> structure for sensor control delays. Second, it adds sensorDelays()
> method to the CameraSensorHelper and overrides it where
> sensor-specific delays are known. And finally, it replaces hardcoded
> delays in rkisp1 pipeline handler with those obtained from the
> CameraSensorHelper via IPA.
>
> I'm not completely sure this is the best solution from the
> architecture viewpoint, thus "RFC". Any feedback is welcome!


I had a similar patch locally, but I decided to swap it in the end so that the delays are defined as 
members of the CameraSensorProperties class, which is available from the PipelineHandlers already - 
so setting the delays in the rkisp1 (and all the other...) pipeline handlers becomes:


     /* Initialize the camera properties. */
     data->properties_ = data->sensor_->properties();

     unsigned int gainDelay = data->properties_.get(properties::GainDelay).value();
     unsigned int exposureDelay = data->properties_.get(properties::ExposureDelay).value();
     std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
         { V4L2_CID_ANALOGUE_GAIN, { gainDelay, false } },
         { V4L2_CID_EXPOSURE, { exposureDelay, false } },
     };


Unless the delays are going to also be needed by the IPA module I think that's probably a bit 
cleaner - I can share the patches to show the full changeset. Do you anticipate the IPA's needing 
the delay values?


Thanks

Dan


>
> Mikhail Rudenko (3):
>    ipa: core: add IPASensorDelays structure
>    libcamera: libipa: camera_sensor: Add sensorDelays method
>    ipa: rkisp1: Pass sensor delays from IPA to pipeline handler
>
>   include/libcamera/ipa/core.mojom         | 35 ++++++++++++++
>   include/libcamera/ipa/rkisp1.mojom       |  3 +-
>   src/ipa/libipa/camera_sensor_helper.cpp  | 59 ++++++++++++++++++++++++
>   src/ipa/libipa/camera_sensor_helper.h    |  5 ++
>   src/ipa/rkisp1/rkisp1.cpp                |  8 +++-
>   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 27 +++++------
>   6 files changed, 118 insertions(+), 19 deletions(-)
>
> --
> 2.46.0