[libcamera-devel,v2,16/16] libcamera: pipeline: uvcvideo: Add format information and validation
diff mbox series

Message ID 20190612004359.15772-17-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Add support for format information and validation
Related show

Commit Message

Niklas Söderlund June 12, 2019, 12:43 a.m. UTC 
Extend the uvcvideo pipeline with format information and validation. The
format information is gathered by enumerating the v4l2 device. This
enumeration approach is valid for UVC as it has a static and simple
media graph.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/pipeline/uvcvideo.cpp | 41 ++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 10 deletions(-)

Comments

Jacopo Mondi June 13, 2019, 5:30 p.m. UTC | #1 
Hi Niklas,

On Wed, Jun 12, 2019 at 02:43:59AM +0200, Niklas Söderlund wrote:
> Extend the uvcvideo pipeline with format information and validation. The
> format information is gathered by enumerating the v4l2 device. This
> enumeration approach is valid for UVC as it has a static and simple
> media graph.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/pipeline/uvcvideo.cpp | 41 ++++++++++++++++++++++-------
>  1 file changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 45260f34c8f5daba..7ca9d0eefe8223e8 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -5,6 +5,8 @@
>   * uvcvideo.cpp - Pipeline handler for uvcvideo devices
>   */
>
> +#include <algorithm>
> +
>  #include <libcamera/camera.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
> @@ -95,18 +97,33 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
>  	}
>
>  	StreamConfiguration &cfg = config_[0];
> -
> -	/* \todo: Validate the configuration against the device capabilities. */
> +	const StreamFormats &formats = cfg.formats();
>  	const unsigned int pixelFormat = cfg.pixelFormat;
>  	const Size size = cfg.size;
>
> -	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> -	cfg.size = { 640, 480 };
> +	const std::vector<unsigned int> pixelFormats = formats.pixelformats();
> +	auto iter = std::find(pixelFormats.begin(), pixelFormats.end(), pixelFormat);
> +	if (iter == pixelFormats.end()) {
> +		cfg.pixelFormat = pixelFormats.front();
> +		LOG(UVC, Debug)
> +			<< "Adjusting pixel format from " << pixelFormat
> +			<< " to " << cfg.pixelFormat;
> +		status = Adjusted;
> +	}
> +
> +	const std::vector<Size> &formatSizes = formats.sizes(cfg.pixelFormat);
> +	cfg.size = formatSizes.front();
> +	for (const Size &formatsSize : formatSizes) {
> +		if (formatsSize > size)
> +			break;
> +
> +		cfg.size = formatsSize;
> +	}

Even this selects not the closest size, but the smallest closest one, I
think it's fine for now.
>
> -	if (cfg.pixelFormat != pixelFormat || cfg.size != size) {
> +	if (cfg.size != size) {
>  		LOG(UVC, Debug)
> -			<< "Adjusting configuration from " << cfg.toString()
> -			<< " to " << cfg.size.toString() << "-YUYV";
> +			<< "Adjusting size from " << size.toString()
> +			<< " to " << cfg.size.toString();
>  		status = Adjusted;
>  	}
>
> @@ -123,14 +140,18 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
>  CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
> +	UVCCameraData *data = cameraData(camera);
>  	CameraConfiguration *config = new UVCCameraConfiguration();
>
>  	if (roles.empty())
>  		return config;
>
> -	StreamConfiguration cfg{};
> -	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> -	cfg.size = { 640, 480 };
> +	ImageFormats v4l2formats = data->video_->formats();
> +	StreamFormats formats(v4l2formats.data());

Do we want to be able to construct a StreamFormat from an ImageFormat?


Minors apart
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> +	StreamConfiguration cfg(formats);
> +
> +	cfg.pixelFormat = formats.pixelformats().front();
> +	cfg.size = formats.sizes(cfg.pixelFormat).back();
>  	cfg.bufferCount = 4;
>
>  	config->addConfiguration(cfg);
> --
> 2.21.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Niklas Söderlund June 16, 2019, 1:17 p.m. UTC | #2 
Hi Jacopo,

Thanks for your feedback.

On 2019-06-13 19:30:10 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Wed, Jun 12, 2019 at 02:43:59AM +0200, Niklas Söderlund wrote:
> > Extend the uvcvideo pipeline with format information and validation. The
> > format information is gathered by enumerating the v4l2 device. This
> > enumeration approach is valid for UVC as it has a static and simple
> > media graph.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/libcamera/pipeline/uvcvideo.cpp | 41 ++++++++++++++++++++++-------
> >  1 file changed, 31 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index 45260f34c8f5daba..7ca9d0eefe8223e8 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -5,6 +5,8 @@
> >   * uvcvideo.cpp - Pipeline handler for uvcvideo devices
> >   */
> >
> > +#include <algorithm>
> > +
> >  #include <libcamera/camera.h>
> >  #include <libcamera/request.h>
> >  #include <libcamera/stream.h>
> > @@ -95,18 +97,33 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
> >  	}
> >
> >  	StreamConfiguration &cfg = config_[0];
> > -
> > -	/* \todo: Validate the configuration against the device capabilities. */
> > +	const StreamFormats &formats = cfg.formats();
> >  	const unsigned int pixelFormat = cfg.pixelFormat;
> >  	const Size size = cfg.size;
> >
> > -	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> > -	cfg.size = { 640, 480 };
> > +	const std::vector<unsigned int> pixelFormats = formats.pixelformats();
> > +	auto iter = std::find(pixelFormats.begin(), pixelFormats.end(), pixelFormat);
> > +	if (iter == pixelFormats.end()) {
> > +		cfg.pixelFormat = pixelFormats.front();
> > +		LOG(UVC, Debug)
> > +			<< "Adjusting pixel format from " << pixelFormat
> > +			<< " to " << cfg.pixelFormat;
> > +		status = Adjusted;
> > +	}
> > +
> > +	const std::vector<Size> &formatSizes = formats.sizes(cfg.pixelFormat);
> > +	cfg.size = formatSizes.front();
> > +	for (const Size &formatsSize : formatSizes) {
> > +		if (formatsSize > size)
> > +			break;
> > +
> > +		cfg.size = formatsSize;
> > +	}
> 
> Even this selects not the closest size, but the smallest closest one, I
> think it's fine for now.

