[libcamera-devel] [PATCH 2/2] ipa: rkisp1: Replace event-based ops with dedicated functions
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Wed Mar 23 12:54:46 CET 2022
Hi Umang,
On Tue, Mar 08, 2022 at 02:45:20PM +0530, Umang Jain via libcamera-devel wrote:
> The IPARkISP1Interface currently uses event-type based structures in
> order to communicate with the pipeline-handler (and vice-versa).
> Replace the event based structures with dedicated functions associated
> to each operation.
>
> The translated naming scheme of operations to dedicated functions:
> ActionV4L2Set => setSensorControls
> ActionParamFilled => paramsBufferReady
> ActionMetadata => statsBufferReady
> EventSignalStatBuffer => processStatsBuffer()
> EventQueueRequest => queueRequest()
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
With the change requested by Laurent,
Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> include/libcamera/ipa/rkisp1.mojom | 31 ++-------
> src/ipa/rkisp1/rkisp1.cpp | 82 +++++++-----------------
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 71 ++++++++------------
> 3 files changed, 56 insertions(+), 128 deletions(-)
>
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index c3a6d8e1..a0b92b84 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -8,28 +8,6 @@ module ipa.rkisp1;
>
> import "include/libcamera/ipa/core.mojom";
>
> -enum RkISP1Operations {
> - ActionV4L2Set = 1,
> - ActionParamFilled = 2,
> - ActionMetadata = 3,
> - EventSignalStatBuffer = 4,
> - EventQueueRequest = 5,
> -};
> -
> -struct RkISP1Event {
> - RkISP1Operations op;
> - uint32 frame;
> - uint32 bufferId;
> - libcamera.ControlList controls;
> - libcamera.ControlList sensorControls;
> -};
> -
> -struct RkISP1Action {
> - RkISP1Operations op;
> - libcamera.ControlList controls;
> - libcamera.ControlList sensorControls;
> -};
> -
> interface IPARkISP1Interface {
> init(libcamera.IPASettings settings,
> uint32 hwRevision)
> @@ -45,9 +23,14 @@ interface IPARkISP1Interface {
> mapBuffers(array<libcamera.IPABuffer> buffers);
> unmapBuffers(array<uint32> ids);
>
> - [async] processEvent(RkISP1Event ev);
> + [async] queueRequest(uint32 frame, uint32 bufferId,
> + libcamera.ControlList reqControls);
> + [async] processStatsBuffer(uint32 frame, uint32 bufferId,
> + libcamera.ControlList sensorControls);
> };
>
> interface IPARkISP1EventInterface {
> - queueFrameAction(uint32 frame, RkISP1Action action);
> + paramsBufferReady(uint32 frame);
> + setSensorControls(uint32 frame, libcamera.ControlList sensorControls);
> + statsBufferReady(uint32 frame, libcamera.ControlList metadata);
> };
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 129afddd..dc00c1f8 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -51,14 +51,12 @@ public:
> const std::map<uint32_t, ControlInfoMap> &entityControls) override;
> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> void unmapBuffers(const std::vector<unsigned int> &ids) override;
> - void processEvent(const RkISP1Event &event) override;
>
> + void queueRequest(const uint32_t frame, const uint32_t bufferId,
> + const ControlList &controls) override;
> + void processStatsBuffer(const uint32_t frame, const uint32_t bufferId,
> + const ControlList &sensorControls) override;
> private:
> - void queueRequest(unsigned int frame, rkisp1_params_cfg *params,
> - const ControlList &controls);
> - void updateStatistics(unsigned int frame,
> - const rkisp1_stat_buffer *stats);
> -
> void setControls(unsigned int frame);
> void metadataReady(unsigned int frame, unsigned int aeState);
>
> @@ -231,60 +229,34 @@ void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
> }
> }
>
> -void IPARkISP1::processEvent(const RkISP1Event &event)
> -{
> - switch (event.op) {
> - case EventSignalStatBuffer: {
> - unsigned int frame = event.frame;
> - unsigned int bufferId = event.bufferId;
> -
> - const rkisp1_stat_buffer *stats =
> - reinterpret_cast<rkisp1_stat_buffer *>(
> - mappedBuffers_.at(bufferId).planes()[0].data());
> -
> - context_.frameContext.sensor.exposure =
> - event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> - context_.frameContext.sensor.gain =
> - camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> -
> - updateStatistics(frame, stats);
> - break;
> - }
> - case EventQueueRequest: {
> - unsigned int frame = event.frame;
> - unsigned int bufferId = event.bufferId;
> -
> - rkisp1_params_cfg *params =
> - reinterpret_cast<rkisp1_params_cfg *>(
> - mappedBuffers_.at(bufferId).planes()[0].data());
> -
> - queueRequest(frame, params, event.controls);
> - break;
> - }
> - default:
> - LOG(IPARkISP1, Error) << "Unknown event " << event.op;
> - break;
> - }
> -}
> -
> -void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,
> +void IPARkISP1::queueRequest(const uint32_t frame, const uint32_t bufferId,
> [[maybe_unused]] const ControlList &controls)
> {
> + rkisp1_params_cfg *params =
> + reinterpret_cast<rkisp1_params_cfg *>(
> + mappedBuffers_.at(bufferId).planes()[0].data());
> +
> /* Prepare parameters buffer. */
> memset(params, 0, sizeof(*params));
>
> for (auto const &algo : algorithms_)
> algo->prepare(context_, params);
>
> - RkISP1Action op;
> - op.op = ActionParamFilled;
> -
> - queueFrameAction.emit(frame, op);
> + paramsBufferReady.emit(frame);
> }
>
> -void IPARkISP1::updateStatistics(unsigned int frame,
> - const rkisp1_stat_buffer *stats)
> +void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,
> + const ControlList &sensorControls)
> {
> + const rkisp1_stat_buffer *stats =
> + reinterpret_cast<rkisp1_stat_buffer *>(
> + mappedBuffers_.at(bufferId).planes()[0].data());
> +
> + context_.frameContext.sensor.exposure =
> + sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> + context_.frameContext.sensor.gain =
> + camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> +
> unsigned int aeState = 0;
>
> for (auto const &algo : algorithms_)
> @@ -297,18 +269,14 @@ void IPARkISP1::updateStatistics(unsigned int frame,
>
> void IPARkISP1::setControls(unsigned int frame)
> {
> - RkISP1Action op;
> - op.op = ActionV4L2Set;
> -
> uint32_t exposure = context_.frameContext.agc.exposure;
> uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);
>
> ControlList ctrls(ctrls_);
> ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));
> ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));
> - op.sensorControls = ctrls;
>
> - queueFrameAction.emit(frame, op);
> + setSensorControls.emit(frame, ctrls);
> }
>
> void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)
> @@ -318,11 +286,7 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)
> if (aeState)
> ctrls.set(controls::AeLocked, aeState == 2);
>
> - RkISP1Action op;
> - op.op = ActionMetadata;
> - op.controls = ctrls;
> -
> - queueFrameAction.emit(frame, op);
> + statsBufferReady.emit(frame, ctrls);
> }
>
> } /* namespace ipa::rkisp1 */
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 8cca8a15..d395e335 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -104,8 +104,9 @@ public:
> std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
>
> private:
> - void queueFrameAction(unsigned int frame,
> - const ipa::rkisp1::RkISP1Action &action);
> + void paramFilled(unsigned int frame);
> + void setSensorControls(unsigned int frame,
> + const ControlList &sensorControls);
>
> void metadataReady(unsigned int frame, const ControlList &metadata);
> };
> @@ -316,8 +317,9 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
> if (!ipa_)
> return -ENOENT;
>
> - ipa_->queueFrameAction.connect(this,
> - &RkISP1CameraData::queueFrameAction);
> + ipa_->setSensorControls.connect(this, &RkISP1CameraData::setSensorControls);
> + ipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled);
> + ipa_->statsBufferReady.connect(this, &RkISP1CameraData::metadataReady);
>
> int ret = ipa_->init(IPASettings{ "", sensor_->model() }, hwRevision);
> if (ret < 0) {
> @@ -328,39 +330,27 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
> return 0;
> }
>
> -void RkISP1CameraData::queueFrameAction(unsigned int frame,
> - const ipa::rkisp1::RkISP1Action &action)
> +void RkISP1CameraData::paramFilled(unsigned int frame)
> {
> - switch (action.op) {
> - case ipa::rkisp1::ActionV4L2Set: {
> - const ControlList &controls = action.sensorControls;
> - delayedCtrls_->push(controls);
> - break;
> - }
> - case ipa::rkisp1::ActionParamFilled: {
> - PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
> - RkISP1FrameInfo *info = frameInfo_.find(frame);
> - if (!info)
> - break;
> + PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
> + RkISP1FrameInfo *info = frameInfo_.find(frame);
> + if (!info)
> + return;
>
> - pipe->param_->queueBuffer(info->paramBuffer);
> - pipe->stat_->queueBuffer(info->statBuffer);
> + pipe->param_->queueBuffer(info->paramBuffer);
> + pipe->stat_->queueBuffer(info->statBuffer);
>
> - if (info->mainPathBuffer)
> - mainPath_->queueBuffer(info->mainPathBuffer);
> + if (info->mainPathBuffer)
> + mainPath_->queueBuffer(info->mainPathBuffer);
>
> - if (info->selfPathBuffer)
> - selfPath_->queueBuffer(info->selfPathBuffer);
> + if (info->selfPathBuffer)
> + selfPath_->queueBuffer(info->selfPathBuffer);
> +}
>
> - break;
> - }
> - case ipa::rkisp1::ActionMetadata:
> - metadataReady(frame, action.controls);
> - break;
> - default:
> - LOG(RkISP1, Error) << "Unknown action " << action.op;
> - break;
> - }
> +void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
> + const ControlList &sensorControls)
> +{
> + delayedCtrls_->push(sensorControls);
> }
>
> void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
> @@ -865,13 +855,8 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
> if (!info)
> return -ENOENT;
>
> - ipa::rkisp1::RkISP1Event ev;
> - ev.op = ipa::rkisp1::EventQueueRequest;
> - ev.frame = data->frame_;
> - ev.bufferId = info->paramBuffer->cookie();
> - ev.controls = request->controls();
> - data->ipa_->processEvent(ev);
> -
> + data->ipa_->queueRequest(data->frame_, info->paramBuffer->cookie(),
> + request->controls());
> data->frame_++;
>
> return 0;
> @@ -1120,12 +1105,8 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
> if (data->frame_ <= buffer->metadata().sequence)
> data->frame_ = buffer->metadata().sequence + 1;
>
> - ipa::rkisp1::RkISP1Event ev;
> - ev.op = ipa::rkisp1::EventSignalStatBuffer;
> - ev.frame = info->frame;
> - ev.bufferId = info->statBuffer->cookie();
> - ev.sensorControls = data->delayedCtrls_->get(buffer->metadata().sequence);
> - data->ipa_->processEvent(ev);
> + data->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),
> + data->delayedCtrls_->get(buffer->metadata().sequence));
> }
>
> REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)
> --
> 2.31.1
>
More information about the libcamera-devel
mailing list