Hi all,
On 06/24/2013 02:57 PM, Hans Verkuil wrote:
On Sun June 23 2013 00:46:57 Sakari Ailus wrote:
On Wed, Jun 19, 2013 at 08:32:35AM +0200, Hans Verkuil wrote:
On Tue June 18 2013 23:22:33 Laurent Pinchart wrote:
On Friday 07 June 2013 00:38:04 Sakari Ailus wrote:
On Mon, May 13, 2013 at 02:14:43PM +0200, Hans Verkuil wrote:
Currently the event API allows for a payload of up to 64 bytes. Sometimes
we would like to pass larger payloads to userspace such as metadata
associated with a particular video stream.
A typical example of that would be object detection events.
This RFC describes one approach for doing this.
The event framework has the nice property of being able to use from within
interrupts. Copying large payloads does not fit into that framework, so
such payloads should be adminstrated separately.
But does it really need to be reflected in user space that much, i.e. by
introducing a separate ioctl for de-queueing the payload ?
In addition, I wouldn't allow large payloads to be filled in from
interrupt context: a worker queue would be much more appropriate.
How large really is "large"? 65 bytes? 64 kiB?
More than 64 bytes, which is what v4l2_event supports today.
This seems some artificial distinction which is likely inaccurate across
various systems. For many of them there is not much difference whether
we copy 64 B or 1 kB. Of course it have to be dealt in the kernel from
what context events of what size are passed from a device driver to the
v4l2-core. But once the event object has been queued in the core it
is not really relevant from what context it has been queued, is it ?
In general events have no or small payloads. So for the majority 64 bytes
is enough.
The newer CPUs tend to be faster and faster and the same applies to memory.
I guess threaded interrupt handlers are still nice. But using a mutex in
order to serialise access to the struct will force drivers to use threaded
interrupt handlers should they want to generate large events.
But there are still lots of embedded systems out there that are not fast. Or
that run at reduced frequency to preserve power, etc., etc.
Again, IMHO it's not the API design that should make the border, rather
a driver for a particular device/system, provided with a non-blocking API
for small/fast events and with a blocking one for the larger payload events.
Interrupts should be fast, and 64 bytes seems a reasonable limit to me.
Note that the event API is only useful for relatively low-bandwidth data
since the data is always copied. When dealing with high-bandwidth data the
data should either be a separate plane or become a special stream I/O
buffer type.
The userspace API enhancements in order to achieve this would be the
following:
- Any event that has a large payload would specify a payload_sequence
counter and a payload size value (in bytes).
- A new VIDIOC_DQEVENT_PAYLOAD ioctl would be added which passes the event
type, the payload_sequence counter and a pointer to a buffer to the
kernel, and the payload is returned, or an error is returned if the
payload data is no longer available.
Do you think we should have a separate IOCTL for this? The downside is that
to dequeue events, the application would need to try both in the worst case
just to obtain an event.
Yes, I'm also not convinced doubling the number of ioctls and adding all
complexity related with matching an event and the payload is a good
approach.
The system has more data to deal with and we're complicating the API and
adding
more overhead for this case ?
I think it'd be nicer to be able to fit that into the same IOCTL. There are
plenty of reserved fields and, actually, the event data as well: we could
consider the large-payload event a meta-event which would contain the
required information to pass the event data to the user space. The type of
such an event could be V4L2_EVENT_LARGE, for instance.
The problem is that userspace doesn't know in advance how much memory it would
need to allocate to store the event payload. Using two ioctls allow retrieving
the size first, and then the payload. We could work around the problem by
passing the maximum size to userspace and forcing preallocation of large
enough buffers.
The problem is also that you don't know what event you will get when calling
DQEVENT. So you would have to figure out the maximum of the payload sizes of all
subscribed events which is rather awkward.
Is it? The application knows which events it has subscribed, and the maximum
event size is the maximum of that of maximum of different event types.
I side Sakari here. Since we cannot pass the information about a payload
buffer
size to the kernel in same ioctl it looks acceptable to me to have user
space
provided with an API to retrieve the maximum event's payload size,
preallocating
the buffers and using them when de-queueing any event.
Also, couldn't an application subscribe to "large payload" event(s) on
one file
handle and to any other events, like currently supported, on a separate
file
handle ?
Optional enhancements:
- Have VIDIOC_SUBSCRIBE_EVENT return the maximum payload size (lets apps
preallocate payload memory, but it might be overkill).
Why so? We could use a reserved field as well. The size would be zero if the
maximum would be less than 64.
I'm undecided here. Implementing support for this in an application is probably
the best way to discover whether or not it is useful to supply the maximum
payload size to the user. I suspect that this is actually useful.
I agree. Otherwise the application would have to figure it out through other
ways which are less generic and also more difficult. On the other hand, the
maximum event size could be dependent on unrelated parameters, such as image
size. If the user changes the image size without changing the event
subscription this could be a problem. (Well, there's an obvious solution,
too, if we run into this: provide an event on it, just as on control
parameters. :-))
Oops, that might be indeed a bit of a problem. But first someone needs
to come
up with such a flexible payload event... Just poking to see if there is
anyone
already waiting with such one out there... :)
A meta-event. You're evil :-)
I do think that in practice there is always an upper worst case bound. Although
it a probably a good idea to allow for a case where no such bound can be given.
Regards,
Sylwester
--
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
