Re: [RFC v4] Multi-plane buffer support for V4L2 API

2010-07-17 Thread Hans Verkuil
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

2010-07-13 Thread Pawel Osciak
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

2010-07-12 Thread Pawel Osciak
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

2010-07-12 Thread Hans Verkuil
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

2010-07-12 Thread Mauro Carvalho Chehab
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

2010-07-10 Thread Hans Verkuil
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,