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

[Kieran Bingham]

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:

	field (we might need to consider interlaced somewhere?)
	colorspace,
	flags
	ycbcr_enc, hsv_enc
	quantization
	xfer_func

Should we be handling any of that information here too?

Or should we add it on an as-required basis?

--
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