[libcamera-devel] [PATCH v3 13/13] libcamera: ipa: Add support for CameraSensorInfo

Jacopo Mondi jacopo at jmondi.org
Fri Apr 24 23:53:04 CEST 2020


Add support for camera sensor information in the libcamera IPA protocol.

Define a new 'struct ipa_sensor_info' structure in the IPA context and
use it to perform translation between the C and the C++ API.

Update the IPAInterface::configure() operation to accept a new
CameraSensorInfo parameter and port all users of that function to
the new interface.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 include/ipa/ipa_interface.h                 | 21 +++++++-
 src/ipa/libipa/ipa_interface_wrapper.cpp    | 19 ++++++-
 src/ipa/libipa/ipa_interface_wrapper.h      |  1 +
 src/ipa/rkisp1/rkisp1.cpp                   | 13 ++++-
 src/ipa/vimc/vimc.cpp                       |  4 +-
 src/libcamera/include/ipa_context_wrapper.h |  4 +-
 src/libcamera/ipa_context_wrapper.cpp       | 24 +++++++--
 src/libcamera/ipa_interface.cpp             | 60 +++++++++++++++++++++
 src/libcamera/pipeline/rkisp1/rkisp1.cpp    |  9 +++-
 src/libcamera/proxy/ipa_proxy_linux.cpp     |  4 +-
 src/libcamera/proxy/ipa_proxy_thread.cpp    |  9 ++--
 test/ipa/ipa_wrappers_test.cpp              | 21 +++++++-
 12 files changed, 173 insertions(+), 16 deletions(-)

diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
index e65844bc7b34..a7acc09c4715 100644
--- a/include/ipa/ipa_interface.h
+++ b/include/ipa/ipa_interface.h
@@ -18,6 +18,22 @@ struct ipa_context {
 	const struct ipa_context_ops *ops;
 };
 
