[libcamera-devel] [PATCH v2] pipeline: ipa: raspberrypi: Handle failures during IPA configuration

Laurent Pinchart laurent.pinchart at ideasonboard.com
Tue Dec 1 02:55:43 CET 2020


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.

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) {
>  		/*

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list