RE: vb2: holding buffers until after start_streaming()

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

 



Hello,

On Monday, June 20, 2011 3:28 AM Pawel Osciak wrote:

> On Fri, Jun 17, 2011 at 11:57, Jonathan Corbet <corbet@xxxxxxx> wrote:
> > Here's another videobuf2 question...I've been trying to track down some
> > weird behavior, the roots of which were in the fact that
> start_streaming()
> > gets called even though no buffers have been queued.  This behavior is
> > quite explicit in the code:
> >
> >        /*
> >         * Let driver notice that streaming state has been enabled.
> >         */
> >        ret = call_qop(q, start_streaming, q);
> >        if (ret) {
> >                dprintk(1, "streamon: driver refused to start
> streaming\n");
> >                return ret;
> >        }
> >
> >        q->streaming = 1;
> >
> >        /*
> >         * 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);
> >
> > Pretty much every v4l2 capture application I've ever encountered passes
> all
> > of its buffers to VIDIOC_QBUF before starting streaming for a reason - it
> > makes little sense to start if there's nothing to stream to.  It's really
> > tempting to reorder that code, but...  it seems you must have done things
> > this way for a reason.  Why did you need to reorder the operations in
> this
> > way?
> >
> 
> I don't see a reason why these couldn't be reordered (Marek should be
> able to confirm, he wrote those lines). But this wouldn't fix
> everything, as the V4L2 API permits streamon without queuing any
> buffers first (for capture devices). So even reordered, it's possible
> for start_streaming to be called without passing any buffers to the
> driver first.

The problem is the fact that you cannot guarantee the opposite order
in all cases. Even if you swap __enqueue_in_driver and 
call_qop(start_streaming), user might call respective ioctl in the
opposite order and you will end with start_streaming before 
__enqueue_in_driver. Calling VIDIOC_STREAMON without previous call
to VIDIOC_QBUF is legal from v4l2 api definition.

Because of that I decided to call start_streaming first, before the 
__enqueue_in_driver() to ensure the drivers will get their methods
called always in the same order, whatever used does. 

Start_streaming was designed to perform time consuming operations
like enabling the power, configuring the pipeline, setting up the
tuner, etc. Some of these can fail and it is really good to report
the failure asap.

If you cannot start your hardware (the dma engine) without queued
buffers then you probably need to move dma starting routine to the
first buffer_queue call. The problem is much more complex than it
initially looks. 

Please note that in videobuf2 buffer_queue method is allowed to sleep,
unlike it was designed in old videobuf.

Usually drivers require at least two buffers and always keep at 
least one in the dma engine, which overwrites it with incoming frames
until next buffer have been queued. However there are also devices
(like camera sensors) that might be used to capture only one single
frame or a few consecutive frames (for example a series of pictures).
They need to dequeue the last buffer once it got filled with video
data, so the design with overwriting the buffer makes no sense.
Right now it is really driver dependent and no generic solution 
exist.

We have been discussing it but no consensus has been made yet, so
right now I've decided to keep the current design. We probably needs
some additional flag somewhere to configure the driver either to
continuously overwrite last buffer until next one has been queued
or stop the dma engine and return the buffer to user. Once


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