[libcamera-devel] [PATCH v2 3/3] libcamera: pipeline: uvc: Use the device to validate formats

[Jacopo Mondi]

Hi Kieran,

On Thu, May 23, 2019 at 02:59:00PM +0100, Kieran Bingham wrote:
> UVC pipelines are highly variable, and we can not define their
> configuration restrictions within the UVC pipeline handler directly.
>
> Update the UVCCameraConfiguration to store the UVCCameraData (storing
> a reference to the camera as a means of borrowing a reference to the
> camera data).
>
> We then validate the configuration by the tryFormat() operation on the
> UVC V4L2Device.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/libcamera/pipeline/uvcvideo.cpp | 36 +++++++++++++++++++++--------
>  1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 45260f34c8f5..df321c6e64a6 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -42,9 +42,18 @@ public:
>  class UVCCameraConfiguration : public CameraConfiguration
>  {
>  public:
> -	UVCCameraConfiguration();
> +	UVCCameraConfiguration(Camera *camera, UVCCameraData *data);

Minor: do you need 'camera' in UVCCameraConfiguration or just 'data' ?
You can pass data to the UVCCameraConfiguration constructor at
generateConfiguration() time, and save storing one shared pointer.

Apart from this minor thing:
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>
>  	Status validate() override;
> +
> +private:
> +	/*
> +	 * The UVCCameraConfiguration instance is guaranteed to be valid as long
> +	 * as the corresponding Camera instance is valid. In order to borrow a
> +	 * reference to the camera data, store a new reference to the camera.
> +	 */
> +	std::shared_ptr<Camera> camera_;
> +	const UVCCameraData *data_;
>  };
>
>  class PipelineHandlerUVC : public PipelineHandler
> @@ -76,9 +85,12 @@ private:
>  	}
>  };
>
> -UVCCameraConfiguration::UVCCameraConfiguration()
> +UVCCameraConfiguration::UVCCameraConfiguration(Camera *camera,
> +					       UVCCameraData *data)
>  	: CameraConfiguration()
>  {
> +	camera_ = camera->shared_from_this();
> +	data_ = data;
>  }
>
>  CameraConfiguration::Status UVCCameraConfiguration::validate()
> @@ -96,17 +108,20 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
>
>  	StreamConfiguration &cfg = config_[0];
>
> -	/* \todo: Validate the configuration against the device capabilities. */
> -	const unsigned int pixelFormat = cfg.pixelFormat;
> -	const Size size = cfg.size;
> +	V4L2DeviceFormat format;
> +	format.fourcc = cfg.pixelFormat;
> +	format.size = cfg.size;
>
> -	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> -	cfg.size = { 640, 480 };
> +	/* Validate the format on the device. */
> +	data_->video_->tryFormat(&format);
>
> -	if (cfg.pixelFormat != pixelFormat || cfg.size != size) {
> +	if (cfg.pixelFormat != format.fourcc || cfg.size != format.size) {
>  		LOG(UVC, Debug)
>  			<< "Adjusting configuration from " << cfg.toString()
> -			<< " to " << cfg.size.toString() << "-YUYV";
> +			<< " to " << format.toString();
> +
> +		cfg.pixelFormat = format.fourcc;
> +		cfg.size = format.size;
>  		status = Adjusted;
>  	}
>
> @@ -123,7 +138,8 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
>  CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
> -	CameraConfiguration *config = new UVCCameraConfiguration();
> +	UVCCameraData *data = cameraData(camera);
> +	CameraConfiguration *config = new UVCCameraConfiguration(camera, data);
>
>  	if (roles.empty())
>  		return config;
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190524/da02bbbd/attachment-0001.sig>