Re: [RFC v2 0/7] V4L2 file handles and event interface

2010-01-19 Thread Laurent Pinchart
Hi Hans,

On Monday 18 January 2010 14:07:33 Hans Verkuil wrote:
 On Tue, 22 Dec 2009, Sakari Ailus wrote:
  Hi,
 
  Here's the second version of the V4L2 file handle and event interface
  patchset. Still RFC since I'd like to get more feedback on it.
 
  The first patch adds the V4L2 file handle support and the rest are for
  V4L2 events.
 
  The patchset works with the OMAP 3 ISP driver. Patches for OMAP 3 ISP are
  not part of this patchset but are available in Gitorious (branch is
  called events):
 
  git://gitorious.org/omap3camera/mainline.git event
 
  The major change since the last one v4l2_fh structure is now part of
  driver's own file handle. It's used as file-private_data as well. I did
  this based on Hans Verkuil's suggestion. Sequence numbers and event queue
  length limitation is there as well. There are countless of smaller
  changes, too.
 
  A few notes on the patches:
 
  - I don't like the locking too much. Perhaps the file handle specific
  lock (events-lock) could be dropped in favour of the lock for
  v4l2_file_handle in video_device?
 
  - Poll. The V4L2 specifiction says:
 
  When the application did not call VIDIOC_QBUF or
  VIDIOC_STREAMON yet the poll() function succeeds, but sets the
  POLLERR flag in the revents field.
 
   The current events for OMAP 3 ISP are related to streaming but not all
  might be in future. For example there might be some radio or DVB related
  events.
 
 I know for sure that we will have to be able to handle events when not
 streaming. E.g. events that tell when a HDMI connector was plugged in
 or when the EDID was read will need to arrive whether streaming is on
 or off.

I agree with you. The V4L2 specification will then need to be changed, 
otherwise we won't be able to poll() for events. poll() wouldn't return 
immediately anymore if no buffer is queued or if the device isn't streaming. 
That might break existing applications (although we could argue that some of 
those applications are somehow broken already if they rely on such a weird 
feature).

If we want to avoid disturbing existing applications we could still return 
POLLERR immediately when not streaming if no event has been subscribed to.

  - Sequence numbers are local to file handles.
 
 That is how it should be.
 
  - Subscribing V4L2_EVENT_ALL causes any other events to be unsubscribed.
 
  - If V4L2_EVENT_ALL has been subscribed, unsubscribing any one of the
  events leads to V4L2_EVENT_ALL to be unsubscribed. This problem would be
  difficult to work around since this would require the event system to be
  aware of the driver private events as well.
 
 Good point. Perhaps attempting to unsubscribe a single event when EVENT_ALL
 has been subscribed should result in an error? I.e., you can only
 unsubscribe ALL when you subscribed to ALL in the first place.

-- 
Regards,

Laurent Pinchart
--
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


Re: [RFC v2 0/7] V4L2 file handles and event interface

2010-01-19 Thread Hans Verkuil

 Hi Hans,

 On Monday 18 January 2010 14:07:33 Hans Verkuil wrote:
 On Tue, 22 Dec 2009, Sakari Ailus wrote:
  Hi,
 
  Here's the second version of the V4L2 file handle and event interface
  patchset. Still RFC since I'd like to get more feedback on it.
 
  The first patch adds the V4L2 file handle support and the rest are for
  V4L2 events.
 
  The patchset works with the OMAP 3 ISP driver. Patches for OMAP 3 ISP
 are
  not part of this patchset but are available in Gitorious (branch is
  called events):
 
 git://gitorious.org/omap3camera/mainline.git event
 
  The major change since the last one v4l2_fh structure is now part of
  driver's own file handle. It's used as file-private_data as well. I
 did
  this based on Hans Verkuil's suggestion. Sequence numbers and event
 queue
  length limitation is there as well. There are countless of smaller
  changes, too.
 
  A few notes on the patches:
 
  - I don't like the locking too much. Perhaps the file handle specific
  lock (events-lock) could be dropped in favour of the lock for
  v4l2_file_handle in video_device?
 
  - Poll. The V4L2 specifiction says:
 
 When the application did not call VIDIOC_QBUF or
 VIDIOC_STREAMON yet the poll() function succeeds, but sets the
 POLLERR flag in the revents field.
 
   The current events for OMAP 3 ISP are related to streaming but not
 all
  might be in future. For example there might be some radio or DVB
 related
  events.

 I know for sure that we will have to be able to handle events when not
 streaming. E.g. events that tell when a HDMI connector was plugged in
 or when the EDID was read will need to arrive whether streaming is on
 or off.

 I agree with you. The V4L2 specification will then need to be changed,
 otherwise we won't be able to poll() for events. poll() wouldn't return
 immediately anymore if no buffer is queued or if the device isn't
 streaming.
 That might break existing applications (although we could argue that some
 of
 those applications are somehow broken already if they rely on such a weird
 feature).

 If we want to avoid disturbing existing applications we could still return
 POLLERR immediately when not streaming if no event has been subscribed to.

