Re: [patches!] Re: v4l2/kernel-2.5

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



>  struct v4l2_capability has a name field: "Canonical name for this
>  device. This name is descriptive, but it is also unique for each device.
>  It can be used to build a menu of available devices for a device-select
>  user interface." The "unique" clause was added because a list of
>  identical names is not very useful, something like "BT878 (Hauppauge
>  WinTV) #2" is expected. (I'd prefer "PCI Slot 2" etc, but I've been told
>  it's next to impossible.) Anyway, to help driver specific apps identify
>  the driver it would be really nice to have another name field which
>  contains the driver name ("bttv"), only the driver name, and never
>  changes.

How about this:

struct v4l2_capability
{
	__u8	driver[16];	/* i.e. "bttv" */
	__u8	card[32];	/* i.e. "Hauppauge WinTV" */
	__u8	bus_info[32];	/* "PCI:" + pci_dev->slot_name */
	__u32   version;        /* should use KERNEL_VERSION() */
	__u32	capabilities;	/* Device capabilities */
	__u32	reserved[4];
};

slot_name is the same lspci prints in column #0, that is the best you
can get I think.  Which physical slot on the motherboard this actually
is indeed next-to-impossible to figure in some portable and reliable
way.

>  Gerd added a version field. It wasn't clearly stated, so I assume this
>  refers to the driver version, not the API version.

Yes.

>  If so, it's rather pointless to require KERNEL_VERSION() since the
>  versioning scheme is already a private matter between the driver and
>  its apps. Recommendation ok. If this is intended as API version we
>  must also define what the numbers actually mean.

Why it is a private matter?  The user might want to know this too,
for example because issue $foo in driver $bar has been fixed in version
0.42.  IMHO applications should be able to display to version to the
user.  This requires a fixed way to specify the version number so the
driver can display it even without knowing that particular driver ...

>  There are two fields containing flags, 'capabilities' and:
>  _u32   flags;          /* Feature flags, see below */
>  What is the purpose of this field, no flags seem to be defined.

Forgot to delete that one.

>  struct v4l2_audio: What is capability V4L2_AUDCAP_LOUDNESS and mode
>  V4L2_AUDMODE_LOUDNESS?  These didn't exist before.

Redundant, there is also a control for this.  Don't know where they came
from.  Deleted.

>  Supposedly V4L2_AUDCAP_EFFECTS refers to the V4L2_AUDMODE_STEREO_*
>  modes. They were never fully explained, does anyone know what they
>  do, or can provide a pointer to docs? If they're intentionally vague
>  a menu control may be more appropriate.

Some pseudo-stereo effects.  Yes, a control menu likely works better for
these.

>  Btw there's no stereo capability flag.

Fixed.

>  struct v4l2_tuner: We have an index field here. How are tuners numbered:
>  a) Sequentially 0 ... n, so they can be enumerated with VIDIOC_G_TUNER;

Most useful IMHO.

>  c) No numbering scheme, only v4l2_input.tuner -> v4l2_tuner.index.

That is required anyway ...

>  rangelow, rangehigh, frequency: We have to units, 62.5 kHz and 62.5 Hz
>  depending on V4L2_TUNER_CAP_LOW. Originally v4l2 just used 1 Hz (0 ... 4
>  GHz in __u32)

Really?  IIRC the V4L2_TUNER_CAP_LOW was there all the time ...
At least I hav't changed that last weeks.

>  rxsubchans is defined as the received audio subprograms. I suppose this
>  field is valid only when this input (and its tuner) is currently
>  selected, similar to Gerd's suggestion regarding input status flags
>  (V4L2_IN_ST_*).

Yes.  It simply might not work otherwise on current hardware.

>  The various audio flags are a bit confusing. Here's what I think, please
>  correct me if I'm wrong.
>  
>  Valid combinations in rxsubchans are MONO, STEREO, MONO|LANG2, MONO|SAP,
>  STEREO|SAP. Which of MONO|LANG2 or MONO|SAP (since LANG2=SAP) applies
>  for proper display depends on the capability field. Or if that's
>  inconclusive on the current video standard (BTSC goes with M/NTSC).

I'd make that depend on the video standard.  If you put this into a
menu, it would IMHO be useful if the user finds the "well known" terms
there.  i.e. use "SAP" for BTSC/NTSC and use "LANG2" for PAL-BG
bilingual mode.

