[libcamera-devel] [RFC PATCH v2] ipa: ipu3: Introduce IPAConfigInfo in IPC

[Jacopo Mondi]

Hi Umang,

On Thu, Apr 29, 2021 at 01:55:57PM +0530, Umang Jain wrote:

[snip]

> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -635,7 +635,14 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> > >
> > >   	std::map<uint32_t, ControlInfoMap> entityControls;
> > >   	entityControls.emplace(0, data->cio2_.sensor()->controls());
> > > -	data->ipa_->configure(entityControls, config->imguConfig().bds);
> > > +
> > > +	ipa::ipu3::IPAConfigInfo configInfo;
> > > +	configInfo.sensorInfo = sensorInfo;
> > > +	configInfo.entityControls = entityControls;
> > Why a copy when you can populate this one directly ?
> Originally it was your suggested way only, but Laurent wanted to use named
> initializers for safety - as there are couple of `Sizes` parameters and one
> can get the ordering wrong in directly populating them. IPAConfigInfo is
> generated from mojom framework and doesn't yet support named initiazers
> hence, this is what I ended up as closest implementation.

What I meant was:

why do this
        std::map<uint32_t, ControlInfoMap> entityControls;
	entityControls.emplace(0, data->cio2_.sensor()->controls());
        ....
        configInfo.entityControls = entityControls;

Instead of
        configInfo.entityControls.emplace(0, data->cio2_.sensor()->controls());

and avoid the copy ?

Thanks
   j


> >
> > Overall the idea is good. I would refrain from premature optimizations
> > like the one suggested by Jean-Micheal for the moment though, it's a
> > bit too early in my opinion, and the effort for each pipeline handler
> > to define its own exchange structures is limited.
> Yes - overall idea is good, but I would be comfortable doing that when we
> have got some level of stability for the closed source IPA
> >
> > Thanks
> >     j
> >
> > > +	configInfo.bdsOutputSize = config->imguConfig().bds;
> > > +	configInfo.iif = config->imguConfig().iif;
> > > +
> > > +	data->ipa_->configure(configInfo);
> > >
> > >   	return 0;
> > >   }
> > > --
> > > 2.26.2
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel at lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
>