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

2217 diff mbox series

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@ideasonboard.com>
---
 src/libcamera/pipeline/vimc.cpp | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

Hi Laurent,

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@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 ?

> +	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@jmondi.org>

Thanks
   j

>  	cfg.pixelFormat = V4L2_PIX_FMT_RGB24;
>  	cfg.size = { 1920, 1080 };
>  	cfg.bufferCount = 4;
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

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@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@jmondi.org>
> 
> >  	cfg.pixelFormat = V4L2_PIX_FMT_RGB24;
> >  	cfg.size = { 1920, 1080 };
> >  	cfg.bufferCount = 4;