>  If so, should the driver set rxsubchans to MONO|STEREO|LANG2 (all of
>  them should appear in a menu) or 0 (nothing detected)?

IMHO all of them.  I would see rxsubchans as a set of useful choices at
the moment.  If the hardware can disturgish between STEREO + LANG1+LANG2
being transmitted it can clear the bits for the modes currently not
available, otherwise all should be set.

>  In the capability (to receive and decode) field all flags can appear in
>  any combination. Typically I'd expect MONO, MONO|SAP, MONO|STEREO|SAP or
>  MONO|STEREO|LANG1|LANG2.

Yes.

>  Btw what about surround? There are tv cards with surround decoder but
>  the API does not mention this once. No input/output types, capability,
>  rxsubchans, audmod flag. Is surround decoding purely a function of the
>  PCM sampling interface?

I don't want to put that in until someone really needs this.  I think
this could be handled using the ALSA mixer API just fine.  Maybe v4l2
controls work too.

>  struct v4l2_tuner signal: "The signal strength if known." I assume this
>  field is valid only when this input (and its tuner) is currently
>  selected, as above.

Yes.

>  Which values should it assume, higher is better?

Higher is better.

>  "if known" is ambiguous: really no signal or unknown?

Use "-1" for unknown?

>  The afc field is of type __u32 but defined as: "If negative, tune
>  higher; if positive, tune lower".

Changed to __s32, thanks.

>  Actually afc may never settle. Any suggestions what apps are supposed
>  to do?

Don't think this (scanning algo) belongs into the API spec.  A hint
about this issue to app drivers should be enougth.

>  Video standards. I must repeat my criticism of the link between
>  v4l2_input, v4l2_output, and v4l2_standard.
>  
>  The VIDIOC_ENUMSTD ioctl is ok, one item for each standard distinguished
>  by the hardware (e.g. tuner PAL-B/G or PAL-I; video decoder PAL-B/G/H/I,
>  NTSC-M, SECAM-L, ...). Originally there were inputset and outputset
>  fields, and I proposed "to add .standardset in v4l2_input/v4l2_output
>  and remove the .input/outputset here. Rationale is that we have the
>  .tuner link and .audioset there too. This change would root everything
>  in the input/output enum."
>  
>  Now we have a std (v4l2_std_id) field in v4l2_input/v4l2_output which
>  must be looked up. Also VIDIOC_G_STD and VIDIOC_S_STD take a v4l2_std_id
>  pointer, remnant of v4l2 0.20.

v4l2_std_id isn't a pointer, it is a bitfield.  If your input can handle
PAL+NTSC, just do v4l2_input->std = V4L2_STD_PAL | V4L2_STD_NTSC.

>  No idea what the purpose of VIDIOC_QUERYSTD _IOR ('V', 63,
>  v4l2_std_id) is.

Ask the driver what he thinks the current TV norm of the signal on
current input is.  Depending on how good the card is in detecting video
standards this might return V4L2_STD_ALL, V4L2_STD_625_50, V4L2_STD_NTSC
or V4L2_STD_PAL_I ...

>  Standards should work like this: each bit in .standardset corresponds
>  to one enumerated standard akin to .audioset. VIDIOC_G|S_STD take an
>  int like VIDIOC_G|S_INPUT|OUTPUT, index of the respective
>  standard|input|output.

Again: v4l2_std_id _is_ a bitfield.

>  struct v4l2_queryctrl: The step field is __s32 like minimum, maximum,
>  and default_value but it cannot actually be negative (minimum >=
>  maximum?). Maybe it should be changed to __u32.

why step can't be negative?  Maybe for some "smaller values are
better"-kind of control?

>  One of the predefined controls demands explanation:
>  V4L2_CID_AUDIO_LOUDNESS boolean "Audio Loudness mode." What is this and
>  how is it related, if at all, to struct v4l2_audio
>  V4L2_AUDMODE_LOUDNESS?

See above ...

  Gerd

-- 
You can't please everybody.  And usually if you _try_ to please
everybody, the end result is one big mess.
				-- Linus Torvalds, 2002-04-20





[Index of Archives]     [Linux DVB]     [Video Disk Recorder]     [Asterisk]     [Photo]     [DCCP]     [Netdev]     [Xorg]     [Util Linux NG]     [Xfree86]     [Free Photo Albums]     [Fedora Users]     [Fedora Women]     [ALSA Users]     [ALSA Devel]     [Linux USB]

Powered by Linux