[linux-dvb] Re: More V4 Video API Q's

[Johannes Stezenbach <js@linuxtv.org>]

Rob.McConnell@Zarlink.Com wrote:
> Johannes Stezenbach <js@linuxtv.org> wrote on 05/10/2004 23:42:39:
> > On Mon, Sep 27, 2004 at 03:29:56PM +0000, Rob.McConnell@Zarlink.Com
> > > 3) I would like to see a DVB_VIDEO_EXIT or DVB_VIDEO_DEINIT ioctl that
> > > unblocks all processes sleeping on wait queues so that they can be
> woken up
> > > prior to the driver being closed.  For example,  if you have multiple
> > > threads that access the video device and one is blocked on the
> > > DVB_VIDEO_GET_EVENT ioctl, then when the user-space application calls
> > > "close", the actual close function will NOT be called until all threads
> > > have exited.  By calling DVB_VIDEO_EXIT or DVB_VIDEO_DEINIT first, all
> > > threads can be terminated in a controlled fashion and the close syscall
> can
> > > then complete.  I have used this method on current drivers and works
> very
> > > well indeed.  Maybe we should reflect this ioctl in other drivers as
> well?
> >
> > That sounds totally bogus to me. If a thread blocking in some ioctl
> > prevents you from doing a close() in another thread (which could then
> > wake up the other thread) then our or your locking is broken.
> > Or maybe I didn't understand what you mean...
> 
> The close() syscall will not actually call the code (release method) to
> close the device down until the usage count in the "file" structure reaches
> 0.  This means if you either opened the device multiple times or called
> clone()/fork() on the fd, then the usage count will be incremented.  Now in
> you have called clone() (by calling "pthread_create" say), then the usage
> count will have been incremented.  If the thread is blocking on an event,
> then a close() syscall in the parent process will not evoke the release
> method until the thread has exited.

Threads share file descriptors, there is no increment in use count
after pthread_create() like it is after fork(). IOW, the
CLONE_FILES flag is passed to clone() when a new thread is
created. So you should either avoid opening the device twice,
or use poll() with some other fd you can use to signal your
thread to exit. Either way, it is a problem of your app and
your proposed API extension is the wrong way to solve it.


> A few other comments to make on the video API:
> 
> (i) We have the ioctl "DVB_VIDEO_SET_SOURCE" that takes a structure
> "dvb_video_source".  However, the desired format type is only relevant to
> memory inputs, not demux.  Wouldn't it be cleaner to have the format and fd
> as a union?
> 
> e.g. struct dvb_video_source {
>         enum dvb_video_source_type type;
>         union {
>             enum dvb_video_source_format format;
>             int fd;
>         }u;
>      }
> 
> (ii) Shouldn't the play state "enum dvb_video_play_state" have another
> entry for when the decoder is in still picture mode?

Both points are worth considering.

Regards,
Johannes