I think this is a very reasonable approach.

Regards,

 Hans


  - Sequence numbers are local to file handles.

 That is how it should be.

  - Subscribing V4L2_EVENT_ALL causes any other events to be
 unsubscribed.
 
  - If V4L2_EVENT_ALL has been subscribed, unsubscribing any one of the
  events leads to V4L2_EVENT_ALL to be unsubscribed. This problem would
 be
  difficult to work around since this would require the event system to
 be
  aware of the driver private events as well.

 Good point. Perhaps attempting to unsubscribe a single event when
 EVENT_ALL
 has been subscribed should result in an error? I.e., you can only
 unsubscribe ALL when you subscribed to ALL in the first place.

 --
 Regards,

 Laurent Pinchart



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

--
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


Re: [RFC v2 0/7] V4L2 file handles and event interface

2010-01-18 Thread Hans Verkuil

Hi Sakari,

Some notes on this as well (and that concludes my review):

On Tue, 22 Dec 2009, Sakari Ailus wrote:


Hi,

Here's the second version of the V4L2 file handle and event interface 
patchset. Still RFC since I'd like to get more feedback on it.


The first patch adds the V4L2 file handle support and the rest are for V4L2 
events.


The patchset works with the OMAP 3 ISP driver. Patches for OMAP 3 ISP are not 
part of this patchset but are available in Gitorious (branch is called 
events):


git://gitorious.org/omap3camera/mainline.git event

The major change since the last one v4l2_fh structure is now part of driver's 
own file handle. It's used as file-private_data as well. I did this based on 
Hans Verkuil's suggestion. Sequence numbers and event queue length limitation 
is there as well. There are countless of smaller changes, too.


A few notes on the patches:

- I don't like the locking too much. Perhaps the file handle specific lock 
(events-lock) could be dropped in favour of the lock for v4l2_file_handle in 
video_device?


- Poll. The V4L2 specifiction says:

When the application did not call VIDIOC_QBUF or
VIDIOC_STREAMON yet the poll() function succeeds, but sets the
POLLERR flag in the revents field.

 The current events for OMAP 3 ISP are related to streaming but not all 
might be in future. For example there might be some radio or DVB related 
events.


I know for sure that we will have to be able to handle events when not
streaming. E.g. events that tell when a HDMI connector was plugged in
or when the EDID was read will need to arrive whether streaming is on
or off.


- Sequence numbers are local to file handles.


That is how it should be.


- Subscribing V4L2_EVENT_ALL causes any other events to be unsubscribed.

- If V4L2_EVENT_ALL has been subscribed, unsubscribing any one of the events 
leads to V4L2_EVENT_ALL to be unsubscribed. This problem would be difficult 
to work around since this would require the event system to be aware of the 
driver private events as well.


Good point. Perhaps attempting to unsubscribe a single event when EVENT_ALL
has been subscribed should result in an error? I.e., you can only unsubscribe
ALL when you subscribed to ALL in the first place.

Regards,

Hans

- If events are missed, the sequence number is incremented in any case. This 
way the user space knows events have been missed.


Comments would be very, very welcome.

Cheers,

--
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com


--
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


[RFC v2 0/7] V4L2 file handles and event interface

2009-12-22 Thread Sakari Ailus

Hi,

Here's the second version of the V4L2 file handle and event interface 
patchset. Still RFC since I'd like to get more feedback on it.


The first patch adds the V4L2 file handle support and the rest are for 
V4L2 events.


The patchset works with the OMAP 3 ISP driver. Patches for OMAP 3 ISP 
are not part of this patchset but are available in Gitorious (branch is 
called events):


git://gitorious.org/omap3camera/mainline.git event

The major change since the last one v4l2_fh structure is now part of 
driver's own file handle. It's used as file-private_data as well. I did 
this based on Hans Verkuil's suggestion. Sequence numbers and event 
queue length limitation is there as well. There are countless of smaller 
changes, too.


A few notes on the patches:

- I don't like the locking too much. Perhaps the file handle specific 
lock (events-lock) could be dropped in favour of the lock for 
v4l2_file_handle in video_device?


- Poll. The V4L2 specifiction says:

When the application did not call VIDIOC_QBUF or
VIDIOC_STREAMON yet the poll() function succeeds, but sets the
POLLERR flag in the revents field.

  The current events for OMAP 3 ISP are related to streaming but not 
all might be in future. For example there might be some radio or DVB 
related events.


- Sequence numbers are local to file handles.

- Subscribing V4L2_EVENT_ALL causes any other events to be unsubscribed.

- If V4L2_EVENT_ALL has been subscribed, unsubscribing any one of the 
events leads to V4L2_EVENT_ALL to be unsubscribed. This problem would be 
difficult to work around since this would require the event system to be 
aware of the driver private events as well.


- If events are missed, the sequence number is incremented in any case. 
This way the user space knows events have been missed.


Comments would be very, very welcome.

Cheers,

--
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
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