[libcamera-devel] [PATCH v5 7/7] libcamera: ipa: Add support for CameraSensorInfo
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Apr 28 18:58:42 CEST 2020
Hi Jacopo,
Thank you for the patch.
On Tue, Apr 28, 2020 at 11:19:34AM +0200, Jacopo Mondi wrote:
> 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>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/ipa/ipa_interface.h | 27 +++++++-
> src/ipa/libipa/ipa_interface_wrapper.cpp | 19 +++++-
> src/ipa/libipa/ipa_interface_wrapper.h | 1 +
> src/ipa/rkisp1/rkisp1.cpp | 12 +++-
> src/ipa/vimc/vimc.cpp | 3 +-
> src/libcamera/include/ipa_context_wrapper.h | 3 +-
> src/libcamera/ipa_context_wrapper.cpp | 23 ++++++-
> src/libcamera/ipa_interface.cpp | 73 +++++++++++++++++++++
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 ++-
> src/libcamera/proxy/ipa_proxy_linux.cpp | 3 +-
> src/libcamera/proxy/ipa_proxy_thread.cpp | 8 ++-
> test/ipa/ipa_wrappers_test.cpp | 22 ++++++-
> 12 files changed, 188 insertions(+), 16 deletions(-)
>
> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> index ef3d6507b692..a2d5e3dd1f51 100644
> --- a/include/ipa/ipa_interface.h
> +++ b/include/ipa/ipa_interface.h
> @@ -22,6 +22,27 @@ struct ipa_settings {
> const char *configuration_file;
> };
>
> +struct ipa_sensor_info {
> + const char *model;
> + uint8_t bits_per_pixel;
> + struct {
> + uint32_t width;
> + uint32_t height;
> + } active_area;
> + struct {
> + int32_t left;
> + int32_t top;
> + uint32_t width;
> + uint32_t height;
> + } analog_crop;
> + struct {
> + uint32_t width;
> + uint32_t height;
> + } output_size;
> + uint64_t pixel_rate;
> + uint32_t line_length;
> +};
> +
> struct ipa_stream {
> unsigned int id;
> unsigned int pixel_format;
> @@ -75,6 +96,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,
> @@ -125,6 +147,8 @@ struct IPAOperationData {
> std::vector<ControlList> controls;
> };
>
> +struct CameraSensorInfo;
> +
> class IPAInterface
> {
> public:
> @@ -134,7 +158,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 9596cb20cd66..21d8c98bddee 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
> @@ -115,6 +116,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,
> @@ -124,6 +126,21 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,
>
> ctx->serializer_.reset();
>
> + /* Translate the IPA sensor info. */
> + CameraSensorInfo sensorInfo{};
> + sensorInfo.model = sensor_info->model;
> + 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_size.width,
> + sensor_info->output_size.height };
> + sensorInfo.pixelRate = sensor_info->pixel_rate;
> + sensorInfo.lineLength = sensor_info->line_length;
> +
> /* Translate the IPA stream configurations map. */
> std::map<unsigned int, IPAStream> ipaStreams;
>
> @@ -149,7 +166,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 78ccf0f59d42..56507aafd81e 100644
> --- a/src/ipa/libipa/ipa_interface_wrapper.h
> +++ b/src/ipa/libipa/ipa_interface_wrapper.h
> @@ -31,6 +31,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 9a347e527cd2..bfa88418fa7b 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -36,7 +36,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 +67,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
> + * before 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 f29bc504d8c8..9271f2d8acff 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -37,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 64395b4a450b..0db022ef5a1b 100644
> --- a/src/libcamera/include/ipa_context_wrapper.h
> +++ b/src/libcamera/include/ipa_context_wrapper.h
> @@ -22,7 +22,8 @@ public:
> int init(const IPASettings &settings) 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 5bd5d916ccc0..0bd3a1aeca95 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"
>
> /**
> @@ -107,17 +108,33 @@ 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 = {};
> + sensor_info.model = sensorInfo.model.c_str();
> + 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_size.width = sensorInfo.outputSize.width;
> + sensor_info.output_size.height = sensorInfo.outputSize.height;
> + sensor_info.pixel_rate = sensorInfo.pixelRate;
> + sensor_info.line_length = sensorInfo.lineLength;
> +
> /* Translate the IPA stream configurations map. */
> struct ipa_stream c_streams[streamConfig.size()];
>
> @@ -157,7 +174,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 616f20fbf54f..c890eadaf6c8 100644
> --- a/src/libcamera/ipa_interface.cpp
> +++ b/src/libcamera/ipa_interface.cpp
> @@ -102,6 +102,74 @@
> * empty string)
> */
>
> +/**
> + * \struct ipa_sensor_info
> + * \brief Camera sensor information for the IPA context operations
> + * \sa libcamera::CameraSensorInfo
> + *
> + * \var ipa_sensor_info::model
> + * \brief The camera sensor model 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::active_area
> + * \brief The camera sensor pixel array active size
> + * \sa libcamera::CameraSensorInfo::activeAreaSize
> + *
> + * \var ipa_sensor_info::analog_crop.left
> + * \brief The left coordinate of the analog crop rectangle, relative to the
> + * pixel array active area
> + * \sa libcamera::CameraSensorInfo::analogCrop
> + *
> + * \var ipa_sensor_info::analog_crop.top
> + * \brief The top coordinate of the analog crop rectangle, relative to the pixel
> + * array active area
> + * \sa libcamera::CameraSensorInfo::analogCrop
> + *
> + * \var ipa_sensor_info::analog_crop.width
> + * \brief The horizontal size of the analog crop rectangle
> + * \sa libcamera::CameraSensorInfo::analogCrop
> + *
> + * \var ipa_sensor_info::analog_crop.height
> + * \brief The vertical size of the analog crop rectangle
> + * \sa libcamera::CameraSensorInfo::analogCrop
> + *
> + * \var ipa_sensor_info::analog_crop
> + * \brief The analog crop rectangle
> + * \sa libcamera::CameraSensorInfo::analogCrop
> + *
> + * \var ipa_sensor_info::output_size.width
> + * \brief The horizontal size of the output image
> + * \sa libcamera::CameraSensorInfo::outputSize
> + *
> + * \var ipa_sensor_info::output_size.height
> + * \brief The vertical size of the output image
> + * \sa libcamera::CameraSensorInfo::outputSize
> + *
> + * \var ipa_sensor_info::output_size
> + * \brief The size of the output image
> + * \sa libcamera::CameraSensorInfo::outputSize
> + *
> + * \var ipa_sensor_info::pixel_rate
> + * \brief The number of pixel produced in a second
> + * \sa libcamera::CameraSensorInfo::pixelRate
> + *
> + * \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
> @@ -481,6 +549,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
> *
> @@ -488,6 +557,10 @@ namespace libcamera {
> * the camera's streams and the sensor settings. The meaning of the numerical
> * keys in the \a streamConfig and \a entityControls maps is defined by the IPA
> * protocol.
> + *
> + * The \a sensorInfo conveys information about the camera sensor settings that
> + * the pipeline handler has selected for the configuration. The IPA may use
> + * that information to tune its algorithms.
> */
>
> /**
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index fde445b99a46..1a34ffe60ab4 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -822,6 +822,14 @@ int PipelineHandlerRkISP1::start(Camera *camera)
> activeCamera_ = camera;
>
> /* Inform IPA of stream configuration and sensor controls. */
> + CameraSensorInfo sensorInfo = {};
> + ret = data->sensor_->sensorInfo(&sensorInfo);
> + if (ret) {
> + /* \todo Turn this in an hard failure. */
> + LOG(RkISP1, Warning) << "Camera sensor information not available";
> + sensorInfo = {};
> + }
> +
> std::map<unsigned int, IPAStream> streamConfig;
> streamConfig[0] = {
> .pixelFormat = data->stream_.configuration().pixelFormat,
> @@ -831,7 +839,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 cd8ac824679d..9e0f44cf314f 100644
> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> @@ -29,7 +29,8 @@ public:
> int init(const IPASettings &settings) 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 be82fde34bd9..81d2d68ee715 100644
> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
> @@ -29,7 +29,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 +127,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 21bf51a2e046..4de132123525 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"
> @@ -66,9 +67,17 @@ 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 size "
> + << sensorInfo.outputSize.toString();
> + }
> +
> /* Verify streamConfig. */
> if (streamConfig.size() != 2) {
> cerr << "configure(): Invalid number of streams "
> @@ -293,13 +302,22 @@ protected:
> int ret;
>
> /* Test configure(). */
> + CameraSensorInfo sensorInfo{
> + .model = "sensor",
> + .bitsPerPixel = 8,
> + .activeAreaSize = { 2576, 1956 },
> + .analogCrop = { 8, 8, 2560, 1940 },
> + .outputSize = { 2560, 1940 },
> + .pixelRate = 96000000,
> + .lineLength = 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;
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list