[libcamera-devel] [PATCH v4 14/37] libcamera: ipa_context_wrapper: Remove ipa_context_wrapper
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Nov 20 18:09:29 CET 2020
Hi Paul,
Thank you for the patch.
On Fri, Nov 06, 2020 at 07:36:44PM +0900, Paul Elder wrote:
> Since ipa_context has been replaced with custom IPAInterfaces, it is not
> longer needed. Remove it.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
The documentation of the IPAManager class still references the IPA
context wrapper and ipa_context. With that fixed (probably in 13/37),
squashing this patch with 13/37 or keeping it separate,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> No change in v4
>
> No change in v3
>
> New in v2
> ---
> .../libcamera/internal/ipa_context_wrapper.h | 52 ---
> include/libcamera/internal/meson.build | 1 -
> include/libcamera/ipa/ipa_interface.h | 106 -------
> src/libcamera/ipa_context_wrapper.cpp | 297 ------------------
> 4 files changed, 456 deletions(-)
> delete mode 100644 include/libcamera/internal/ipa_context_wrapper.h
> delete mode 100644 src/libcamera/ipa_context_wrapper.cpp
>
> diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h
> deleted file mode 100644
> index 8f767e84..00000000
> --- a/include/libcamera/internal/ipa_context_wrapper.h
> +++ /dev/null
> @@ -1,52 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * ipa_context_wrapper.h - Image Processing Algorithm context wrapper
> - */
> -#ifndef __LIBCAMERA_INTERNAL_IPA_CONTEXT_WRAPPER_H__
> -#define __LIBCAMERA_INTERNAL_IPA_CONTEXT_WRAPPER_H__
> -
> -#include <libcamera/ipa/ipa_interface.h>
> -
> -#include "libcamera/internal/control_serializer.h"
> -
> -namespace libcamera {
> -
> -class IPAContextWrapper final : public IPAInterface
> -{
> -public:
> - IPAContextWrapper(struct ipa_context *context);
> - ~IPAContextWrapper();
> -
> - 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;
> -
> - void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> - void unmapBuffers(const std::vector<unsigned int> &ids) override;
> -
> - virtual void processEvent(const IPAOperationData &data) override;
> -
> -private:
> - static void queue_frame_action(void *ctx, unsigned int frame,
> - struct ipa_operation_data &data);
> - static const struct ipa_callback_ops callbacks_;
> -
> - void doQueueFrameAction(unsigned int frame,
> - const IPAOperationData &data);
> -
> - struct ipa_context *ctx_;
> - IPAInterface *intf_;
> -
> - ControlSerializer serializer_;
> -};
> -
> -} /* namespace libcamera */
> -
> -#endif /* __LIBCAMERA_INTERNAL_IPA_CONTEXT_WRAPPER_H__ */
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 6500fe2a..592635a9 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -21,7 +21,6 @@ libcamera_internal_headers = files([
> 'event_dispatcher_poll.h',
> 'file.h',
> 'formats.h',
> - 'ipa_context_wrapper.h',
> 'ipa_manager.h',
> 'ipa_module.h',
> 'ipa_proxy.h',
> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
> index 322b7079..48766b22 100644
> --- a/include/libcamera/ipa/ipa_interface.h
> +++ b/include/libcamera/ipa/ipa_interface.h
> @@ -10,111 +10,6 @@
> #include <stddef.h>
> #include <stdint.h>
>
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -struct ipa_context {
> - const struct ipa_context_ops *ops;
> -};
> -
> -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;
> - unsigned int width;
> - unsigned int height;
> -};
> -
> -struct ipa_control_info_map {
> - unsigned int id;
> - const uint8_t *data;
> - size_t size;
> -};
> -
> -struct ipa_buffer_plane {
> - int dmabuf;
> - size_t length;
> -};
> -
> -struct ipa_buffer {
> - unsigned int id;
> - unsigned int num_planes;
> - struct ipa_buffer_plane planes[3];
> -};
> -
> -struct ipa_control_list {
> - const uint8_t *data;
> - unsigned int size;
> -};
> -
> -struct ipa_operation_data {
> - unsigned int operation;
> - const uint32_t *data;
> - unsigned int num_data;
> - const struct ipa_control_list *lists;
> - unsigned int num_lists;
> -};
> -
> -struct ipa_callback_ops {
> - void (*queue_frame_action)(void *cb_ctx, unsigned int frame,
> - struct ipa_operation_data &data);
> -};
> -
> -struct ipa_context_ops {
> - void (*destroy)(struct ipa_context *ctx);
> - void *(*get_interface)(struct ipa_context *ctx);
> - void (*init)(struct ipa_context *ctx,
> - const struct ipa_settings *settings);
> - int (*start)(struct ipa_context *ctx);
> - void (*stop)(struct ipa_context *ctx);
> - 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);
> - void (*map_buffers)(struct ipa_context *ctx,
> - const struct ipa_buffer *buffers,
> - size_t num_buffers);
> - void (*unmap_buffers)(struct ipa_context *ctx, const unsigned int *ids,
> - size_t num_buffers);
> - void (*process_event)(struct ipa_context *ctx,
> - const struct ipa_operation_data *data);
> -};
> -
> -struct ipa_context *ipaCreate();
> -
> -#ifdef __cplusplus
> -}
> -
> #include <map>
> #include <vector>
>
> @@ -170,6 +65,5 @@ public:
> };
>
> } /* namespace libcamera */
> -#endif
>
> #endif /* __LIBCAMERA_IPA_INTERFACE_H__ */
> diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
> deleted file mode 100644
> index 231300ce..00000000
> --- a/src/libcamera/ipa_context_wrapper.cpp
> +++ /dev/null
> @@ -1,297 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * ipa_context_wrapper.cpp - Image Processing Algorithm context wrapper
> - */
> -
> -#include "libcamera/internal/ipa_context_wrapper.h"
> -
> -#include <vector>
> -
> -#include <libcamera/controls.h>
> -
> -#include "libcamera/internal/byte_stream_buffer.h"
> -#include "libcamera/internal/camera_sensor.h"
> -#include "libcamera/internal/utils.h"
> -
> -/**
> - * \file ipa_context_wrapper.h
> - * \brief Image Processing Algorithm context wrapper
> - */
> -
> -namespace libcamera {
> -
> -/**
> - * \class IPAContextWrapper
> - * \brief Wrap an ipa_context and expose it as an IPAInterface
> - *
> - * The IPAContextWrapper class wraps an ipa_context, provided by an IPA module, and
> - * exposes an IPAInterface. This mechanism is used for IPAs that are not
> - * isolated in a separate process to allow direct calls from pipeline handler
> - * using the IPAInterface API instead of the lower-level ipa_context API.
> - *
> - * The IPAInterface methods are converted to the ipa_context API by translating
> - * all C++ arguments into plain C structures or byte arrays that contain no
> - * pointer, as required by the ipa_context API.
> - */
> -
> -/**
> - * \brief Construct an IPAContextWrapper instance that wraps the \a context
> - * \param[in] context The IPA module context
> - *
> - * Ownership of the \a context is passed to the IPAContextWrapper. The context remains
> - * valid for the whole lifetime of the wrapper and is destroyed automatically
> - * with it.
> - */
> -IPAContextWrapper::IPAContextWrapper(struct ipa_context *context)
> - : ctx_(context), intf_(nullptr)
> -{
> - if (!ctx_)
> - return;
> -
> - bool forceCApi = !!utils::secure_getenv("LIBCAMERA_IPA_FORCE_C_API");
> -
> - if (!forceCApi && ctx_ && ctx_->ops->get_interface) {
> - intf_ = reinterpret_cast<IPAInterface *>(ctx_->ops->get_interface(ctx_));
> - intf_->queueFrameAction.connect(this, &IPAContextWrapper::doQueueFrameAction);
> - return;
> - }
> -
> - ctx_->ops->register_callbacks(ctx_, &IPAContextWrapper::callbacks_,
> - this);
> -}
> -
> -IPAContextWrapper::~IPAContextWrapper()
> -{
> - if (!ctx_)
> - return;
> -
> - ctx_->ops->destroy(ctx_);
> -}
> -
> -int IPAContextWrapper::init(const IPASettings &settings)
> -{
> - if (intf_)
> - return intf_->init(settings);
> -
> - if (!ctx_)
> - return 0;
> -
> - struct ipa_settings c_settings;
> - c_settings.configuration_file = settings.configurationFile.c_str();
> -
> - ctx_->ops->init(ctx_, &c_settings);
> -
> - return 0;
> -}
> -
> -int IPAContextWrapper::start()
> -{
> - if (intf_)
> - return intf_->start();
> -
> - if (!ctx_)
> - return 0;
> -
> - return ctx_->ops->start(ctx_);
> -}
> -
> -void IPAContextWrapper::stop()
> -{
> - if (intf_)
> - return intf_->stop();
> -
> - if (!ctx_)
> - return;
> -
> - 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)
> -{
> - if (intf_)
> - return intf_->configure(sensorInfo, streamConfig,
> - entityControls, ipaConfig, result);
> -
> - 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()];
> -
> - unsigned int i = 0;
> - for (const auto &stream : streamConfig) {
> - struct ipa_stream *c_stream = &c_streams[i];
> - unsigned int id = stream.first;
> - const IPAStream &ipaStream = stream.second;
> -
> - c_stream->id = id;
> - c_stream->pixel_format = ipaStream.pixelFormat;
> - c_stream->width = ipaStream.size.width;
> - c_stream->height = ipaStream.size.height;
> -
> - ++i;
> - }
> -
> - /* Translate the IPA entity controls map. */
> - struct ipa_control_info_map c_info_maps[entityControls.size()];
> - std::vector<std::vector<uint8_t>> data(entityControls.size());
> -
> - i = 0;
> - for (const auto &info : entityControls) {
> - struct ipa_control_info_map &c_info_map = c_info_maps[i];
> - unsigned int id = info.first;
> - const ControlInfoMap &infoMap = info.second;
> -
> - size_t infoMapSize = serializer_.binarySize(infoMap);
> - data[i].resize(infoMapSize);
> - ByteStreamBuffer byteStream(data[i].data(), data[i].size());
> - serializer_.serialize(infoMap, byteStream);
> -
> - c_info_map.id = id;
> - c_info_map.data = byteStream.base();
> - c_info_map.size = byteStream.size();
> -
> - ++i;
> - }
> -
> - /* \todo Translate the ipaConfig and reponse */
> - ctx_->ops->configure(ctx_, &sensor_info, c_streams, streamConfig.size(),
> - c_info_maps, entityControls.size());
> -}
> -
> -void IPAContextWrapper::mapBuffers(const std::vector<IPABuffer> &buffers)
> -{
> - if (intf_)
> - return intf_->mapBuffers(buffers);
> -
> - if (!ctx_)
> - return;
> -
> - struct ipa_buffer c_buffers[buffers.size()];
> -
> - for (unsigned int i = 0; i < buffers.size(); ++i) {
> - struct ipa_buffer &c_buffer = c_buffers[i];
> - const IPABuffer &buffer = buffers[i];
> - const std::vector<FrameBuffer::Plane> &planes = buffer.planes;
> -
> - c_buffer.id = buffer.id;
> - c_buffer.num_planes = planes.size();
> -
> - for (unsigned int j = 0; j < planes.size(); ++j) {
> - const FrameBuffer::Plane &plane = planes[j];
> - c_buffer.planes[j].dmabuf = plane.fd.fd();
> - c_buffer.planes[j].length = plane.length;
> - }
> - }
> -
> - ctx_->ops->map_buffers(ctx_, c_buffers, buffers.size());
> -}
> -
> -void IPAContextWrapper::unmapBuffers(const std::vector<unsigned int> &ids)
> -{
> - if (intf_)
> - return intf_->unmapBuffers(ids);
> -
> - if (!ctx_)
> - return;
> -
> - ctx_->ops->unmap_buffers(ctx_, ids.data(), ids.size());
> -}
> -
> -void IPAContextWrapper::processEvent(const IPAOperationData &data)
> -{
> - if (intf_)
> - return intf_->processEvent(data);
> -
> - if (!ctx_)
> - return;
> -
> - struct ipa_operation_data c_data;
> - c_data.operation = data.operation;
> - c_data.data = data.data.data();
> - c_data.num_data = data.data.size();
> -
> - struct ipa_control_list control_lists[data.controls.size()];
> - c_data.lists = control_lists;
> - c_data.num_lists = data.controls.size();
> -
> - std::size_t listsSize = 0;
> - for (const auto &list : data.controls)
> - listsSize += serializer_.binarySize(list);
> -
> - std::vector<uint8_t> binaryData(listsSize);
> - ByteStreamBuffer byteStreamBuffer(binaryData.data(), listsSize);
> -
> - unsigned int i = 0;
> - for (const auto &list : data.controls) {
> - struct ipa_control_list &c_list = control_lists[i];
> - c_list.size = serializer_.binarySize(list);
> - ByteStreamBuffer b = byteStreamBuffer.carveOut(c_list.size);
> -
> - serializer_.serialize(list, b);
> -
> - c_list.data = b.base();
> - }
> -
> - ctx_->ops->process_event(ctx_, &c_data);
> -}
> -
> -void IPAContextWrapper::doQueueFrameAction(unsigned int frame,
> - const IPAOperationData &data)
> -{
> - IPAInterface::queueFrameAction.emit(frame, data);
> -}
> -
> -void IPAContextWrapper::queue_frame_action(void *ctx, unsigned int frame,
> - struct ipa_operation_data &data)
> -{
> - IPAContextWrapper *_this = static_cast<IPAContextWrapper *>(ctx);
> - IPAOperationData opData;
> -
> - opData.operation = data.operation;
> - for (unsigned int i = 0; i < data.num_data; ++i)
> - opData.data.push_back(data.data[i]);
> -
> - for (unsigned int i = 0; i < data.num_lists; ++i) {
> - const struct ipa_control_list &c_list = data.lists[i];
> - ByteStreamBuffer b(c_list.data, c_list.size);
> - opData.controls.push_back(_this->serializer_.deserialize<ControlList>(b));
> - }
> -
> - _this->doQueueFrameAction(frame, opData);
> -}
> -
> -#ifndef __DOXYGEN__
> -/*
> - * This construct confuses Doxygen and makes it believe that all members of the
> - * operations is a member of IPAContextWrapper. It must thus be hidden.
> - */
> -const struct ipa_callback_ops IPAContextWrapper::callbacks_ = {
> - .queue_frame_action = &IPAContextWrapper::queue_frame_action,
> -};
> -#endif
> -
> -} /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list