[libcamera-devel] [PATCH v2] pipeline: ipa: raspberrypi: Handle failures during IPA configuration
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Fri Dec 4 11:39:27 CET 2020
Hi Naush,
On Tue, Dec 01, 2020 at 03:55:43AM +0200, Laurent Pinchart wrote:
> Hi Naush,
>
> Thank you for the patch.
>
> On Fri, Nov 27, 2020 at 02:56:12PM +0000, Naushir Patuck wrote:
> > If the IPA fails during configuration, return an error flag to the
> > pipeline handler and fail the use case gracefully.
> >
> > At present, the IPA configuration can fail for the following reasons:
> > - The sensor is not recognised, and fails to open a CamHelper object.
> > - The pipeline handler did not pass in controls for the ISP and sensor.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> Wouldn't it be simpler to modify the configure() function to return an
> int ? How about the following ? If it works for you I'll submit a proper
> patch.
>From the perspective of the IPC changes, Naush's solution is more easily
translatable. When the IPA interface will become customizable, there
isn't a way to specify which output parameters should be return values
vs return parameters. Thus keeping them all as return parameters is the
better solution here.
So imo this is fine to merge as-is.
Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Author: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Date: Tue Dec 1 03:54:18 2020 +0200
>
> libcamera: ipa_interface: Make configure() return an int
>
> It's useful for the IPAInterface::configure() function to be able to
> return a status. Make it return an int, to avoid forcing IPAs to return
> a status encoded in the IPAOperationData in a custom way.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h
> index 8f767e844221..a00b5e7b92eb 100644
> --- a/include/libcamera/internal/ipa_context_wrapper.h
> +++ b/include/libcamera/internal/ipa_context_wrapper.h
> @@ -22,11 +22,11 @@ public:
> int init(const IPASettings &settings) override;
> int start() override;
> void stop() override;
> - void configure(const CameraSensorInfo &sensorInfo,
> - const std::map<unsigned int, IPAStream> &streamConfig,
> - const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - const IPAOperationData &ipaConfig,
> - IPAOperationData *result) override;
> + int configure(const CameraSensorInfo &sensorInfo,
> + const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + const IPAOperationData &ipaConfig,
> + IPAOperationData *result) override;
>
> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> void unmapBuffers(const std::vector<unsigned int> &ids) override;
> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
> index 322b7079070e..1d8cf8dc1c56 100644
> --- a/include/libcamera/ipa/ipa_interface.h
> +++ b/include/libcamera/ipa/ipa_interface.h
> @@ -95,12 +95,12 @@ struct ipa_context_ops {
> void (*register_callbacks)(struct ipa_context *ctx,
> const struct ipa_callback_ops *callbacks,
> void *cb_ctx);
> - void (*configure)(struct ipa_context *ctx,
> - const struct ipa_sensor_info *sensor_info,
> - const struct ipa_stream *streams,
> - unsigned int num_streams,
> - const struct ipa_control_info_map *maps,
> - unsigned int num_maps);
> + int (*configure)(struct ipa_context *ctx,
> + const struct ipa_sensor_info *sensor_info,
> + const struct ipa_stream *streams,
> + unsigned int num_streams,
> + const struct ipa_control_info_map *maps,
> + unsigned int num_maps);
> void (*map_buffers)(struct ipa_context *ctx,
> const struct ipa_buffer *buffers,
> size_t num_buffers);
> @@ -156,11 +156,11 @@ public:
> virtual int start() = 0;
> virtual void stop() = 0;
>
> - virtual void configure(const CameraSensorInfo &sensorInfo,
> - const std::map<unsigned int, IPAStream> &streamConfig,
> - const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - const IPAOperationData &ipaConfig,
> - IPAOperationData *result) = 0;
> + virtual int configure(const CameraSensorInfo &sensorInfo,
> + const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + const IPAOperationData &ipaConfig,
> + IPAOperationData *result) = 0;
>
> virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
> virtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;
> diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
> index cee532e3a747..74d7bc6e1c9b 100644
> --- a/src/ipa/libipa/ipa_interface_wrapper.cpp
> +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
> @@ -115,12 +115,12 @@ void IPAInterfaceWrapper::register_callbacks(struct ipa_context *_ctx,
> ctx->cb_ctx_ = cb_ctx;
> }
>
> -void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,
> - const struct ipa_sensor_info *sensor_info,
> - const struct ipa_stream *streams,
> - unsigned int num_streams,
> - const struct ipa_control_info_map *maps,
> - unsigned int num_maps)
> +int IPAInterfaceWrapper::configure(struct ipa_context *_ctx,
> + const struct ipa_sensor_info *sensor_info,
> + const struct ipa_stream *streams,
> + unsigned int num_streams,
> + const struct ipa_control_info_map *maps,
> + unsigned int num_maps)
> {
> IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
>
> @@ -168,8 +168,8 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,
>
> /* \todo Translate the ipaConfig and result. */
> IPAOperationData ipaConfig;
> - ctx->ipa_->configure(sensorInfo, ipaStreams, entityControls, ipaConfig,
> - nullptr);
> + return ctx->ipa_->configure(sensorInfo, ipaStreams, entityControls,
> + ipaConfig, nullptr);
> }
>
> void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,
> diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h
> index a1c701599b56..acd3160039b1 100644
> --- a/src/ipa/libipa/ipa_interface_wrapper.h
> +++ b/src/ipa/libipa/ipa_interface_wrapper.h
> @@ -30,12 +30,12 @@ private:
> static void register_callbacks(struct ipa_context *ctx,
> const struct ipa_callback_ops *callbacks,
> void *cb_ctx);
> - static void configure(struct ipa_context *ctx,
> - const struct ipa_sensor_info *sensor_info,
> - const struct ipa_stream *streams,
> - unsigned int num_streams,
> - const struct ipa_control_info_map *maps,
> - unsigned int num_maps);
> + static int configure(struct ipa_context *ctx,
> + const struct ipa_sensor_info *sensor_info,
> + const struct ipa_stream *streams,
> + unsigned int num_streams,
> + const struct ipa_control_info_map *maps,
> + unsigned int num_maps);
> static void map_buffers(struct ipa_context *ctx,
> const struct ipa_buffer *c_buffers,
> size_t num_buffers);
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 9853a343c892..75f7af3430ef 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -81,11 +81,11 @@ public:
> int start() override { return 0; }
> void stop() override {}
>
> - 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;
> + int 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;
> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> void unmapBuffers(const std::vector<unsigned int> &ids) override;
> void processEvent(const IPAOperationData &event) override;
> @@ -191,14 +191,14 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
> mode_.line_length = 1e9 * sensorInfo.lineLength / sensorInfo.pixelRate;
> }
>
> -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)
> +int 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)
> {
> if (entityControls.empty())
> - return;
> + return -EINVAL;
>
> result->operation = 0;
>
> @@ -314,6 +314,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> }
>
> lastMode_ = mode_;
> + return 0;
> }
>
> void IPARPi::mapBuffers(const std::vector<IPABuffer> &buffers)
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 07d7f1b2ddd8..78d78c5ac521 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -40,11 +40,11 @@ public:
> int start() override { return 0; }
> void stop() override {}
>
> - void configure(const CameraSensorInfo &info,
> - const std::map<unsigned int, IPAStream> &streamConfig,
> - const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - const IPAOperationData &ipaConfig,
> - IPAOperationData *response) override;
> + int configure(const CameraSensorInfo &info,
> + const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + const IPAOperationData &ipaConfig,
> + IPAOperationData *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;
> @@ -79,27 +79,27 @@ private:
> * assemble one. Make sure the reported sensor information are relevant
> * before accessing them.
> */
> -void IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
> - [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> - const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - [[maybe_unused]] const IPAOperationData &ipaConfig,
> - [[maybe_unused]] IPAOperationData *result)
> +int IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
> + [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + [[maybe_unused]] const IPAOperationData &ipaConfig,
> + [[maybe_unused]] IPAOperationData *result)
> {
> if (entityControls.empty())
> - return;
> + return -EINVAL;
>
> ctrls_ = entityControls.at(0);
>
> const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
> if (itExp == ctrls_.end()) {
> LOG(IPARkISP1, Error) << "Can't find exposure control";
> - return;
> + return -EINVAL;
> }
>
> const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);
> if (itGain == ctrls_.end()) {
> LOG(IPARkISP1, Error) << "Can't find gain control";
> - return;
> + return -EINVAL;
> }
>
> autoExposure_ = true;
> @@ -117,6 +117,7 @@ void IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
> << " Gain: " << minGain_ << "-" << maxGain_;
>
> setControls(0);
> + return 0;
> }
>
> void IPARkISP1::mapBuffers(const std::vector<IPABuffer> &buffers)
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index cf8411359e40..79c8c2fb8927 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -37,11 +37,11 @@ public:
> int start() override;
> void stop() override;
>
> - void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
> - [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> - [[maybe_unused]] const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - [[maybe_unused]] const IPAOperationData &ipaConfig,
> - [[maybe_unused]] IPAOperationData *result) override {}
> + int configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
> + [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> + [[maybe_unused]] const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + [[maybe_unused]] const IPAOperationData &ipaConfig,
> + [[maybe_unused]] IPAOperationData *result) override { return 0; }
> void mapBuffers([[maybe_unused]] const std::vector<IPABuffer> &buffers) override {}
> void unmapBuffers([[maybe_unused]] const std::vector<unsigned int> &ids) override {}
> void processEvent([[maybe_unused]] const IPAOperationData &event) override {}
> diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
> index 231300ce0bec..f63ad830c003 100644
> --- a/src/libcamera/ipa_context_wrapper.cpp
> +++ b/src/libcamera/ipa_context_wrapper.cpp
> @@ -108,18 +108,18 @@ void IPAContextWrapper::stop()
> ctx_->ops->stop(ctx_);
> }
>
> -void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,
> - const std::map<unsigned int, IPAStream> &streamConfig,
> - const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - const IPAOperationData &ipaConfig,
> - IPAOperationData *result)
> +int IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,
> + const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + const IPAOperationData &ipaConfig,
> + IPAOperationData *result)
> {
> if (intf_)
> return intf_->configure(sensorInfo, streamConfig,
> entityControls, ipaConfig, result);
>
> if (!ctx_)
> - return;
> + return 0;
>
> serializer_.reset();
>
> @@ -178,8 +178,9 @@ void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,
> }
>
> /* \todo Translate the ipaConfig and reponse */
> - ctx_->ops->configure(ctx_, &sensor_info, c_streams, streamConfig.size(),
> - c_info_maps, entityControls.size());
> + return ctx_->ops->configure(ctx_, &sensor_info, c_streams,
> + streamConfig.size(), c_info_maps,
> + entityControls.size());
> }
>
> void IPAContextWrapper::mapBuffers(const std::vector<IPABuffer> &buffers)
> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> index 23fc56d7d48e..516a8ecd4b53 100644
> --- a/src/libcamera/ipa_interface.cpp
> +++ b/src/libcamera/ipa_interface.cpp
> @@ -347,6 +347,8 @@
> * \param[in] num_maps The number of entries in the \a maps array
> *
> * \sa libcamera::IPAInterface::configure()
> + *
> + * \return 0 on success or a negative error code on failure
> */
>
> /**
> @@ -573,6 +575,8 @@ namespace libcamera {
> * 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 on failure
> */
>
> /**
> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> index b78a0e4535f5..ed250ce79c17 100644
> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> @@ -32,11 +32,11 @@ public:
> }
> int start() override { return 0; }
> void stop() override {}
> - void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
> - [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> - [[maybe_unused]] const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - [[maybe_unused]] const IPAOperationData &ipaConfig,
> - [[maybe_unused]] IPAOperationData *result) override {}
> + int configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
> + [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> + [[maybe_unused]] const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + [[maybe_unused]] const IPAOperationData &ipaConfig,
> + [[maybe_unused]] IPAOperationData *result) override { return 0; }
> void mapBuffers([[maybe_unused]] const std::vector<IPABuffer> &buffers) override {}
> void unmapBuffers([[maybe_unused]] const std::vector<unsigned int> &ids) override {}
> void processEvent([[maybe_unused]] const IPAOperationData &event) override {}
> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
> index eead2883708d..fd91726c4840 100644
> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
> @@ -29,11 +29,11 @@ public:
> int start() override;
> void stop() override;
>
> - void configure(const CameraSensorInfo &sensorInfo,
> - const std::map<unsigned int, IPAStream> &streamConfig,
> - const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - const IPAOperationData &ipaConfig,
> - IPAOperationData *result) override;
> + int configure(const CameraSensorInfo &sensorInfo,
> + const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + const IPAOperationData &ipaConfig,
> + IPAOperationData *result) override;
> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> void unmapBuffers(const std::vector<unsigned int> &ids) override;
> void processEvent(const IPAOperationData &event) override;
> @@ -132,14 +132,14 @@ void IPAProxyThread::stop()
> thread_.wait();
> }
>
> -void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,
> - const std::map<unsigned int, IPAStream> &streamConfig,
> - const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - const IPAOperationData &ipaConfig,
> - IPAOperationData *result)
> +int IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,
> + const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + const IPAOperationData &ipaConfig,
> + IPAOperationData *result)
> {
> - ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,
> - result);
> + return ipa_->configure(sensorInfo, streamConfig, entityControls,
> + ipaConfig, result);
> }
>
> void IPAProxyThread::mapBuffers(const std::vector<IPABuffer> &buffers)
> diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp
> index 59d991cbbf6a..ad0fb0386865 100644
> --- a/test/ipa/ipa_wrappers_test.cpp
> +++ b/test/ipa/ipa_wrappers_test.cpp
> @@ -67,55 +67,63 @@ public:
> report(Op_stop, TestPass);
> }
>
> - void configure(const CameraSensorInfo &sensorInfo,
> - const std::map<unsigned int, IPAStream> &streamConfig,
> - const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - [[maybe_unused]] const IPAOperationData &ipaConfig,
> - [[maybe_unused]] IPAOperationData *result) override
> + int configure(const CameraSensorInfo &sensorInfo,
> + const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> + [[maybe_unused]] const IPAOperationData &ipaConfig,
> + [[maybe_unused]] IPAOperationData *result) override
> {
> /* Verify sensorInfo. */
> if (sensorInfo.outputSize.width != 2560 ||
> sensorInfo.outputSize.height != 1940) {
> cerr << "configure(): Invalid sensor info size "
> << sensorInfo.outputSize.toString();
> + report(Op_configure, TestFail);
> + return -EINVAL;
> }
>
> /* Verify streamConfig. */
> if (streamConfig.size() != 2) {
> cerr << "configure(): Invalid number of streams "
> << streamConfig.size() << endl;
> - return report(Op_configure, TestFail);
> + report(Op_configure, TestFail);
> + return -EINVAL;
> }
>
> auto iter = streamConfig.find(1);
> if (iter == streamConfig.end()) {
> cerr << "configure(): No configuration for stream 1" << endl;
> - return report(Op_configure, TestFail);
> + report(Op_configure, TestFail);
> + return -EINVAL;
> }
> const IPAStream *stream = &iter->second;
> if (stream->pixelFormat != V4L2_PIX_FMT_YUYV ||
> stream->size != Size{ 1024, 768 }) {
> cerr << "configure(): Invalid configuration for stream 1" << endl;
> - return report(Op_configure, TestFail);
> + report(Op_configure, TestFail);
> + return -EINVAL;
> }
>
> iter = streamConfig.find(2);
> if (iter == streamConfig.end()) {
> cerr << "configure(): No configuration for stream 2" << endl;
> - return report(Op_configure, TestFail);
> + report(Op_configure, TestFail);
> + return -EINVAL;
> }
> stream = &iter->second;
> if (stream->pixelFormat != V4L2_PIX_FMT_NV12 ||
> stream->size != Size{ 800, 600 }) {
> cerr << "configure(): Invalid configuration for stream 2" << endl;
> - return report(Op_configure, TestFail);
> + report(Op_configure, TestFail);
> + return -EINVAL;
> }
>
> /* Verify entityControls. */
> auto ctrlIter = entityControls.find(42);
> if (ctrlIter == entityControls.end()) {
> cerr << "configure(): Controls not found" << endl;
> - return report(Op_configure, TestFail);
> + report(Op_configure, TestFail);
> + return -EINVAL;
> }
>
> const ControlInfoMap &infoMap = ctrlIter->second;
> @@ -124,10 +132,12 @@ public:
> infoMap.count(V4L2_CID_CONTRAST) != 1 ||
> infoMap.count(V4L2_CID_SATURATION) != 1) {
> cerr << "configure(): Invalid control IDs" << endl;
> - return report(Op_configure, TestFail);
> + report(Op_configure, TestFail);
> + return -EINVAL;
> }
>
> report(Op_configure, TestPass);
> + return 0;
> }
>
> void mapBuffers(const std::vector<IPABuffer> &buffers) override
>
> > ---
> > include/libcamera/ipa/raspberrypi.h | 1 +
> > src/ipa/raspberrypi/raspberrypi.cpp | 12 +++++++++++-
> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 +++++
> > 3 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> > index 2b55dbfc..86c97ffa 100644
> > --- a/include/libcamera/ipa/raspberrypi.h
> > +++ b/include/libcamera/ipa/raspberrypi.h
> > @@ -21,6 +21,7 @@ enum ConfigParameters {
> > IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
> > IPA_CONFIG_SENSOR = (1 << 2),
> > IPA_CONFIG_DROP_FRAMES = (1 << 3),
> > + IPA_CONFIG_FAILED = (1 << 4),
> > };
> >
> > enum Operations {
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 9853a343..57dd9c61 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -197,8 +197,11 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> > const IPAOperationData &ipaConfig,
> > IPAOperationData *result)
> > {
> > - if (entityControls.empty())
> > + if (entityControls.size() != 2) {
> > + LOG(IPARPI, Error) << "No ISP or sensor controls found.";
> > + result->operation = RPi::IPA_CONFIG_FAILED;
> > return;
> > + }
> >
> > result->operation = 0;
> >
> > @@ -217,6 +220,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> > if (!helper_) {
> > helper_ = std::unique_ptr<RPiController::CamHelper>(RPiController::CamHelper::Create(cameraName));
> >
> > + if (!helper_) {
> > + LOG(IPARPI, Error) << "Could not create camera helper for "
> > + << cameraName;
> > + result->operation = RPi::IPA_CONFIG_FAILED;
> > + return;
> > + }
> > +
> > /*
> > * Pass out the sensor config to the pipeline handler in order
> > * to setup the staggered writer class.
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 6fcdf557..76252806 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1194,6 +1194,11 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
> > ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
> > &result);
> >
> > + if (result.operation & RPi::IPA_CONFIG_FAILED) {
> > + LOG(RPI, Error) << "IPA configuration failed!";
> > + return -EPIPE;
> > + }
> > +
> > unsigned int resultIdx = 0;
> > if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {
> > /*
More information about the libcamera-devel
mailing list