List:       linux-video
Subject:    [video4linux] new API questions, concerns, and suggestions
From:       Aaron Colwell <colwaar () charlie ! cns ! iit ! edu>
Date:       1998-07-30 17:29:54
[Download message RAW]

Most of my comments will center around the mmap and stream capture part of
the API seeing and this is the part of the API that I am most interested
in. The idea of having capture queues in my opinion is a great idea. I
just have a few questions an concerns about it. The first concern is that
I think that there should be some sort of confiurable limit on the memory
usage by the driver. This could be configured at driver load time. A
default memory limit can be specified, but if more or less memory is
needed then that can be specified on driver load. My reason for suggesting
this is that right now anyone could request 100 buffers for 1024x768
captures and eat up all the memory. Dont forget that the memory that is
used for DMA is non-swappable and could lead to a nice exploit to bring
down the system. I also was reading up on memory allocation
in the kernel and I discovered 2 interesting things. Most of this
information comes from the "Linux Device Drivers" book. 
1. The kernel can only return 128k of consecutive pages in a kmalloc or
get_free_pages call(p. 291). This can pose problems for captures that
require more memory than 128k, like a 320x200 24bit RGB capture. 
2. I noticed that the bttv driver uses vmalloc to allocate its DMA buffer.
Is this correct? I thought that vmalloc only returned a block of memory
that is contiguous in virtual memory and not necessarily in physical
memory(p.157). This would cause serious problems if vmalloc allocated
space that was not contiguous in physical memory. 

Apparently there are solutions to the problem, but you have to tell the
kernel to reserve memory at boot time and then use some sort of
"allocator" module to use large DMA buffers. In my opinion this issue
needs to be ironed out before we even get into issues about how the
buffers need to be managed. We need the buffers before we can manage them!
:)

Another question I have concerns handling of buffers. The way I understand
the proposal, buffers are requested, but not created through a
VIDIOC_REQBUFS ioctl. I assume that this will set up some sort of array or
list that will hold the buffer pointers and buffer status information.
VIDIOC_QUERYBUF is then called to get the information about the status of
the buffer and the offset to perform a mmap call. When mmap is called the
buffer is actually allocated in memory and mapped to user space. Streaming
capture they occurs in the following manner. Buffers are put into the
capture queue by calling VIDIOC_QBUF ioctl. Buffers can be waited for by
using select or the VIDIOC_NEXTBUF calls. When a buffer is filled I use a
VIDIOC_DQBUF to get information about which buffer is ready and I use that
information to access the data from the correct mmaped buffer. Once I'm
done with the buffer I put it back into the buffer queue using
VIDIOC_QBUF. I hope that this is the correct process that you are
describing for stream capture.

1. We try and use the the buffer index number for the offset value passed
to the mmap call. This seems a little more intuitive to me and would make
looking up what buffer the user wants a lot easier. The driver can then
change the offset to what it needs to be when it does the remap in the
driver code.

2. It would be nice for there to be a way to notify the application that
frames are being dropped. The select and VIDIOC_NEXTBUF could return and
when the application calls VIDIOC_DQBUF and sees that there is nothing in
the "done" queue it could make some sort of call to see if frames were
dropped or assume (which we all know what happens when people assume) that
frames were dropped. I feel that this is sort of important because even
though it is the job of the application to make sure its has buffers
queued up for capturing a program could get "stuck" waiting for data that
is never going to come. Perhaps we could just add a call that reports the
number of buffers in each queue. The app could call this function and see
if there is anything in the capture queue.

3. I had more comments, but after some thought and closer reading of the
proposal, I realized that the way that you specify it here is a good way
of doing things. It is very obvious that this is pretty well thought out. 


Again I want to commend Bill and the others on the new proposal. It adds a
lot of features that needed to be added and makes it a much more useful
API for things other then just watching TV and capturing a frame or 2. I'm
going to try adding some of this functionallity to my bttv driver and see
how it works out.

Well thats all for now. Comments are welcomed.

Aaron

------------
To unsubscribe from this list send mail to majordomo@phunk.org with the
line "unsubscribe video4linux" without the quotes in the body of the
message.

List:       linux-video
Subject:    Re: [video4linux] new API questions, concerns, and suggestions
From:       alexb () jetsuite ! com (Alex Bakaev)
Date:       1998-07-30 18:29:53
[Download message RAW]

Aaron Colwell wrote:
[snip]
> 1. The kernel can only return 128k of consecutive pages in a kmalloc or
> get_free_pages call(p. 291). This can pose problems for captures that
> require more memory than 128k, like a 320x200 24bit RGB capture.

