[libcamera-devel] [PATCH v2 4/8] libcamera: pipeline: uvcvideo: Translate from V4L2 to DRM pixel formats

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tue, Mar 17, 2020 at 04:52:35AM +0100, Niklas Söderlund wrote:
> When generating a camera configuration pixel formats directly from the
> video device are used that contains V4L2 pixel formats. Translate the
> pixel formats to DRM before using them i the camera configuration.

s/them i/them in/

> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/uvcvideo.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 320da2685795c041..14f7ddb18a765834 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -154,7 +154,14 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
>  		return config;
>  
>  	ImageFormats v4l2formats = data->video_->formats();
> -	StreamFormats formats(v4l2formats.data());
> +	std::map<PixelFormat, std::vector<SizeRange>> deviceformats;

s/deviceformats/deviceFormats/

> +	for (const auto &it : v4l2formats.data()) {
> +		PixelFormat pixelformat = V4L2VideoDevice::toPixelFormat(it.first);

s/pixelformat/pixelFormat/

Maybe data->video_->toPixelFormat(it.first) ?

> +		const std::vector<SizeRange> &ranges = it.second;
> +		deviceformats[pixelformat] = ranges;

		deviceFormats.emplace(pixelFormat, ranges);

or maybe even

		deviceFormats.emplace(pixelFormat, it.second);

> +	}

I'm pretty sure you won't like that, but the C++ way of doing this would
be as follows :-)

        std::map<unsigned int, std::vector<SizeRange>> v4l2Formats = data->video_->formats();;
        std::map<PixelFormat, std::vector<SizeRange>> deviceformats;
        std::transform(v4l2Formats.begin(), v4l2Formats.end(),
                       std::inserter(deviceformats, deviceformats.begin()),
                       [&](const decltype(v4l2Formats)::value_type &format) {
                               return decltype(deviceformats)::value_type{
                                       data->video_->toPixelFormat(format.first),
                                       format.second
                               };
                       });

I think I've developed a Stockholm syndrome for C++...

Either way,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>


> +
> +	StreamFormats formats(deviceformats);
>  	StreamConfiguration cfg(formats);
>  
>  	cfg.pixelFormat = formats.pixelformats().front();

-- 
Regards,

Laurent Pinchart