On Tue July 10 2012 05:17:41 Ezequiel Garcia wrote:
> Hey Mauro,
> 
> On Fri, Jul 6, 2012 at 11:41 AM, Ezequiel Garcia <elezegar...@gmail.com> 
> wrote:
> > On Thu, Jul 5, 2012 at 9:01 PM, Mauro Carvalho Chehab
> > <mche...@redhat.com> wrote:
> >> Em 05-07-2012 19:36, Sylwester Nawrocki escreveu:
> >>> On 07/06/2012 12:11 AM, Mauro Carvalho Chehab wrote:
> >>>>> +static int vidioc_dqbuf(struct file *file, void *priv, struct 
> >>>>> v4l2_buffer *p)
> >>>>> +{
> >>>>> +   struct stk1160 *dev = video_drvdata(file);
> >>>>> +
> >>>>> +   if (!stk1160_is_owner(dev, file))
> >>>>> +           return -EBUSY;
> >>>>> +
> >>>>> +   return vb2_dqbuf(&dev->vb_vidq, p, file->f_flags&  O_NONBLOCK);

Take a look at the latest videobuf2-core.h: I've added helper functions
that check the owner. You can probably simplify the driver code quite a bit
by using those helpers.

> >>>>
> >>>> Why to use O_NONBLOCK here? it should be doing whatever userspace wants.
> >>>
> >>> This is OK, since the third argument to vb2_dqbuf() is a boolean 
> >>> indicating
> >>> whether this call should be blocking or not. And a "& O_NONBLOCK" masks 
> >>> this
> >>> information out from file->f_flags.
> >>
> >> Ah! OK then.
> >>
> >> It might be better to initialize it during vb2 initialization, at open,
> >> instead of requiring this argument every time vb_dqbuf() is called.

You can't do this at open since the application can change the NONBLOCK mode
after open. So the current approach is correct.

Regards,

        Hans

> Currently stk1160 doesn't implement an open call, but uses v4l2_fh_open 
> instead.
> I'm not sure I should add a separate open, or perhaps you would accept
> to initialize this non-block flag in vidioc_reqbufs.
> 
> On the other hand, many drivers are doing it at dqbuf, like here at stk1160,
> and I was wondering: is it *that* bad?
> 
> Thanks,
> Ezequiel.
> 
--
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

Reply via email to