[libcamera-devel] [PATCH 06/13] libcamera: pipeline: rkisp1: Export stream formats to applicaitons

[Niklas Söderlund]

Hello Jacopo and Laurent,

On 2020-08-20 18:04:35 +0300, Laurent Pinchart wrote:
> Hello,
> 
> s/applicaitons/applications/ in the subject line.
> 
> On Thu, Aug 20, 2020 at 10:46:13AM +0200, Jacopo Mondi wrote:
> > Hi Niklas,
> >    a few questions
> > 
> > On Thu, Aug 13, 2020 at 02:52:39AM +0200, Niklas Söderlund wrote:
> > > The information about stream format is available but not exported to
> > > applications, fix this.
> > >
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > > ---
> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > index 9d5a52d352aefabc..8e0f6db5faa96928 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > @@ -585,7 +585,13 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera
> > >  	if (roles.empty())
> > >  		return config;
> > >
> > > -	StreamConfiguration cfg{};
> > > +	std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
> > > +	for (const PixelFormat &format : RKISP1_RSZ_MP_FORMATS)
> > > +		streamFormats[format] =
> > > +			{ { RKISP1_RSZ_MP_SRC_MIN, RKISP1_RSZ_MP_SRC_MAX } };
> > > +
> > 
> > Can the ISP freely up-scale? Otherwise the size should be capped to
> > the max sensor size
> 
> Even if the ISP can upscale I'm not sure we should allow it. I'd prefer
> capping the size.

This is a great idea!

> 
> > Can all formats be produced from all input formats ? Otherwise the
> > formats that can be produced should be matched with the ones produced
> > by the sensor.

For the ones supported today, yes. For the future when we add RAW 
rudimentary my tests suggest we can't convert between the different RGB 
layouts and can only support the one produced by the sensor.

> > 
> > As a general note, should we try to report the available stream
> > formats by inspeting the ones that can be produced by the video device
> > or can we live with a static enumeration in the pipeline handler ?
> 
> We don't need to query the kernel if we know the information already,
> but we should only report formats that can actually be produced for the
> connected sensor.

I would love to have a kernel API where we could enumerate this with all 
constraints added by the pipeline configuration. I fear this will never 
happen and we need to have knowledge about this in user-space.

> 
> > > +	StreamFormats formats(streamFormats);
> > > +	StreamConfiguration cfg(formats);
> > >  	cfg.pixelFormat = formats::NV12;
> > >  	cfg.size = data->sensor_->resolution();
> > >
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund