[libcamera-devel,22/23] ipa: remove libipa

9632 diff mbox series

As every pipeline and have its own proxy, IPAInterfaceWrapper is no
longer necessary. Since it's the only member of libipa, remove libipa
completely.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/ipa/libipa/ipa_interface_wrapper.cpp | 285 -----------------------
 src/ipa/libipa/ipa_interface_wrapper.h   |  61 -----
 src/ipa/libipa/meson.build               |  15 --
 src/ipa/meson.build                      |   2 -
 src/ipa/raspberrypi/meson.build          |   2 -
 src/ipa/raspberrypi/raspberrypi.cpp      |   2 -
 6 files changed, 367 deletions(-)
 delete mode 100644 src/ipa/libipa/ipa_interface_wrapper.cpp
 delete mode 100644 src/ipa/libipa/ipa_interface_wrapper.h
 delete mode 100644 src/ipa/libipa/meson.build

Hi Paul,

Thanks for your patch.

On 2020-09-15 23:20:37 +0900, Paul Elder wrote:
> As every pipeline and have its own proxy, IPAInterfaceWrapper is no
> longer necessary. Since it's the only member of libipa, remove libipa
> completely.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

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

> ---
>  src/ipa/libipa/ipa_interface_wrapper.cpp | 285 -----------------------
>  src/ipa/libipa/ipa_interface_wrapper.h   |  61 -----
>  src/ipa/libipa/meson.build               |  15 --
>  src/ipa/meson.build                      |   2 -
>  src/ipa/raspberrypi/meson.build          |   2 -
>  src/ipa/raspberrypi/raspberrypi.cpp      |   2 -
>  6 files changed, 367 deletions(-)
>  delete mode 100644 src/ipa/libipa/ipa_interface_wrapper.cpp
>  delete mode 100644 src/ipa/libipa/ipa_interface_wrapper.h
>  delete mode 100644 src/ipa/libipa/meson.build
> 
> diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
> deleted file mode 100644
> index cee532e3..00000000
> --- a/src/ipa/libipa/ipa_interface_wrapper.cpp
> +++ /dev/null
> @@ -1,285 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * ipa_interface_wrapper.cpp - Image Processing Algorithm interface wrapper
> - */
> -
> -#include "ipa_interface_wrapper.h"
> -
> -#include <map>
> -#include <string.h>
> -#include <unistd.h>
> -#include <vector>
> -
> -#include <libcamera/ipa/ipa_interface.h>
> -
> -#include "libcamera/internal/byte_stream_buffer.h"
> -#include "libcamera/internal/camera_sensor.h"
> -
> -/**
> - * \file ipa_interface_wrapper.h
> - * \brief Image Processing Algorithm interface wrapper
> - */
> -
> -namespace libcamera {
> -
> -/**
> - * \class IPAInterfaceWrapper
> - * \brief Wrap an IPAInterface and expose it as an ipa_context
> - *
> - * This class implements the ipa_context API based on a provided IPAInterface.
> - * It helps IPAs that implement the IPAInterface API to provide the external
> - * ipa_context API.
> - *
> - * To use the wrapper, an IPA module simple creates a new instance of its
> - * IPAInterface implementation, and passes it to the constructor of the
> - * IPAInterfaceWrapper. As IPAInterfaceWrapper inherits from ipa_context, the
> - * constructed wrapper can then be directly returned from the IPA module's
> - * ipaCreate() function.
> - *
> - * \code{.cpp}
> - * class MyIPA : public IPAInterface
> - * {
> - * 	...
> - * };
> - *
> - * struct ipa_context *ipaCreate()
> - * {
> - * 	return new IPAInterfaceWrapper(std::make_unique<MyIPA>());
> - * }
> - * \endcode
> - *
> - * The wrapper takes ownership of the IPAInterface and will automatically
> - * delete it when the wrapper is destroyed.
> - */
> -
> -/**
> - * \brief Construct an IPAInterfaceWrapper wrapping \a interface
> - * \param[in] interface The interface to wrap
> - */
> -IPAInterfaceWrapper::IPAInterfaceWrapper(std::unique_ptr<IPAInterface> interface)
> -	: ipa_(std::move(interface)), callbacks_(nullptr), cb_ctx_(nullptr)
> -{
> -	ops = &operations_;
> -
> -	ipa_->queueFrameAction.connect(this, &IPAInterfaceWrapper::queueFrameAction);
> -}
> -
> -void IPAInterfaceWrapper::destroy(struct ipa_context *_ctx)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> -
> -	delete ctx;
> -}
> -
> -void *IPAInterfaceWrapper::get_interface(struct ipa_context *_ctx)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> -
> -	return ctx->ipa_.get();
> -}
> -
> -void IPAInterfaceWrapper::init(struct ipa_context *_ctx,
> -			       const struct ipa_settings *settings)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> -
> -	IPASettings ipaSettings{
> -		.configurationFile = settings->configuration_file
> -	};
> -	ctx->ipa_->init(ipaSettings);
> -}
> -
> -int IPAInterfaceWrapper::start(struct ipa_context *_ctx)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> -
> -	return ctx->ipa_->start();
> -}
> -
> -void IPAInterfaceWrapper::stop(struct ipa_context *_ctx)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> -
> -	ctx->ipa_->stop();
> -}
> -
> -void IPAInterfaceWrapper::register_callbacks(struct ipa_context *_ctx,
> -					     const struct ipa_callback_ops *callbacks,
> -					     void *cb_ctx)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> -
> -	ctx->callbacks_ = callbacks;
> -	ctx->cb_ctx_ = cb_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,
> -				    unsigned int num_maps)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_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;
> -
> -	for (unsigned int i = 0; i < num_streams; ++i) {
> -		const struct ipa_stream &stream = streams[i];
> -
> -		ipaStreams[stream.id] = {
> -			stream.pixel_format,
> -			Size(stream.width, stream.height),
> -		};
> -	}
> -
> -	/* Translate the IPA entity controls map. */
> -	std::map<unsigned int, const ControlInfoMap &> entityControls;
> -	std::map<unsigned int, ControlInfoMap> infoMaps;
> -
> -	for (unsigned int i = 0; i < num_maps; ++i) {
> -		const struct ipa_control_info_map &ipa_map = maps[i];
> -		ByteStreamBuffer byteStream(ipa_map.data, ipa_map.size);
> -		unsigned int id = ipa_map.id;
> -
> -		infoMaps[id] = ctx->serializer_.deserialize<ControlInfoMap>(byteStream);
> -		entityControls.emplace(id, infoMaps[id]);
> -	}
> -
> -	/* \todo Translate the ipaConfig and result. */
> -	IPAOperationData ipaConfig;
> -	ctx->ipa_->configure(sensorInfo, ipaStreams, entityControls, ipaConfig,
> -			     nullptr);
> -}
> -
> -void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,
> -				      const struct ipa_buffer *_buffers,
> -				      size_t num_buffers)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> -	std::vector<IPABuffer> buffers(num_buffers);
> -
> -	for (unsigned int i = 0; i < num_buffers; ++i) {
> -		const struct ipa_buffer &_buffer = _buffers[i];
> -		IPABuffer &buffer = buffers[i];
> -		std::vector<FrameBuffer::Plane> &planes = buffer.planes;
> -
> -		buffer.id = _buffer.id;
> -
> -		planes.resize(_buffer.num_planes);
> -		for (unsigned int j = 0; j < _buffer.num_planes; ++j) {
> -			planes[j].fd = FileDescriptor(_buffer.planes[j].dmabuf);
> -			planes[j].length = _buffer.planes[j].length;
> -		}
> -	}
> -
> -	ctx->ipa_->mapBuffers(buffers);
> -}
> -
> -void IPAInterfaceWrapper::unmap_buffers(struct ipa_context *_ctx,
> -					const unsigned int *_ids,
> -					size_t num_buffers)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> -	std::vector<unsigned int> ids(_ids, _ids + num_buffers);
> -	ctx->ipa_->unmapBuffers(ids);
> -}
> -
> -void IPAInterfaceWrapper::process_event(struct ipa_context *_ctx,
> -					const struct ipa_operation_data *data)
> -{
> -	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> -	IPAOperationData opData;
> -
> -	opData.operation = data->operation;
> -
> -	opData.data.resize(data->num_data);
> -	memcpy(opData.data.data(), data->data,
> -	       data->num_data * sizeof(*data->data));
> -
> -	opData.controls.resize(data->num_lists);
> -	for (unsigned int i = 0; i < data->num_lists; ++i) {
> -		const struct ipa_control_list *c_list = &data->lists[i];
> -		ByteStreamBuffer byteStream(c_list->data, c_list->size);
> -		opData.controls[i] = ctx->serializer_.deserialize<ControlList>(byteStream);
> -	}
> -
> -	ctx->ipa_->processEvent(opData);
> -}
> -
> -void IPAInterfaceWrapper::queueFrameAction(unsigned int frame,
> -					   const IPAOperationData &data)
> -{
> -	if (!callbacks_)
> -		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();
> -	}
> -
> -	callbacks_->queue_frame_action(cb_ctx_, frame, c_data);
> -}
> -
> -#ifndef __DOXYGEN__
> -/*
> - * This construct confuses Doygen and makes it believe that all members of the
> - * operations is a member of IPAInterfaceWrapper. It must thus be hidden.
> - */
> -const struct ipa_context_ops IPAInterfaceWrapper::operations_ = {
> -	.destroy = &IPAInterfaceWrapper::destroy,
> -	.get_interface = &IPAInterfaceWrapper::get_interface,
> -	.init = &IPAInterfaceWrapper::init,
> -	.start = &IPAInterfaceWrapper::start,
> -	.stop = &IPAInterfaceWrapper::stop,
> -	.register_callbacks = &IPAInterfaceWrapper::register_callbacks,
> -	.configure = &IPAInterfaceWrapper::configure,
> -	.map_buffers = &IPAInterfaceWrapper::map_buffers,
> -	.unmap_buffers = &IPAInterfaceWrapper::unmap_buffers,
> -	.process_event = &IPAInterfaceWrapper::process_event,
> -};
> -#endif
> -
> -} /* namespace libcamera */
> diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h
> deleted file mode 100644
> index a1c70159..00000000
> --- a/src/ipa/libipa/ipa_interface_wrapper.h
> +++ /dev/null
> @@ -1,61 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * ipa_interface_wrapper.h - Image Processing Algorithm interface wrapper
> - */
> -#ifndef __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__
> -#define __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__
> -
> -#include <memory>
> -
> -#include <libcamera/ipa/ipa_interface.h>
> -
> -#include "libcamera/internal/control_serializer.h"
> -
> -namespace libcamera {
> -
> -class IPAInterfaceWrapper : public ipa_context
> -{
> -public:
> -	IPAInterfaceWrapper(std::unique_ptr<IPAInterface> interface);
> -
> -private:
> -	static void destroy(struct ipa_context *ctx);
> -	static void *get_interface(struct ipa_context *ctx);
> -	static void init(struct ipa_context *ctx,
> -			 const struct ipa_settings *settings);
> -	static int start(struct ipa_context *ctx);
> -	static void stop(struct ipa_context *ctx);
> -	static void register_callbacks(struct ipa_context *ctx,
> -				       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,
> -			      unsigned int num_maps);
> -	static void map_buffers(struct ipa_context *ctx,
> -				const struct ipa_buffer *c_buffers,
> -				size_t num_buffers);
> -	static void unmap_buffers(struct ipa_context *ctx,
> -				  const unsigned int *ids,
> -				  size_t num_buffers);
> -	static void process_event(struct ipa_context *ctx,
> -				  const struct ipa_operation_data *data);
> -
> -	static const struct ipa_context_ops operations_;
> -
> -	void queueFrameAction(unsigned int frame, const IPAOperationData &data);
> -
> -	std::unique_ptr<IPAInterface> ipa_;
> -	const struct ipa_callback_ops *callbacks_;
> -	void *cb_ctx_;
> -
> -	ControlSerializer serializer_;
> -};
> -
> -} /* namespace libcamera */
> -
> -#endif /* __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__ */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> deleted file mode 100644
> index 22626405..00000000
> --- a/src/ipa/libipa/meson.build
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -# SPDX-License-Identifier: CC0-1.0
> -
> -libipa_headers = files([
> -    'ipa_interface_wrapper.h',
> -])
> -
> -libipa_sources = files([
> -    'ipa_interface_wrapper.cpp',
> -])
> -
> -libipa_includes = include_directories('..')
> -
> -libipa = static_library('ipa', libipa_sources,
> -                        include_directories : ipa_includes,
> -                        dependencies : libcamera_dep)
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index 5a5de267..b11e5d23 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -15,8 +15,6 @@ config_h.set('IPA_CONFIG_DIR',
>  config_h.set('IPA_MODULE_DIR',
>               '"' + join_paths(get_option('prefix'), ipa_install_dir) + '"')
>  
> -subdir('libipa')
> -
>  ipa_sign = files('ipa-sign.sh')
>  
>  ipas = ['raspberrypi', 'rkisp1', 'vimc']
> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> index 9445cd09..e912c668 100644
> --- a/src/ipa/raspberrypi/meson.build
> +++ b/src/ipa/raspberrypi/meson.build
> @@ -10,7 +10,6 @@ rpi_ipa_deps = [
>  
>  rpi_ipa_includes = [
>      ipa_includes,
> -    libipa_includes,
>      include_directories('controller')
>  ]
>  
> @@ -46,7 +45,6 @@ mod = shared_module(ipa_name,
>                      name_prefix : '',
>                      include_directories : rpi_ipa_includes,
>                      dependencies : rpi_ipa_deps,
> -                    link_with : libipa,
>                      install : true,
>                      install_dir : ipa_install_dir)
>  
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index e09caa8d..ec04b23d 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -23,8 +23,6 @@
>  #include <libcamera/request.h>
>  #include <libcamera/span.h>
>  
> -#include <libipa/ipa_interface_wrapper.h>
> -
>  #include "libcamera/internal/camera_sensor.h"
>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/utils.h"
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel