| Message ID | 20200915142038.28757-19-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thanks for your work. On 2020-09-15 23:20:33 +0900, Paul Elder wrote: > Now that we can generate custom functions and data structures with mojo, > switch the raspberrypi pipeline handler and IPA to use the custom data > structures as defined in the mojom data definition file. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/ipa/raspberrypi.h | 37 +++--- > src/ipa/raspberrypi/raspberrypi.cpp | 108 +++++++++------- > .../pipeline/raspberrypi/raspberrypi.cpp | 120 +++++++++++------- > 3 files changed, 155 insertions(+), 110 deletions(-) > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h > index ed2b12d5..e58d1cbc 100644 > --- a/include/libcamera/ipa/raspberrypi.h > +++ b/include/libcamera/ipa/raspberrypi.h > @@ -23,22 +23,27 @@ enum RPiIpaMask { > namespace libcamera { > > /* List of controls handled by the Raspberry Pi IPA */ > -static const ControlInfoMap RPiControls = { > - { &controls::AeEnable, ControlInfo(false, true) }, > - { &controls::ExposureTime, ControlInfo(0, 999999) }, > - { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) }, > - { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) }, > - { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) }, > - { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) }, > - { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > - { &controls::AwbEnable, ControlInfo(false, true) }, > - { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > - { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) }, > - { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > - { &controls::Contrast, ControlInfo(0.0f, 32.0f) }, > - { &controls::Saturation, ControlInfo(0.0f, 32.0f) }, > - { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) }, > - { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) }, > +static ControlInfoMap RPiControls; > + > +inline void initializeRPiControls() > +{ > + RPiControls = { > + { &controls::AeEnable, ControlInfo(false, true) }, > + { &controls::ExposureTime, ControlInfo(0, 999999) }, > + { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) }, > + { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) }, > + { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) }, > + { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) }, > + { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > + { &controls::AwbEnable, ControlInfo(false, true) }, > + { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > + { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) }, > + { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > + { &controls::Contrast, ControlInfo(0.0f, 32.0f) }, > + { &controls::Saturation, ControlInfo(0.0f, 32.0f) }, > + { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) }, > + { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) }, > + }; Why do you nee to move this to a function? > }; > > } /* namespace libcamera */ > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > index 4557016c..e09caa8d 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -19,6 +19,7 @@ > #include <libcamera/ipa/ipa_interface.h> > #include <libcamera/ipa/ipa_module_info.h> > #include <libcamera/ipa/raspberrypi.h> > +#include <libcamera/ipa/raspberrypi_generated.h> > #include <libcamera/request.h> > #include <libcamera/span.h> > > @@ -60,7 +61,7 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(IPARPI) > > -class IPARPi : public IPAInterface > +class IPARPi : public IPARPiInterface > { > public: > IPARPi() > @@ -68,6 +69,7 @@ public: > frame_count_(0), check_count_(0), hide_count_(0), > mistrust_count_(0), lsTable_(nullptr) > { > + initializeRPiControls(); > } > > ~IPARPi() > @@ -82,12 +84,12 @@ public: > > void configure(const CameraSensorInfo &sensorInfo, > const std::map<unsigned int, IPAStream> &streamConfig, > - const std::map<unsigned int, const ControlInfoMap &> &entityControls, > - const IPAOperationData &data, > - IPAOperationData *response) override; > + const std::map<unsigned int, ControlInfoMap> &entityControls, Is it by design or a effect/limitation of mojo we can't keep const & ? > + const RPiConfigureParams &data, > + RPiConfigureParams *response) override; > void mapBuffers(const std::vector<IPABuffer> &buffers) override; > void unmapBuffers(const std::vector<unsigned int> &ids) override; > - void processEvent(const IPAOperationData &event) override; > + void processEvent(const RPiEventParams &event) override; > > private: > void setMode(const CameraSensorInfo &sensorInfo); > @@ -143,6 +145,11 @@ private: > unsigned int mistrust_count_; > /* LS table allocation passed in from the pipeline handler. */ > FileDescriptor lsTableHandle_; > + /* > + * LS table allocation passed in from the pipeline handler, > + * in the context of the pipeline handler. > + */ > + int32_t lsTableHandlePH_; > void *lsTable_; > }; > > @@ -192,15 +199,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo) > > void IPARPi::configure(const CameraSensorInfo &sensorInfo, > [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig, > - const std::map<unsigned int, const ControlInfoMap &> &entityControls, > - const IPAOperationData &ipaConfig, > - IPAOperationData *result) > + const std::map<unsigned int, ControlInfoMap> &entityControls, > + const RPiConfigureParams &ipaConfig, > + RPiConfigureParams *result) > { > if (entityControls.empty()) > return; > > - result->operation = 0; > - > unicam_ctrls_ = entityControls.at(0); > isp_ctrls_ = entityControls.at(1); > /* Setup a metadata ControlList to output metadata. */ > @@ -222,11 +227,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, > helper_->GetDelays(exposureDelay, gainDelay); > sensorMetadata = helper_->SensorEmbeddedDataPresent(); > > - result->data.push_back(gainDelay); > - result->data.push_back(exposureDelay); > - result->data.push_back(sensorMetadata); > + RPiConfigurePayload payload = {}; > + payload.op_ = RPI_IPA_CONFIG_STAGGERED_WRITE; > + payload.staggeredWriteResult_.gainDelay_ = gainDelay; > + payload.staggeredWriteResult_.exposureDelay_ = exposureDelay; > + payload.staggeredWriteResult_.sensorMetadata_ = sensorMetadata; So much nicer, I really like this! > > - result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE; > + result->payload_.push_back(payload); > } > > /* Re-assemble camera mode using the sensor info. */ > @@ -274,15 +281,23 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, > if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) { > ControlList ctrls(unicam_ctrls_); > applyAGC(&agcStatus, ctrls); > - result->controls.push_back(ctrls); > > - result->operation |= RPI_IPA_CONFIG_SENSOR; > + RPiConfigurePayload payload = {}; > + payload.op_ = RPI_IPA_CONFIG_SENSOR; > + payload.controls_ = ctrls; > + > + result->payload_.push_back(payload); > } > > lastMode_ = mode_; > > /* Store the lens shading table pointer and handle if available. */ > - if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) { > + auto lens = std::find_if(ipaConfig.payload_.begin(), > + ipaConfig.payload_.end(), > + [] (const RPiConfigurePayload &p) { > + return p.op_ == RPI_IPA_CONFIG_LS_TABLE; > + }); I think this warrents a helper, is it possible to have a base class for RPiConfigureParams which could be shared between pipelines to have such helpers or do we think that is to restricitve and should do so for each IPA protocol implementation? > + if (lens != ipaConfig.payload_.end()) { > /* Remove any previous table, if there was one. */ > if (lsTable_) { > munmap(lsTable_, MAX_LS_GRID_SIZE); > @@ -290,7 +305,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, > } > > /* Map the LS table buffer into user space. */ > - lsTableHandle_ = FileDescriptor(ipaConfig.data[0]); > + lsTableHandle_ = FileDescriptor(lens->lsTableHandle_); > + lsTableHandlePH_ = lens->lsTableHandleStatic_; > if (lsTableHandle_.isValid()) { > lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE, > MAP_SHARED, lsTableHandle_.fd(), 0); > @@ -335,11 +351,11 @@ void IPARPi::unmapBuffers(const std::vector<unsigned int> &ids) > } > } > > -void IPARPi::processEvent(const IPAOperationData &event) > +void IPARPi::processEvent(const RPiEventParams &event) > { > - switch (event.operation) { > + switch (event.ev_) { > case RPI_IPA_EVENT_SIGNAL_STAT_READY: { > - unsigned int bufferId = event.data[0]; > + unsigned int bufferId = event.bufferId_; > > if (++check_count_ != frame_count_) /* assert here? */ > LOG(IPARPI, Error) << "WARNING: Prepare/Process mismatch!!!"; > @@ -348,17 +364,17 @@ void IPARPi::processEvent(const IPAOperationData &event) > > reportMetadata(); > > - IPAOperationData op; > - op.operation = RPI_IPA_ACTION_STATS_METADATA_COMPLETE; > - op.data = { bufferId & RPiIpaMask::ID }; > - op.controls = { libcameraMetadata_ }; > + RPiActionParams op; > + op.op_ = RPI_IPA_ACTION_STATS_METADATA_COMPLETE; > + op.statsComplete_.bufferId_ = { bufferId & RPiIpaMask::ID }; > + op.statsComplete_.controls_ = { libcameraMetadata_ }; > queueFrameAction.emit(0, op); > break; > } > > case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE: { > - unsigned int embeddedbufferId = event.data[0]; > - unsigned int bayerbufferId = event.data[1]; > + unsigned int embeddedbufferId = event.ispPrepare_.embeddedbufferId_; > + unsigned int bayerbufferId = event.ispPrepare_.bayerbufferId_; > > /* > * At start-up, or after a mode-switch, we may want to > @@ -368,23 +384,23 @@ void IPARPi::processEvent(const IPAOperationData &event) > prepareISP(embeddedbufferId); > > /* Ready to push the input buffer into the ISP. */ > - IPAOperationData op; > + RPiActionParams op; > if (++frame_count_ > hide_count_) > - op.operation = RPI_IPA_ACTION_RUN_ISP; > + op.op_ = RPI_IPA_ACTION_RUN_ISP; > else > - op.operation = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME; > - op.data = { bayerbufferId & RPiIpaMask::ID }; > + op.op_ = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME; > + op.bufferId_ = { bayerbufferId & RPiIpaMask::ID }; > queueFrameAction.emit(0, op); > break; > } > > case RPI_IPA_EVENT_QUEUE_REQUEST: { > - queueRequest(event.controls[0]); > + queueRequest(event.controls_); > break; > } > > default: > - LOG(IPARPI, Error) << "Unknown event " << event.operation; > + LOG(IPARPI, Error) << "Unknown event " << event.ev_; > break; > } > } > @@ -489,6 +505,8 @@ void IPARPi::queueRequest(const ControlList &controls) > /* Clear the return metadata buffer. */ > libcameraMetadata_.clear(); > > + LOG(IPARPI, Info) << "Request ctrl length: " << controls.size(); > + > for (auto const &ctrl : controls) { > LOG(IPARPI, Info) << "Request ctrl: " > << controls::controls.at(ctrl.first)->name() > @@ -698,9 +716,9 @@ void IPARPi::queueRequest(const ControlList &controls) > > void IPARPi::returnEmbeddedBuffer(unsigned int bufferId) > { > - IPAOperationData op; > - op.operation = RPI_IPA_ACTION_EMBEDDED_COMPLETE; > - op.data = { bufferId & RPiIpaMask::ID }; > + RPiActionParams op; > + op.op_ = RPI_IPA_ACTION_EMBEDDED_COMPLETE; > + op.bufferId_ = { bufferId & RPiIpaMask::ID }; > queueFrameAction.emit(0, op); > } > > @@ -763,9 +781,9 @@ void IPARPi::prepareISP(unsigned int bufferId) > applyDPC(dpcStatus, ctrls); > > if (!ctrls.empty()) { > - IPAOperationData op; > - op.operation = RPI_IPA_ACTION_V4L2_SET_ISP; > - op.controls.push_back(ctrls); > + RPiActionParams op; > + op.op_ = RPI_IPA_ACTION_V4L2_SET_ISP; > + op.controls_ = ctrls; > queueFrameAction.emit(0, op); > } > } > @@ -823,9 +841,9 @@ void IPARPi::processStats(unsigned int bufferId) > ControlList ctrls(unicam_ctrls_); > applyAGC(&agcStatus, ctrls); > > - IPAOperationData op; > - op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED; > - op.controls.push_back(ctrls); > + RPiActionParams op; > + op.op_ = RPI_IPA_ACTION_V4L2_SET_STAGGERED; > + op.controls_ = ctrls; > queueFrameAction.emit(0, op); > } > } > @@ -1056,7 +1074,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls) > .grid_width = w, > .grid_stride = w, > .grid_height = h, > - .dmabuf = lsTableHandle_.fd(), > + .dmabuf = lsTableHandlePH_, > .ref_transform = 0, > .corner_sampled = 1, > .gain_format = GAIN_FORMAT_U4P10 > @@ -1136,9 +1154,9 @@ const struct IPAModuleInfo ipaModuleInfo = { > "raspberrypi", > }; > > -struct ipa_context *ipaCreate() > +IPAInterface *ipaCreate() > { > - return new IPAInterfaceWrapper(std::make_unique<IPARPi>()); > + return new IPARPi(); > } > > }; /* extern "C" */ > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index ce43af34..70bb6fcc 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -15,7 +15,9 @@ > #include <libcamera/control_ids.h> > #include <libcamera/file_descriptor.h> > #include <libcamera/formats.h> > +#include <libcamera/ipa/ipa_proxy_raspberrypi.h> > #include <libcamera/ipa/raspberrypi.h> > +#include <libcamera/ipa/raspberrypi_generated.h> > #include <libcamera/logging.h> > #include <libcamera/property_ids.h> > #include <libcamera/request.h> > @@ -296,7 +298,7 @@ public: > int loadIPA(); > int configureIPA(); > > - void queueFrameAction(unsigned int frame, const IPAOperationData &action); > + void queueFrameAction(unsigned int frame, const RPiActionParams &action); > > /* bufferComplete signal handlers. */ > void unicamBufferDequeue(FrameBuffer *buffer); > @@ -307,6 +309,8 @@ public: > void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream); > void handleState(); > > + std::unique_ptr<IPAProxyRPi> ipa_; > + > CameraSensor *sensor_; > /* Array of Unicam and ISP device streams and associated buffers/streams. */ > RPiDevice<Unicam, 2> unicam_; > @@ -506,6 +510,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() > PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager) > : PipelineHandler(manager), unicam_(nullptr), isp_(nullptr) > { > + initializeRPiControls(); > } > > CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, > @@ -1103,7 +1108,9 @@ void RPiCameraData::frameStarted(uint32_t sequence) > > int RPiCameraData::loadIPA() > { > - ipa_ = IPAManager::createIPA(pipe_, 1, 1); > + std::unique_ptr<IPAProxy> ptr = IPAManager::createIPA(pipe_, 1, 1); > + ipa_ = std::unique_ptr<IPAProxyRPi>{static_cast<IPAProxyRPi*>(std::move(ptr).release())}; > + > if (!ipa_) > return -ENOENT; > > @@ -1119,8 +1126,8 @@ int RPiCameraData::loadIPA() > int RPiCameraData::configureIPA() > { > std::map<unsigned int, IPAStream> streamConfig; > - std::map<unsigned int, const ControlInfoMap &> entityControls; > - IPAOperationData ipaConfig = {}; > + std::map<unsigned int, ControlInfoMap> entityControls; > + RPiConfigureParams ipaConfig; > > /* Get the device format to pass to the IPA. */ > V4L2DeviceFormat sensorFormat; > @@ -1145,8 +1152,11 @@ int RPiCameraData::configureIPA() > return -ENOMEM; > > /* Allow the IPA to mmap the LS table via the file descriptor. */ > - ipaConfig.operation = RPI_IPA_CONFIG_LS_TABLE; > - ipaConfig.data = { static_cast<unsigned int>(lsTable_.fd()) }; > + RPiConfigurePayload payload; > + payload.op_ = RPI_IPA_CONFIG_LS_TABLE; > + payload.lsTableHandle_ = lsTable_; > + payload.lsTableHandleStatic_ = lsTable_.fd(); > + ipaConfig.payload_.push_back(payload); > } > > CameraSensorInfo sensorInfo = {}; > @@ -1157,53 +1167,68 @@ int RPiCameraData::configureIPA() > } > > /* Ready the IPA - it must know about the sensor resolution. */ > - IPAOperationData result; > + RPiConfigureParams results; > > ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, > - &result); > + &results); > > - if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) { > - /* > - * Setup our staggered control writer with the sensor default > - * gain and exposure delays. > - */ > - if (!staggeredCtrl_) { > - staggeredCtrl_.init(unicam_[Unicam::Image].dev(), > - { { V4L2_CID_ANALOGUE_GAIN, result.data[0] }, > - { V4L2_CID_EXPOSURE, result.data[1] } }); > - sensorMetadata_ = result.data[2]; > + for (RPiConfigurePayload &result : results.payload_) { > + if (result.op_ == RPI_IPA_CONFIG_STAGGERED_WRITE) { > + > + /* > + * Setup our staggered control writer with the sensor default > + * gain and exposure delays. > + */ > + if (!staggeredCtrl_) { > + staggeredCtrl_.init(unicam_[Unicam::Image].dev(), > + { { V4L2_CID_ANALOGUE_GAIN, > + result.staggeredWriteResult_.gainDelay_ }, > + { V4L2_CID_EXPOSURE, > + result.staggeredWriteResult_.exposureDelay_ } }); > + sensorMetadata_ = result.staggeredWriteResult_.sensorMetadata_; > + } > + > + /* Configure the H/V flip controls based on the sensor rotation. */ > + ControlList ctrls(unicam_[Unicam::Image].dev()->controls()); > + int32_t rotation = sensor_->properties().get(properties::Rotation); > + ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation)); > + ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation)); > + unicam_[Unicam::Image].dev()->setControls(&ctrls); > } > - } > > - if (result.operation & RPI_IPA_CONFIG_SENSOR) { > - const ControlList &ctrls = result.controls[0]; > - if (!staggeredCtrl_.set(ctrls)) > - LOG(RPI, Error) << "V4L2 staggered set failed"; > + if (result.op_ == RPI_IPA_CONFIG_SENSOR) { > + const ControlList &ctrls = result.controls_; > + if (!staggeredCtrl_.set(ctrls)) > + LOG(RPI, Error) << "V4L2 staggered set failed"; > + } > } > > return 0; > } > > void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, > - const IPAOperationData &action) > + const RPiActionParams &action) > { > /* > * The following actions can be handled when the pipeline handler is in > * a stopped state. > */ > - switch (action.operation) { > + switch (action.op_) { > case RPI_IPA_ACTION_V4L2_SET_STAGGERED: { > - const ControlList &controls = action.controls[0]; > + const ControlList &controls = action.controls_; > if (!staggeredCtrl_.set(controls)) > LOG(RPI, Error) << "V4L2 staggered set failed"; > goto done; > } > > case RPI_IPA_ACTION_V4L2_SET_ISP: { > - ControlList controls = action.controls[0]; > + ControlList controls = action.controls_; > isp_[Isp::Input].dev()->setControls(&controls); > goto done; > } > + > + default: > + break; > } > > if (state_ == State::Stopped) > @@ -1213,20 +1238,20 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, > * The following actions must not be handled when the pipeline handler > * is in a stopped state. > */ > - switch (action.operation) { > + switch (action.op_) { > case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: { > - unsigned int bufferId = action.data[0]; > + unsigned int bufferId = action.statsComplete_.bufferId_; > FrameBuffer *buffer = isp_[Isp::Stats].getBuffers()->at(bufferId).get(); > > handleStreamBuffer(buffer, &isp_[Isp::Stats]); > /* Fill the Request metadata buffer with what the IPA has provided */ > - requestQueue_.front()->metadata() = std::move(action.controls[0]); > + requestQueue_.front()->metadata() = std::move(action.statsComplete_.controls_); > state_ = State::IpaComplete; > break; > } > > case RPI_IPA_ACTION_EMBEDDED_COMPLETE: { > - unsigned int bufferId = action.data[0]; > + unsigned int bufferId = action.bufferId_; > FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers()->at(bufferId).get(); > handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]); > break; > @@ -1234,20 +1259,17 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, > > case RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME: > case RPI_IPA_ACTION_RUN_ISP: { > - unsigned int bufferId = action.data[0]; > + unsigned int bufferId = action.bufferId_; > FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers()->at(bufferId).get(); > > - LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << buffer->cookie() > - << ", timestamp: " << buffer->metadata().timestamp; > - > isp_[Isp::Input].dev()->queueBuffer(buffer); > - dropFrame_ = (action.operation == RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME) ? true : false; > + dropFrame_ = (action.op_ == RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME) ? true : false; > ispOutputCount_ = 0; > break; > } > > default: > - LOG(RPI, Error) << "Unknown action " << action.operation; > + LOG(RPI, Error) << "Unknown action " << action.op_; > break; > } > > @@ -1347,10 +1369,10 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer) > > /* If this is a stats output, hand it to the IPA now. */ > if (stream == &isp_[Isp::Stats]) { > - IPAOperationData op; > - op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY; > - op.data = { RPiIpaMask::STATS | buffer->cookie() }; > - ipa_->processEvent(op); > + RPiEventParams ev; > + ev.ev_ = RPI_IPA_EVENT_SIGNAL_STAT_READY; > + ev.bufferId_ = { RPiIpaMask::STATS | buffer->cookie() }; > + ipa_->processEvent(ev); > } > > handleState(); > @@ -1493,7 +1515,7 @@ void RPiCameraData::checkRequestCompleted() > void RPiCameraData::tryRunPipeline() > { > FrameBuffer *bayerBuffer, *embeddedBuffer; > - IPAOperationData op; > + RPiEventParams ev; > > /* If any of our request or buffer queues are empty, we cannot proceed. */ > if (state_ != State::Idle || requestQueue_.empty() || > @@ -1548,9 +1570,9 @@ void RPiCameraData::tryRunPipeline() > * queue the ISP output buffer listed in the request to start the HW > * pipeline. > */ > - op.operation = RPI_IPA_EVENT_QUEUE_REQUEST; > - op.controls = { request->controls() }; > - ipa_->processEvent(op); > + ev.ev_ = RPI_IPA_EVENT_QUEUE_REQUEST; > + ev.controls_ = { request->controls() }; > + ipa_->processEvent(ev); > > /* Queue up any ISP buffers passed into the request. */ > for (auto &stream : isp_) { > @@ -1569,10 +1591,10 @@ void RPiCameraData::tryRunPipeline() > << " Bayer buffer id: " << bayerBuffer->cookie() > << " Embedded buffer id: " << embeddedBuffer->cookie(); > > - op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE; > - op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(), > - RPiIpaMask::BAYER_DATA | bayerBuffer->cookie() }; > - ipa_->processEvent(op); > + ev.ev_ = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE; > + ev.ispPrepare_.embeddedbufferId_ = RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(); > + ev.ispPrepare_.bayerbufferId_ = RPiIpaMask::BAYER_DATA | bayerBuffer->cookie(); > + ipa_->processEvent(ev); > } > > void RPiCameraData::tryFlushQueues() > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, Thank you for the reivew. On Sat, Sep 19, 2020 at 02:42:34PM +0200, Niklas Söderlund wrote: > Hi Paul, > > Thanks for your work. > > On 2020-09-15 23:20:33 +0900, Paul Elder wrote: > > Now that we can generate custom functions and data structures with mojo, > > switch the raspberrypi pipeline handler and IPA to use the custom data > > structures as defined in the mojom data definition file. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > include/libcamera/ipa/raspberrypi.h | 37 +++--- > > src/ipa/raspberrypi/raspberrypi.cpp | 108 +++++++++------- > > .../pipeline/raspberrypi/raspberrypi.cpp | 120 +++++++++++------- > > 3 files changed, 155 insertions(+), 110 deletions(-) > > > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h > > index ed2b12d5..e58d1cbc 100644 > > --- a/include/libcamera/ipa/raspberrypi.h > > +++ b/include/libcamera/ipa/raspberrypi.h > > @@ -23,22 +23,27 @@ enum RPiIpaMask { > > namespace libcamera { > > > > /* List of controls handled by the Raspberry Pi IPA */ > > -static const ControlInfoMap RPiControls = { > > - { &controls::AeEnable, ControlInfo(false, true) }, > > - { &controls::ExposureTime, ControlInfo(0, 999999) }, > > - { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) }, > > - { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) }, > > - { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) }, > > - { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) }, > > - { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > > - { &controls::AwbEnable, ControlInfo(false, true) }, > > - { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > - { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) }, > > - { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > > - { &controls::Contrast, ControlInfo(0.0f, 32.0f) }, > > - { &controls::Saturation, ControlInfo(0.0f, 32.0f) }, > > - { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) }, > > - { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) }, > > +static ControlInfoMap RPiControls; > > + > > +inline void initializeRPiControls() > > +{ > > + RPiControls = { > > + { &controls::AeEnable, ControlInfo(false, true) }, > > + { &controls::ExposureTime, ControlInfo(0, 999999) }, > > + { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) }, > > + { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) }, > > + { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) }, > > + { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) }, > > + { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > > + { &controls::AwbEnable, ControlInfo(false, true) }, > > + { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > + { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) }, > > + { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > > + { &controls::Contrast, ControlInfo(0.0f, 32.0f) }, > > + { &controls::Saturation, ControlInfo(0.0f, 32.0f) }, > > + { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) }, > > + { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) }, > > + }; > > Why do you nee to move this to a function? Otherwise RPiControls gets constructed too early, and if an error happens in ControlInfoMap::generateIdmap() and it tries to LOG, it'll cause a segfault. So it's either construct RPiControls later, or remove the error LOG (or add protection to LOG?). > > }; > > > > } /* namespace libcamera */ > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > > index 4557016c..e09caa8d 100644 > > --- a/src/ipa/raspberrypi/raspberrypi.cpp > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > > @@ -19,6 +19,7 @@ > > #include <libcamera/ipa/ipa_interface.h> > > #include <libcamera/ipa/ipa_module_info.h> > > #include <libcamera/ipa/raspberrypi.h> > > +#include <libcamera/ipa/raspberrypi_generated.h> > > #include <libcamera/request.h> > > #include <libcamera/span.h> > > > > @@ -60,7 +61,7 @@ namespace libcamera { > > > > LOG_DEFINE_CATEGORY(IPARPI) > > > > -class IPARPi : public IPAInterface > > +class IPARPi : public IPARPiInterface > > { > > public: > > IPARPi() > > @@ -68,6 +69,7 @@ public: > > frame_count_(0), check_count_(0), hide_count_(0), > > mistrust_count_(0), lsTable_(nullptr) > > { > > + initializeRPiControls(); > > } > > > > ~IPARPi() > > @@ -82,12 +84,12 @@ public: > > > > void configure(const CameraSensorInfo &sensorInfo, > > const std::map<unsigned int, IPAStream> &streamConfig, > > - const std::map<unsigned int, const ControlInfoMap &> &entityControls, > > - const IPAOperationData &data, > > - IPAOperationData *response) override; > > + const std::map<unsigned int, ControlInfoMap> &entityControls, > > Is it by design or a effect/limitation of mojo we can't keep const & ? Yes, mojo (the language) doesn't support const nor & in array definitions. > > + const RPiConfigureParams &data, > > + RPiConfigureParams *response) override; > > void mapBuffers(const std::vector<IPABuffer> &buffers) override; > > void unmapBuffers(const std::vector<unsigned int> &ids) override; > > - void processEvent(const IPAOperationData &event) override; > > + void processEvent(const RPiEventParams &event) override; > > > > private: > > void setMode(const CameraSensorInfo &sensorInfo); > > @@ -143,6 +145,11 @@ private: > > unsigned int mistrust_count_; > > /* LS table allocation passed in from the pipeline handler. */ > > FileDescriptor lsTableHandle_; > > + /* > > + * LS table allocation passed in from the pipeline handler, > > + * in the context of the pipeline handler. > > + */ > > + int32_t lsTableHandlePH_; > > void *lsTable_; > > }; > > > > @@ -192,15 +199,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo) > > > > void IPARPi::configure(const CameraSensorInfo &sensorInfo, > > [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig, > > - const std::map<unsigned int, const ControlInfoMap &> &entityControls, > > - const IPAOperationData &ipaConfig, > > - IPAOperationData *result) > > + const std::map<unsigned int, ControlInfoMap> &entityControls, > > + const RPiConfigureParams &ipaConfig, > > + RPiConfigureParams *result) > > { > > if (entityControls.empty()) > > return; > > > > - result->operation = 0; > > - > > unicam_ctrls_ = entityControls.at(0); > > isp_ctrls_ = entityControls.at(1); > > /* Setup a metadata ControlList to output metadata. */ > > @@ -222,11 +227,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, > > helper_->GetDelays(exposureDelay, gainDelay); > > sensorMetadata = helper_->SensorEmbeddedDataPresent(); > > > > - result->data.push_back(gainDelay); > > - result->data.push_back(exposureDelay); > > - result->data.push_back(sensorMetadata); > > + RPiConfigurePayload payload = {}; > > + payload.op_ = RPI_IPA_CONFIG_STAGGERED_WRITE; > > + payload.staggeredWriteResult_.gainDelay_ = gainDelay; > > + payload.staggeredWriteResult_.exposureDelay_ = exposureDelay; > > + payload.staggeredWriteResult_.sensorMetadata_ = sensorMetadata; > > So much nicer, I really like this! \o/ > > > > - result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE; > > + result->payload_.push_back(payload); > > } > > > > /* Re-assemble camera mode using the sensor info. */ > > @@ -274,15 +281,23 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, > > if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) { > > ControlList ctrls(unicam_ctrls_); > > applyAGC(&agcStatus, ctrls); > > - result->controls.push_back(ctrls); > > > > - result->operation |= RPI_IPA_CONFIG_SENSOR; > > + RPiConfigurePayload payload = {}; > > + payload.op_ = RPI_IPA_CONFIG_SENSOR; > > + payload.controls_ = ctrls; > > + > > + result->payload_.push_back(payload); > > } > > > > lastMode_ = mode_; > > > > /* Store the lens shading table pointer and handle if available. */ > > - if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) { > > + auto lens = std::find_if(ipaConfig.payload_.begin(), > > + ipaConfig.payload_.end(), > > + [] (const RPiConfigurePayload &p) { > > + return p.op_ == RPI_IPA_CONFIG_LS_TABLE; > > + }); > > I think this warrents a helper, is it possible to have a base class for > RPiConfigureParams which could be shared between pipelines to have such > helpers or do we think that is to restricitve and should do so for each > IPA protocol implementation? I don't think that's the right solution. I think the proper way to make this nicer is with a better design of the RPi IPA interface. CONFIG_LS_TABLE is only used pipeline -> IPA and CONFIG_STAGGERED_WRITE and CONFIG_SENSOR are IPA -> pipeline. Perhaps a separate configure() input and output parameter type? Like: struct RPiConfigInput { bool lsTableIsValid = false; FileDescriptor lsTableHandle; int32 lsTableHandleStatic = -1; }; struct RPiConfigOutput { bool staggeredWriteIsValid = false; RPiStaggeredWritePayload staggeredWriteResult; bool configSensor = false; ControlList controls; }; I discussed with Laurent about using nullable fields in structs (like mojo supports) but that had more demerits than embedding did imo so flags it is. Then this and the next hunk would become: @@ -274,15 +281,23 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) { ControlList ctrls(unicam_ctrls_); applyAGC(&agcStatus, ctrls); - result->controls.push_back(ctrls); - result->operation |= RPI_IPA_CONFIG_SENSOR; + result->configSensor_ = true; + result->controls_ = ctrls; } lastMode_ = mode_; /* Store the lens shading table pointer and handle if available. */ - if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) { + if (ipaConfig.lsTableIsValid_) { /* Remove any previous table, if there was one. */ if (lsTable_) { munmap(lsTable_, MAX_LS_GRID_SIZE); @@ -290,7 +305,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, } /* Map the LS table buffer into user space. */ - lsTableHandle_ = FileDescriptor(ipaConfig.data[0]); + lsTableHandle_ = FileDescriptor(ipaConfig.lsTableHandle_); + lsTableHandlePH_ = ipaConfig.lsTableHandleStatic_; if (lsTableHandle_.isValid()) { lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, lsTableHandle_.fd(), 0); Or something along those lines. I had just directly copied the old IPA interface with IPAOperationData, so even I haven't yet used the custom IPA interface to its fullest potential. Paul > > + if (lens != ipaConfig.payload_.end()) { > > /* Remove any previous table, if there was one. */ > > if (lsTable_) { > > munmap(lsTable_, MAX_LS_GRID_SIZE); > > @@ -290,7 +305,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, > > } > > > > /* Map the LS table buffer into user space. */ > > - lsTableHandle_ = FileDescriptor(ipaConfig.data[0]); > > + lsTableHandle_ = FileDescriptor(lens->lsTableHandle_); > > + lsTableHandlePH_ = lens->lsTableHandleStatic_; > > if (lsTableHandle_.isValid()) { > > lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE, > > MAP_SHARED, lsTableHandle_.fd(), 0); > > @@ -335,11 +351,11 @@ void IPARPi::unmapBuffers(const std::vector<unsigned int> &ids) > > } > > } > > > > -void IPARPi::processEvent(const IPAOperationData &event) > > +void IPARPi::processEvent(const RPiEventParams &event) > > { > > - switch (event.operation) { > > + switch (event.ev_) { > > case RPI_IPA_EVENT_SIGNAL_STAT_READY: { > > - unsigned int bufferId = event.data[0]; > > + unsigned int bufferId = event.bufferId_; > > > > if (++check_count_ != frame_count_) /* assert here? */ > > LOG(IPARPI, Error) << "WARNING: Prepare/Process mismatch!!!"; > > @@ -348,17 +364,17 @@ void IPARPi::processEvent(const IPAOperationData &event) > > > > reportMetadata(); > > > > - IPAOperationData op; > > - op.operation = RPI_IPA_ACTION_STATS_METADATA_COMPLETE; > > - op.data = { bufferId & RPiIpaMask::ID }; > > - op.controls = { libcameraMetadata_ }; > > + RPiActionParams op; > > + op.op_ = RPI_IPA_ACTION_STATS_METADATA_COMPLETE; > > + op.statsComplete_.bufferId_ = { bufferId & RPiIpaMask::ID }; > > + op.statsComplete_.controls_ = { libcameraMetadata_ }; > > queueFrameAction.emit(0, op); > > break; > > } > > > > case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE: { > > - unsigned int embeddedbufferId = event.data[0]; > > - unsigned int bayerbufferId = event.data[1]; > > + unsigned int embeddedbufferId = event.ispPrepare_.embeddedbufferId_; > > + unsigned int bayerbufferId = event.ispPrepare_.bayerbufferId_; > > > > /* > > * At start-up, or after a mode-switch, we may want to > > @@ -368,23 +384,23 @@ void IPARPi::processEvent(const IPAOperationData &event) > > prepareISP(embeddedbufferId); > > > > /* Ready to push the input buffer into the ISP. */ > > - IPAOperationData op; > > + RPiActionParams op; > > if (++frame_count_ > hide_count_) > > - op.operation = RPI_IPA_ACTION_RUN_ISP; > > + op.op_ = RPI_IPA_ACTION_RUN_ISP; > > else > > - op.operation = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME; > > - op.data = { bayerbufferId & RPiIpaMask::ID }; > > + op.op_ = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME; > > + op.bufferId_ = { bayerbufferId & RPiIpaMask::ID }; > > queueFrameAction.emit(0, op); > > break; > > } > > > > case RPI_IPA_EVENT_QUEUE_REQUEST: { > > - queueRequest(event.controls[0]); > > + queueRequest(event.controls_); > > break; > > } > > > > default: > > - LOG(IPARPI, Error) << "Unknown event " << event.operation; > > + LOG(IPARPI, Error) << "Unknown event " << event.ev_; > > break; > > } > > } > > @@ -489,6 +505,8 @@ void IPARPi::queueRequest(const ControlList &controls) > > /* Clear the return metadata buffer. */ > > libcameraMetadata_.clear(); > > > > + LOG(IPARPI, Info) << "Request ctrl length: " << controls.size(); > > + > > for (auto const &ctrl : controls) { > > LOG(IPARPI, Info) << "Request ctrl: " > > << controls::controls.at(ctrl.first)->name() > > @@ -698,9 +716,9 @@ void IPARPi::queueRequest(const ControlList &controls) > > > > void IPARPi::returnEmbeddedBuffer(unsigned int bufferId) > > { > > - IPAOperationData op; > > - op.operation = RPI_IPA_ACTION_EMBEDDED_COMPLETE; > > - op.data = { bufferId & RPiIpaMask::ID }; > > + RPiActionParams op; > > + op.op_ = RPI_IPA_ACTION_EMBEDDED_COMPLETE; > > + op.bufferId_ = { bufferId & RPiIpaMask::ID }; > > queueFrameAction.emit(0, op); > > } > > > > @@ -763,9 +781,9 @@ void IPARPi::prepareISP(unsigned int bufferId) > > applyDPC(dpcStatus, ctrls); > > > > if (!ctrls.empty()) { > > - IPAOperationData op; > > - op.operation = RPI_IPA_ACTION_V4L2_SET_ISP; > > - op.controls.push_back(ctrls); > > + RPiActionParams op; > > + op.op_ = RPI_IPA_ACTION_V4L2_SET_ISP; > > + op.controls_ = ctrls; > > queueFrameAction.emit(0, op); > > } > > } > > @@ -823,9 +841,9 @@ void IPARPi::processStats(unsigned int bufferId) > > ControlList ctrls(unicam_ctrls_); > > applyAGC(&agcStatus, ctrls); > > > > - IPAOperationData op; > > - op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED; > > - op.controls.push_back(ctrls); > > + RPiActionParams op; > > + op.op_ = RPI_IPA_ACTION_V4L2_SET_STAGGERED; > > + op.controls_ = ctrls; > > queueFrameAction.emit(0, op); > > } > > } > > @@ -1056,7 +1074,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls) > > .grid_width = w, > > .grid_stride = w, > > .grid_height = h, > > - .dmabuf = lsTableHandle_.fd(), > > + .dmabuf = lsTableHandlePH_, > > .ref_transform = 0, > > .corner_sampled = 1, > > .gain_format = GAIN_FORMAT_U4P10 > > @@ -1136,9 +1154,9 @@ const struct IPAModuleInfo ipaModuleInfo = { > > "raspberrypi", > > }; > > > > -struct ipa_context *ipaCreate() > > +IPAInterface *ipaCreate() > > { > > - return new IPAInterfaceWrapper(std::make_unique<IPARPi>()); > > + return new IPARPi(); > > } > > > > }; /* extern "C" */ > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index ce43af34..70bb6fcc 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -15,7 +15,9 @@ > > #include <libcamera/control_ids.h> > > #include <libcamera/file_descriptor.h> > > #include <libcamera/formats.h> > > +#include <libcamera/ipa/ipa_proxy_raspberrypi.h> > > #include <libcamera/ipa/raspberrypi.h> > > +#include <libcamera/ipa/raspberrypi_generated.h> > > #include <libcamera/logging.h> > > #include <libcamera/property_ids.h> > > #include <libcamera/request.h> > > @@ -296,7 +298,7 @@ public: > > int loadIPA(); > > int configureIPA(); > > > > - void queueFrameAction(unsigned int frame, const IPAOperationData &action); > > + void queueFrameAction(unsigned int frame, const RPiActionParams &action); > > > > /* bufferComplete signal handlers. */ > > void unicamBufferDequeue(FrameBuffer *buffer); > > @@ -307,6 +309,8 @@ public: > > void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream); > > void handleState(); > > > > + std::unique_ptr<IPAProxyRPi> ipa_; > > + > > CameraSensor *sensor_; > > /* Array of Unicam and ISP device streams and associated buffers/streams. */ > > RPiDevice<Unicam, 2> unicam_; > > @@ -506,6 +510,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() > > PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager) > > : PipelineHandler(manager), unicam_(nullptr), isp_(nullptr) > > { > > + initializeRPiControls(); > > } > > > > CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, > > @@ -1103,7 +1108,9 @@ void RPiCameraData::frameStarted(uint32_t sequence) > > > > int RPiCameraData::loadIPA() > > { > > - ipa_ = IPAManager::createIPA(pipe_, 1, 1); > > + std::unique_ptr<IPAProxy> ptr = IPAManager::createIPA(pipe_, 1, 1); > > + ipa_ = std::unique_ptr<IPAProxyRPi>{static_cast<IPAProxyRPi*>(std::move(ptr).release())}; > > + > > if (!ipa_) > > return -ENOENT; > > > > @@ -1119,8 +1126,8 @@ int RPiCameraData::loadIPA() > > int RPiCameraData::configureIPA() > > { > > std::map<unsigned int, IPAStream> streamConfig; > > - std::map<unsigned int, const ControlInfoMap &> entityControls; > > - IPAOperationData ipaConfig = {}; > > + std::map<unsigned int, ControlInfoMap> entityControls; > > + RPiConfigureParams ipaConfig; > > > > /* Get the device format to pass to the IPA. */ > > V4L2DeviceFormat sensorFormat; > > @@ -1145,8 +1152,11 @@ int RPiCameraData::configureIPA() > > return -ENOMEM; > > > > /* Allow the IPA to mmap the LS table via the file descriptor. */ > > - ipaConfig.operation = RPI_IPA_CONFIG_LS_TABLE; > > - ipaConfig.data = { static_cast<unsigned int>(lsTable_.fd()) }; > > + RPiConfigurePayload payload; > > + payload.op_ = RPI_IPA_CONFIG_LS_TABLE; > > + payload.lsTableHandle_ = lsTable_; > > + payload.lsTableHandleStatic_ = lsTable_.fd(); > > + ipaConfig.payload_.push_back(payload); > > } > > > > CameraSensorInfo sensorInfo = {}; > > @@ -1157,53 +1167,68 @@ int RPiCameraData::configureIPA() > > } > > > > /* Ready the IPA - it must know about the sensor resolution. */ > > - IPAOperationData result; > > + RPiConfigureParams results; > > > > ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, > > - &result); > > + &results); > > > > - if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) { > > - /* > > - * Setup our staggered control writer with the sensor default > > - * gain and exposure delays. > > - */ > > - if (!staggeredCtrl_) { > > - staggeredCtrl_.init(unicam_[Unicam::Image].dev(), > > - { { V4L2_CID_ANALOGUE_GAIN, result.data[0] }, > > - { V4L2_CID_EXPOSURE, result.data[1] } }); > > - sensorMetadata_ = result.data[2]; > > + for (RPiConfigurePayload &result : results.payload_) { > > + if (result.op_ == RPI_IPA_CONFIG_STAGGERED_WRITE) { > > + > > + /* > > + * Setup our staggered control writer with the sensor default > > + * gain and exposure delays. > > + */ > > + if (!staggeredCtrl_) { > > + staggeredCtrl_.init(unicam_[Unicam::Image].dev(), > > + { { V4L2_CID_ANALOGUE_GAIN, > > + result.staggeredWriteResult_.gainDelay_ }, > > + { V4L2_CID_EXPOSURE, > > + result.staggeredWriteResult_.exposureDelay_ } }); > > + sensorMetadata_ = result.staggeredWriteResult_.sensorMetadata_; > > + } > > + > > + /* Configure the H/V flip controls based on the sensor rotation. */ > > + ControlList ctrls(unicam_[Unicam::Image].dev()->controls()); > > + int32_t rotation = sensor_->properties().get(properties::Rotation); > > + ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation)); > > + ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation)); > > + unicam_[Unicam::Image].dev()->setControls(&ctrls); > > } > > - } > > > > - if (result.operation & RPI_IPA_CONFIG_SENSOR) { > > - const ControlList &ctrls = result.controls[0]; > > - if (!staggeredCtrl_.set(ctrls)) > > - LOG(RPI, Error) << "V4L2 staggered set failed"; > > + if (result.op_ == RPI_IPA_CONFIG_SENSOR) { > > + const ControlList &ctrls = result.controls_; > > + if (!staggeredCtrl_.set(ctrls)) > > + LOG(RPI, Error) << "V4L2 staggered set failed"; > > + } > > } > > > > return 0; > > } > > > > void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, > > - const IPAOperationData &action) > > + const RPiActionParams &action) > > { > > /* > > * The following actions can be handled when the pipeline handler is in > > * a stopped state. > > */ > > - switch (action.operation) { > > + switch (action.op_) { > > case RPI_IPA_ACTION_V4L2_SET_STAGGERED: { > > - const ControlList &controls = action.controls[0]; > > + const ControlList &controls = action.controls_; > > if (!staggeredCtrl_.set(controls)) > > LOG(RPI, Error) << "V4L2 staggered set failed"; > > goto done; > > } > > > > case RPI_IPA_ACTION_V4L2_SET_ISP: { > > - ControlList controls = action.controls[0]; > > + ControlList controls = action.controls_; > > isp_[Isp::Input].dev()->setControls(&controls); > > goto done; > > } > > + > > + default: > > + break; > > } > > > > if (state_ == State::Stopped) > > @@ -1213,20 +1238,20 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, > > * The following actions must not be handled when the pipeline handler > > * is in a stopped state. > > */ > > - switch (action.operation) { > > + switch (action.op_) { > > case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: { > > - unsigned int bufferId = action.data[0]; > > + unsigned int bufferId = action.statsComplete_.bufferId_; > > FrameBuffer *buffer = isp_[Isp::Stats].getBuffers()->at(bufferId).get(); > > > > handleStreamBuffer(buffer, &isp_[Isp::Stats]); > > /* Fill the Request metadata buffer with what the IPA has provided */ > > - requestQueue_.front()->metadata() = std::move(action.controls[0]); > > + requestQueue_.front()->metadata() = std::move(action.statsComplete_.controls_); > > state_ = State::IpaComplete; > > break; > > } > > > > case RPI_IPA_ACTION_EMBEDDED_COMPLETE: { > > - unsigned int bufferId = action.data[0]; > > + unsigned int bufferId = action.bufferId_; > > FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers()->at(bufferId).get(); > > handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]); > > break; > > @@ -1234,20 +1259,17 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, > > > > case RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME: > > case RPI_IPA_ACTION_RUN_ISP: { > > - unsigned int bufferId = action.data[0]; > > + unsigned int bufferId = action.bufferId_; > > FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers()->at(bufferId).get(); > > > > - LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << buffer->cookie() > > - << ", timestamp: " << buffer->metadata().timestamp; > > - > > isp_[Isp::Input].dev()->queueBuffer(buffer); > > - dropFrame_ = (action.operation == RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME) ? true : false; > > + dropFrame_ = (action.op_ == RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME) ? true : false; > > ispOutputCount_ = 0; > > break; > > } > > > > default: > > - LOG(RPI, Error) << "Unknown action " << action.operation; > > + LOG(RPI, Error) << "Unknown action " << action.op_; > > break; > > } > > > > @@ -1347,10 +1369,10 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer) > > > > /* If this is a stats output, hand it to the IPA now. */ > > if (stream == &isp_[Isp::Stats]) { > > - IPAOperationData op; > > - op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY; > > - op.data = { RPiIpaMask::STATS | buffer->cookie() }; > > - ipa_->processEvent(op); > > + RPiEventParams ev; > > + ev.ev_ = RPI_IPA_EVENT_SIGNAL_STAT_READY; > > + ev.bufferId_ = { RPiIpaMask::STATS | buffer->cookie() }; > > + ipa_->processEvent(ev); > > } > > > > handleState(); > > @@ -1493,7 +1515,7 @@ void RPiCameraData::checkRequestCompleted() > > void RPiCameraData::tryRunPipeline() > > { > > FrameBuffer *bayerBuffer, *embeddedBuffer; > > - IPAOperationData op; > > + RPiEventParams ev; > > > > /* If any of our request or buffer queues are empty, we cannot proceed. */ > > if (state_ != State::Idle || requestQueue_.empty() || > > @@ -1548,9 +1570,9 @@ void RPiCameraData::tryRunPipeline() > > * queue the ISP output buffer listed in the request to start the HW > > * pipeline. > > */ > > - op.operation = RPI_IPA_EVENT_QUEUE_REQUEST; > > - op.controls = { request->controls() }; > > - ipa_->processEvent(op); > > + ev.ev_ = RPI_IPA_EVENT_QUEUE_REQUEST; > > + ev.controls_ = { request->controls() }; > > + ipa_->processEvent(ev); > > > > /* Queue up any ISP buffers passed into the request. */ > > for (auto &stream : isp_) { > > @@ -1569,10 +1591,10 @@ void RPiCameraData::tryRunPipeline() > > << " Bayer buffer id: " << bayerBuffer->cookie() > > << " Embedded buffer id: " << embeddedBuffer->cookie(); > > > > - op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE; > > - op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(), > > - RPiIpaMask::BAYER_DATA | bayerBuffer->cookie() }; > > - ipa_->processEvent(op); > > + ev.ev_ = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE; > > + ev.ispPrepare_.embeddedbufferId_ = RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(); > > + ev.ispPrepare_.bayerbufferId_ = RPiIpaMask::BAYER_DATA | bayerBuffer->cookie(); > > + ipa_->processEvent(ev); > > } > > > > void RPiCameraData::tryFlushQueues() > > -- > > 2.27.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards, > Niklas Söderlund
diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h index ed2b12d5..e58d1cbc 100644 --- a/include/libcamera/ipa/raspberrypi.h +++ b/include/libcamera/ipa/raspberrypi.h @@ -23,22 +23,27 @@ enum RPiIpaMask { namespace libcamera { /* List of controls handled by the Raspberry Pi IPA */ -static const ControlInfoMap RPiControls = { - { &controls::AeEnable, ControlInfo(false, true) }, - { &controls::ExposureTime, ControlInfo(0, 999999) }, - { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) }, - { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) }, - { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) }, - { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) }, - { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, - { &controls::AwbEnable, ControlInfo(false, true) }, - { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, - { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) }, - { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, - { &controls::Contrast, ControlInfo(0.0f, 32.0f) }, - { &controls::Saturation, ControlInfo(0.0f, 32.0f) }, - { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) }, - { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) }, +static ControlInfoMap RPiControls; + +inline void initializeRPiControls() +{ + RPiControls = { + { &controls::AeEnable, ControlInfo(false, true) }, + { &controls::ExposureTime, ControlInfo(0, 999999) }, + { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) }, + { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) }, + { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) }, + { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) }, + { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, + { &controls::AwbEnable, ControlInfo(false, true) }, + { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, + { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) }, + { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, + { &controls::Contrast, ControlInfo(0.0f, 32.0f) }, + { &controls::Saturation, ControlInfo(0.0f, 32.0f) }, + { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) }, + { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) }, + }; }; } /* namespace libcamera */ diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 4557016c..e09caa8d 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -19,6 +19,7 @@ #include <libcamera/ipa/ipa_interface.h> #include <libcamera/ipa/ipa_module_info.h> #include <libcamera/ipa/raspberrypi.h> +#include <libcamera/ipa/raspberrypi_generated.h> #include <libcamera/request.h> #include <libcamera/span.h> @@ -60,7 +61,7 @@ namespace libcamera { LOG_DEFINE_CATEGORY(IPARPI) -class IPARPi : public IPAInterface +class IPARPi : public IPARPiInterface { public: IPARPi() @@ -68,6 +69,7 @@ public: frame_count_(0), check_count_(0), hide_count_(0), mistrust_count_(0), lsTable_(nullptr) { + initializeRPiControls(); } ~IPARPi() @@ -82,12 +84,12 @@ public: void configure(const CameraSensorInfo &sensorInfo, const std::map<unsigned int, IPAStream> &streamConfig, - const std::map<unsigned int, const ControlInfoMap &> &entityControls, - const IPAOperationData &data, - IPAOperationData *response) override; + const std::map<unsigned int, ControlInfoMap> &entityControls, + const RPiConfigureParams &data, + RPiConfigureParams *response) override; void mapBuffers(const std::vector<IPABuffer> &buffers) override; void unmapBuffers(const std::vector<unsigned int> &ids) override; - void processEvent(const IPAOperationData &event) override; + void processEvent(const RPiEventParams &event) override; private: void setMode(const CameraSensorInfo &sensorInfo); @@ -143,6 +145,11 @@ private: unsigned int mistrust_count_; /* LS table allocation passed in from the pipeline handler. */ FileDescriptor lsTableHandle_; + /* + * LS table allocation passed in from the pipeline handler, + * in the context of the pipeline handler. + */ + int32_t lsTableHandlePH_; void *lsTable_; }; @@ -192,15 +199,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo) void IPARPi::configure(const CameraSensorInfo &sensorInfo, [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig, - const std::map<unsigned int, const ControlInfoMap &> &entityControls, - const IPAOperationData &ipaConfig, - IPAOperationData *result) + const std::map<unsigned int, ControlInfoMap> &entityControls, + const RPiConfigureParams &ipaConfig, + RPiConfigureParams *result) { if (entityControls.empty()) return; - result->operation = 0; - unicam_ctrls_ = entityControls.at(0); isp_ctrls_ = entityControls.at(1); /* Setup a metadata ControlList to output metadata. */ @@ -222,11 +227,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, helper_->GetDelays(exposureDelay, gainDelay); sensorMetadata = helper_->SensorEmbeddedDataPresent(); - result->data.push_back(gainDelay); - result->data.push_back(exposureDelay); - result->data.push_back(sensorMetadata); + RPiConfigurePayload payload = {}; + payload.op_ = RPI_IPA_CONFIG_STAGGERED_WRITE; + payload.staggeredWriteResult_.gainDelay_ = gainDelay; + payload.staggeredWriteResult_.exposureDelay_ = exposureDelay; + payload.staggeredWriteResult_.sensorMetadata_ = sensorMetadata; - result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE; + result->payload_.push_back(payload); } /* Re-assemble camera mode using the sensor info. */ @@ -274,15 +281,23 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) { ControlList ctrls(unicam_ctrls_); applyAGC(&agcStatus, ctrls); - result->controls.push_back(ctrls); - result->operation |= RPI_IPA_CONFIG_SENSOR; + RPiConfigurePayload payload = {}; + payload.op_ = RPI_IPA_CONFIG_SENSOR; + payload.controls_ = ctrls; + + result->payload_.push_back(payload); } lastMode_ = mode_; /* Store the lens shading table pointer and handle if available. */ - if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) { + auto lens = std::find_if(ipaConfig.payload_.begin(), + ipaConfig.payload_.end(), + [] (const RPiConfigurePayload &p) { + return p.op_ == RPI_IPA_CONFIG_LS_TABLE; + }); + if (lens != ipaConfig.payload_.end()) { /* Remove any previous table, if there was one. */ if (lsTable_) { munmap(lsTable_, MAX_LS_GRID_SIZE); @@ -290,7 +305,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, } /* Map the LS table buffer into user space. */ - lsTableHandle_ = FileDescriptor(ipaConfig.data[0]); + lsTableHandle_ = FileDescriptor(lens->lsTableHandle_); + lsTableHandlePH_ = lens->lsTableHandleStatic_; if (lsTableHandle_.isValid()) { lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, lsTableHandle_.fd(), 0); @@ -335,11 +351,11 @@ void IPARPi::unmapBuffers(const std::vector<unsigned int> &ids) } } -void IPARPi::processEvent(const IPAOperationData &event) +void IPARPi::processEvent(const RPiEventParams &event) { - switch (event.operation) { + switch (event.ev_) { case RPI_IPA_EVENT_SIGNAL_STAT_READY: { - unsigned int bufferId = event.data[0]; + unsigned int bufferId = event.bufferId_; if (++check_count_ != frame_count_) /* assert here? */ LOG(IPARPI, Error) << "WARNING: Prepare/Process mismatch!!!"; @@ -348,17 +364,17 @@ void IPARPi::processEvent(const IPAOperationData &event) reportMetadata(); - IPAOperationData op; - op.operation = RPI_IPA_ACTION_STATS_METADATA_COMPLETE; - op.data = { bufferId & RPiIpaMask::ID }; - op.controls = { libcameraMetadata_ }; + RPiActionParams op; + op.op_ = RPI_IPA_ACTION_STATS_METADATA_COMPLETE; + op.statsComplete_.bufferId_ = { bufferId & RPiIpaMask::ID }; + op.statsComplete_.controls_ = { libcameraMetadata_ }; queueFrameAction.emit(0, op); break; } case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE: { - unsigned int embeddedbufferId = event.data[0]; - unsigned int bayerbufferId = event.data[1]; + unsigned int embeddedbufferId = event.ispPrepare_.embeddedbufferId_; + unsigned int bayerbufferId = event.ispPrepare_.bayerbufferId_; /* * At start-up, or after a mode-switch, we may want to @@ -368,23 +384,23 @@ void IPARPi::processEvent(const IPAOperationData &event) prepareISP(embeddedbufferId); /* Ready to push the input buffer into the ISP. */ - IPAOperationData op; + RPiActionParams op; if (++frame_count_ > hide_count_) - op.operation = RPI_IPA_ACTION_RUN_ISP; + op.op_ = RPI_IPA_ACTION_RUN_ISP; else - op.operation = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME; - op.data = { bayerbufferId & RPiIpaMask::ID }; + op.op_ = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME; + op.bufferId_ = { bayerbufferId & RPiIpaMask::ID }; queueFrameAction.emit(0, op); break; } case RPI_IPA_EVENT_QUEUE_REQUEST: { - queueRequest(event.controls[0]); + queueRequest(event.controls_); break; } default: - LOG(IPARPI, Error) << "Unknown event " << event.operation; + LOG(IPARPI, Error) << "Unknown event " << event.ev_; break; } } @@ -489,6 +505,8 @@ void IPARPi::queueRequest(const ControlList &controls) /* Clear the return metadata buffer. */ libcameraMetadata_.clear(); + LOG(IPARPI, Info) << "Request ctrl length: " << controls.size(); + for (auto const &ctrl : controls) { LOG(IPARPI, Info) << "Request ctrl: " << controls::controls.at(ctrl.first)->name() @@ -698,9 +716,9 @@ void IPARPi::queueRequest(const ControlList &controls) void IPARPi::returnEmbeddedBuffer(unsigned int bufferId) { - IPAOperationData op; - op.operation = RPI_IPA_ACTION_EMBEDDED_COMPLETE; - op.data = { bufferId & RPiIpaMask::ID }; + RPiActionParams op; + op.op_ = RPI_IPA_ACTION_EMBEDDED_COMPLETE; + op.bufferId_ = { bufferId & RPiIpaMask::ID }; queueFrameAction.emit(0, op); } @@ -763,9 +781,9 @@ void IPARPi::prepareISP(unsigned int bufferId) applyDPC(dpcStatus, ctrls); if (!ctrls.empty()) { - IPAOperationData op; - op.operation = RPI_IPA_ACTION_V4L2_SET_ISP; - op.controls.push_back(ctrls); + RPiActionParams op; + op.op_ = RPI_IPA_ACTION_V4L2_SET_ISP; + op.controls_ = ctrls; queueFrameAction.emit(0, op); } } @@ -823,9 +841,9 @@ void IPARPi::processStats(unsigned int bufferId) ControlList ctrls(unicam_ctrls_); applyAGC(&agcStatus, ctrls); - IPAOperationData op; - op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED; - op.controls.push_back(ctrls); + RPiActionParams op; + op.op_ = RPI_IPA_ACTION_V4L2_SET_STAGGERED; + op.controls_ = ctrls; queueFrameAction.emit(0, op); } } @@ -1056,7 +1074,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls) .grid_width = w, .grid_stride = w, .grid_height = h, - .dmabuf = lsTableHandle_.fd(), + .dmabuf = lsTableHandlePH_, .ref_transform = 0, .corner_sampled = 1, .gain_format = GAIN_FORMAT_U4P10 @@ -1136,9 +1154,9 @@ const struct IPAModuleInfo ipaModuleInfo = { "raspberrypi", }; -struct ipa_context *ipaCreate() +IPAInterface *ipaCreate() { - return new IPAInterfaceWrapper(std::make_unique<IPARPi>()); + return new IPARPi(); } }; /* extern "C" */ diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index ce43af34..70bb6fcc 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -15,7 +15,9 @@ #include <libcamera/control_ids.h> #include <libcamera/file_descriptor.h> #include <libcamera/formats.h> +#include <libcamera/ipa/ipa_proxy_raspberrypi.h> #include <libcamera/ipa/raspberrypi.h> +#include <libcamera/ipa/raspberrypi_generated.h> #include <libcamera/logging.h> #include <libcamera/property_ids.h> #include <libcamera/request.h> @@ -296,7 +298,7 @@ public: int loadIPA(); int configureIPA(); - void queueFrameAction(unsigned int frame, const IPAOperationData &action); + void queueFrameAction(unsigned int frame, const RPiActionParams &action); /* bufferComplete signal handlers. */ void unicamBufferDequeue(FrameBuffer *buffer); @@ -307,6 +309,8 @@ public: void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream); void handleState(); + std::unique_ptr<IPAProxyRPi> ipa_; + CameraSensor *sensor_; /* Array of Unicam and ISP device streams and associated buffers/streams. */ RPiDevice<Unicam, 2> unicam_; @@ -506,6 +510,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager) : PipelineHandler(manager), unicam_(nullptr), isp_(nullptr) { + initializeRPiControls(); } CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera, @@ -1103,7 +1108,9 @@ void RPiCameraData::frameStarted(uint32_t sequence) int RPiCameraData::loadIPA() { - ipa_ = IPAManager::createIPA(pipe_, 1, 1); + std::unique_ptr<IPAProxy> ptr = IPAManager::createIPA(pipe_, 1, 1); + ipa_ = std::unique_ptr<IPAProxyRPi>{static_cast<IPAProxyRPi*>(std::move(ptr).release())}; + if (!ipa_) return -ENOENT; @@ -1119,8 +1126,8 @@ int RPiCameraData::loadIPA() int RPiCameraData::configureIPA() { std::map<unsigned int, IPAStream> streamConfig; - std::map<unsigned int, const ControlInfoMap &> entityControls; - IPAOperationData ipaConfig = {}; + std::map<unsigned int, ControlInfoMap> entityControls; + RPiConfigureParams ipaConfig; /* Get the device format to pass to the IPA. */ V4L2DeviceFormat sensorFormat; @@ -1145,8 +1152,11 @@ int RPiCameraData::configureIPA() return -ENOMEM; /* Allow the IPA to mmap the LS table via the file descriptor. */ - ipaConfig.operation = RPI_IPA_CONFIG_LS_TABLE; - ipaConfig.data = { static_cast<unsigned int>(lsTable_.fd()) }; + RPiConfigurePayload payload; + payload.op_ = RPI_IPA_CONFIG_LS_TABLE; + payload.lsTableHandle_ = lsTable_; + payload.lsTableHandleStatic_ = lsTable_.fd(); + ipaConfig.payload_.push_back(payload); } CameraSensorInfo sensorInfo = {}; @@ -1157,53 +1167,68 @@ int RPiCameraData::configureIPA() } /* Ready the IPA - it must know about the sensor resolution. */ - IPAOperationData result; + RPiConfigureParams results; ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, - &result); + &results); - if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) { - /* - * Setup our staggered control writer with the sensor default - * gain and exposure delays. - */ - if (!staggeredCtrl_) { - staggeredCtrl_.init(unicam_[Unicam::Image].dev(), - { { V4L2_CID_ANALOGUE_GAIN, result.data[0] }, - { V4L2_CID_EXPOSURE, result.data[1] } }); - sensorMetadata_ = result.data[2]; + for (RPiConfigurePayload &result : results.payload_) { + if (result.op_ == RPI_IPA_CONFIG_STAGGERED_WRITE) { + + /* + * Setup our staggered control writer with the sensor default + * gain and exposure delays. + */ + if (!staggeredCtrl_) { + staggeredCtrl_.init(unicam_[Unicam::Image].dev(), + { { V4L2_CID_ANALOGUE_GAIN, + result.staggeredWriteResult_.gainDelay_ }, + { V4L2_CID_EXPOSURE, + result.staggeredWriteResult_.exposureDelay_ } }); + sensorMetadata_ = result.staggeredWriteResult_.sensorMetadata_; + } + + /* Configure the H/V flip controls based on the sensor rotation. */ + ControlList ctrls(unicam_[Unicam::Image].dev()->controls()); + int32_t rotation = sensor_->properties().get(properties::Rotation); + ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation)); + ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation)); + unicam_[Unicam::Image].dev()->setControls(&ctrls); } - } - if (result.operation & RPI_IPA_CONFIG_SENSOR) { - const ControlList &ctrls = result.controls[0]; - if (!staggeredCtrl_.set(ctrls)) - LOG(RPI, Error) << "V4L2 staggered set failed"; + if (result.op_ == RPI_IPA_CONFIG_SENSOR) { + const ControlList &ctrls = result.controls_; + if (!staggeredCtrl_.set(ctrls)) + LOG(RPI, Error) << "V4L2 staggered set failed"; + } } return 0; } void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, - const IPAOperationData &action) + const RPiActionParams &action) { /* * The following actions can be handled when the pipeline handler is in * a stopped state. */ - switch (action.operation) { + switch (action.op_) { case RPI_IPA_ACTION_V4L2_SET_STAGGERED: { - const ControlList &controls = action.controls[0]; + const ControlList &controls = action.controls_; if (!staggeredCtrl_.set(controls)) LOG(RPI, Error) << "V4L2 staggered set failed"; goto done; } case RPI_IPA_ACTION_V4L2_SET_ISP: { - ControlList controls = action.controls[0]; + ControlList controls = action.controls_; isp_[Isp::Input].dev()->setControls(&controls); goto done; } + + default: + break; } if (state_ == State::Stopped) @@ -1213,20 +1238,20 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, * The following actions must not be handled when the pipeline handler * is in a stopped state. */ - switch (action.operation) { + switch (action.op_) { case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: { - unsigned int bufferId = action.data[0]; + unsigned int bufferId = action.statsComplete_.bufferId_; FrameBuffer *buffer = isp_[Isp::Stats].getBuffers()->at(bufferId).get(); handleStreamBuffer(buffer, &isp_[Isp::Stats]); /* Fill the Request metadata buffer with what the IPA has provided */ - requestQueue_.front()->metadata() = std::move(action.controls[0]); + requestQueue_.front()->metadata() = std::move(action.statsComplete_.controls_); state_ = State::IpaComplete; break; } case RPI_IPA_ACTION_EMBEDDED_COMPLETE: { - unsigned int bufferId = action.data[0]; + unsigned int bufferId = action.bufferId_; FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers()->at(bufferId).get(); handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]); break; @@ -1234,20 +1259,17 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, case RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME: case RPI_IPA_ACTION_RUN_ISP: { - unsigned int bufferId = action.data[0]; + unsigned int bufferId = action.bufferId_; FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers()->at(bufferId).get(); - LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << buffer->cookie() - << ", timestamp: " << buffer->metadata().timestamp; - isp_[Isp::Input].dev()->queueBuffer(buffer); - dropFrame_ = (action.operation == RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME) ? true : false; + dropFrame_ = (action.op_ == RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME) ? true : false; ispOutputCount_ = 0; break; } default: - LOG(RPI, Error) << "Unknown action " << action.operation; + LOG(RPI, Error) << "Unknown action " << action.op_; break; } @@ -1347,10 +1369,10 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer) /* If this is a stats output, hand it to the IPA now. */ if (stream == &isp_[Isp::Stats]) { - IPAOperationData op; - op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY; - op.data = { RPiIpaMask::STATS | buffer->cookie() }; - ipa_->processEvent(op); + RPiEventParams ev; + ev.ev_ = RPI_IPA_EVENT_SIGNAL_STAT_READY; + ev.bufferId_ = { RPiIpaMask::STATS | buffer->cookie() }; + ipa_->processEvent(ev); } handleState(); @@ -1493,7 +1515,7 @@ void RPiCameraData::checkRequestCompleted() void RPiCameraData::tryRunPipeline() { FrameBuffer *bayerBuffer, *embeddedBuffer; - IPAOperationData op; + RPiEventParams ev; /* If any of our request or buffer queues are empty, we cannot proceed. */ if (state_ != State::Idle || requestQueue_.empty() || @@ -1548,9 +1570,9 @@ void RPiCameraData::tryRunPipeline() * queue the ISP output buffer listed in the request to start the HW * pipeline. */ - op.operation = RPI_IPA_EVENT_QUEUE_REQUEST; - op.controls = { request->controls() }; - ipa_->processEvent(op); + ev.ev_ = RPI_IPA_EVENT_QUEUE_REQUEST; + ev.controls_ = { request->controls() }; + ipa_->processEvent(ev); /* Queue up any ISP buffers passed into the request. */ for (auto &stream : isp_) { @@ -1569,10 +1591,10 @@ void RPiCameraData::tryRunPipeline() << " Bayer buffer id: " << bayerBuffer->cookie() << " Embedded buffer id: " << embeddedBuffer->cookie(); - op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE; - op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(), - RPiIpaMask::BAYER_DATA | bayerBuffer->cookie() }; - ipa_->processEvent(op); + ev.ev_ = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE; + ev.ispPrepare_.embeddedbufferId_ = RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(); + ev.ispPrepare_.bayerbufferId_ = RPiIpaMask::BAYER_DATA | bayerBuffer->cookie(); + ipa_->processEvent(ev); } void RPiCameraData::tryFlushQueues()
Now that we can generate custom functions and data structures with mojo, switch the raspberrypi pipeline handler and IPA to use the custom data structures as defined in the mojom data definition file. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- include/libcamera/ipa/raspberrypi.h | 37 +++--- src/ipa/raspberrypi/raspberrypi.cpp | 108 +++++++++------- .../pipeline/raspberrypi/raspberrypi.cpp | 120 +++++++++++------- 3 files changed, 155 insertions(+), 110 deletions(-)