Yes, if DMA device doesn't support scatter-gather. bt848, for example,
doesn't need physically consecutive memory for buffers and risc
programs.

> 2. I noticed that the bttv driver uses vmalloc to allocate its DMA buffer.
> Is this correct? I thought that vmalloc only returned a block of memory
> that is contiguous in virtual memory and not necessarily in physical
> memory(p.157). This would cause serious problems if vmalloc allocated
> space that was not contiguous in physical memory.
> 

But risc programs for bt848 can be compiled in such a way that no memory
needs to be physically contiguous.

> Apparently there are solutions to the problem, but you have to tell the
> kernel to reserve memory at boot time and then use some sort of
> "allocator" module to use large DMA buffers. 

As long as there is a way to allocate page locked memory, that's all
that's needed.

Alex
------------
To unsubscribe from this list send mail to majordomo@phunk.org with the
line "unsubscribe video4linux" without the quotes in the body of the
message.

List:       linux-video
Subject:    Re: [video4linux] new API questions, concerns, and suggestions
From:       Bill Dirks <dirks () rendition ! com>
Date:       1998-07-30 19:24:52
[Download message RAW]

Hello, Aaron.
Thanks for the feedback, I'm so glad to get more comments. :)

Aaron Colwell wrote:
> Most of my comments will center around the mmap and stream capture part of
> the API seeing and this is the part of the API that I am most interested
> in.

Great! That's the part I have the least experience with under Linux!

> I think that there should be some sort of confiurable limit on the memory
> usage by the driver. This could be configured at driver load time. My reason
> for suggesting this is that right now anyone could request 100 buffers for 1024x768
> captures and eat up all the memory. [mem is non-swappable]

Yes. Ultimately the driver is responsible for protecting the system. All
drivers have this type of responsibility.
I put in an explicit request-grant type mechanism so the application
will know in advance how many buffers it will be allowed to allocate.
The driver decides when VIDIOC_REQBUFS is called. It should look at the
frame size and how much memory it is willing to lock down and figure out
the upper limit. I did not spec how the driver makes that decision. I'm
not sure it should be spec'ed. Maybe we should have a standardized
configuration setting that sets a firm upper limit, but the driver may
restrict even more if it wants.

Remember that this is a buffer _mapping_ protocol designed for general
purpose use-- not just allocating system memory, and not just capture
buffers. If it makes sense to do so, the buffer is implicitly allocated
and deallocated when the app maps and unmaps it, but not necessarily.

I can imagine many types of buffers that could be mapped:
1. Pre-existing buffers in system memory
2. Pre-existing buffers in device memory
3. Buffers in system memory that only exist when they are mapped
With different purposes:
1. Input images (for codec or image filter drivers)
2. Output images (like capture buffers)
3. Microcode buffers (device-executable code)
4. Lookup tables
5. Memory mapped device registers
6. Misc buffers for any other kind of general information exchange
   between the driver and the application, settings, performance data,
   many different kinds of sideband information...
How wild is your imagination? :) The latter stuff gets to be rather
device-specific, but the point is how extensible the interface is.

The app requests a buffer type and count of buffers. The driver grants,
does not grant, or partially grants the request. Inexact grants can be
fewer buffers, or maybe can be like the app asks for on-device mem and
driver grants off-device mem (but the app-visible semantics of the
buffer are *exactly* the same, maybe just slower).

> was reading up on memory allocation...discovered 2 interesting things.
> ...from the "Linux Device Drivers" book.
> 1. The kernel can only return 128k of consecutive pages in a kmalloc...
> [what about] captures that require >128k, like a 320x200 24bit RGB?

Most bus-mastering capture cards do not require contiguous memory. The
restriction applies to the old-fashioned motherboard DMA controllers
that were/are used with ISA cards. For a typical PCI bus-mastering card,
the card takes a list of {address;length;} structures that define the
non-contiguous buffer. This is known as a "scatter list" (for DMA to
memory) or a "gather list" (for DMA from memory). The list has to be
contiguous physically.

> 2. I noticed that the bttv driver uses vmalloc to allocate its DMA
> buffer....  I thought that vmalloc only returned a block of memory
> that is contiguous in virtual memory and not necessarily in physical

My driver uses vmalloc() too. There is a way (not hard) to get the bus
address of each page of vmalloc()ed memory. I don't have the code with
me now, but I could send it later. "Linux Device Drivers" does not cover
this, unfortunately.

