Hi Hans,
thank you for the review.
>Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>On Friday 30 July 2010 10:49:41 Pawel Osciak wrote:
<snip>
>> @@ -157,9 +158,23 @@ enum v4l2_buf_type {
>> /* Experimental */
>> V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY = 8,
>> #endif
>> + V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE = 17,
>> + V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE = 18,
>
>Why 17 and 18 instead of 9 and 10?
>
To be able to test for "mplane" versions with a bit operation,
(type & 0x10), and to leave some space for future extensions
to "old" formats. I can go back to 9, 10 though if you prefer.
>> + *
>> + * Multi-planar buffers consist of one or more planes, e.g. an YCbCr
>buffer
>> + * with two planes can have one plane for Y, and another for interleaved
>CbCr
>> + * components. Each plane can reside in a separate memory buffer, or even
>in
>> + * a completely separate memory node (e.g. in embedded devices).
>> + */
>> +struct v4l2_plane {
>> + __u32 bytesused;
>> + __u32 length;
>> + union {
>> + __u32 mem_offset;
>> + unsigned long userptr;
>> + } m;
>> + __u32 data_offset;
>> + __u32 reserved[11];
>> +};
>> +
>> +/**
>> + * struct v4l2_buffer - video buffer info
>> + * @index: id number of the buffer
>> + * @type: buffer type (type == *_MPLANE for multiplanar buffers)
>> + * @bytesused: number of bytes occupied by data in the buffer (payload);
>> + * unused (set to 0) for multiplanar buffers
>> + * @flags: buffer informational flags
>> + * @field: field order of the image in the buffer
>> + * @timestamp: frame timestamp
>> + * @timecode: frame timecode
>> + * @sequence: sequence count of this frame
>> + * @memory: the method, in which the actual video data is passed
>> + * @offset: for non-multiplanar buffers with memory ==
>V4L2_MEMORY_MMAP;
>> + * offset from the start of the device memory for this plane,
>> + * (or a "cookie" that should be passed to mmap() as offset)
>> + * @userptr: for non-multiplanar buffers with memory ==
>V4L2_MEMORY_USERPTR;
>> + * a userspace pointer pointing to this buffer
>> + * @planes: for multiplanar buffers; userspace pointer to the array
>of plane
>> + * info structs for this buffer
>> + * @length: size in bytes of the buffer (NOT its payload) for single-
>plane
>> + * buffers (when type != *_MPLANE); number of planes (and number
>> + * of elements in the planes array) for multi-plane buffers
>
>This is confusing. Just write "number of elements in the planes array".
>
>> + * @input: input number from which the video data has has been captured
>> + *
>> + * Contains data exchanged by application and driver using one of the
>Streaming
>> + * I/O methods.
>> + */
>> struct v4l2_buffer {
>> __u32 index;
>> enum v4l2_buf_type type;
>> @@ -529,6 +606,7 @@ struct v4l2_buffer {
>> union {
>> __u32 offset;
>> unsigned long userptr;
>> + struct v4l2_plane *planes;
>
>Should use the __user attribute.
>
We discussed this already, just for others: since we use the "planes" pointer
both as __user and kernel pointer, it's not worth it. We'd have to do some
obscure #ifdef magic and redefine the struct for parts of kernel code.
The same thing goes for controls pointer in v4l2_ext_controls.
Best regards
--
Pawel Osciak
Linux Platform Group
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
