[{"id":13685,"web_url":"https://patchwork.libcamera.org/comment/13685/","msgid":"<20201112111852.GG1480295@oden.dyn.berto.se>","date":"2020-11-12T11:18:52","subject":"Re: [libcamera-devel] [PATCH v4 27/37] libcamera: pipeline,\n\tipa: vimc: Support the new IPC mechanism","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your patch.\n\nOn 2020-11-06 19:36:57 +0900, Paul Elder wrote:\n> Add support to vimc pipeline handler and IPA for the new IPC mechanism.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v4:\n> - rename Controls to controls\n> - rename IPAVimcCallbackInterface to IPAVimcEventInterface\n> - rename libcamera_generated_headers to libcamera_generated_ipa_headers\n> in meson\n> - use the new header name, vimc_ipa_interface.h\n> \n> Changes in v3:\n> - change namespacing of base ControlInfoMap structure\n> \n> New in v2\n> ---\n> include/libcamera/ipa/meson.build | 1 +\n> include/libcamera/ipa/vimc.h | 8 ++++++++\n> include/libcamera/ipa/vimc.mojom | 12 ++++++++++++\n> src/ipa/vimc/meson.build | 2 +-\n> src/ipa/vimc/vimc.cpp | 16 ++++------------\n> src/libcamera/pipeline/vimc/vimc.cpp | 8 +++++++-\n> 6 files changed, 33 insertions(+), 14 deletions(-)\n> create mode 100644 include/libcamera/ipa/vimc.mojom\n> \n> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> index d49dff7b..70f5dfc8 100644\n> --- a/include/libcamera/ipa/meson.build\n> +++ b/include/libcamera/ipa/meson.build\n> @@ -26,6 +26,7 @@ ipa_mojom_core = custom_target(core_mojom_file.split('.')[0] + '_mojom_module',\n> \n> ipa_mojom_files = [\n> 'raspberrypi.mojom',\n> + 'vimc.mojom',\n> ]\n> \n> ipa_mojoms = []\n> diff --git a/include/libcamera/ipa/vimc.h b/include/libcamera/ipa/vimc.h\n> index 27a4a61d..2c5f3f8f 100644\n> --- a/include/libcamera/ipa/vimc.h\n> +++ b/include/libcamera/ipa/vimc.h\n> @@ -8,6 +8,8 @@\n> #ifndef __LIBCAMERA_IPA_VIMC_H__\n> #define __LIBCAMERA_IPA_VIMC_H__\n> \n> +#include \n> +\n> #ifndef __DOXYGEN__\n> \n> namespace libcamera {\n> @@ -21,6 +23,12 @@ enum IPAOperationCode {\n> \tIPAOperationStop,\n> };\n> \n> +namespace Vimc {\n> +\n> +static ControlInfoMap controls;\n> +\n> +}\n\nMaybe I missed something in a generation script but is this strictly \nneeded or just done in preparation for the future?\n\n> +\n> } /* namespace libcamera */\n> \n> #endif /* __DOXYGEN__ */\n> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n> new file mode 100644\n> index 00000000..d8e9b504\n> --- /dev/null\n> +++ b/include/libcamera/ipa/vimc.mojom\n> @@ -0,0 +1,12 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +\n> +import \"include/libcamera/ipa/core.mojom\";\n> +\n> +interface IPAVimcInterface {\n> +\tinit(IPASettings settings) => (int32 ret);\n> +\tstart() => (int32 ret);\n> +\tstop();\n> +};\n> +\n> +interface IPAVimcEventInterface {\n> +};\n> diff --git a/src/ipa/vimc/meson.build b/src/ipa/vimc/meson.build\n> index 8c9df854..e982ebba 100644\n> --- a/src/ipa/vimc/meson.build\n> +++ b/src/ipa/vimc/meson.build\n> @@ -3,7 +3,7 @@\n> ipa_name = 'ipa_vimc'\n> \n> mod = shared_module(ipa_name,\n> - 'vimc.cpp',\n> + ['vimc.cpp', libcamera_generated_ipa_headers],\n> name_prefix : '',\n> include_directories : [ipa_includes, libipa_includes],\n> dependencies : libcamera_dep,\n> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> index cf841135..fbf7325e 100644\n> --- a/src/ipa/vimc/vimc.cpp\n> +++ b/src/ipa/vimc/vimc.cpp\n> @@ -6,6 +6,7 @@\n> */\n> \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -26,7 +27,7 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(IPAVimc)\n> \n> -class IPAVimc : public IPAInterface\n> +class IPAVimc : public IPAVimcInterface\n> {\n> public:\n> \tIPAVimc();\n> @@ -37,15 +38,6 @@ public:\n> \tint start() override;\n> \tvoid stop() override;\n> \n> -\tvoid configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,\n> -\t\t [[maybe_unused]] const std::map &streamConfig,\n> -\t\t [[maybe_unused]] const std::map &entityControls,\n> -\t\t [[maybe_unused]] const IPAOperationData &ipaConfig,\n> -\t\t [[maybe_unused]] IPAOperationData *result) override {}\n> -\tvoid mapBuffers([[maybe_unused]] const std::vector &buffers) override {}\n> -\tvoid unmapBuffers([[maybe_unused]] const std::vector &ids) override {}\n> -\tvoid processEvent([[maybe_unused]] const IPAOperationData &event) override {}\n> -\n> private:\n> \tvoid initTrace();\n> \tvoid trace(enum IPAOperationCode operation);\n> @@ -141,9 +133,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \t\"vimc\",\n> };\n> \n> -struct ipa_context *ipaCreate()\n> +IPAInterface *ipaCreate()\n> {\n> -\treturn new IPAInterfaceWrapper(std::make_unique());\n> +\treturn new IPAVimc();\n> }\n> }\n> \n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 7416c37c..9dddb891 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -34,6 +34,10 @@\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> \n> +#include \n> +#include \n> +#include \n> +\n> namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(VIMC)\n> @@ -67,6 +71,8 @@ public:\n> \tV4L2VideoDevice *video_;\n> \tV4L2VideoDevice *raw_;\n> \tStream stream_;\n> +\n> +\tstd::unique_ptr ipa_;\n> };\n> \n> class VimcCameraConfiguration : public CameraConfiguration\n> @@ -428,7 +434,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \n> \tstd::unique_ptr data = std::make_unique(this, media);\n> \n> -\tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n> +\tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n> \tif (data->ipa_ != nullptr) {\n> \t\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> \t\tdata->ipa_->init(IPASettings{ conf });\n> -- \n> 2.27.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 04DF1BE082\n\tfor ;\n\tThu, 12 Nov 2020 11:18:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84C4863163;\n\tThu, 12 Nov 2020 12:18:56 +0100 (CET)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EEAB46310F\n\tfor ;\n\tThu, 12 Nov 2020 12:18:54 +0100 (CET)","by mail-lj1-x233.google.com with SMTP id v20so5656131ljk.8\n\tfor ;\n\tThu, 12 Nov 2020 03:18:54 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tn20sm521131lfl.249.2020.11.12.03.18.53\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 12 Nov 2020 03:18:53 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"FgmhXhmR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=bBFKNR3YafcnxcXuqe6JRETPhNTWiQipgn9xGBu3Gqs=;\n\tb=FgmhXhmREiMd0WK0ESd6Fs1cYtattJnwInp5OUdtp6Nzptibg9uux8b5B4FgvI4jfr\n\tOTKvcFoZRioKUKhbz26VoupxnCc1pdYuHrIsTE9WVE6zfg3y7NTLfCFOTJWzm/X8Q/Bv\n\thX0XavE4VstU4G2InjQJNorCWCAnFKyJw4QK9LFkxiR7VnDkszJKUYwHrop4lrQaN7hZ\n\t+7529xqHAbhgmc2bGRNDpnAbtklotOMBqdOAGV7n3bWRjyoFnAc+vDkKctZmtC3Vpm+9\n\tUu0Vqg7xL2t0QcmO8e9oHNV7XcALinwnpDKyQcc0FjeRgTW+DT19Lz5UJU9mWI4Kty6V\n\t0m7g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=bBFKNR3YafcnxcXuqe6JRETPhNTWiQipgn9xGBu3Gqs=;\n\tb=pEkEWxIBzE+D9NXl2LU4NoPUt1oE5epHu7Q20JzXQxbN2D5xHm/ojxhabAKm/v4ZlK\n\tNYAk0uvZa8Bcc2qM1M7HzbLAppD7D+UQ9f60YpCsE7ZMfFG71xmWnSSt0ZMZg5F6YiCw\n\tG0gRn+mLYA7Ho+lWkN42lP1mU+hu87m7Plb2SaKZs01QvnVg8mwqXA2E3a2IITCrvcix\n\teQ68R0pI0tWSuvUy+EIBtiOi9Jb2L0+XpULi9t/ZBwk9yQRTXnEztFWW+HXMwFYdRTrM\n\tjknpCbHwEXLiPlgkI+j80UI0erKtr/nvrz5cu5bU/zx5DcKqsl8GiBjvkJrJQcwFMxHy\n\thrKw==","X-Gm-Message-State":"AOAM532yPUuE4DUOPlMnmxs22KAzTxOlHSnC4cFc9NsoFbiGsbTooPE/\n\tNFyDAlhLuq2s+HhScpcsGnx3O6JhNOx55w==","X-Google-Smtp-Source":"ABdhPJxYj+3+lCoKP4v8bYYVHOEsQPb96QFsJps53bfSWGrwh4HYqGFk6pRoqKAy7//fOshCUe/jzA==","X-Received":"by 2002:a2e:b5bb:: with SMTP id\n\tf27mr5514096ljn.363.1605179934275; \n\tThu, 12 Nov 2020 03:18:54 -0800 (PST)","Date":"Thu, 12 Nov 2020 12:18:52 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Message-ID":"<20201112111852.GG1480295@oden.dyn.berto.se>","References":"<20201106103707.49660-1-paul.elder@ideasonboard.com>\n\t<20201106103707.49660-28-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201106103707.49660-28-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 27/37] libcamera: pipeline,\n\tipa: vimc: Support the new IPC mechanism","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13693,"web_url":"https://patchwork.libcamera.org/comment/13693/","msgid":"<20201113063746.GC1811@pyrite.rasen.tech>","date":"2020-11-13T06:37:46","subject":"Re: [libcamera-devel] [PATCH v4 27/37] libcamera: pipeline,\n\tipa: vimc: Support the new IPC mechanism","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Nov 12, 2020 at 12:18:52PM +0100, Niklas Söderlund wrote:\n> Hi Paul,\n> \n> Thanks for your patch.\n> \n> On 2020-11-06 19:36:57 +0900, Paul Elder wrote:\n> > Add support to vimc pipeline handler and IPA for the new IPC mechanism.\n> > \n> > Signed-off-by: Paul Elder \n> > \n> > ---\n> > Changes in v4:\n> > - rename Controls to controls\n> > - rename IPAVimcCallbackInterface to IPAVimcEventInterface\n> > - rename libcamera_generated_headers to libcamera_generated_ipa_headers\n> > in meson\n> > - use the new header name, vimc_ipa_interface.h\n> > \n> > Changes in v3:\n> > - change namespacing of base ControlInfoMap structure\n> > \n> > New in v2\n> > ---\n> > include/libcamera/ipa/meson.build | 1 +\n> > include/libcamera/ipa/vimc.h | 8 ++++++++\n> > include/libcamera/ipa/vimc.mojom | 12 ++++++++++++\n> > src/ipa/vimc/meson.build | 2 +-\n> > src/ipa/vimc/vimc.cpp | 16 ++++------------\n> > src/libcamera/pipeline/vimc/vimc.cpp | 8 +++++++-\n> > 6 files changed, 33 insertions(+), 14 deletions(-)\n> > create mode 100644 include/libcamera/ipa/vimc.mojom\n> > \n> > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> > index d49dff7b..70f5dfc8 100644\n> > --- a/include/libcamera/ipa/meson.build\n> > +++ b/include/libcamera/ipa/meson.build\n> > @@ -26,6 +26,7 @@ ipa_mojom_core = custom_target(core_mojom_file.split('.')[0] + '_mojom_module',\n> > \n> > ipa_mojom_files = [\n> > 'raspberrypi.mojom',\n> > + 'vimc.mojom',\n> > ]\n> > \n> > ipa_mojoms = []\n> > diff --git a/include/libcamera/ipa/vimc.h b/include/libcamera/ipa/vimc.h\n> > index 27a4a61d..2c5f3f8f 100644\n> > --- a/include/libcamera/ipa/vimc.h\n> > +++ b/include/libcamera/ipa/vimc.h\n> > @@ -8,6 +8,8 @@\n> > #ifndef __LIBCAMERA_IPA_VIMC_H__\n> > #define __LIBCAMERA_IPA_VIMC_H__\n> > \n> > +#include \n> > +\n> > #ifndef __DOXYGEN__\n> > \n> > namespace libcamera {\n> > @@ -21,6 +23,12 @@ enum IPAOperationCode {\n> > \tIPAOperationStop,\n> > };\n> > \n> > +namespace Vimc {\n> > +\n> > +static ControlInfoMap controls;\n> > +\n> > +}\n> \n> Maybe I missed something in a generation script but is this strictly \n> needed or just done in preparation for the future?\n\nThe reason I have this is to designate a default ControlInfoMap for\nControlLists in the de/serializer, in the case that the ControlList\ndoesn't have an info map.\n\nI think there was a short discussion about getting rid of this...? I'm\nnot sure where it went though.\n\n\nPaul\n\n> > +\n> > } /* namespace libcamera */\n> > \n> > #endif /* __DOXYGEN__ */\n> > diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n> > new file mode 100644\n> > index 00000000..d8e9b504\n> > --- /dev/null\n> > +++ b/include/libcamera/ipa/vimc.mojom\n> > @@ -0,0 +1,12 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +\n> > +import \"include/libcamera/ipa/core.mojom\";\n> > +\n> > +interface IPAVimcInterface {\n> > +\tinit(IPASettings settings) => (int32 ret);\n> > +\tstart() => (int32 ret);\n> > +\tstop();\n> > +};\n> > +\n> > +interface IPAVimcEventInterface {\n> > +};\n> > diff --git a/src/ipa/vimc/meson.build b/src/ipa/vimc/meson.build\n> > index 8c9df854..e982ebba 100644\n> > --- a/src/ipa/vimc/meson.build\n> > +++ b/src/ipa/vimc/meson.build\n> > @@ -3,7 +3,7 @@\n> > ipa_name = 'ipa_vimc'\n> > \n> > mod = shared_module(ipa_name,\n> > - 'vimc.cpp',\n> > + ['vimc.cpp', libcamera_generated_ipa_headers],\n> > name_prefix : '',\n> > include_directories : [ipa_includes, libipa_includes],\n> > dependencies : libcamera_dep,\n> > diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> > index cf841135..fbf7325e 100644\n> > --- a/src/ipa/vimc/vimc.cpp\n> > +++ b/src/ipa/vimc/vimc.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> > \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \n> > @@ -26,7 +27,7 @@ namespace libcamera {\n> > \n> > LOG_DEFINE_CATEGORY(IPAVimc)\n> > \n> > -class IPAVimc : public IPAInterface\n> > +class IPAVimc : public IPAVimcInterface\n> > {\n> > public:\n> > \tIPAVimc();\n> > @@ -37,15 +38,6 @@ public:\n> > \tint start() override;\n> > \tvoid stop() override;\n> > \n> > -\tvoid configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,\n> > -\t\t [[maybe_unused]] const std::map &streamConfig,\n> > -\t\t [[maybe_unused]] const std::map &entityControls,\n> > -\t\t [[maybe_unused]] const IPAOperationData &ipaConfig,\n> > -\t\t [[maybe_unused]] IPAOperationData *result) override {}\n> > -\tvoid mapBuffers([[maybe_unused]] const std::vector &buffers) override {}\n> > -\tvoid unmapBuffers([[maybe_unused]] const std::vector &ids) override {}\n> > -\tvoid processEvent([[maybe_unused]] const IPAOperationData &event) override {}\n> > -\n> > private:\n> > \tvoid initTrace();\n> > \tvoid trace(enum IPAOperationCode operation);\n> > @@ -141,9 +133,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> > \t\"vimc\",\n> > };\n> > \n> > -struct ipa_context *ipaCreate()\n> > +IPAInterface *ipaCreate()\n> > {\n> > -\treturn new IPAInterfaceWrapper(std::make_unique());\n> > +\treturn new IPAVimc();\n> > }\n> > }\n> > \n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 7416c37c..9dddb891 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -34,6 +34,10 @@\n> > #include \"libcamera/internal/v4l2_subdevice.h\"\n> > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > namespace libcamera {\n> > \n> > LOG_DEFINE_CATEGORY(VIMC)\n> > @@ -67,6 +71,8 @@ public:\n> > \tV4L2VideoDevice *video_;\n> > \tV4L2VideoDevice *raw_;\n> > \tStream stream_;\n> > +\n> > +\tstd::unique_ptr ipa_;\n> > };\n> > \n> > class VimcCameraConfiguration : public CameraConfiguration\n> > @@ -428,7 +434,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> > \n> > \tstd::unique_ptr data = std::make_unique(this, media);\n> > \n> > -\tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n> > +\tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n> > \tif (data->ipa_ != nullptr) {\n> > \t\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> > \t\tdata->ipa_->init(IPASettings{ conf });\n> > -- \n> > 2.27.0\n> > \n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n> \n> -- \n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 54CCDBE086\n\tfor ;\n\tFri, 13 Nov 2020 06:37:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D262B631B4;\n\tFri, 13 Nov 2020 07:37:55 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 61DA563149\n\tfor ;\n\tFri, 13 Nov 2020 07:37:54 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D20DD31A;\n\tFri, 13 Nov 2020 07:37:52 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Jxvhuv1u\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1605249474;\n\tbh=zcmmXX9undU5nmpxwUuHcpaFBhw7SqG4+g0rirwJMbg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Jxvhuv1u7ALyPEEnq0tk978CEJKtV7ZlHIJT70y2R1oRsSdz/d2AlFYCDo+xpBIAc\n\tv1i98mMO1V3o2jnaiLqkTGNLiNIQCCJMAstQ6IKK2Ij6xEW/RLhNmAF5j7V4jtUBB2\n\t52nwzzJPpi/wq/4ccO9nltEUgmy4lgio4Kech+LY=","Date":"Fri, 13 Nov 2020 15:37:46 +0900","From":"paul.elder@ideasonboard.com","To":"Niklas =?iso-8859-1?q?S=F6derlund?= ","Message-ID":"<20201113063746.GC1811@pyrite.rasen.tech>","References":"<20201106103707.49660-1-paul.elder@ideasonboard.com>\n\t<20201106103707.49660-28-paul.elder@ideasonboard.com>\n\t<20201112111852.GG1480295@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201112111852.GG1480295@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v4 27/37] libcamera: pipeline,\n\tipa: vimc: Support the new IPC mechanism","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13926,"web_url":"https://patchwork.libcamera.org/comment/13926/","msgid":"<20201126154419.GQ3905@pendragon.ideasonboard.com>","date":"2020-11-26T15:44:19","subject":"Re: [libcamera-devel] [PATCH v4 27/37] libcamera: pipeline,\n\tipa: vimc: Support the new IPC mechanism","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Fri, Nov 13, 2020 at 03:37:46PM +0900, paul.elder@ideasonboard.com wrote:\n> On Thu, Nov 12, 2020 at 12:18:52PM +0100, Niklas Söderlund wrote:\n> > On 2020-11-06 19:36:57 +0900, Paul Elder wrote:\n> > > Add support to vimc pipeline handler and IPA for the new IPC mechanism.\n> > > \n> > > Signed-off-by: Paul Elder \n> > > \n> > > ---\n> > > Changes in v4:\n> > > - rename Controls to controls\n> > > - rename IPAVimcCallbackInterface to IPAVimcEventInterface\n> > > - rename libcamera_generated_headers to libcamera_generated_ipa_headers\n> > > in meson\n> > > - use the new header name, vimc_ipa_interface.h\n> > > \n> > > Changes in v3:\n> > > - change namespacing of base ControlInfoMap structure\n> > > \n> > > New in v2\n> > > ---\n> > > include/libcamera/ipa/meson.build | 1 +\n> > > include/libcamera/ipa/vimc.h | 8 ++++++++\n> > > include/libcamera/ipa/vimc.mojom | 12 ++++++++++++\n> > > src/ipa/vimc/meson.build | 2 +-\n> > > src/ipa/vimc/vimc.cpp | 16 ++++------------\n> > > src/libcamera/pipeline/vimc/vimc.cpp | 8 +++++++-\n> > > 6 files changed, 33 insertions(+), 14 deletions(-)\n> > > create mode 100644 include/libcamera/ipa/vimc.mojom\n> > > \n> > > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> > > index d49dff7b..70f5dfc8 100644\n> > > --- a/include/libcamera/ipa/meson.build\n> > > +++ b/include/libcamera/ipa/meson.build\n> > > @@ -26,6 +26,7 @@ ipa_mojom_core = custom_target(core_mojom_file.split('.')[0] + '_mojom_module',\n> > > \n> > > ipa_mojom_files = [\n> > > 'raspberrypi.mojom',\n> > > + 'vimc.mojom',\n> > > ]\n> > > \n> > > ipa_mojoms = []\n> > > diff --git a/include/libcamera/ipa/vimc.h b/include/libcamera/ipa/vimc.h\n> > > index 27a4a61d..2c5f3f8f 100644\n> > > --- a/include/libcamera/ipa/vimc.h\n> > > +++ b/include/libcamera/ipa/vimc.h\n> > > @@ -8,6 +8,8 @@\n> > > #ifndef __LIBCAMERA_IPA_VIMC_H__\n> > > #define __LIBCAMERA_IPA_VIMC_H__\n> > > \n> > > +#include \n> > > +\n> > > #ifndef __DOXYGEN__\n> > > \n> > > namespace libcamera {\n> > > @@ -21,6 +23,12 @@ enum IPAOperationCode {\n> > > \tIPAOperationStop,\n> > > };\n> > > \n> > > +namespace Vimc {\n> > > +\n> > > +static ControlInfoMap controls;\n> > > +\n> > > +}\n> > \n> > Maybe I missed something in a generation script but is this strictly \n> > needed or just done in preparation for the future?\n> \n> The reason I have this is to designate a default ControlInfoMap for\n> ControlLists in the de/serializer, in the case that the ControlList\n> doesn't have an info map.\n> \n> I think there was a short discussion about getting rid of this...? I'm\n> not sure where it went though.\n\nI'm not sure either, but I would really really like to avoid adding this\n:-) It's not taking the right direction.\n\n> > > +\n> > > } /* namespace libcamera */\n> > > \n> > > #endif /* __DOXYGEN__ */\n> > > diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n> > > new file mode 100644\n> > > index 00000000..d8e9b504\n> > > --- /dev/null\n> > > +++ b/include/libcamera/ipa/vimc.mojom\n> > > @@ -0,0 +1,12 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +\n> > > +import \"include/libcamera/ipa/core.mojom\";\n> > > +\n> > > +interface IPAVimcInterface {\n> > > +\tinit(IPASettings settings) => (int32 ret);\n> > > +\tstart() => (int32 ret);\n> > > +\tstop();\n> > > +};\n> > > +\n> > > +interface IPAVimcEventInterface {\n> > > +};\n> > > diff --git a/src/ipa/vimc/meson.build b/src/ipa/vimc/meson.build\n> > > index 8c9df854..e982ebba 100644\n> > > --- a/src/ipa/vimc/meson.build\n> > > +++ b/src/ipa/vimc/meson.build\n> > > @@ -3,7 +3,7 @@\n> > > ipa_name = 'ipa_vimc'\n> > > \n> > > mod = shared_module(ipa_name,\n> > > - 'vimc.cpp',\n> > > + ['vimc.cpp', libcamera_generated_ipa_headers],\n> > > name_prefix : '',\n> > > include_directories : [ipa_includes, libipa_includes],\n> > > dependencies : libcamera_dep,\n> > > diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> > > index cf841135..fbf7325e 100644\n> > > --- a/src/ipa/vimc/vimc.cpp\n> > > +++ b/src/ipa/vimc/vimc.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \n> > > #include \n> > > @@ -26,7 +27,7 @@ namespace libcamera {\n> > > \n> > > LOG_DEFINE_CATEGORY(IPAVimc)\n> > > \n> > > -class IPAVimc : public IPAInterface\n> > > +class IPAVimc : public IPAVimcInterface\n> > > {\n> > > public:\n> > > \tIPAVimc();\n> > > @@ -37,15 +38,6 @@ public:\n> > > \tint start() override;\n> > > \tvoid stop() override;\n> > > \n> > > -\tvoid configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,\n> > > -\t\t [[maybe_unused]] const std::map &streamConfig,\n> > > -\t\t [[maybe_unused]] const std::map &entityControls,\n> > > -\t\t [[maybe_unused]] const IPAOperationData &ipaConfig,\n> > > -\t\t [[maybe_unused]] IPAOperationData *result) override {}\n> > > -\tvoid mapBuffers([[maybe_unused]] const std::vector &buffers) override {}\n> > > -\tvoid unmapBuffers([[maybe_unused]] const std::vector &ids) override {}\n> > > -\tvoid processEvent([[maybe_unused]] const IPAOperationData &event) override {}\n> > > -\n> > > private:\n> > > \tvoid initTrace();\n> > > \tvoid trace(enum IPAOperationCode operation);\n> > > @@ -141,9 +133,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> > > \t\"vimc\",\n> > > };\n> > > \n> > > -struct ipa_context *ipaCreate()\n> > > +IPAInterface *ipaCreate()\n> > > {\n> > > -\treturn new IPAInterfaceWrapper(std::make_unique());\n> > > +\treturn new IPAVimc();\n> > > }\n> > > }\n> > > \n> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > index 7416c37c..9dddb891 100644\n> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > @@ -34,6 +34,10 @@\n> > > #include \"libcamera/internal/v4l2_subdevice.h\"\n> > > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > > \n> > > +#include \n> > > +#include \n> > > +#include \n\nI before V :-)\n\nReviewed-by: Laurent Pinchart \n\n> > > +\n> > > namespace libcamera {\n> > > \n> > > LOG_DEFINE_CATEGORY(VIMC)\n> > > @@ -67,6 +71,8 @@ public:\n> > > \tV4L2VideoDevice *video_;\n> > > \tV4L2VideoDevice *raw_;\n> > > \tStream stream_;\n> > > +\n> > > +\tstd::unique_ptr ipa_;\n> > > };\n> > > \n> > > class VimcCameraConfiguration : public CameraConfiguration\n> > > @@ -428,7 +434,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> > > \n> > > \tstd::unique_ptr data = std::make_unique(this, media);\n> > > \n> > > -\tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n> > > +\tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n> > > \tif (data->ipa_ != nullptr) {\n> > > \t\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> > > \t\tdata->ipa_->init(IPASettings{ conf });","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D6C15BE176\n\tfor ;\n\tThu, 26 Nov 2020 15:44:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 58C8C6346B;\n\tThu, 26 Nov 2020 16:44:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 16F7F6344B\n\tfor ;\n\tThu, 26 Nov 2020 16:44:29 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 81448A1B;\n\tThu, 26 Nov 2020 16:44:28 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RIYJrDrs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606405468;\n\tbh=J5ZbB838nBbKNianHjC5q+WBxTi1XLQ4xhZin3/eTtY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RIYJrDrsK9ffFe1w+sxf8D2TlweHKi3xLqn1WYFaQ9QEQYRQ2FU8q93zjZ0CN1yzk\n\t5QaJ7t8at2OzRJ9MOY/g4kIH5RQIiVTvnvJG0KOd6lWvbtg9JbgJlqzJyvubIuFsxh\n\t0rNzeVasZp66XChzYNh/vk03VADv4ZqJFfRJcUFQ=","Date":"Thu, 26 Nov 2020 17:44:19 +0200","From":"Laurent Pinchart ","To":"paul.elder@ideasonboard.com","Message-ID":"<20201126154419.GQ3905@pendragon.ideasonboard.com>","References":"<20201106103707.49660-1-paul.elder@ideasonboard.com>\n\t<20201106103707.49660-28-paul.elder@ideasonboard.com>\n\t<20201112111852.GG1480295@oden.dyn.berto.se>\n\t<20201113063746.GC1811@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201113063746.GC1811@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v4 27/37] libcamera: pipeline,\n\tipa: vimc: Support the new IPC mechanism","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]