[libcamera-devel] [PATCH 1/2] libcamera: v4l2-format: Add V4L2DeviceFormat

[Kieran Bingham]

Hi Jacopo,

On 28/01/2019 16:20, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Mon, Jan 28, 2019 at 03:57:45PM +0000, Kieran Bingham wrote:
>> Hi Jacopo,
>>
>> One more thing...
>>
>> On 28/01/2019 15:42, Kieran Bingham wrote:
>>> Hi Jacopo,
>>>
>>> On 28/01/2019 15:11, Jacopo Mondi wrote:
>>>> Add a V4L2DeviceFormat class aimed to be used to provide format configuration
>>>> requests to a V4L2Device.
>>>>
>>>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>>>
>>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>>
>>>> ---
>>>>  src/libcamera/include/v4l2_device.h | 14 ++++++++
>>>>  src/libcamera/v4l2_device.cpp       | 56 +++++++++++++++++++++++++++++
>>>>  2 files changed, 70 insertions(+)
>>>>
>>>> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
>>>> index c67ebbf..c70959e 100644
>>>> --- a/src/libcamera/include/v4l2_device.h
>>>> +++ b/src/libcamera/include/v4l2_device.h
>>>> @@ -53,6 +53,20 @@ struct V4L2Capability final : v4l2_capability {
>>>>  	}
>>>>  };
>>>>
>>>> +class V4L2DeviceFormat
>>>> +{
>>>> +public:
>>>> +	uint32_t width;
>>>> +	uint32_t height;
>>>> +	uint32_t fourcc;
>>>
>>> I might have called this pixelFormat, but it's internal and it can
>>> change if we later define a PixelFormat class or such.
>>>
>>> Currently we will store a fourcc so this is accurate.
>>
>>
>> struct v4l2_pix_format *pix stores the following extra information:
> 
> Not only v4l2_pix_format, but v4l2_pix_format_mplane and
> v4l2_mbus_framefmt too...
> 
>>
>> 	field (we might need to consider interlaced somewhere?)
>> 	colorspace,
>> 	flags
>> 	ycbcr_enc, hsv_enc
>> 	quantization
>> 	xfer_func
> 
> Right now, I cannot tell. I'm sure there are use cases where those
> informations are relevant.
> 
>>
>> Should we be handling any of that information here too?
>>
>> Or should we add it on an as-required basis?
> 
> I think they just need to be copied back and forth from the ioctl
> arguments, once the V4L2DeviceFormat has been expanded to include
> these fields.
> 
> I'll leave it for later, but if anyone has other opinions we can look
> into that now.

Leaving it until it is needed is fine with me.
--
Kieran


> 
> Thanks
>   j
> 
>>
>> --
>> Kieran
>>
>>
>>
>>>
>>>> +
>>>> +	struct {
>>>> +		uint32_t size;
>>>> +		uint32_t bpl;
>>>> +	} planesFmt[3];
>>>> +	unsigned int planes;
>>>> +};
>>>> +
>>>>  class MediaEntity;
>>>>  class V4L2Device
>>>>  {
>>>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
>>>> index 408f9b9..d6143f2 100644
>>>> --- a/src/libcamera/v4l2_device.cpp
>>>> +++ b/src/libcamera/v4l2_device.cpp
>>>> @@ -80,6 +80,62 @@ LOG_DEFINE_CATEGORY(V4L2)
>>>>   * \return True if the device provides Streaming I/O IOCTLs
>>>>   */
>>>>
>>>> +/**
>>>> + * \class V4L2DeviceFormat
>>>> + * \brief The V4L2 device image format and sizes
>>>> + *
>>>> + * Describes the image format and image sizes to be programmed on a V4L2
>>>> + * video device. The image format is defined by fourcc code as defined by
>>>> + * the V4L2 APIs with the V4L2_PIX_FMT_ macros, a visible width and height
>>>> + * and a variable number of planes (1 to 3) with variable sizes and line
>>>> + * strides.
>>>> + *
>>>> + * Formats defined as 'single planar' by the V4L2 APIs are represented with
>>>> + * V4L2DeviceFormat instances with a single plane
>>>> + * (V4L2DeviceFormat::planes = 1). Semi-planar and multiplanar formats use
>>>> + * 2 and 3 planes respectively.
>>>> + *
>>>> + * V4L2DeviceFormat defines the exchange format between components that
>>>> + * receive image configuration requests from applications and a V4L2Device.
>>>> + * The V4L2Device validates and applies the requested size and format to
>>>> + * the device driver.
>>>> + */
>>>> +
>>>> +/**
>>>> + * \var V4L2DeviceFormat::width
>>>> + * \brief The image width
>>>> + */
>>>> +
>>>> +/**
>>>> + * \var V4L2DeviceFormat::height
>>>> + * \brief The image height
>>>> + */
>>>> +
>>>> +/**
>>>> + * \var V4L2DeviceFormat::fourcc> + * \brief The pixel encoding scheme
>>>> + *
>>>> + * The fourcc code, as defined by the V4L2 APIs with the V4L2_PIX_FMT_ macros,
>>>> + * that identifies the image format pixel encoding scheme.
>>>> + */
>>>> +
>>>> +/**
>>>> + * \var V4L2DeviceFormat::planesFmt
>>>> + * \brief The per-plane size information
>>>> + *
>>>> + * Images are stored in memory in one or more data planes. Each data plane
>>>> + * has a specific size and line length, which could differ from the image
>>>> + * visible sizes to accommodate line or plane padding data.
>>>> + *
>>>> + * Only the first V4L2DeviceFormat::planes entries are considered valid.
>>>> + *
>>>> + */
>>>> +
>>>> +/**
>>>> + * \var V4L2DeviceFormat::planes
>>>> + * \brief The number of valid data planes
>>>> + */
>>>> +
>>>>  /**
>>>>   * \class V4L2Device
>>>>   * \brief V4L2Device object and API
>>>>
>>>
>>
>> --
>> Regards
>> --
>> Kieran

-- 
Regards
--
Kieran