From patchwork Fri Apr 24 21:53:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 3531 Return-Path: Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 67DE762F5E for ; Fri, 24 Apr 2020 23:50:17 +0200 (CEST) X-Originating-IP: 87.3.55.240 Received: from uno.homenet.telecomitalia.it (host240-55-dynamic.3-87-r.retail.telecomitalia.it [87.3.55.240]) (Authenticated sender: jacopo@jmondi.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 3367A60002; Fri, 24 Apr 2020 21:50:16 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Fri, 24 Apr 2020 23:53:04 +0200 Message-Id: <20200424215304.558317-14-jacopo@jmondi.org> X-Mailer: git-send-email 2.26.1 In-Reply-To: <20200424215304.558317-1-jacopo@jmondi.org> References: <20200424215304.558317-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 13/13] libcamera: ipa: Add support for CameraSensorInfo X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Apr 2020 21:50:17 -0000 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 --- 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 #include +#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 &streamConfig, + virtual void configure(const CameraSensorInfo &sensorInfo, + const std::map &streamConfig, const std::map &entityControls) = 0; virtual void mapBuffers(const std::vector &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 #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 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 #include +#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 &streamConfig, + void configure(const CameraSensorInfo &info, + const std::map &streamConfig, const std::map &entityControls) override; void mapBuffers(const std::vector &buffers) override; void unmapBuffers(const std::vector &ids) override; @@ -66,7 +68,14 @@ private: uint32_t maxGain_; }; -void IPARkISP1::configure(const std::map &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 &streamConfig, const std::map &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 +#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 &streamConfig, + void configure(const CameraSensorInfo &sensorInfo, + const std::map &streamConfig, const std::map &entityControls) override {} void mapBuffers(const std::vector &buffers) override {} void unmapBuffers(const std::vector &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 +#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 &streamConfig, + void configure(const CameraSensorInfo &sensorInfo, + const std::map &streamConfig, const std::map &entityControls) override; void mapBuffers(const std::vector &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 #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 &streamConfig, +void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo, + const std::map &streamConfig, const std::map &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 &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 streamConfig; streamConfig[0] = { .pixelFormat = data->stream_.configuration().pixelFormat, @@ -831,7 +838,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) std::map 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 #include +#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 &streamConfig, + void configure(const CameraSensorInfo &sensorInfo, + const std::map &streamConfig, const std::map &entityControls) override {} void mapBuffers(const std::vector &buffers) override {} void unmapBuffers(const std::vector &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 #include +#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 &streamConfig, + void configure(const CameraSensorInfo &sensorInfo, + const std::map &streamConfig, const std::map &entityControls) override; void mapBuffers(const std::vector &buffers) override; void unmapBuffers(const std::vector &ids) override; @@ -126,10 +128,11 @@ void IPAProxyThread::stop() thread_.wait(); } -void IPAProxyThread::configure(const std::map &streamConfig, +void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo, + const std::map &streamConfig, const std::map &entityControls) { - ipa_->configure(streamConfig, entityControls); + ipa_->configure(sensorInfo, streamConfig, entityControls); } void IPAProxyThread::mapBuffers(const std::vector &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 #include +#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 &streamConfig, + void configure(const CameraSensorInfo &sensorInfo, + const std::map &streamConfig, const std::map &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 config{ { 1, { V4L2_PIX_FMT_YUYV, { 1024, 768 } } }, { 2, { V4L2_PIX_FMT_NV12, { 800, 600 } } }, }; std::map controlInfo; controlInfo.emplace(42, subdev_->controls()); - ret = INVOKE(configure, config, controlInfo); + ret = INVOKE(configure, sensorInfo, config, controlInfo); if (ret == TestFail) return TestFail;