Hi Gerd/Michael again,
some comments on the current API:
> #define V4L2_MAJOR_VERSION 0
> #define V4L2_MINOR_VERSION 20
<nitpicking> increase this to 21 please. ;-). </nitpicking>
> enum v4l2_buf_type {
> V4L2_BUF_TYPE_VIDEO_CAPTURE = 1,
> V4L2_BUF_TYPE_VIDEO_OUTPUT = 2,
> V4L2_BUF_TYPE_VIDEO_OVERLAY = 3,
> V4L2_BUF_TYPE_VBI_CAPTURE = 4,
> V4L2_BUF_TYPE_VBI_OUTPUT = 5,
> V4L2_BUF_TYPE_PRIVATE = 0x80,
> };
In the old approach, there was a separate VIDEO_CODECIN/OUT, I kind of
understand why it was removed, but that makes some other parts a it
harder.
> struct v4l2_fmtdesc
> {
> __u32 index; /* Format number */
> enum v4l2_buf_type type; /* buffer type */
> __u8 description[32]; /* Description string */
> __u32 pixelformat; /* Format fourcc */
> __u32 reserved[4];
> };
We see the type here, and the 'flag' field is removed too. Now, if I
understand correctly, we need to define separate picture formats for
input and output. That's fine, though I actually expected a flag instead
of a type field for this (i.e., flag the thing as input-only, or
input|output, etc.). That's fine, now let's get back to the
compressed/codec thing. For gstreamer, we actually used this field to
define the mime type of the stream: if flag&compressed, mimetype =
sprintf("video/%4.4s", fourcc), else, mimetype="video/raw",type=fourcc.
I think this was particularly useful. therefore, I'd propose to change
this into:
#define V4L2_FMT_FLAG_COMPRESSED 0x0001
struct v4l2_fmtdesc
{
__u32 index; /* Format number */
enum v4l2_buf_type type; /* buffer type */
__u32 flags;
__u8 description[32]; /* Description string */
__u32 pixelformat; /* Format fourcc */
__u32 reserved[4];
};
There's probably more flags that can be useful, but this flag was
particularly useful in gstreamer. It makes video4linux useful for both
compressed capture as well as non-compressed, since we can distinguish
the two, and we actually *need* to (in the application). With this, we
could write a universal v4l2 element that supports any format that might
ever be there... Now, that's much harder.
The other solution (which I like too, but you probably don't ;-) ) is to
add CODECIN/OUT as v4l2_buf_type, like it as in the old v4l2.
> struct v4l2_capability
> {
> __u8 name[32]; /* Descriptive, and unique */
> __u32 capabilities; /* Device capabilities */
> __u32 flags; /* Feature flags, see below */
> __u32 reserved[4];
> };
>
> /* Values for 'capabilities' field */
> #define V4L2_CAP_VIDEO_CAPTURE 0x00000001 /* Is a video capture device */
> #define V4L2_CAP_VIDEO_OUTPUT 0x00000002 /* Is a video output device */
> #define V4L2_CAP_VIDEO_OVERLAY 0x00000004 /* Can do video overlay */
> #define V4L2_CAP_VBI_CAPTURE 0x00000010 /* Is a VBI capture device */
> #define V4L2_CAP_VBI_OUTPUT 0x00000020 /* Is a VBI output device */
> #define V4L2_CAP_RDS_CAPTURE 0x00000100 /* RDS data capture */
>
> #define V4L2_CAP_TUNER 0x00010000 /* Has a tuner */
I'd like to see V4L2_CAP_AUDIO be added here, is that possible?
> #if 0
> /* ### generic compression settings don't work, there is too much
> * ### codec-specific stuff. Maybe reuse that for MPEG codec settings
> * ### later ... */
> struct v4l2_compression
Just rename it to v4l2_mpegcompression?
> struct v4l2_window
> {
> struct v4l2_rect w;
> __u32 chromakey;
> struct v4l2_clip *clips;
> __u32 clipcount;
> void *bitmap;
> };
This is not right, w can't have unsigned x/y parameters, they *need* to
be signed because the window can be offscreen:
struct v4l2_window
{
struct {
__s32 x, y;
__u32 width, height;
} w;
__u32 chromakey;
struct v4l2_clip *clips;
__u32 clipcount;
void *bitmap;
};
sounds better to me.
>
> /*
> * C A P T U R E P A R A M E T E R S
> */
> struct v4l2_captureparm
> {
> __u32 capability; /* Supported modes */
> __u32 capturemode; /* Current mode */
> struct v4l2_fract timeperframe; /* Time per frame in .1us units */
> __u32 extendedmode; /* Driver-specific extensions */
> __u32 readbuffers; /* # of buffers for read */
> __u32 reserved[4];
> };
> /* Flags for 'capability' and 'capturemode' fields */
> #define V4L2_MODE_HIGHQUALITY 0x0001 /* High quality imaging mode */
> #define V4L2_CAP_TIMEPERFRAME 0x1000 /* timeperframe field is supported */
>
> struct v4l2_outputparm
> {
> __u32 capability; /* Supported modes */
> __u32 outputmode; /* Current mode */
> struct v4l2_fract timeperframe; /* Time per frame in seconds */
> __u32 extendedmode; /* Driver-specific extensions */
> __u32 writebuffers; /* # of buffers for write */
> __u32 reserved[4];
> };
I must have missed something, what's this for?
> struct v4l2_crop {
> enum v4l2_buf_type type;
> struct v4l2_rect c;
> };
I'm seeing some problems for cropped capture. For cropping, you
basically first need to set the capture size/format, since cropping
depends on the format being used. But after that, if the application
sets cropping rectangles, the size changes. Is that how it's supposed to
work (and is the application then supposed to re-request size by G_FMT),
or is this indeed a problem?
> struct v4l2_enumstd
> {
> __u32 index;
> struct v4l2_standard std;
> __u32 inputset; /* set of inputs that */
> /* support this standard */
> __u32 outputset; /* set of outputs that */
> /* support this standard */
> __u32 reserved[2];
> };
inputset/outputset is just the same as inputs/outputs, or are they a
bitfield where (1<<x) means that input/output x supports this std?
I think this is all... I like some of your changes, as I've said before.
Let's hope this still will make it into 2.5/6.x.
Ronald
