From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: Hans Verkuil <hansverk@cisco.com>
Cc: Pawel Osciak <pawel@osciak.com>,
Linux Media Mailing List <linux-media@vger.kernel.org>,
Sakari Ailus <sakari.ailus@iki.fi>,
Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Mauro Carvalho Chehab <mchehab@infradead.org>
Subject: Re: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
Date: Mon, 22 Aug 2011 15:54:03 +0200 (CEST) [thread overview]
Message-ID: <Pine.LNX.4.64.1108221448030.29246@axis700.grange> (raw)
In-Reply-To: <201108221316.27491.hansverk@cisco.com>
We discussed a bit more with Hans on IRC, and below is my attempt of a
summary. Hans, please, correct me, if I misunderstood anything. Pawel,
Sakari, Laurent: please, reply, whether you're ok with this.
On Mon, 22 Aug 2011, Hans Verkuil wrote:
> On Monday, August 22, 2011 12:40:25 Guennadi Liakhovetski wrote:
[snip]
> > It would be good if you also could have a look at my reply to this Pawel's
> > mail:
> >
> > http://article.gmane.org/gmane.linux.drivers.video-input-
> infrastructure/36905
> >
> > and, specifically, at the vb2_parse_planes() function in it. That's my
> > understanding of what would be needed, if we preserve .queue_setup() and
> > use your last suggestion to include struct v4l2_format in struct
> > v4l2_create_buffers.
>
> vb2_parse_planes can be useful as a utility function that 'normal' drivers can
> call from the queue_setup. But vb2 should not parse the format directly, it
> should just pass it on to the driver through the queue_setup function.
>
> You also mention: "All frame-format fields like fourcc code, width, height,
> colorspace are only input from the user. If the user didn't fill them in, they
> should not be used."
>
> I disagree with that. The user should fill in a full format description, just
> as with S/TRY_FMT. That's the information that the driver will use to set up
> the buffers. It could have weird rules like: if the fourcc is this, and the
> size is less than that, then we can allocate in this memory bank.
>
> It is also consistent with REQBUFS: there too the driver uses a full format
> (i.e. the last set format).
>
> I would modify queue_setup to something like this:
>
> int (*queue_setup)(struct vb2_queue *q, struct v4l2_format *fmt,
> unsigned int *num_buffers,
> unsigned int *num_planes, unsigned int sizes[],
> void *alloc_ctxs[]);
>
> Whether fmt is left to NULL in the reqbufs case, or whether the driver has to
> call g_fmt first before calling vb2 is something that could be decided by what
> is easiest to implement.
1. VIDIOC_CREATE_BUFS passes struct v4l2_create_buffers from the user to
the kernel, in which struct v4l2_format is embedded. The user _must_
fill in .type member of struct v4l2_format. For .type ==
V4L2_BUF_TYPE_VIDEO_CAPTURE or V4L2_BUF_TYPE_VIDEO_OUTPUT .fmt.pix is
used, for .type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE or
V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE .fmt.pix_mp is used. In both these
cases the user _must_ fill in .width, .height, .pixelformat, .field,
.colorspace by possibly calling VIDIOC_G_FMT or VIDIOC_TRY_FMT. The
user also _may_ optionally fill in any further buffer-size related
fields, if it believes to have any special requirements to them. On
a successful return from the ioctl() .count and .index fields are
filled in by the kernel, .format stays unchanged. The user has to call
VIDIOC_QUERYBUF to retrieve specific buffer information.
2. Videobuf2 drivers, that implement .vidioc_create_bufs() operation, call
vb2_create_bufs() with a pointer to struct v4l2_create_buffers as a
second argument. vb2_create_bufs() in turn calls the .queue_setup()
driver callback, whose prototype is modified as follows:
int (*queue_setup)(struct vb2_queue *q, const struct v4l2_format *fmt,
unsigned int *num_buffers,
unsigned int *num_planes, unsigned int sizes[],
void *alloc_ctxs[]);
with &create->format as a second argument. As pointed out above, this
struct is not modified by V4L, instead, the usual arguments 3-6 are
filled in by the driver, which are then used by vb2_create_bufs() to
call __vb2_queue_alloc().
3. vb2_reqbufs() shall call .queue_setup() with fmt == NULL, which will be
a signal to the driver to use the current format.
4. We keep .queue_setup(), because its removal would inevitably push a
part of the common code from vb2_reqbufs() and vb2_create_bufs() down
into drivers, thus creating code redundancy and increasing its
complexity.
You have 24 hours to object, before I proceed with the next version;-)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
next prev parent reply other threads:[~2011-08-22 13:54 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-05 7:47 [PATCH 0/6 v4] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management Guennadi Liakhovetski
2011-08-06 18:42 ` Sakari Ailus
2011-08-17 8:41 ` Guennadi Liakhovetski
2011-08-17 12:13 ` Sakari Ailus
2011-08-06 21:51 ` Pawel Osciak
2011-08-08 9:16 ` Hans Verkuil
2011-08-08 11:40 ` Laurent Pinchart
2011-08-08 12:40 ` Hans Verkuil
2011-08-08 22:06 ` Laurent Pinchart
2011-08-08 22:46 ` Sakari Ailus
2011-08-09 7:26 ` Hans Verkuil
2011-08-09 23:37 ` Sakari Ailus
2011-08-10 6:25 ` Hans Verkuil
2011-08-11 11:09 ` Sakari Ailus
2011-08-15 11:28 ` Guennadi Liakhovetski
2011-08-15 11:36 ` Hans Verkuil
2011-08-15 13:45 ` Guennadi Liakhovetski
2011-08-16 13:13 ` Guennadi Liakhovetski
2011-08-16 16:14 ` Pawel Osciak
2011-08-17 9:11 ` Guennadi Liakhovetski
2011-08-17 9:14 ` Laurent Pinchart
2011-08-17 15:29 ` Pawel Osciak
2011-08-22 10:06 ` Hans Verkuil
2011-08-22 10:40 ` Guennadi Liakhovetski
2011-08-22 11:16 ` Hans Verkuil
2011-08-22 13:54 ` Guennadi Liakhovetski [this message]
2011-08-22 14:01 ` Hans Verkuil
2011-08-22 15:42 ` Laurent Pinchart
2011-08-22 15:52 ` Hans Verkuil
2011-08-22 17:21 ` Laurent Pinchart
2011-08-23 6:31 ` Hans Verkuil
2011-08-24 4:05 ` Pawel Osciak
2011-08-24 6:44 ` Hans Verkuil
2011-08-17 13:22 ` Marek Szyprowski
2011-08-17 14:57 ` Pawel Osciak
2011-08-18 5:49 ` Marek Szyprowski
2011-08-05 7:47 ` [PATCH 2/6 v4] V4L: add a new videobuf2 buffer state VB2_BUF_STATE_PREPARED Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 3/6 v4] V4L: vb2: change .queue_setup() argument to unsigned int Guennadi Liakhovetski
2011-08-05 21:36 ` [PATCH 3/6 v5] V4L: vb2: prepare to support multi-size buffers Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 4/6 v4] V4L: vb2: add support for buffers of different sizes on a single queue Guennadi Liakhovetski
2011-08-06 18:56 ` Sakari Ailus
2011-08-17 8:44 ` Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 5/6 v4] V4L: sh-mobile-ceu-camera: prepare to support multi-size buffers Guennadi Liakhovetski
2011-08-05 21:39 ` [PATCH 5/6 v5] " Guennadi Liakhovetski
2011-08-05 7:47 ` [PATCH 6/6 v4] V4L: soc-camera: add 2 new ioctl() handlers Guennadi Liakhovetski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.64.1108221448030.29246@axis700.grange \
--to=g.liakhovetski@gmx.de \
--cc=hansverk@cisco.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@infradead.org \
--cc=pawel@osciak.com \
--cc=sakari.ailus@iki.fi \
--cc=sakari.ailus@maxwell.research.nokia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.