Re: [RFC v4] Multi-plane buffer support for V4L2 API
On Tuesday 13 July 2010 11:43:47 Pawel Osciak wrote: Hi Mauro, thanks for taking the time to look at this. Mauro Carvalho Chehab mche...@redhat.com wrote: With Hans proposed changes that you've already acked, I think the proposal is ok, except for one detail: 4. Format enumeration -- struct v4l2_fmtdesc, used for format enumeration, does include the v4l2_buf_type enum as well, so the new types can be handled properly here as well. For drivers supporting both versions of the API, 1-plane formats should be returned for multiplanar buffer types as well, for consistency. In other words, for multiplanar buffer types, the formats returned are a superset of those returned when enumerating with the old buffer types. We shouldn't mix types here. If the userspace is asking for multi-planar types, the driver should return just the multi-planar formats. If the userspace wants to know about both, it will just call for both types of formats. Yes. Although the idea here is that we wanted to be able to use single-planar formats with either the old API or the new multiplane API. In the new API, you could just set num_planes=1. So multiplanar API != multiplanar format. When you enum_fmt for mutliplanar types, you get all formats you can use with the multiplanar API and not all formats that have num_planes 1. This can simplify applications - they don't have to switch between APIs when switching between formats. They may even choose not to use the old API at all (if a driver allows it). Do we want to lose the ability to use multiplanar API for single-plane formats? I'm very much opposed to making num_planes=1 a special case in the multiplanar API. Just like the extended control API can still handle 'normal' controls (well, they should, at least :-) ), so should the multiplanar API be a superset of the normal API. Anything else would make applications unnecessarily complex. Any driver that has multiplanar formats should be using the videobuf2 framework. Which will hopefully make it very easy to have support for both 1-plane and multiplanar APIs. Probably the only place where you need to do something special is in ENUM_FMT: the multiplanar stream type will enumerate all formats, the 'old' stream types will only enumerate the 1-plane formats. So minimal impact to the driver, but nicely consistent towards the application. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC v4] Multi-plane buffer support for V4L2 API
Hi Mauro, thanks for taking the time to look at this. Mauro Carvalho Chehab mche...@redhat.com wrote: With Hans proposed changes that you've already acked, I think the proposal is ok, except for one detail: 4. Format enumeration -- struct v4l2_fmtdesc, used for format enumeration, does include the v4l2_buf_type enum as well, so the new types can be handled properly here as well. For drivers supporting both versions of the API, 1-plane formats should be returned for multiplanar buffer types as well, for consistency. In other words, for multiplanar buffer types, the formats returned are a superset of those returned when enumerating with the old buffer types. We shouldn't mix types here. If the userspace is asking for multi-planar types, the driver should return just the multi-planar formats. If the userspace wants to know about both, it will just call for both types of formats. Yes. Although the idea here is that we wanted to be able to use single-planar formats with either the old API or the new multiplane API. In the new API, you could just set num_planes=1. So multiplanar API != multiplanar format. When you enum_fmt for mutliplanar types, you get all formats you can use with the multiplanar API and not all formats that have num_planes 1. This can simplify applications - they don't have to switch between APIs when switching between formats. They may even choose not to use the old API at all (if a driver allows it). Do we want to lose the ability to use multiplanar API for single-plane formats? Best regards -- Pawel Osciak Linux Platform Group Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC v4] Multi-plane buffer support for V4L2 API
Hi Hans, thank you for your comments as always. Hans Verkuil wrote hverk...@xs4all.nl: Hi Pawel, Looks good, but I have a few small suggestions: On Friday 09 July 2010 20:59:45 Pawel Osciak wrote: (snip) struct v4l2_format { enum v4l2_buf_type type; union { struct v4l2_pix_format pix; /* V4L2_BUF_TYPE_VIDEO_CAPTURE */ + struct v4l2_pix_format_mplane mp_pix; /* V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE */ I would probably go with pix_mp to be consistent with the name of the struct. ok. +/** + * struct v4l2_pix_format_mplane - multiplanar format definition + * @pix_fmt: definition of an image format for all planes + * @plane_fmt: per-plane information + */ +struct v4l2_pix_format_mplane { + struct v4l2_pix_format pix_fmt; + struct v4l2_plane_formatplane_fmt[VIDEO_MAX_PLANES]; +} __attribute__ ((packed)); How do you know how many planes there are? I wonder whether it wouldn't be smarter to just copy the relevant fields from struct v4l2_pix_format to struct v4l2_pix_format_mplane instead of embedded that struct. That way you can 1) add a 'planes' field and 2) get rid of the no longer needed bytesperline and sizeimage fields. And I think the priv field should also go, just have a reserved[2] instead. By mean planes you mean a field indicating the number of planes in the current format, right? Number of planes can be inferred from fourcc, but you are right, it's still useful to have to have a field for that. What do you think of this: /** * struct v4l2_pix_format_mplane - multiplanar format definition * @width: image width in pixels * @height: image height in pixels * @pixelformat:little endian four character code (fourcc) * @field: field order (for interlaced video) * @colorspace: supplemental to pixelformat * @plane_fmt: per-plane information * @num_planes: number of planes for this format and number of valid * elements in plane_fmt array */ struct v4l2_pix_format_mplane { __u32 width; __u32 height; __u32 pixelformat; enum v4l2_field field; enum v4l2_colorspacecolorspace; struct v4l2_plane_formatplane_fmt[VIDEO_MAX_PLANES]; __u8num_planes; __u8reserved[11]; } __attribute__ ((packed)); v4l2_plane_format stays the same (see below). 8 * struct v4l2_plane_format + 3 * 4 + 2 * enum + 12 * 1 = 8 * 20 + 40 = 200 The plane format struct is as follows: +/** + * struct v4l2_plane_format - additional, per-plane format definition + * @sizeimage: maximum size in bytes required for data, for which + * this plane will be used + * @bytesperline: distance in bytes between the leftmost pixels in two + * adjacent lines + */ +struct v4l2_plane_format { + __u32 sizeimage; + __u16 bytesperline; + __u16 reserved[7]; +} __attribute__ ((packed)); Note that bytesperline is u16 now, but that shouldn't hurt. Fitting everything into v4l2_format's union (which is 200 bytes long): v4l2_pix_format shouldn't be larger than 40 bytes. 8 * struct v4l2_plane_format + struct v4l2_pix_format = 8 * 20 + 40 = 200 (snip) 2. Plane description struct -- +/** + * struct v4l2_plane - plane info for multiplanar buffers + * @bytesused: number of bytes occupied by data in the plane (payload) + * @mem_off: when memory in the associated struct v4l2_buffer is + * V4L2_MEMORY_MMAP, equals the offset from the start of the + * device memory for this plane (or is a cookie that should be + * passed to mmap() called on the video node) + * @userptr: when memory is V4L2_MEMORY_USERPTR, a userspace pointer pointing + * to this plane + * @length:size of this plane (NOT the payload) in bytes + * @data_off: offset in plane to the start of data/end of header, if relevant + * + * Multi-plane buffers consist of two or more planes, e.g. an YCbCr buffer + * with two planes has one plane for Y, and another for interleaved CbCr + * components. Each plane can reside in a separate memory buffer, or in + * a completely separate memory chip even (e.g. in embedded devices). + */ +struct v4l2_plane { + __u32 bytesused; + __u32 length; + union { + __u32 mem_off; Rename to mem_offset. This prevents confusion since 'off' can also be interpreted as the opposite of 'on'. + unsigned long userptr; + } m; + __u32 data_off; Rename to data_offset. Ok to both. Best regards -- Pawel
Re: [RFC v4] Multi-plane buffer support for V4L2 API
On Monday 12 July 2010 11:14:30 Pawel Osciak wrote: Hi Hans, thank you for your comments as always. Hans Verkuil wrote hverk...@xs4all.nl: Hi Pawel, Looks good, but I have a few small suggestions: On Friday 09 July 2010 20:59:45 Pawel Osciak wrote: (snip) struct v4l2_format { enum v4l2_buf_type type; union { struct v4l2_pix_format pix; /* V4L2_BUF_TYPE_VIDEO_CAPTURE */ + struct v4l2_pix_format_mplane mp_pix; /* V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE */ I would probably go with pix_mp to be consistent with the name of the struct. ok. +/** + * struct v4l2_pix_format_mplane - multiplanar format definition + * @pix_fmt: definition of an image format for all planes + * @plane_fmt: per-plane information + */ +struct v4l2_pix_format_mplane { + struct v4l2_pix_format pix_fmt; + struct v4l2_plane_formatplane_fmt[VIDEO_MAX_PLANES]; +} __attribute__ ((packed)); How do you know how many planes there are? I wonder whether it wouldn't be smarter to just copy the relevant fields from struct v4l2_pix_format to struct v4l2_pix_format_mplane instead of embedded that struct. That way you can 1) add a 'planes' field and 2) get rid of the no longer needed bytesperline and sizeimage fields. And I think the priv field should also go, just have a reserved[2] instead. By mean planes you mean a field indicating the number of planes in the current format, right? Right. Number of planes can be inferred from fourcc, but you are right, it's still useful to have to have a field for that. You really don't want to have a switch on the fourcc just to determine the number of planes. Creating a separate field will make life much easier. What do you think of this: /** * struct v4l2_pix_format_mplane - multiplanar format definition * @width:image width in pixels * @height: image height in pixels * @pixelformat: little endian four character code (fourcc) * @field:field order (for interlaced video) * @colorspace: supplemental to pixelformat * @plane_fmt:per-plane information * @num_planes: number of planes for this format and number of valid *elements in plane_fmt array */ struct v4l2_pix_format_mplane { __u32 width; __u32 height; __u32 pixelformat; enum v4l2_field field; enum v4l2_colorspacecolorspace; struct v4l2_plane_formatplane_fmt[VIDEO_MAX_PLANES]; __u8num_planes; __u8reserved[11]; } __attribute__ ((packed)); v4l2_plane_format stays the same (see below). 8 * struct v4l2_plane_format + 3 * 4 + 2 * enum + 12 * 1 = 8 * 20 + 40 = 200 Looks good. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v4] Multi-plane buffer support for V4L2 API
Hi Pawel, Em 09-07-2010 15:59, Pawel Osciak escreveu: Hello, This is the fourth version of the multi-plane API extensions proposal. I think that we have reached a stage at which it is more or less finalized. Rationale can be found at the beginning of the original thread: http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/11212 With Hans proposed changes that you've already acked, I think the proposal is ok, except for one detail: 4. Format enumeration -- struct v4l2_fmtdesc, used for format enumeration, does include the v4l2_buf_type enum as well, so the new types can be handled properly here as well. For drivers supporting both versions of the API, 1-plane formats should be returned for multiplanar buffer types as well, for consistency. In other words, for multiplanar buffer types, the formats returned are a superset of those returned when enumerating with the old buffer types. We shouldn't mix types here. If the userspace is asking for multi-planar types, the driver should return just the multi-planar formats. If the userspace wants to know about both, it will just call for both types of formats. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v4] Multi-plane buffer support for V4L2 API
Hi Pawel, Looks good, but I have a few small suggestions: On Friday 09 July 2010 20:59:45 Pawel Osciak wrote: Hello, This is the fourth version of the multi-plane API extensions proposal. I think that we have reached a stage at which it is more or less finalized. Rationale can be found at the beginning of the original thread: http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/11212 === I. Multiplanar API description/negotiation === 1. Maximum number of planes -- We've chosen the maximum number of planes to be 8 per buffer: +#define VIDEO_MAX_PLANES 8 2. Capability checks -- If a driver supports multiplanar API, it can turn on one (or both) of the new capability flags: +/* Is a video capture device that supports multiplanar formats */ +#define V4L2_CAP_VIDEO_CAPTURE_MPLANE 0x1000 +/* Is a video output device that supports multiplanar formats */ +#define V4L2_CAP_VIDEO_OUTPUT_MPLANE 0x2000 - any combination of those flags is valid; - any combinations with the old, non-multiplanar V4L2_CAP_VIDEO_CAPTURE and V4L2_CAP_VIDEO_OUTPUT flags are also valid; - the new flags indicate, that a driver supports the new API, but it does NOT have to actually use multiplanar formats; it is perfectly possible and valid to use the new API for 1-plane buffers as well; using the new API for both 1-planar and n-planar formats makes the applications simpler, as they do not have to fall back to the old API in the former case. 3. Describing multiplanar formats -- To describe multiplanar formats, we have to extend the format struct: struct v4l2_format { enum v4l2_buf_type type; union { struct v4l2_pix_format pix; /* V4L2_BUF_TYPE_VIDEO_CAPTURE */ + struct v4l2_pix_format_mplane mp_pix; /* V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE */ I would probably go with pix_mp to be consistent with the name of the struct. struct v4l2_window win; /* V4L2_BUF_TYPE_VIDEO_OVERLAY */ struct v4l2_vbi_format vbi; /* V4L2_BUF_TYPE_VBI_CAPTURE */ struct v4l2_sliced_vbi_format sliced; /* V4L2_BUF_TYPE_SLICED_VBI_CAPTURE */ __u8raw_data[200]; /* user-defined */ } fmt; }; We need a new buffer type to recognize when to use mp_pix instead of pix. Or, actually, two buffer types, for both OUTPUT and CAPTURE: enum v4l2_buf_type { /* ... */ + V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE = 9, + V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE = 10, /* ... */ }; For those buffer types, we use struct v4l2_pix_format_mplane: +/** + * struct v4l2_pix_format_mplane - multiplanar format definition + * @pix_fmt: definition of an image format for all planes + * @plane_fmt: per-plane information + */ +struct v4l2_pix_format_mplane { + struct v4l2_pix_format pix_fmt; + struct v4l2_plane_formatplane_fmt[VIDEO_MAX_PLANES]; +} __attribute__ ((packed)); How do you know how many planes there are? I wonder whether it wouldn't be smarter to just copy the relevant fields from struct v4l2_pix_format to struct v4l2_pix_format_mplane instead of embedded that struct. That way you can 1) add a 'planes' field and 2) get rid of the no longer needed bytesperline and sizeimage fields. And I think the priv field should also go, just have a reserved[2] instead. The pix_fmt member is to be used in the same way as in the old API. Its fields: - width, height, field, colorspace, priv retain their old meaning; - pixelformat is still fourcc, but new fourcc values have to be introduced for multiplanar formats; - bytesperline, sizeimage lose their meanings and are replaced by their counterparts in the new, per-plane format struct. The plane format struct is as follows: +/** + * struct v4l2_plane_format - additional, per-plane format definition + * @sizeimage: maximum size in bytes required for data, for which + * this plane will be used + * @bytesperline: distance in bytes between the leftmost pixels in two + * adjacent lines + */ +struct v4l2_plane_format { + __u32 sizeimage; + __u16 bytesperline; + __u16 reserved[7]; +} __attribute__ ((packed)); Note that bytesperline is u16 now, but that shouldn't hurt. Fitting everything into v4l2_format's union (which is 200 bytes long): v4l2_pix_format shouldn't be larger than 40 bytes. 8 * struct v4l2_plane_format + struct v4l2_pix_format = 8 * 20 + 40 = 200 4. Format enumeration -- struct v4l2_fmtdesc,