[libcamera-devel] [PATCH v4 14/37] libcamera: ipa_context_wrapper: Remove ipa_context_wrapper

Niklas Söderlund niklas.soderlund at ragnatech.se
Thu Nov 12 11:56:03 CET 2020


Hi Paul,

Thanks for your work.

On 2020-11-06 19:36:44 +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>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> 
> ---
> 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 */
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund


More information about the libcamera-devel mailing list