| Message ID | 20201112085915.3053-3-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush Thanks for the patch. Only some minor nitpicks... On Thu, 12 Nov 2020 at 08:59, Naushir Patuck <naush@raspberrypi.com> wrote: > > This change allows controls passed into pipeline_hanlder::start to be > forwarded onto ipa::start(). We also add a return channel if the > pipeline handle must action any of these controls, e.g. setting the > analogue gain or shutter speed in the sensor device. s/hanlder/handler/ (though strictly the class is PipelineHandler) Is IPAContextWrapper::start more accurate, instead of ipa::start? s/pipeline handle/pipeline handler/ (sorry!) > > todo: The IPA interface wrapper needs a translation for passing > IPAOperationData into start() and configure() > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > include/libcamera/internal/ipa_context_wrapper.h | 3 ++- > include/libcamera/ipa/ipa_interface.h | 3 ++- > src/ipa/libipa/ipa_interface_wrapper.cpp | 4 +++- > src/ipa/raspberrypi/raspberrypi.cpp | 3 ++- > src/ipa/rkisp1/rkisp1.cpp | 3 ++- > src/ipa/vimc/vimc.cpp | 6 ++++-- > src/libcamera/ipa_context_wrapper.cpp | 6 ++++-- > src/libcamera/ipa_interface.cpp | 7 +++++++ > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++-- > src/libcamera/pipeline/vimc/vimc.cpp | 3 ++- > src/libcamera/proxy/ipa_proxy_linux.cpp | 3 ++- > src/libcamera/proxy/ipa_proxy_thread.cpp | 13 ++++++++----- > test/ipa/ipa_interface_test.cpp | 3 ++- > test/ipa/ipa_wrappers_test.cpp | 5 +++-- > 15 files changed, 49 insertions(+), 22 deletions(-) > > diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h > index 8f767e84..1878a615 100644 > --- a/include/libcamera/internal/ipa_context_wrapper.h > +++ b/include/libcamera/internal/ipa_context_wrapper.h > @@ -20,7 +20,8 @@ public: > ~IPAContextWrapper(); > > int init(const IPASettings &settings) override; > - int start() override; > + int start(const IPAOperationData &ipaConfig, > + IPAOperationData *result) override; > void stop() override; > void configure(const CameraSensorInfo &sensorInfo, > const std::map<unsigned int, IPAStream> &streamConfig, > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h > index 322b7079..b44e2538 100644 > --- a/include/libcamera/ipa/ipa_interface.h > +++ b/include/libcamera/ipa/ipa_interface.h > @@ -153,7 +153,8 @@ public: > virtual ~IPAInterface() = default; > > virtual int init(const IPASettings &settings) = 0; > - virtual int start() = 0; > + virtual int start(const IPAOperationData &ipaConfig, > + IPAOperationData *result) = 0; > virtual void stop() = 0; > > virtual void configure(const CameraSensorInfo &sensorInfo, > diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp > index cee532e3..fb2da9d3 100644 > --- a/src/ipa/libipa/ipa_interface_wrapper.cpp > +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp > @@ -95,7 +95,9 @@ int IPAInterfaceWrapper::start(struct ipa_context *_ctx) > { > IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx); > > - return ctx->ipa_->start(); > + /* \todo Translate the ipaConfig and result. */ > + IPAOperationData ipaConfig; In other places (though not always) you've tended to add "= {}" to the ipaConfig object. As far as I can see, it's only the "operation" field that might be uninitialised. Does this matter? > + return ctx->ipa_->start(ipaConfig, nullptr); > } > > void IPAInterfaceWrapper::stop(struct ipa_context *_ctx) > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > index f338ff8b..7a07b477 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -77,7 +77,8 @@ public: > } > > int init(const IPASettings &settings) override; > - int start() override { return 0; } > + int start([[maybe_unused]] const IPAOperationData &ipaConfig, > + [[maybe_unused]] IPAOperationData *result) override { return 0; } > void stop() override {} > > void configure(const CameraSensorInfo &sensorInfo, > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 07d7f1b2..0b8a9096 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -37,7 +37,8 @@ public: > { > return 0; > } > - int start() override { return 0; } > + int start([[maybe_unused]] const IPAOperationData &ipaConfig, > + [[maybe_unused]] IPAOperationData *result) override { return 0; } > void stop() override {} > > void configure(const CameraSensorInfo &info, > diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp > index cf841135..ed26331d 100644 > --- a/src/ipa/vimc/vimc.cpp > +++ b/src/ipa/vimc/vimc.cpp > @@ -34,7 +34,8 @@ public: > > int init(const IPASettings &settings) override; > > - int start() override; > + int start(const IPAOperationData &ipaConfig, > + IPAOperationData *result) override; > void stop() override; > > void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo, > @@ -82,7 +83,8 @@ int IPAVimc::init(const IPASettings &settings) > return 0; > } > > -int IPAVimc::start() > +int IPAVimc::start([[maybe_unused]] const IPAOperationData &ipaConfig, > + [[maybe_unused]] IPAOperationData *result) > { > trace(IPAOperationStart); > > diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp > index 231300ce..de96a606 100644 > --- a/src/libcamera/ipa_context_wrapper.cpp > +++ b/src/libcamera/ipa_context_wrapper.cpp > @@ -86,14 +86,16 @@ int IPAContextWrapper::init(const IPASettings &settings) > return 0; > } > > -int IPAContextWrapper::start() > +int IPAContextWrapper::start(const IPAOperationData &ipaConfig, > + IPAOperationData *result) > { > if (intf_) > - return intf_->start(); > + return intf_->start(ipaConfig, result); > > if (!ctx_) > return 0; > > + /* \todo Translate the ipaConfig and reponse */ > return ctx_->ops->start(ctx_); > } s/reponse/response/ > > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp > index 23fc56d7..282c3c0f 100644 > --- a/src/libcamera/ipa_interface.cpp > +++ b/src/libcamera/ipa_interface.cpp > @@ -536,10 +536,17 @@ namespace libcamera { > /** > * \fn IPAInterface::start() > * \brief Start the IPA > + * \param[in] ipaConfig Pipeline-handler-specific configuration data > + * \param[out] result Pipeline-handler-specific configuration result > * > * This method informs the IPA module that the camera is about to be started. > * The IPA module shall prepare any resources it needs to operate. > * > + * The \a ipaConfig and \a result parameters carry custom data passed by the > + * pipeline handler to the IPA and back. The pipeline handler may set the \a > + * result parameter to null if the IPA protocol doesn't need to pass a result > + * back through the configure() function. > + * > * \return 0 on success or a negative error code otherwise > */ > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index ddb30e49..a8e4997a 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -747,7 +747,9 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont > } > > /* Start the IPA. */ > - ret = data->ipa_->start(); > + IPAOperationData ipaConfig, result; Another IPAOperationData with undefined "operation" field. Does it matter? > + ipaConfig.controls.emplace_back(*controls); > + ret = data->ipa_->start(ipaConfig, &result); > if (ret) { > LOG(RPI, Error) > << "Failed to start IPA for " << camera->id(); > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 2e8d2930..a6c82a48 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -832,7 +832,8 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c > if (ret) > return ret; > > - ret = data->ipa_->start(); > + IPAOperationData ipaConfig = {}; > + ret = data->ipa_->start(ipaConfig, nullptr); > if (ret) { > freeBuffers(camera); > LOG(RkISP1, Error) > @@ -911,7 +912,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c > std::map<unsigned int, const ControlInfoMap &> entityControls; > entityControls.emplace(0, data->sensor_->controls()); > > - IPAOperationData ipaConfig; > + ipaConfig = {}; > data->ipa_->configure(sensorInfo, streamConfig, entityControls, > ipaConfig, nullptr); > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index d81b8598..b4773cf5 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -322,7 +322,8 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con > if (ret < 0) > return ret; > > - ret = data->ipa_->start(); > + IPAOperationData ipaConfig = {}; > + ret = data->ipa_->start(ipaConfig, nullptr); > if (ret) { > data->video_->releaseBuffers(); > return ret; > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp > index b78a0e45..619976e5 100644 > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp > @@ -30,7 +30,8 @@ public: > { > return 0; > } > - int start() override { return 0; } > + int start([[maybe_unused]] const IPAOperationData &ipaConfig, > + [[maybe_unused]] IPAOperationData *result) override { return 0; } > void stop() override {} > void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo, > [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig, > diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp > index eead2883..9a81b6e7 100644 > --- a/src/libcamera/proxy/ipa_proxy_thread.cpp > +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp > @@ -26,7 +26,8 @@ public: > IPAProxyThread(IPAModule *ipam); > > int init(const IPASettings &settings) override; > - int start() override; > + int start(const IPAOperationData &ipaConfig, > + IPAOperationData *result) override; > void stop() override; > > void configure(const CameraSensorInfo &sensorInfo, > @@ -50,9 +51,9 @@ private: > ipa_ = ipa; > } > > - int start() > + int start(const IPAOperationData &ipaConfig, IPAOperationData *result) > { > - return ipa_->start(); > + return ipa_->start(ipaConfig, result); > } > > void stop() > @@ -111,12 +112,14 @@ int IPAProxyThread::init(const IPASettings &settings) > return 0; > } > > -int IPAProxyThread::start() > +int IPAProxyThread::start(const IPAOperationData &ipaConfig, > + IPAOperationData *result) > { > running_ = true; > thread_.start(); > > - return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking); > + return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking, > + ipaConfig, result); > } > > void IPAProxyThread::stop() > diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp > index 67488409..6222938f 100644 > --- a/test/ipa/ipa_interface_test.cpp > +++ b/test/ipa/ipa_interface_test.cpp > @@ -120,7 +120,8 @@ protected: > } > > /* Test start of IPA module. */ > - ipa_->start(); > + IPAOperationData ipaConfig; Another uninitialised "operation" field. Does it matter? Should we make them all look the same? Other than these nitpicks: Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Tested-by: David Plowman <david.plowman@raspberrypi.com> Best regards David > + ipa_->start(ipaConfig, nullptr); > timer.start(1000); > while (timer.isRunning() && trace_ != IPAOperationStart) > dispatcher->processEvents(); > diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp > index 59d991cb..7b8ae77b 100644 > --- a/test/ipa/ipa_wrappers_test.cpp > +++ b/test/ipa/ipa_wrappers_test.cpp > @@ -56,7 +56,8 @@ public: > return 0; > } > > - int start() override > + int start([[maybe_unused]] const IPAOperationData &ipaConfig, > + [[maybe_unused]] IPAOperationData *result) override > { > report(Op_start, TestPass); > return 0; > @@ -376,7 +377,7 @@ protected: > return TestFail; > } > > - ret = INVOKE(start); > + ret = INVOKE(start, ipaConfig, nullptr); > if (ret == TestFail) { > cerr << "Failed to run start()"; > return TestFail; > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h index 8f767e84..1878a615 100644 --- a/include/libcamera/internal/ipa_context_wrapper.h +++ b/include/libcamera/internal/ipa_context_wrapper.h @@ -20,7 +20,8 @@ public: ~IPAContextWrapper(); int init(const IPASettings &settings) override; - int start() override; + int start(const IPAOperationData &ipaConfig, + IPAOperationData *result) override; void stop() override; void configure(const CameraSensorInfo &sensorInfo, const std::map<unsigned int, IPAStream> &streamConfig, diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h index 322b7079..b44e2538 100644 --- a/include/libcamera/ipa/ipa_interface.h +++ b/include/libcamera/ipa/ipa_interface.h @@ -153,7 +153,8 @@ public: virtual ~IPAInterface() = default; virtual int init(const IPASettings &settings) = 0; - virtual int start() = 0; + virtual int start(const IPAOperationData &ipaConfig, + IPAOperationData *result) = 0; virtual void stop() = 0; virtual void configure(const CameraSensorInfo &sensorInfo, diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp index cee532e3..fb2da9d3 100644 --- a/src/ipa/libipa/ipa_interface_wrapper.cpp +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp @@ -95,7 +95,9 @@ int IPAInterfaceWrapper::start(struct ipa_context *_ctx) { IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx); - return ctx->ipa_->start(); + /* \todo Translate the ipaConfig and result. */ + IPAOperationData ipaConfig; + return ctx->ipa_->start(ipaConfig, nullptr); } void IPAInterfaceWrapper::stop(struct ipa_context *_ctx) diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index f338ff8b..7a07b477 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -77,7 +77,8 @@ public: } int init(const IPASettings &settings) override; - int start() override { return 0; } + int start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) override { return 0; } void stop() override {} void configure(const CameraSensorInfo &sensorInfo, diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 07d7f1b2..0b8a9096 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -37,7 +37,8 @@ public: { return 0; } - int start() override { return 0; } + int start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) override { return 0; } void stop() override {} void configure(const CameraSensorInfo &info, diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp index cf841135..ed26331d 100644 --- a/src/ipa/vimc/vimc.cpp +++ b/src/ipa/vimc/vimc.cpp @@ -34,7 +34,8 @@ public: int init(const IPASettings &settings) override; - int start() override; + int start(const IPAOperationData &ipaConfig, + IPAOperationData *result) override; void stop() override; void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo, @@ -82,7 +83,8 @@ int IPAVimc::init(const IPASettings &settings) return 0; } -int IPAVimc::start() +int IPAVimc::start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) { trace(IPAOperationStart); diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp index 231300ce..de96a606 100644 --- a/src/libcamera/ipa_context_wrapper.cpp +++ b/src/libcamera/ipa_context_wrapper.cpp @@ -86,14 +86,16 @@ int IPAContextWrapper::init(const IPASettings &settings) return 0; } -int IPAContextWrapper::start() +int IPAContextWrapper::start(const IPAOperationData &ipaConfig, + IPAOperationData *result) { if (intf_) - return intf_->start(); + return intf_->start(ipaConfig, result); if (!ctx_) return 0; + /* \todo Translate the ipaConfig and reponse */ return ctx_->ops->start(ctx_); } diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index 23fc56d7..282c3c0f 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -536,10 +536,17 @@ namespace libcamera { /** * \fn IPAInterface::start() * \brief Start the IPA + * \param[in] ipaConfig Pipeline-handler-specific configuration data + * \param[out] result Pipeline-handler-specific configuration result * * This method informs the IPA module that the camera is about to be started. * The IPA module shall prepare any resources it needs to operate. * + * The \a ipaConfig and \a result parameters carry custom data passed by the + * pipeline handler to the IPA and back. The pipeline handler may set the \a + * result parameter to null if the IPA protocol doesn't need to pass a result + * back through the configure() function. + * * \return 0 on success or a negative error code otherwise */ diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index ddb30e49..a8e4997a 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -747,7 +747,9 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont } /* Start the IPA. */ - ret = data->ipa_->start(); + IPAOperationData ipaConfig, result; + ipaConfig.controls.emplace_back(*controls); + ret = data->ipa_->start(ipaConfig, &result); if (ret) { LOG(RPI, Error) << "Failed to start IPA for " << camera->id(); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 2e8d2930..a6c82a48 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -832,7 +832,8 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c if (ret) return ret; - ret = data->ipa_->start(); + IPAOperationData ipaConfig = {}; + ret = data->ipa_->start(ipaConfig, nullptr); if (ret) { freeBuffers(camera); LOG(RkISP1, Error) @@ -911,7 +912,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c std::map<unsigned int, const ControlInfoMap &> entityControls; entityControls.emplace(0, data->sensor_->controls()); - IPAOperationData ipaConfig; + ipaConfig = {}; data->ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, nullptr); diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index d81b8598..b4773cf5 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -322,7 +322,8 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con if (ret < 0) return ret; - ret = data->ipa_->start(); + IPAOperationData ipaConfig = {}; + ret = data->ipa_->start(ipaConfig, nullptr); if (ret) { data->video_->releaseBuffers(); return ret; diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp index b78a0e45..619976e5 100644 --- a/src/libcamera/proxy/ipa_proxy_linux.cpp +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp @@ -30,7 +30,8 @@ public: { return 0; } - int start() override { return 0; } + int start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) override { return 0; } void stop() override {} void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo, [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig, diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp index eead2883..9a81b6e7 100644 --- a/src/libcamera/proxy/ipa_proxy_thread.cpp +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp @@ -26,7 +26,8 @@ public: IPAProxyThread(IPAModule *ipam); int init(const IPASettings &settings) override; - int start() override; + int start(const IPAOperationData &ipaConfig, + IPAOperationData *result) override; void stop() override; void configure(const CameraSensorInfo &sensorInfo, @@ -50,9 +51,9 @@ private: ipa_ = ipa; } - int start() + int start(const IPAOperationData &ipaConfig, IPAOperationData *result) { - return ipa_->start(); + return ipa_->start(ipaConfig, result); } void stop() @@ -111,12 +112,14 @@ int IPAProxyThread::init(const IPASettings &settings) return 0; } -int IPAProxyThread::start() +int IPAProxyThread::start(const IPAOperationData &ipaConfig, + IPAOperationData *result) { running_ = true; thread_.start(); - return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking); + return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking, + ipaConfig, result); } void IPAProxyThread::stop() diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 67488409..6222938f 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -120,7 +120,8 @@ protected: } /* Test start of IPA module. */ - ipa_->start(); + IPAOperationData ipaConfig; + ipa_->start(ipaConfig, nullptr); timer.start(1000); while (timer.isRunning() && trace_ != IPAOperationStart) dispatcher->processEvents(); diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp index 59d991cb..7b8ae77b 100644 --- a/test/ipa/ipa_wrappers_test.cpp +++ b/test/ipa/ipa_wrappers_test.cpp @@ -56,7 +56,8 @@ public: return 0; } - int start() override + int start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) override { report(Op_start, TestPass); return 0; @@ -376,7 +377,7 @@ protected: return TestFail; } - ret = INVOKE(start); + ret = INVOKE(start, ipaConfig, nullptr); if (ret == TestFail) { cerr << "Failed to run start()"; return TestFail;
This change allows controls passed into pipeline_hanlder::start to be forwarded onto ipa::start(). We also add a return channel if the pipeline handle must action any of these controls, e.g. setting the analogue gain or shutter speed in the sensor device. todo: The IPA interface wrapper needs a translation for passing IPAOperationData into start() and configure() Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- include/libcamera/internal/ipa_context_wrapper.h | 3 ++- include/libcamera/ipa/ipa_interface.h | 3 ++- src/ipa/libipa/ipa_interface_wrapper.cpp | 4 +++- src/ipa/raspberrypi/raspberrypi.cpp | 3 ++- src/ipa/rkisp1/rkisp1.cpp | 3 ++- src/ipa/vimc/vimc.cpp | 6 ++++-- src/libcamera/ipa_context_wrapper.cpp | 6 ++++-- src/libcamera/ipa_interface.cpp | 7 +++++++ src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++-- src/libcamera/pipeline/vimc/vimc.cpp | 3 ++- src/libcamera/proxy/ipa_proxy_linux.cpp | 3 ++- src/libcamera/proxy/ipa_proxy_thread.cpp | 13 ++++++++----- test/ipa/ipa_interface_test.cpp | 3 ++- test/ipa/ipa_wrappers_test.cpp | 5 +++-- 15 files changed, 49 insertions(+), 22 deletions(-)