Gerd Knorr wrote:
>
> > 2) Identification:
> > A driver supporting DMA to userspace will set the V4L2_FLAG_DMA flag, as
> > returned by QUERYCAP.
>
> Hmm, V4L2_FLAG_DMA is a bad name. Capture to mmap()'ed kernel buffers is
> DMA too...
V4L2_FLAG_USERDMA ?
> > struct v4l2_dmabuffer *dmabuf;
>
> > struct v4l2_dmabuffer
> > {
> > void *start;
> > unsigned long size;
> > unsigned long stride;
> > unsigned long reserved[2];
> > };
> >
> > The dmabuf pointer passed by the application is a pointer to an array of
> > struct v4l2_dmabuffer, with one element for each plane in the color
> > format (for the current formats, that would be one plane for the packed
> > formats, and 3 for the planar formats).
> >
> > *start is the start address of the buffer.
> > size is the size in bytes of the buffer.
> > stride is the length in bytes of a line in the buffer.
>
> Hmm, you mix up buffers + format here. "stride" is redundant with
> v4l2_pix_format.bytesperline, which is a bad idea IMHO.
>
> Why the extra struct in the first place? Why not simply put a pointer
> to the userspace buffer into the struct v4l2_buffer?
>
> Gerd
OK - this was an attempt to work around a weakness in the original API.
The original API requires planar captures to be in a contiguous chunk of
memory with the same bytesperline for all planes. I have seen quite a
few apps for which this is not true - hence the extra structure to allow
the user app to pass a detailed description of where and how to dma the
data. The size element is there so the user can map a bigger chunk of
memory than is currently necessary so that the format can be changed
later on.
I don't think this is such a big problem, as the elements in this
structure are all very tightly coupled to the target memory, rather than
the format. Perhaps the following alteration would be better.
struct v4l2_dmabuffer
{
void *start;
unsigned long size;
unsigned long stride;
unsigned long flags;
unsigned long reserved[2];
};
#define V4L2_FLAG_DMABUFFER_BYTESPERLINE_OVERRIDE
where setting the override flag would lock the bytesperline for any
formats being written to that memory block.
If you see this as a serious problem, there is always the possibility
that Bill would let us take the existing API apart to allow multiple
strides for the different planes.
-justin
