[libcamera-devel] [PATCH v3 12/23] libcamera: ipu3: Drop DelayedControls

[Kieran Bingham]

Quoting Jacopo Mondi via libcamera-devel (2022-06-30 14:38:51)
> Now that the CameraSensor class exposes the DelayedControl interface
> use the sensor to operate controls.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 85e5f8a70408..a5a35b6585c6 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -26,7 +26,6 @@
>  #include "libcamera/internal/camera.h"
>  #include "libcamera/internal/camera_lens.h"
>  #include "libcamera/internal/camera_sensor.h"
> -#include "libcamera/internal/delayed_controls.h"
>  #include "libcamera/internal/device_enumerator.h"
>  #include "libcamera/internal/framebuffer.h"
>  #include "libcamera/internal/ipa_manager.h"
> @@ -74,7 +73,6 @@ public:
>         bool supportsFlips_;
>         Transform rotationTransform_;
>  
> -       std::unique_ptr<DelayedControls> delayedCtrls_;
>         IPU3Frames frameInfos_;
>  
>         std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
> @@ -785,8 +783,6 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
>         if (ret)
>                 goto error;
>  
> -       data->delayedCtrls_->reset();
> -
>         /*
>          * Start the ImgU video devices, buffers will be queued to the
>          * ImgU output and viewfinder when requests will be queued.
> @@ -1136,9 +1132,6 @@ int PipelineHandlerIPU3::registerCameras()
>                         { V4L2_CID_EXPOSURE, { 2, false } },
>                 };
>  

params can be removed above too.


> -               data->delayedCtrls_ =
> -                       std::make_unique<DelayedControls>(cio2->sensor()->device(),
> -                                                         params);
>                 data->cio2_.frameStart().connect(data.get(),
>                                                  &IPU3CameraData::frameStart);

Does the FrameStart event only come from the cio2_ video device?
It seems like something that should/could already be handled directly
by the CameraSensor class? (But maybe the sensor subdevs don't expose it
as it's an event determined by the CSI2 recievers?)

> @@ -1258,9 +1251,11 @@ void IPU3CameraData::setSensorControls([[maybe_unused]] unsigned int id,
>                                        const ControlList &sensorControls,
>                                        const ControlList &lensControls)
>  {
> -       delayedCtrls_->push(sensorControls);
> +       CameraSensor *sensor = cio2_.sensor();
> +
> +       sensor->pushControls(sensorControls);
>  
> -       CameraLens *focusLens = cio2_.sensor()->focusLens();
> +       CameraLens *focusLens = sensor->focusLens();
>         if (!focusLens)
>                 return;
>  
> @@ -1375,7 +1370,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>         request->metadata().set(controls::SensorTimestamp,
>                                 buffer->metadata().timestamp);
>  
> -       info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);
> +       info->effectiveSensorControls = cio2_.sensor()->getControls(buffer->metadata().sequence);
>  
>         if (request->findBuffer(&rawStream_))
>                 pipe()->completeBuffer(request, buffer);
> @@ -1441,7 +1436,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
>   */
>  void IPU3CameraData::frameStart(uint32_t sequence)
>  {
> -       delayedCtrls_->applyControls(sequence);
> +       cio2_.sensor()->frameStart(sequence);

I wonder if this could be tied into the CameraSensor class directly
somehow ... but this is fine I think for now.


>  
>         if (processingRequests_.empty())
>                 return;
> -- 
> 2.36.1
>