Yes, closes smallest one is selected, I think that is fine for now.

> >
> > -	if (cfg.pixelFormat != pixelFormat || cfg.size != size) {
> > +	if (cfg.size != size) {
> >  		LOG(UVC, Debug)
> > -			<< "Adjusting configuration from " << cfg.toString()
> > -			<< " to " << cfg.size.toString() << "-YUYV";
> > +			<< "Adjusting size from " << size.toString()
> > +			<< " to " << cfg.size.toString();
> >  		status = Adjusted;
> >  	}
> >
> > @@ -123,14 +140,18 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
> >  CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
> >  	const StreamRoles &roles)
> >  {
> > +	UVCCameraData *data = cameraData(camera);
> >  	CameraConfiguration *config = new UVCCameraConfiguration();
> >
> >  	if (roles.empty())
> >  		return config;
> >
> > -	StreamConfiguration cfg{};
> > -	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> > -	cfg.size = { 640, 480 };
> > +	ImageFormats v4l2formats = data->video_->formats();
> > +	StreamFormats formats(v4l2formats.data());
> 
> Do we want to be able to construct a StreamFormat from an ImageFormat?

I don't think so. I think it's a special case for UVC where the 
StreamFormats will be built directly from the ImageFormats. If I'm wrong 
and this will turn into a common thing I would agree that this would be 
neat.

> 
> 
> Minors apart
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> Thanks
>   j
> 
> > +	StreamConfiguration cfg(formats);
> > +
> > +	cfg.pixelFormat = formats.pixelformats().front();
> > +	cfg.size = formats.sizes(cfg.pixelFormat).back();
> >  	cfg.bufferCount = 4;
> >
> >  	config->addConfiguration(cfg);
> > --
> > 2.21.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index 45260f34c8f5daba..7ca9d0eefe8223e8 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -5,6 +5,8 @@ 
  * uvcvideo.cpp - Pipeline handler for uvcvideo devices
  */
 
+#include <algorithm>
+
 #include <libcamera/camera.h>
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
@@ -95,18 +97,33 @@  CameraConfiguration::Status UVCCameraConfiguration::validate()
 	}
 
 	StreamConfiguration &cfg = config_[0];
-
-	/* \todo: Validate the configuration against the device capabilities. */
+	const StreamFormats &formats = cfg.formats();
 	const unsigned int pixelFormat = cfg.pixelFormat;
 	const Size size = cfg.size;
 
-	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
-	cfg.size = { 640, 480 };
+	const std::vector<unsigned int> pixelFormats = formats.pixelformats();
+	auto iter = std::find(pixelFormats.begin(), pixelFormats.end(), pixelFormat);
+	if (iter == pixelFormats.end()) {
+		cfg.pixelFormat = pixelFormats.front();
+		LOG(UVC, Debug)
+			<< "Adjusting pixel format from " << pixelFormat
+			<< " to " << cfg.pixelFormat;
+		status = Adjusted;
+	}
+
+	const std::vector<Size> &formatSizes = formats.sizes(cfg.pixelFormat);
+	cfg.size = formatSizes.front();
+	for (const Size &formatsSize : formatSizes) {
+		if (formatsSize > size)
+			break;
+
+		cfg.size = formatsSize;
+	}
 
-	if (cfg.pixelFormat != pixelFormat || cfg.size != size) {
+	if (cfg.size != size) {
 		LOG(UVC, Debug)
-			<< "Adjusting configuration from " << cfg.toString()
-			<< " to " << cfg.size.toString() << "-YUYV";
+			<< "Adjusting size from " << size.toString()
+			<< " to " << cfg.size.toString();
 		status = Adjusted;
 	}
 
@@ -123,14 +140,18 @@  PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
 CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
 	const StreamRoles &roles)
 {
+	UVCCameraData *data = cameraData(camera);
 	CameraConfiguration *config = new UVCCameraConfiguration();
 
 	if (roles.empty())
 		return config;
 
-	StreamConfiguration cfg{};
-	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
-	cfg.size = { 640, 480 };
+	ImageFormats v4l2formats = data->video_->formats();
+	StreamFormats formats(v4l2formats.data());
+	StreamConfiguration cfg(formats);
+
+	cfg.pixelFormat = formats.pixelformats().front();
+	cfg.size = formats.sizes(cfg.pixelFormat).back();
 	cfg.bufferCount = 4;
 
 	config->addConfiguration(cfg);