> Apparently there are solutions to the problem, but you have to tell the
> kernel to reserve memory at boot time and then use some sort of
> "allocator" module to use large DMA buffers.

I sure as heck hope v4l drivers don't have to do that.

> Another question I have concerns handling of buffers. The way I understand
> the proposal, buffers are requested, but not created through a
> VIDIOC_REQBUFS ioctl.

Yes. Creation sematics are separate, and could be different for
different buffers.

> I assume that this will set up some sort of array or
> list that will hold the buffer pointers and buffer status information.

Probably an array of structures each with a struct video_buffer plus
associated driver-internal info like the vmalloc() address, pointer to
the scatter list, etc.

> VIDIOC_QUERYBUF is then called to get the information about the status of
> the buffer and the offset to perform a mmap call.

Not exactly. VIDIOC_QUERYBUF is called to get the length and offset
value that must be passed to mmap() to map the buffer. The length will
be the amount of memory to be mapped, of course.

The offset was originally used for mapping files to say where in the
file to map. We're not mapping a file so we use it for a different
purpose. The offset value specifies to the driver which buffer to map.
The offset carries some driver-defined,
only-the-driver-knows-what-it-means code number to the driver's mmap()
method. The mmap() method looks at the offset value and maps the buffer
specified. For example, a driver might make offset values 1 - 99 mean
capture buffers, 100 is the microcode buffer, 101 is
<I-don't-know--whatever>, etc. The app must pass the offset and length
exactly as provided.

VIDIOC_QUERYBUF calls implicitly refer to the most recent VIDIOC_REQBUFS
call. Indices passed to QUERYBUF should be 0...count-1. I should say
that in the spec.

> When mmap is called the
> buffer is actually allocated in memory and mapped to user space. 

In the typical case for streaming capture buffers, the buffers would be
allocated when they are mapped, yes.


> Streaming capture then occurs in the following manner. [...]

Correct.

> 1. We try and use the the buffer index number for the offset value passed
> to the mmap call. This seems a little more intuitive to me and would make
> looking up what buffer the user wants a lot easier. The driver can then
> change the offset to what it needs to be when it does the remap in the
> driver code.

The meaning of offset is entirely up to the driver.
VIDIOC_QBUF does not specify which buffer is to be queued. The driver
queues buffers in any order it wants.

> 2. It would be nice for there to be a way to notify the application that
> frames are being dropped.

Latest spec has this.

> The select and VIDIOC_NEXTBUF could return and
> when the application calls VIDIOC_DQBUF and sees that there is nothing in
> the "done" queue it could make some sort of call to see if frames were
> dropped or assume (which we all know what happens when people assume) that
> frames were dropped. I feel that this is sort of important because even
> though it is the job of the application to make sure its has buffers
> queued up for capturing a program could get "stuck" waiting for data that
> is never going to come.

If you QBUF each buffer after you're done with it then all buffers are
always in the capture queue or the done queue, except for the one you
may be currently reading. If an application is queueing buffers at a
metered rate to control the capture rate, then obviously it is assuming
the responsibility of queuing buffers as needed. Those are the only two
cases I can think of.

Anyway, to be sure you're not 'stuck' call VIDIOC_QBUF; any an unqueued
buffer is queued. Call it in a loop until it fails to guarantee all
available buffers are queued.
while(ioctl(fd, VIDIOC_QBUF, 0) == 0)
	continue;

> Perhaps we could just add a call that reports the
> number of buffers in each queue. The app could call this function and see
> if there is anything in the capture queue.

Well, adds another ioctl, plus it's not reliable because you have a race
condition. Is it really needed? I'm really hesitant about it.


> 3. I had more comments, but after some thought and closer reading of the
> proposal, I realized that the way that you specify it here is a good way
> of doing things. It is very obvious that this is pretty well thought out.

Wow! Thanks! Worked really hard on it!


> Again I want to commend Bill and the others on the new proposal. It adds a
> lot of features that needed to be added and makes it a much more useful
> API for things other then just watching TV and capturing a frame or 2. I'm
> going to try adding some of this functionality to my bttv driver and see
> how it works out.

Thanks. Great. I haven't actually gone through the exercise of trying to
code it to see how it plays in the real world.

We need to modify videodev.h, and videodev.c for multiple opens. Need to
coordinate that work somehow. What happens to the old videodev.[ch]?

Bill.
------------
To unsubscribe from this list send mail to majordomo@phunk.org with the
line "unsubscribe video4linux" without the quotes in the body of the
message.