| Message ID | 20220308091520.34607-3-umang.jain@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. 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@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); I'd name this metadataReady, as it reports the request metadata computed by the IPA. The metadata is computed from the stats (among other things), but this event doesn't indicate that stats are ready. With this fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > }; > 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)
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@ideasonboard.com> With the change requested by Laurent, Reviewed-by: Paul Elder <paul.elder@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 >
On 3/15/22 14:24, Laurent Pinchart wrote: > Hi Umang, > > Thank you for the patch. > > 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@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); > I'd name this metadataReady, as it reports the request metadata computed > by the IPA. The metadata is computed from the stats (among other > things), but this event doesn't indicate that stats are ready. > > With this fixed, Address locally however, metadataReady() is a private member of IPARkISP1 class hence having the same signal name made the compliers unhappy: ../src/ipa/rkisp1/rkisp1.cpp: In member function ‘void libcamera::ipa::rkisp1::IPARkISP1::metadataReady(unsigned int, unsigned int)’: ../src/ipa/rkisp1/rkisp1.cpp:289:9: error: invalid use of member function ‘void libcamera::ipa::rkisp1::IPARkISP1::metadataReady(unsigned int, unsigned int)’ (did you forget the ‘()’ ?) 289 | metadataReady.emit(frame, ctrls); | ^~~~~~~~~~~~~ ../src/ipa/rkisp1/rkisp1.cpp:282:44: error: unused parameter ‘frame’ [-Werror=unused-parameter] 282 | void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState) | ~~~~~~~~~~~~~^~~~~ cc1plus: all warnings being treated as errors [173/334] Generating doxygen with a custom command Hence I am forced to rename IPARkISP1::metadataReady() private function to IPARkISP1::prepareMetadata() . I'll attach a final patch in few minutes. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> }; >> 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)
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)
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@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(-)