+struct ipa_sensor_info {
+#define CAMERA_SENSOR_NAME_LEN 32
+	char name[CAMERA_SENSOR_NAME_LEN];
+	uint8_t bits_per_pixel;
+	uint32_t active_area_width;
+	uint32_t active_area_height;
+	int32_t analog_crop_left;
+	int32_t analog_crop_top;
+	uint32_t analog_crop_width;
+	uint32_t analog_crop_height;
+	uint32_t output_width;
+	uint32_t output_height;
+	uint32_t pixel_clock;
+	uint32_t line_length;
+};
+
 struct ipa_stream {
 	unsigned int id;
 	unsigned int pixel_format;
@@ -70,6 +86,7 @@ struct ipa_context_ops {
 				   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,
@@ -96,6 +113,7 @@ struct ipa_context *ipaCreate();
 #include <libcamera/geometry.h>
 #include <libcamera/signal.h>
 
+#include "camera_sensor.h"
 #include "v4l2_controls.h"
 
 namespace libcamera {
@@ -125,7 +143,8 @@ public:
 	virtual int start() = 0;
 	virtual void stop() = 0;
 
-	virtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,
+	virtual void configure(const CameraSensorInfo &sensorInfo,
+			       const std::map<unsigned int, IPAStream> &streamConfig,
 			       const std::map<unsigned int, const ControlInfoMap &> &entityControls) = 0;
 
 	virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
index f50f93a0185a..049b9dd1eefc 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.cpp
+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
@@ -15,6 +15,7 @@
 #include <ipa/ipa_interface.h>
 
 #include "byte_stream_buffer.h"
+#include "camera_sensor.h"
 
 /**
  * \file ipa_interface_wrapper.h
@@ -111,6 +112,7 @@ void IPAInterfaceWrapper::register_callbacks(struct ipa_context *_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,
@@ -120,6 +122,21 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,
 
 	ctx->serializer_.reset();
 
+	/* Translate the IPA sensor info. */
+	CameraSensorInfo sensorInfo{};
+	sensorInfo.name = sensor_info->name;
+	sensorInfo.bitsPerPixel = sensor_info->bits_per_pixel;
+	sensorInfo.activeAreaSize = { sensor_info->active_area_width,
+				      sensor_info->active_area_height, };
+	sensorInfo.analogCrop = { sensor_info->analog_crop_left,
+				  sensor_info->analog_crop_top,
+				  sensor_info->analog_crop_width,
+				  sensor_info->analog_crop_height, };
+	sensorInfo.outputSize = { sensor_info->output_width,
+				  sensor_info->output_height, };
+	sensorInfo.pixelClock = sensor_info->pixel_clock;
+	sensorInfo.lineLength = sensor_info->line_length;
+
 	/* Translate the IPA stream configurations map. */
 	std::map<unsigned int, IPAStream> ipaStreams;
 
@@ -145,7 +162,7 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,
 		entityControls.emplace(id, infoMaps[id]);
 	}
 
-	ctx->ipa_->configure(ipaStreams, entityControls);
+	ctx->ipa_->configure(sensorInfo, ipaStreams, entityControls);
 }
 
 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 e4bc6dd4535d..febd6e66d0c4 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.h
+++ b/src/ipa/libipa/ipa_interface_wrapper.h
@@ -30,6 +30,7 @@ private:
 				       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,
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index acbbe58c7a2e..8b081359afff 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -22,6 +22,7 @@
 #include <libcamera/request.h>
 #include <libipa/ipa_interface_wrapper.h>
 
+#include "camera_sensor.h"
 #include "log.h"
 #include "utils.h"
 
@@ -36,7 +37,8 @@ public:
 	int start() override { return 0; }
 	void stop() override {}
 
-	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
+	void configure(const CameraSensorInfo &info,
+		       const std::map<unsigned int, IPAStream> &streamConfig,
 		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
@@ -66,7 +68,14 @@ private:
 	uint32_t maxGain_;
 };
 
-void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
+/**
+ * \todo The RkISP1 pipeline currently provides an empty CameraSensorInfo
+ * if the connected sensor does not provide enough information to properly
+ * assemble one. Make sure the reported sensor information are relevant
+ * befor accessing them.
+ */
+void IPARkISP1::configure(const CameraSensorInfo &info,
+			  const std::map<unsigned int, IPAStream> &streamConfig,
 			  const std::map<unsigned int, const ControlInfoMap &> &entityControls)
 {
 	if (entityControls.empty())
diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
index d2267e11737f..b8aadd8588cb 100644
--- a/src/ipa/vimc/vimc.cpp
+++ b/src/ipa/vimc/vimc.cpp
@@ -19,6 +19,7 @@
 
 #include <libipa/ipa_interface_wrapper.h>
 
+#include "camera_sensor.h"
 #include "log.h"
 
 namespace libcamera {
@@ -36,7 +37,8 @@ public:
 	int start() override;
 	void stop() override;
 
-	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
+	void configure(const CameraSensorInfo &sensorInfo,
+		       const std::map<unsigned int, IPAStream> &streamConfig,
 		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
 	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h
index 0a48bfe732c8..e11e7b6a894d 100644
--- a/src/libcamera/include/ipa_context_wrapper.h
+++ b/src/libcamera/include/ipa_context_wrapper.h
@@ -9,6 +9,7 @@
 
 #include <ipa/ipa_interface.h>
 
+#include "camera_sensor.h"
 #include "control_serializer.h"
 
 namespace libcamera {
@@ -22,7 +23,8 @@ public:
 	int init() override;
 	int start() override;
 	void stop() override;
-	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
+	void configure(const CameraSensorInfo &sensorInfo,
+		       const std::map<unsigned int, IPAStream> &streamConfig,
 		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;
 
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
index ab6ce396b88a..b1ff83aa8be8 100644
--- a/src/libcamera/ipa_context_wrapper.cpp
+++ b/src/libcamera/ipa_context_wrapper.cpp
@@ -12,6 +12,7 @@
 #include <libcamera/controls.h>
 
 #include "byte_stream_buffer.h"
+#include "camera_sensor.h"
 #include "utils.h"
 
 /**
@@ -104,17 +105,34 @@ void IPAContextWrapper::stop()
 	ctx_->ops->stop(ctx_);
 }
 
-void IPAContextWrapper::configure(const std::map<unsigned int, IPAStream> &streamConfig,
+void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,
+				  const std::map<unsigned int, IPAStream> &streamConfig,
 				  const std::map<unsigned int, const ControlInfoMap &> &entityControls)
 {
 	if (intf_)
-		return intf_->configure(streamConfig, entityControls);
+		return intf_->configure(sensorInfo, streamConfig, entityControls);
 
 	if (!ctx_)
 		return;
 
 	serializer_.reset();
 
+	/* Translate the camera sensor info. */
+	struct ipa_sensor_info sensor_info = {};
+	strncpy(sensor_info.name, sensorInfo.name.c_str(),
+		sensorInfo.name.length());
+	sensor_info.bits_per_pixel = sensorInfo.bitsPerPixel;
+	sensor_info.active_area_width = sensorInfo.activeAreaSize.width;
+	sensor_info.active_area_height = sensorInfo.activeAreaSize.height;
+	sensor_info.analog_crop_left = sensorInfo.analogCrop.x;
+	sensor_info.analog_crop_top = sensorInfo.analogCrop.y;
+	sensor_info.analog_crop_width = sensorInfo.analogCrop.width;
+	sensor_info.analog_crop_height = sensorInfo.analogCrop.height;
+	sensor_info.output_width = sensorInfo.outputSize.width;
+	sensor_info.output_height = sensorInfo.outputSize.height;
+	sensor_info.pixel_clock = sensorInfo.pixelClock;
+	sensor_info.line_length = sensorInfo.lineLength;
+
 	/* Translate the IPA stream configurations map. */
 	struct ipa_stream c_streams[streamConfig.size()];
 
@@ -154,7 +172,7 @@ void IPAContextWrapper::configure(const std::map<unsigned int, IPAStream> &strea
 		++i;
 	}
 
-	ctx_->ops->configure(ctx_, c_streams, streamConfig.size(),
+	ctx_->ops->configure(ctx_, &sensor_info, c_streams, streamConfig.size(),
 			     c_info_maps, entityControls.size());
 }
 
diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
index 890d4340e4f3..22a3567bc1d0 100644
--- a/src/libcamera/ipa_interface.cpp
+++ b/src/libcamera/ipa_interface.cpp
@@ -92,6 +92,65 @@
  * \brief The IPA context operations
  */
 
+/**
+ * \struct ipa_sensor_info
+ * \brief Camera sensor information for the IPA context operations
+ * \sa libcamera::CameraSensorInfo
+ *
+ * \def CAMERA_SENSOR_NAME_LEN
+ * \brief The camera sensor name maximum length
+ *
+ * \var ipa_sensor_info::name
+ * \brief The camera sensor name
+ * \todo Remove this field as soon as no IPA depends on it anymore
+ *
+ * \var ipa_sensor_info::bits_per_pixel
+ * \brief The camera sensor image format bit depth
+ * \sa libcamera::CameraSensorInfo::bitsPerPixel
+ *
+ * \var ipa_sensor_info::active_area_width
+ * \brief The camera sensor pixel array active area width
+ * \sa libcamera::CameraSensorInfo::activeAreaSize
+ *
+ * \var ipa_sensor_info::active_area_height
+ * \brief The camera sensor pixel array active area height
+ * \sa libcamera::CameraSensorInfo::activeAreaSize
+ *
+ * \var ipa_sensor_info::analog_crop_left
+ * \brief The horizontal displacement from the active area top-left corner of
+ * the cropped portion of the pixel array matrix
+ * \sa libcamera::CameraSensorInfo::analogCrop
+ *
+ * \var ipa_sensor_info::analog_crop_top
+ * \brief The vertical displacement from the active area top-left corner of
+ * the cropped portion of the pixel array matrix
+ * \sa libcamera::CameraSensorInfo::analogCrop
+ *
+ * \var ipa_sensor_info::analog_crop_width
+ * \brief The horizontal size of the cropped portion of the pixel array matrix
+ * \sa libcamera::CameraSensorInfo::analogCrop
+ *
+ * \var ipa_sensor_info::analog_crop_height
+ * \brief The vertical size of the cropped portion of the pixel array matrix
+ * \sa libcamera::CameraSensorInfo::analogCrop
+ *
+ * \var ipa_sensor_info::output_width
+ * \brief The horizontal size of the output image
+ * \sa libcamera::CameraSensorInfo::outputSize
+ *
+ * \var ipa_sensor_info::output_height
+ * \brief The vertical size of the output image
+ * \sa libcamera::CameraSensorInfo::outputSize
+ *
+ * \var ipa_sensor_info::pixel_clock
+ * \brief the pixel clock out frequency in Hz
+ * \sa libcamera::CameraSensorInfo::pixelClock
+ *
+ * \var ipa_sensor_info::line_length
+ * \brief The full line length, including blanking, in pixel units
+ * \sa libcamera::CameraSensorInfo::lineLength
+ */
+
 /**
  * \struct ipa_stream
  * \brief Stream information for the IPA context operations
@@ -447,6 +506,7 @@ namespace libcamera {
 /**
  * \fn IPAInterface::configure()
  * \brief Configure the IPA stream and sensor settings
+ * \param[in] sensorInfo Camera sensor information
  * \param[in] streamConfig Configuration of all active streams
  * \param[in] entityControls Controls provided by the pipeline entities
  *
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index f42264361785..169492e8313f 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -822,6 +822,13 @@ int PipelineHandlerRkISP1::start(Camera *camera)
 	activeCamera_ = camera;
 
 	/* Inform IPA of stream configuration and sensor controls. */
+	CameraSensorInfo sensorInfo = {};
+	ret = data->sensor_->sensorInfo(&sensorInfo);
+	if (ret) {
+		LOG(RkISP1, Info) << "Camera sensor information not available";
+		sensorInfo = {};
+	}
+
 	std::map<unsigned int, IPAStream> streamConfig;
 	streamConfig[0] = {
 		.pixelFormat = data->stream_.configuration().pixelFormat,
@@ -831,7 +838,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
 	std::map<unsigned int, const ControlInfoMap &> entityControls;
 	entityControls.emplace(0, data->sensor_->controls());
 
-	data->ipa_->configure(streamConfig, entityControls);
+	data->ipa_->configure(sensorInfo, streamConfig, entityControls);
 
 	return ret;
 }
diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
index 2aa80b946704..54b1abc4727b 100644
--- a/src/libcamera/proxy/ipa_proxy_linux.cpp
+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
@@ -10,6 +10,7 @@
 #include <ipa/ipa_interface.h>
 #include <ipa/ipa_module_info.h>
 
+#include "camera_sensor.h"
 #include "ipa_module.h"
 #include "ipa_proxy.h"
 #include "ipc_unixsocket.h"
@@ -29,7 +30,8 @@ public:
 	int init() override { return 0; }
 	int start() override { return 0; }
 	void stop() override {}
-	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
+	void configure(const CameraSensorInfo &sensorInfo,
+		       const std::map<unsigned int, IPAStream> &streamConfig,
 		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
 	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
index 368ccca1cf60..b06734371b39 100644
--- a/src/libcamera/proxy/ipa_proxy_thread.cpp
+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
@@ -10,6 +10,7 @@
 #include <ipa/ipa_interface.h>
 #include <ipa/ipa_module_info.h>
 
+#include "camera_sensor.h"
 #include "ipa_context_wrapper.h"
 #include "ipa_module.h"
 #include "ipa_proxy.h"
@@ -29,7 +30,8 @@ public:
 	int start() override;
 	void stop() override;
 
-	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
+	void configure(const CameraSensorInfo &sensorInfo,
+		       const std::map<unsigned int, IPAStream> &streamConfig,
 		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
@@ -126,10 +128,11 @@ void IPAProxyThread::stop()
 	thread_.wait();
 }
 
-void IPAProxyThread::configure(const std::map<unsigned int, IPAStream> &streamConfig,
+void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,
+			       const std::map<unsigned int, IPAStream> &streamConfig,
 			       const std::map<unsigned int, const ControlInfoMap &> &entityControls)
 {
-	ipa_->configure(streamConfig, entityControls);
+	ipa_->configure(sensorInfo, streamConfig, entityControls);
 }
 
 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 fae1d56b67c7..2cd10f981ecc 100644
--- a/test/ipa/ipa_wrappers_test.cpp
+++ b/test/ipa/ipa_wrappers_test.cpp
@@ -15,6 +15,7 @@
 #include <libcamera/controls.h>
 #include <libipa/ipa_interface_wrapper.h>
 
+#include "camera_sensor.h"
 #include "device_enumerator.h"
 #include "ipa_context_wrapper.h"
 #include "media_device.h"
@@ -60,9 +61,19 @@ public:
 		report(Op_stop, TestPass);
 	}
 
-	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
+	void configure(const CameraSensorInfo &sensorInfo,
+		       const std::map<unsigned int, IPAStream> &streamConfig,
 		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override
 	{
+		/* Verify sensorInfo. */
+		if (sensorInfo.outputSize.width != 2560 ||
+		    sensorInfo.outputSize.height != 1940) {
+			cerr << "configure(): Invalid sensor info sizes: ("
+			     << sensorInfo.outputSize.width << "x"
+			     << sensorInfo.outputSize.height << ")" << endl;
+			return report(Op_configure, TestFail);
+		}
+
 		/* Verify streamConfig. */
 		if (streamConfig.size() != 2) {
 			cerr << "configure(): Invalid number of streams "
@@ -287,13 +298,19 @@ protected:
 		int ret;
 
 		/* Test configure(). */
+		CameraSensorInfo sensorInfo = { "sensor", 8,
+						{ 2576, 1956 },
+						{ 8, 8, 2560, 1940 },
+						{ 2560, 1940 },
+						96000000,
+						2918, };
 		std::map<unsigned int, IPAStream> config{
 			{ 1, { V4L2_PIX_FMT_YUYV, { 1024, 768 } } },
 			{ 2, { V4L2_PIX_FMT_NV12, { 800, 600 } } },
 		};
 		std::map<unsigned int, const ControlInfoMap &> controlInfo;
 		controlInfo.emplace(42, subdev_->controls());
-		ret = INVOKE(configure, config, controlInfo);
+		ret = INVOKE(configure, sensorInfo, config, controlInfo);
 		if (ret == TestFail)
 			return TestFail;
 
-- 
2.26.1



More information about the libcamera-devel mailing list