Re: [RFC PATCH v3 1/6] media: v4l2: Extend pixel formats to unify single/multi-planar handling (and more)

[Hans Verkuil <hverkuil@xs4all.nl>]

On 10/8/19 11:11 AM, Boris Brezillon wrote:
> This is part of the multiplanar and singleplanar unification process.
> v4l2_ext_pix_format is supposed to work for both cases.
> 
> We also add the concept of modifiers already employed in DRM to expose
> HW-specific formats (like tiled or compressed formats) and allow
> exchanging this information with the DRM subsystem in a consistent way.
> 
> Note that V4L2_BUF_TYPE_VIDEO[_OUTPUT]_OVERLAY and
> V4L2_BUF_TYPE_VIDEO_{CAPTURE,OUTPUT}_MPLANE types are no longer accepted
> in v4l2_ext_format and will be rejected if you use the {G,S,TRY}EXT_FMT
> ioctls. V4L2_BUF_TYPE_VIDEO_{CAPTURE,OUTPUT}_MPLANE is dropped as part
> of the multiplanar/singleplanar unification.
> V4L2_BUF_TYPE_VIDEO[_OUTPUT]_OVERLAY seems to be used mostly on old
> drivers and supporting it would require some extra rework.
> 
> New hooks have been added to v4l2_ioctl_ops to support those new ioctls
> in drivers, but, in the meantime, the core takes care of converting
> {S,G,TRY}_EXT_FMT requests into {S,G,TRY}_FMT so that old drivers can
> still work if the userspace app/lib uses the new ioctls.
> The conversion is also done the other around to allow userspace
> apps/libs using {S,G,TRY}_FMT to work with drivers implementing the
> _ext_ hooks.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---

<snip>

>  
> +#define VIDIOC_G_EXT_FMT	_IOWR('V', 104, struct v4l2_ext_format)
> +#define VIDIOC_S_EXT_FMT	_IOWR('V', 105, struct v4l2_ext_format)
> +#define VIDIOC_TRY_EXT_FMT	_IOWR('V', 106, struct v4l2_ext_format)
>  /* Reminder: when adding new ioctls please add support for them to
>     drivers/media/v4l2-core/v4l2-compat-ioctl32.c as well! */
>  
> 

Since we're extending g/s/try_fmt, we should also provide a replacement for
enum_fmt, esp. given this thread:

https://www.mail-archive.com/linux-media@vger.kernel.org/msg150871.html

So here is a preliminary suggestion:

struct v4l2_ext_fmtdesc {
        __u32               index;             /* Format number      */
        __u32               type;              /* enum v4l2_buf_type */
	__u32               which;             /* enum v4l2_subdev_format_whence, ignored if mbus_code == 0 */
        __u32               mbus_code;         /* Mediabus Code (set to 0 if n/a) */
        __u32               flags;
        __u8                description[32];   /* Description string */
        __u32               pixelformat;       /* Format fourcc      */
};

This would solve (I think) the issue raised in the thread since you can now get
just for formats that are valid for the given mediabus code and the which field.

Other improvements that could be made is to return more information about the
format (similar to struct v4l2_format_info). In particular v4l2_pixel_encoding
and mem/comp_planes would be useful for userspace to know.

Finally, we can also add a new ioctl that combines ENUM_FMT/ENUM_FRAMESIZES/ENUM_FRAMEINTERVALS
and returns an array of all valid formats/sizes/intervals, requiring just a single ioctl
to obtain all this information.

Regards,

	Hans