[libcamera-devel] [PATCH] libcamera: pipeline: vimc: Support format enumeration

[Laurent Pinchart]

Hi Jacopo,

On Fri, Oct 25, 2019 at 01:33:58PM +0200, Jacopo Mondi wrote:
> On Fri, Oct 25, 2019 at 06:20:16AM +0300, Laurent Pinchart wrote:
> > Fill the StreamConfiguration with all supported formats. The list of
> > supported formats is currently hardcoded based on the limits of the vimc
> > driver.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/vimc.cpp | 33 ++++++++++++++++++++++++---------
> >  1 file changed, 24 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index dcdaef120ad1..c16ae4cb76b5 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -103,6 +103,16 @@ private:
> >  	}
> >  };
> >
> > +namespace {
> > +
> > +constexpr std::array<unsigned int, 3> pixelformats{
> 
> pixelformats {
> 
> Doesn't checkstyle.py complains ?

It doesn't, so I'll fix it :-)

> > +	V4L2_PIX_FMT_BGR24,
> > +	V4L2_PIX_FMT_RGB24,
> > +	V4L2_PIX_FMT_ARGB32,
> > +};
> > +
> > +} /* namespace */
> > +
> >  VimcCameraConfiguration::VimcCameraConfiguration()
> >  	: CameraConfiguration()
> >  {
> > @@ -110,12 +120,6 @@ VimcCameraConfiguration::VimcCameraConfiguration()
> >
> >  CameraConfiguration::Status VimcCameraConfiguration::validate()
> >  {
> > -	static const std::array<unsigned int, 3> formats{
> > -		V4L2_PIX_FMT_BGR24,
> > -		V4L2_PIX_FMT_RGB24,
> > -		V4L2_PIX_FMT_ARGB32,
> > -	};
> > -
> >  	Status status = Valid;
> >
> >  	if (config_.empty())
> > @@ -130,8 +134,8 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
> >  	StreamConfiguration &cfg = config_[0];
> >
> >  	/* Adjust the pixel format. */
> > -	if (std::find(formats.begin(), formats.end(), cfg.pixelFormat) ==
> > -	    formats.end()) {
> > +	if (std::find(pixelformats.begin(), pixelformats.end(), cfg.pixelFormat) ==
> > +	    pixelformats.end()) {
> >  		LOG(VIMC, Debug) << "Adjusting format to RGB24";
> >  		cfg.pixelFormat = V4L2_PIX_FMT_RGB24;
> >  		status = Adjusted;
> > @@ -170,7 +174,18 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
> >  	if (roles.empty())
> >  		return config;
> >
> > -	StreamConfiguration cfg{};
> > +	ImageFormats formats;
> > +
> > +	for (unsigned int pixelformat : pixelformats) {
> > +		/* The scaler hardcodes a x3 scale-up ratio. */
> > +		std::vector<SizeRange> sizes{
> > +			SizeRange{ 48, 48, 4096, 2160 }
> > +		};
> > +		formats.addFormat(pixelformat, sizes);
> > +	}
> > +
> > +	StreamConfiguration cfg(formats.data());
> > +
> 
> Looks good, small detail pointed out above apart
> 
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> >  	cfg.pixelFormat = V4L2_PIX_FMT_RGB24;
> >  	cfg.size = { 1920, 1080 };
> >  	cfg.bufferCount = 4;

-- 
Regards,

Laurent Pinchart