[1/1] libcamera: camera: Fix up the AeEnable control during Camera::start()

23411 diff mbox series

In Camera::queueRequest() the control list is updated transparently by
converting AeEnable into ExposureTimeMode and AnalogueGainMode
controls.

However, this was not happening during Camera::start(), meaning that
setting AeEnable there was having no effect. It should behave the same
here too.

Fixes: 7abd4139051c ("libcamera: camera: Pre-process AeEnable control")
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 include/libcamera/camera.h |  2 ++
 src/libcamera/camera.cpp   | 57 +++++++++++++++++++++++++-------------
 2 files changed, 40 insertions(+), 19 deletions(-)

Quoting David Plowman (2025-05-21 14:53:27)
> In Camera::queueRequest() the control list is updated transparently by
> converting AeEnable into ExposureTimeMode and AnalogueGainMode
> controls.
> 
> However, this was not happening during Camera::start(), meaning that
> setting AeEnable there was having no effect. It should behave the same
> here too.
> 
> Fixes: 7abd4139051c ("libcamera: camera: Pre-process AeEnable control")
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>

Looks good to me.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  include/libcamera/camera.h |  2 ++
>  src/libcamera/camera.cpp   | 57 +++++++++++++++++++++++++-------------
>  2 files changed, 40 insertions(+), 19 deletions(-)
> 
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 94cee7bd..b24a2974 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -165,6 +165,8 @@ private:
>         friend class FrameBufferAllocator;
>         int exportFrameBuffers(Stream *stream,
>                                std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> +
> +       void patchControlList(ControlList &controls);
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index c180a5fd..23b9207a 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -1266,6 +1266,33 @@ std::unique_ptr<Request> Camera::createRequest(uint64_t cookie)
>         return request;
>  }
>  
> +/**
> + * \brief Patch a control list that contains the AeEnable control
> + * \param[inout] controls The control list to be patched
> + *
> + * The control list is patched in place, turning the AeEnable control into
> + * the equivalent ExposureTimeMode/AnalogueGainMode controls.
> + */
> +void Camera::patchControlList(ControlList &controls)
> +{
> +       const auto &aeEnable = controls.get(controls::AeEnable);
> +       if (aeEnable) {
> +               if (_d()->controlInfo_.count(controls::AnalogueGainMode.id()) &&
> +                   !controls.contains(controls::AnalogueGainMode.id())) {
> +                       controls.set(controls::AnalogueGainMode,
> +                                    *aeEnable ? controls::AnalogueGainModeAuto
> +                                              : controls::AnalogueGainModeManual);
> +               }
> +
> +               if (_d()->controlInfo_.count(controls::ExposureTimeMode.id()) &&
> +                   !controls.contains(controls::ExposureTimeMode.id())) {
> +                       controls.set(controls::ExposureTimeMode,
> +                                    *aeEnable ? controls::ExposureTimeModeAuto
> +                                              : controls::ExposureTimeModeManual);
> +               }
> +       }
> +}
> +
>  /**
>   * \brief Queue a request to the camera
>   * \param[in] request The request to queue to the camera
> @@ -1329,23 +1356,7 @@ int Camera::queueRequest(Request *request)
>         }
>  
>         /* Pre-process AeEnable. */
> -       ControlList &controls = request->controls();
> -       const auto &aeEnable = controls.get(controls::AeEnable);
> -       if (aeEnable) {
> -               if (_d()->controlInfo_.count(controls::AnalogueGainMode.id()) &&
> -                   !controls.contains(controls::AnalogueGainMode.id())) {
> -                       controls.set(controls::AnalogueGainMode,
> -                                    *aeEnable ? controls::AnalogueGainModeAuto
> -                                              : controls::AnalogueGainModeManual);
> -               }
> -
> -               if (_d()->controlInfo_.count(controls::ExposureTimeMode.id()) &&
> -                   !controls.contains(controls::ExposureTimeMode.id())) {
> -                       controls.set(controls::ExposureTimeMode,
> -                                    *aeEnable ? controls::ExposureTimeModeAuto
> -                                              : controls::ExposureTimeModeManual);
> -               }
> -       }
> +       patchControlList(request->controls());
>  
>         d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
>                                ConnectionTypeQueued, request);
> @@ -1383,8 +1394,16 @@ int Camera::start(const ControlList *controls)
>  
>         ASSERT(d->requestSequence_ == 0);
>  
> -       ret = d->pipe_->invokeMethod(&PipelineHandler::start,
> -                                    ConnectionTypeBlocking, this, controls);
> +       if (controls) {
> +               ControlList copy(*controls);
> +               patchControlList(copy);
> +               ret = d->pipe_->invokeMethod(&PipelineHandler::start,
> +                                            ConnectionTypeBlocking, this, &copy);
> +       }
> +       else
> +               ret = d->pipe_->invokeMethod(&PipelineHandler::start,
> +                                            ConnectionTypeBlocking, this, nullptr);
> +
>         if (ret)
>                 return ret;
>  
> -- 
> 2.39.5
>