[libcamera-devel] [PATCH v5 6/9] ipa: vimc: Add IPAOperationCode to init() parameter list
Jacopo Mondi
jacopo at jmondi.org
Wed Oct 12 15:41:46 CEST 2022
I understand the VIMC IPA doesn't do anything useful per se, but it
seems we're using it as test module rather than a skeleton IPA.
IOW do we need this if enum serialization is already tested ? Won't it
confuse IPA implementer that might look into VIMC as a reference ?
On Tue, Oct 11, 2022 at 07:58:56PM +0900, Paul Elder via libcamera-devel wrote:
> For the purpose of testing serializing/deserializing enums in function
> parameters, add IPAOperationCode to the parameter list of init().
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
> No change in v5
>
> Changes in v4:
> - remove [TEST] from commit subject
>
> Changes in v3:
> - pass enum by value as opposed to by const reference
>
> No changes in v2
> ---
> include/libcamera/ipa/vimc.mojom | 2 +-
> src/ipa/vimc/vimc.cpp | 6 ++++--
> src/libcamera/pipeline/vimc/vimc.cpp | 2 +-
> test/ipa/ipa_interface_test.cpp | 2 +-
> 4 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> index 718b9674..16149787 100644
> --- a/include/libcamera/ipa/vimc.mojom
> +++ b/include/libcamera/ipa/vimc.mojom
> @@ -18,7 +18,7 @@ enum IPAOperationCode {
> };
>
> interface IPAVimcInterface {
> - init(libcamera.IPASettings settings) => (int32 ret);
> + init(libcamera.IPASettings settings, IPAOperationCode code) => (int32 ret);
>
> configure(libcamera.IPACameraSensorInfo sensorInfo,
> map<uint32, libcamera.IPAStream> streamConfig,
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index 85afb279..5d494b63 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -31,7 +31,7 @@ public:
> IPAVimc();
> ~IPAVimc();
>
> - int init(const IPASettings &settings) override;
> + int init(const IPASettings &settings, const ipa::vimc::IPAOperationCode code) override;
>
> int start() override;
> void stop() override;
> @@ -66,7 +66,7 @@ IPAVimc::~IPAVimc()
> ::close(fd_);
> }
>
> -int IPAVimc::init(const IPASettings &settings)
> +int IPAVimc::init(const IPASettings &settings, const ipa::vimc::IPAOperationCode code)
> {
> trace(ipa::vimc::IPAOperationInit);
>
> @@ -74,6 +74,8 @@ int IPAVimc::init(const IPASettings &settings)
> << "initializing vimc IPA with configuration file "
> << settings.configurationFile;
>
> + LOG(IPAVimc, Debug) << "Got opcode " << code;
> +
> File conf(settings.configurationFile);
> if (!conf.open(File::OpenModeFlag::ReadOnly)) {
> LOG(IPAVimc, Error) << "Failed to open configuration file";
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index d2f2e460..df749bf7 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -471,7 +471,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
> data->ipa_->paramsBufferReady.connect(data.get(), &VimcCameraData::paramsBufferReady);
>
> std::string conf = data->ipa_->configurationFile("vimc.conf");
> - data->ipa_->init(IPASettings{ conf, data->sensor_->model() });
> + data->ipa_->init(IPASettings{ conf, data->sensor_->model() }, ipa::vimc::IPAOperationInit);
>
> /* Create and register the camera. */
> std::set<Stream *> streams{ &data->stream_ };
> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
> index 6b93e976..cd20348a 100644
> --- a/test/ipa/ipa_interface_test.cpp
> +++ b/test/ipa/ipa_interface_test.cpp
> @@ -106,7 +106,7 @@ protected:
>
> /* Test initialization of IPA module. */
> std::string conf = ipa_->configurationFile("vimc.conf");
> - int ret = ipa_->init(IPASettings{ conf, "vimc" });
> + int ret = ipa_->init(IPASettings{ conf, "vimc" }, ipa::vimc::IPAOperationInit);
> if (ret < 0) {
> cerr << "IPA interface init() failed" << endl;
> return TestFail;
> --
> 2.30.2
>
More information about the libcamera-devel
mailing list