RE: [PATCH/RFC] media: vb2: change queue initialization order

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello,

On Wednesday, June 29, 2011 3:26 PM wrote:

> On Wed, 29 Jun 2011 11:49:06 +0200
> Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
> 
> > This patch introduces VB2_STREAMON_WITHOUT_BUFFERS io flag and changes
> > the order of operations during stream on operation. Now the buffer are
> > first queued to the driver and then the start_streaming method is called.
> 
> Thanks for addressing this.  But I do have to wonder if this flag is really
> necessary.  The effort to set a "they want to start streaming" flag and
> start things for real in buf_queue() is not *that* great; if doing so
> avoids causing failures in applications which are following the rules, it
> seems worth it.
> 
> >  	/*
> > +	 * If any buffers were queued before streamon,
> > +	 * we can now pass them to driver for processing.
> > +	 */
> > +	list_for_each_entry(vb, &q->queued_list, queued_entry)
> > +		__enqueue_in_driver(vb);
> > +
> > +	/*
> >  	 * Let driver notice that streaming state has been enabled.
> >  	 */
> >  	ret = call_qop(q, start_streaming, q);
> 
> I do still wonder why this is an issue - why not pass the buffers through
> to the driver at VIDIOC_QBUF time?  I assume there must be a reason for
> doing things this way, I'd like to understand what it is.

I want to delay giving the ownership of the buffers to the driver until it
is certain that start_streaming method will be called. This way I achieve
a well defined states of the queued buffers:

1. successful start_streaming() -> the driver is processing the queue buffers
2. unsuccessful start_streaming() -> the driver is responsible to discard all
   queued buffers
3. stop_streaming() called -> the driver has finished or discarded all queued
   buffers

If we assume that the buffers are given to the driver in QBUF then we need
to address somehow the following call sequence:

1. REQBUFS(n1) -> allocated n1 buffers
2. QBUF(buffer 1) -> gives buffer to the driver
3. QBUF(buffer 2) -> gives buffer to the driver
4. REQBUFS(n2) -> frees all buffers (driver still keeps 2 already queued
                  buffers) and allocates n2 new buffers

There is no way to tell the driver to discard the old buffers. Additional 
call to stop_streaming method might solve it, but I really don't like
to abuse it here if delaying the ownership transfer just solves the issue.

If you ask why there is a start_streaming callback at all then the reason 
is simple - I would like to have a single place in the driver, where the
streaming is started (streaming might be started as a result of STREAMON
ioctl, read() call or recently discussed poll()).

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux