[libcamera-devel] [PATCH v2 6/7] [TEST] ipa: vimc: Add IPAOperationCode to init() parameter list
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Aug 19 10:13:52 CEST 2022
Hi Paul,
On Fri, Aug 19, 2022 at 04:44:56PM +0900, paul.elder at ideasonboard.com wrote:
> On Fri, Aug 19, 2022 at 03:55:04AM +0300, Laurent Pinchart wrote:
> > On Thu, Aug 18, 2022 at 03:49:21PM +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>
> > >
> > > ---
> > > 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..6bf39a1c 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;
> >
> > Should we do for enums the same thing we do for integers, and pass by
> > value instead of const reference ?
>
> Hm, tbh I think we should. I guess I have to fix that too then :/
>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> That means that... this is approved for merging...? I wasn't sure how we
> felt about modifying the vimc IPA interface in such dummy-test ways
I'm fine with that, the vimc API is meant for this purpose. I expect
we'll refactor it later to make it look less "dummy", but we'll keep an
anum and a Flags somewhere in any case for the purpose of testing. For
now, this is fine.
> > > 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 153cf849..983bd514 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 3c0df843..b9fa15cd 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;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list