[libcamera-devel] [PATCH 3/3] libcamera: v4l2_device: Add support for META_CAPTURE devices

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2019-02-19 17:56:20 +0100, Jacopo Mondi wrote:
> Add support for devices that provides video meta-data to v4l2_device.cpp
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/include/v4l2_device.h |  4 ++++
>  src/libcamera/v4l2_device.cpp       | 12 ++++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index 1d31d1b403bc..52eb6785cc15 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -53,6 +53,10 @@ struct V4L2Capability final : v4l2_capability {
>  		return device_caps() & (V4L2_CAP_VIDEO_CAPTURE |
>  					V4L2_CAP_VIDEO_CAPTURE_MPLANE);
>  	}
> +	bool isMeta() const
> +	{
> +		return device_caps() & V4L2_CAP_META_CAPTURE;
> +	}
>  	bool isOutput() const
>  	{
>  		return device_caps() & (V4L2_CAP_VIDEO_OUTPUT |
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 24e115554a99..7fe6476bf035 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -79,6 +79,12 @@ LOG_DEFINE_CATEGORY(V4L2)
>   * \return True if the device can output video frames
>   */
>  
> +/**
> + * \fn bool V4L2Capability::isMeta()
> + * \brief Identify if the device is capable of providing video meta-data
> + * \return True if the device can provide video meta-data
> + */
> +
>  /**
>   * \fn bool V4L2Capability::hasStreaming()
>   * \brief Determine if the device can perform Streaming I/O
> @@ -280,7 +286,7 @@ int V4L2Device::open()
>  		<< "Opened device " << caps_.bus_info() << ": "
>  		<< caps_.driver() << ": " << caps_.card();
>  
> -	if (!caps_.isCapture() && !caps_.isOutput()) {
> +	if (!caps_.isCapture() && !caps_.isOutput() && !caps_.isMeta()) {
>  		LOG(V4L2, Debug) << "Device is not a supported type";
>  		return -EINVAL;
>  	}
> @@ -294,10 +300,12 @@ int V4L2Device::open()
>  		bufferType_ = caps_.isMultiplanar()
>  			    ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
>  			    : V4L2_BUF_TYPE_VIDEO_CAPTURE;
> -	else
> +	else if (caps_.isOutput())
>  		bufferType_ = caps_.isMultiplanar()
>  			    ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE
>  			    : V4L2_BUF_TYPE_VIDEO_OUTPUT;
> +	else
> +		bufferType_ = V4L2_BUF_TYPE_META_CAPTURE;

I would rearrange all changes in this patch to V4L2Device::open() into 
something like this

    if (!caps_.hasStreaming()) {
        ...
    }

    if (caps_.isCapture())
        bufferType_ = ...
    else if (caps_.isOutput())
        bufferType_ = ...
    else if (caps_.isMeta())
        bufferType_ = ...
    else {
        LOG(V4L2, Debug) << "Device is not a supported type";
        return -EINVAL;
    }

It would eliminate the current practise of checking the same thing twice 
in the same function. There might be bonus points if you can figure out 
a nice way to turn it into a switch statement ;-)

>  
>  	/*
>  	 *  We wait for Read notifications on CAPTURE devices (POLLIN), and
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund