[libcamera-devel] [PATCH v6 10/11] libcamera: pipeline, ipa: vimc: Support the new IPC mechanism
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sun Dec 27 15:55:40 CET 2020
Hi Paul,
Thanks for your patch.
On 2020-12-24 17:16:12 +0900, Paul Elder wrote:
> Add support to vimc pipeline handler and IPA for the new IPC mechanism.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> ---
> Changes in v6:
> - move the enum and const from the header to the mojom file
> - remove the per-pipeline ControlInfoMap
> - since vimc.h has nothing in it anymore, remove it
> - use the namespace in the pipeline handler and IPA
>
> Changes in v5:
> - use the new generated header naming scheme
> - add dummy event, since event interfaces are now required to have at
> least one event
>
> Changes in v4:
> - rename Controls to controls
> - rename IPAVimcCallbackInterface to IPAVimcEventInterface
> - rename libcamera_generated_headers to libcamera_generated_ipa_headers
> in meson
> - use the new header name, vimc_ipa_interface.h
>
> Changes in v3:
> - change namespacing of base ControlInfoMap structure
>
> New in v2
> ---
> include/libcamera/ipa/meson.build | 1 +
> include/libcamera/ipa/vimc.h | 28 ---------------------
> include/libcamera/ipa/vimc.mojom | 24 ++++++++++++++++++
> src/ipa/vimc/meson.build | 2 +-
> src/ipa/vimc/vimc.cpp | 37 ++++++++++------------------
> src/libcamera/pipeline/vimc/vimc.cpp | 10 +++++---
> 6 files changed, 46 insertions(+), 56 deletions(-)
> delete mode 100644 include/libcamera/ipa/vimc.h
> create mode 100644 include/libcamera/ipa/vimc.mojom
>
> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> index 785c1241..67e3f049 100644
> --- a/include/libcamera/ipa/meson.build
> +++ b/include/libcamera/ipa/meson.build
> @@ -56,6 +56,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
>
> ipa_mojom_files = [
> 'raspberrypi.mojom',
> + 'vimc.mojom',
> ]
>
> ipa_mojoms = []
> diff --git a/include/libcamera/ipa/vimc.h b/include/libcamera/ipa/vimc.h
> deleted file mode 100644
> index 27a4a61d..00000000
> --- a/include/libcamera/ipa/vimc.h
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * vimc.h - Vimc Image Processing Algorithm module
> - */
> -
> -#ifndef __LIBCAMERA_IPA_VIMC_H__
> -#define __LIBCAMERA_IPA_VIMC_H__
> -
> -#ifndef __DOXYGEN__
> -
> -namespace libcamera {
> -
> -#define VIMC_IPA_FIFO_PATH "/tmp/libcamera_ipa_vimc_fifo"
> -
> -enum IPAOperationCode {
> - IPAOperationNone,
> - IPAOperationInit,
> - IPAOperationStart,
> - IPAOperationStop,
> -};
> -
> -} /* namespace libcamera */
> -
> -#endif /* __DOXYGEN__ */
> -
> -#endif /* __LIBCAMERA_IPA_VIMC_H__ */
> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> new file mode 100644
> index 00000000..165d9401
> --- /dev/null
> +++ b/include/libcamera/ipa/vimc.mojom
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +
> +module ipa.vimc;
> +
> +import "include/libcamera/ipa/core.mojom";
> +
> +const string VimcIPAFIFOPath = "/tmp/libcamera_ipa_vimc_fifo";
> +
> +enum IPAOperationCode {
> + IPAOperationNone,
> + IPAOperationInit,
> + IPAOperationStart,
> + IPAOperationStop,
> +};
> +
> +interface IPAVimcInterface {
> + init(IPASettings settings) => (int32 ret);
> + start() => (int32 ret);
> + stop();
> +};
> +
> +interface IPAVimcEventInterface {
> + dummyEvent(uint32 val);
> +};
> diff --git a/src/ipa/vimc/meson.build b/src/ipa/vimc/meson.build
> index 8c9df854..e982ebba 100644
> --- a/src/ipa/vimc/meson.build
> +++ b/src/ipa/vimc/meson.build
> @@ -3,7 +3,7 @@
> ipa_name = 'ipa_vimc'
>
> mod = shared_module(ipa_name,
> - 'vimc.cpp',
> + ['vimc.cpp', libcamera_generated_ipa_headers],
> name_prefix : '',
> include_directories : [ipa_includes, libipa_includes],
> dependencies : libcamera_dep,
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index 436a5559..13681d88 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -5,7 +5,7 @@
> * ipa_vimc.cpp - Vimc Image Processing Algorithm module
> */
>
> -#include <libcamera/ipa/vimc.h>
> +#include <libcamera/ipa/vimc_ipa_interface.h>
>
> #include <fcntl.h>
> #include <string.h>
> @@ -24,7 +24,7 @@ namespace libcamera {
>
> LOG_DEFINE_CATEGORY(IPAVimc)
>
> -class IPAVimc : public IPAInterface
> +class IPAVimc : public ipa::vimc::IPAVimcInterface
> {
> public:
> IPAVimc();
> @@ -32,22 +32,12 @@ public:
>
> int init(const IPASettings &settings) override;
>
> - int start(const IPAOperationData &data,
> - IPAOperationData *result) override;
> + int start() override;
> void stop() override;
>
> - void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
> - [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> - [[maybe_unused]] const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> - [[maybe_unused]] const IPAOperationData &ipaConfig,
> - [[maybe_unused]] IPAOperationData *result) override {}
> - void mapBuffers([[maybe_unused]] const std::vector<IPABuffer> &buffers) override {}
> - void unmapBuffers([[maybe_unused]] const std::vector<unsigned int> &ids) override {}
> - void processEvent([[maybe_unused]] const IPAOperationData &event) override {}
> -
> private:
> void initTrace();
> - void trace(enum IPAOperationCode operation);
> + void trace(enum ipa::vimc::IPAOperationCode operation);
>
> int fd_;
> };
> @@ -66,7 +56,7 @@ IPAVimc::~IPAVimc()
>
> int IPAVimc::init(const IPASettings &settings)
> {
> - trace(IPAOperationInit);
> + trace(ipa::vimc::IPAOperationInit);
>
> LOG(IPAVimc, Debug)
> << "initializing vimc IPA with configuration file "
> @@ -81,10 +71,9 @@ int IPAVimc::init(const IPASettings &settings)
> return 0;
> }
>
> -int IPAVimc::start([[maybe_unused]] const IPAOperationData &data,
> - [[maybe_unused]] IPAOperationData *result)
> +int IPAVimc::start()
> {
> - trace(IPAOperationStart);
> + trace(ipa::vimc::IPAOperationStart);
>
> LOG(IPAVimc, Debug) << "start vimc IPA!";
>
> @@ -93,7 +82,7 @@ int IPAVimc::start([[maybe_unused]] const IPAOperationData &data,
>
> void IPAVimc::stop()
> {
> - trace(IPAOperationStop);
> + trace(ipa::vimc::IPAOperationStop);
>
> LOG(IPAVimc, Debug) << "stop vimc IPA!";
> }
> @@ -101,11 +90,11 @@ void IPAVimc::stop()
> void IPAVimc::initTrace()
> {
> struct stat fifoStat;
> - int ret = stat(VIMC_IPA_FIFO_PATH, &fifoStat);
> + int ret = stat(ipa::vimc::VimcIPAFIFOPath.c_str(), &fifoStat);
> if (ret)
> return;
>
> - ret = ::open(VIMC_IPA_FIFO_PATH, O_WRONLY);
> + ret = ::open(ipa::vimc::VimcIPAFIFOPath.c_str(), O_WRONLY);
> if (ret < 0) {
> ret = errno;
> LOG(IPAVimc, Error) << "Failed to open vimc IPA test FIFO: "
> @@ -116,7 +105,7 @@ void IPAVimc::initTrace()
> fd_ = ret;
> }
>
> -void IPAVimc::trace(enum IPAOperationCode operation)
> +void IPAVimc::trace(enum ipa::vimc::IPAOperationCode operation)
> {
> if (fd_ < 0)
> return;
> @@ -141,9 +130,9 @@ const struct IPAModuleInfo ipaModuleInfo = {
> "vimc",
> };
>
> -struct ipa_context *ipaCreate()
> +IPAInterface *ipaCreate()
> {
> - return new IPAInterfaceWrapper(std::make_unique<IPAVimc>());
> + return new IPAVimc();
> }
> }
>
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 8bda746f..d258090e 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -34,6 +34,9 @@
> #include "libcamera/internal/v4l2_subdevice.h"
> #include "libcamera/internal/v4l2_videodevice.h"
>
> +#include <libcamera/ipa/vimc_ipa_interface.h>
> +#include <libcamera/ipa/vimc_ipa_proxy.h>
> +
> namespace libcamera {
>
> LOG_DEFINE_CATEGORY(VIMC)
> @@ -56,6 +59,8 @@ public:
> std::unique_ptr<V4L2VideoDevice> video_;
> std::unique_ptr<V4L2VideoDevice> raw_;
> Stream stream_;
> +
> + std::unique_ptr<ipa::vimc::IPAProxyVimc> ipa_;
> };
>
> class VimcCameraConfiguration : public CameraConfiguration
> @@ -311,8 +316,7 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con
> if (ret < 0)
> return ret;
>
> - IPAOperationData ipaData = {};
> - ret = data->ipa_->start(ipaData, nullptr);
> + ret = data->ipa_->start();
> if (ret) {
> data->video_->releaseBuffers();
> return ret;
> @@ -418,7 +422,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>
> std::unique_ptr<VimcCameraData> data = std::make_unique<VimcCameraData>(this, media);
>
> - data->ipa_ = IPAManager::createIPA(this, 0, 0);
> + data->ipa_ = IPAManager::createIPA<ipa::vimc::IPAProxyVimc>(this, 0, 0);
> if (data->ipa_ != nullptr) {
> std::string conf = data->ipa_->configurationFile("vimc.conf");
> data->ipa_->init(IPASettings{ conf });
> --
> 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