[libcamera-devel] [PATCH v4 27/37] libcamera: pipeline, ipa: vimc: Support the new IPC mechanism
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Nov 26 16:44:19 CET 2020
Hi Paul,
On Fri, Nov 13, 2020 at 03:37:46PM +0900, paul.elder at ideasonboard.com wrote:
> On Thu, Nov 12, 2020 at 12:18:52PM +0100, Niklas Söderlund wrote:
> > On 2020-11-06 19:36:57 +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>
> > >
> > > ---
> > > 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 | 8 ++++++++
> > > include/libcamera/ipa/vimc.mojom | 12 ++++++++++++
> > > src/ipa/vimc/meson.build | 2 +-
> > > src/ipa/vimc/vimc.cpp | 16 ++++------------
> > > src/libcamera/pipeline/vimc/vimc.cpp | 8 +++++++-
> > > 6 files changed, 33 insertions(+), 14 deletions(-)
> > > create mode 100644 include/libcamera/ipa/vimc.mojom
> > >
> > > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> > > index d49dff7b..70f5dfc8 100644
> > > --- a/include/libcamera/ipa/meson.build
> > > +++ b/include/libcamera/ipa/meson.build
> > > @@ -26,6 +26,7 @@ ipa_mojom_core = custom_target(core_mojom_file.split('.')[0] + '_mojom_module',
> > >
> > > ipa_mojom_files = [
> > > 'raspberrypi.mojom',
> > > + 'vimc.mojom',
> > > ]
> > >
> > > ipa_mojoms = []
> > > diff --git a/include/libcamera/ipa/vimc.h b/include/libcamera/ipa/vimc.h
> > > index 27a4a61d..2c5f3f8f 100644
> > > --- a/include/libcamera/ipa/vimc.h
> > > +++ b/include/libcamera/ipa/vimc.h
> > > @@ -8,6 +8,8 @@
> > > #ifndef __LIBCAMERA_IPA_VIMC_H__
> > > #define __LIBCAMERA_IPA_VIMC_H__
> > >
> > > +#include <libcamera/controls.h>
> > > +
> > > #ifndef __DOXYGEN__
> > >
> > > namespace libcamera {
> > > @@ -21,6 +23,12 @@ enum IPAOperationCode {
> > > IPAOperationStop,
> > > };
> > >
> > > +namespace Vimc {
> > > +
> > > +static ControlInfoMap controls;
> > > +
> > > +}
> >
> > Maybe I missed something in a generation script but is this strictly
> > needed or just done in preparation for the future?
>
> The reason I have this is to designate a default ControlInfoMap for
> ControlLists in the de/serializer, in the case that the ControlList
> doesn't have an info map.
>
> I think there was a short discussion about getting rid of this...? I'm
> not sure where it went though.
I'm not sure either, but I would really really like to avoid adding this
:-) It's not taking the right direction.
> > > +
> > > } /* namespace libcamera */
> > >
> > > #endif /* __DOXYGEN__ */
> > > diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> > > new file mode 100644
> > > index 00000000..d8e9b504
> > > --- /dev/null
> > > +++ b/include/libcamera/ipa/vimc.mojom
> > > @@ -0,0 +1,12 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +
> > > +import "include/libcamera/ipa/core.mojom";
> > > +
> > > +interface IPAVimcInterface {
> > > + init(IPASettings settings) => (int32 ret);
> > > + start() => (int32 ret);
> > > + stop();
> > > +};
> > > +
> > > +interface IPAVimcEventInterface {
> > > +};
> > > 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 cf841135..fbf7325e 100644
> > > --- a/src/ipa/vimc/vimc.cpp
> > > +++ b/src/ipa/vimc/vimc.cpp
> > > @@ -6,6 +6,7 @@
> > > */
> > >
> > > #include <libcamera/ipa/vimc.h>
> > > +#include <libcamera/ipa/vimc_ipa_interface.h>
> > >
> > > #include <fcntl.h>
> > > #include <string.h>
> > > @@ -26,7 +27,7 @@ namespace libcamera {
> > >
> > > LOG_DEFINE_CATEGORY(IPAVimc)
> > >
> > > -class IPAVimc : public IPAInterface
> > > +class IPAVimc : public IPAVimcInterface
> > > {
> > > public:
> > > IPAVimc();
> > > @@ -37,15 +38,6 @@ public:
> > > 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);
> > > @@ -141,9 +133,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 7416c37c..9dddb891 100644
> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > > @@ -34,6 +34,10 @@
> > > #include "libcamera/internal/v4l2_subdevice.h"
> > > #include "libcamera/internal/v4l2_videodevice.h"
> > >
> > > +#include <libcamera/ipa/vimc.h>
> > > +#include <libcamera/ipa/vimc_ipa_interface.h>
> > > +#include <libcamera/ipa/ipa_proxy_vimc.h>
I before V :-)
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > +
> > > namespace libcamera {
> > >
> > > LOG_DEFINE_CATEGORY(VIMC)
> > > @@ -67,6 +71,8 @@ public:
> > > V4L2VideoDevice *video_;
> > > V4L2VideoDevice *raw_;
> > > Stream stream_;
> > > +
> > > + std::unique_ptr<IPAProxyVimc> ipa_;
> > > };
> > >
> > > class VimcCameraConfiguration : public CameraConfiguration
> > > @@ -428,7 +434,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<IPAProxyVimc>(this, 0, 0);
> > > if (data->ipa_ != nullptr) {
> > > std::string conf = data->ipa_->configurationFile("vimc.conf");
> > > data->ipa_->init(IPASettings{ conf });
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list