Am Dienstag, den 04.08.2009, 20:40 -0300 schrieb Mauro Carvalho Chehab: > Em Tue, 4 Aug 2009 16:19:46 -0400 > Michael Krufky <mkrufky@xxxxxxxxxxxxxx> escreveu: > > > On Tue, Aug 4, 2009 at 3:58 PM, Alex Deucher<alexdeucher@xxxxxxxxx> wrote: > > > On Tue, Aug 4, 2009 at 3:50 PM, Michael Krufky<mkrufky@xxxxxxxxxxxxxx> wrote: > > >> On Tue, Aug 4, 2009 at 3:47 PM, Alex Deucher<alexdeucher@xxxxxxxxx> wrote: > > >>> On Tue, Aug 4, 2009 at 3:33 PM, Michael Krufky<mkrufky@xxxxxxxxxxx> wrote: > > >>>> Mauro, > > >>>> > > >>>> Please pull from: > > >>>> > > >>>> http://kernellabs.com/hg/~mkrufky/cx23885 > > >>>> > > >>>> for the following fixes: > > >>>> > > >>>> - cx23885: Enable mplayer pvr:// usage > > >>> > > >>> I'm not too familiar with mplayer's v4l support, but why not fix > > >>> mplayer rather than adding a fake audio interface to the driver. > > >>> Wouldn't that potentially confuse users or other apps? > > >> > > >> Thats a good question, Alex. > > >> > > >> The answer, for now, is conformity amongst the v4l2 drivers. > > >> > > >> Mplayer has been around for a good long time, and any v4l2 mpeg > > >> encoder that doesnt do this is broken in the vraious userspace > > >> applications. > > >> > > >> I agree that we should fix userspace also -- but fix the kernel first, > > >> so that older userspace works as-is. > > > > > > er... yeah, but you are re-enforcing broken behavior, rather than > > > "fixing" more drivers, why not submit a mplayer patch and tell users > > > they need a newer version of mplayer/etc. to work with their card? > > > Otherwise we'll end up with tons of drivers with fake audio interfaces > > > and mplayer will never get fixed and users will complain that the > > > audio interface doesn't work. > > > > You don't really have the full picture here, Alex. The applications > > expect to see an audio input, and rightfully so. > > > > This particular driver doesn't expose all of the functionality that is > > available from the raw video device onto the encoder device. > > > > Little by little, we are fixing up the cx23885-417 driver to fully > > expose all featuresets, but in the meanwhile, we do what is needed to > > make things work. > > > > The problem is not mplayer, the real problem is incomplete analog > > video (and analog audio) support in the cx23885 driver, itself. As > > THAT support improves, you will see small hacks like this disappear. > > There are some separate issues that came with this patch: > > 1) V4L2 API (chapter 1.5) states that: > > Drivers must implement all input ioctls when the device has one > or more inputs, all output ioctls when the device has one or more outputs. > When the device has any audio inputs or outputs the driver must set the > V4L2_CAP_AUDIO flag in the struct v4l2_capability returned by the > VIDIOC_QUERYCAP ioctl. > > My understanding is that, except for webcams without any audio input, all other > V4L2 devices should implement VIDIOC_G_AUDIO, VIDIOC_ENUMAUDIO and VIDIOC_S_AUDIO. > > So, your patch should have implemented VIDIOC_S_AUDIO as well. > > 2) Based on the above understanding, your comments are wrong, since the device > has one audio input (otherwise, no audio would be streamed). > > So, it is not mplayer that requires those functions, but V4L2 API. So, please > remove the comments. The same fix applies to pvrusb2 comments. > > 3) On this piece of code: > > + strncpy(vin->name, "CX23885 Audio", 14); > + vin->capability = V4L2_AUDCAP_STEREO; > > the "magic" size above is very ugly. Please find another way for it, like: > > #define CX23885_INPUT_NAME "CX23885 Audio" > strncpy(vin->name, CX23885_INPUT_NAME, sizeof(CX23885_INPUT_NAME) + 1); > > 4) mplayer should be fixed to not require audio inputs for devices without > V4L2_CAP_AUDIO. > > I just tested it here with a webcam, where I have this parameter at .mplayer/config: > > tv = "driver=v4l2:device=/dev/video0:norm=PAL-M:chanlist=us-bcast:alsa=1:adevice=hw.1:audiorate=32000:immediatemode=0:amode=1" > > With a device without audio cap: > > $ v4l2-ctl -D > Driver info: > Driver name : em28xx > Card type : Silvercrest Webcam 1.3mpix > Bus info : usb-0000:00:1d.7-7 > Driver version: 258 > Capabilities : 0x05000041 > Video Capture > Sliced VBI Capture > Read/Write > Streaming > > it is still requesting for audio input, so mplayer has a small non-compliance with V4L2 API. > > 5) There is one trouble with VIDIOC_G_AUDIO and VIDIOC_ENUMAUDIO: > > As stated at V4L2 API, at chapter 6.2.10, added on 2003-06-19: > > 3. The audio input and output interface was found to be incomplete. > > Previously the VIDIOC_G_AUDIO ioctl would enumerate the available audio inputs. > An ioctl to determine the current audio input, if more than one combines with > the current video input, did not exist. So VIDIOC_G_AUDIO was renamed to > VIDIOC_G_AUDIO_OLD, this ioctl will be removed in the future. The VIDIOC_ENUMAUDIO > ioctl was added to enumerate audio inputs, while VIDIOC_G_AUDIO now reports > the current audio input. > > There are very few drivers that are respecting this change. On almost all drivers, > VIDIOC_G_AUDIO is still working like VIDIOC_G_AUDIO_OLD. > > For example, this is what bttv driver is doing: > > static int bttv_g_audio(struct file *file, void *priv, struct v4l2_audio *a) > { > if (unlikely(a->index)) > return -EINVAL; > > strcpy(a->name, "audio"); > return 0; > } > > The same V4L2 violation is found at au0828, av7110, cx18, cx231xx, cx23885, cx88, > dsbr100, em28xx, ivtv, mxb, radio-tea5764, saa7134, sn9c102. > > It is correct on: gspca (yet, it assumes that all webcams have audio), hdpvr, > pvrusb2, radio-aimslab, radio-aztech, radio-cadet, radio-gemtek, radio-maestro, > radio-maxiradio, radio-mr800, radio-rtrack2, radio-sf16fmi, radio-sf16fmr2, > radio-si470x, radio-terratec, radio-trust, radio-typhoon, radio-zoltrix, > tea575x-tuner, usbvision. > > For sure all drivers should behave the same for VIDIOC_G_AUDIO. So, we need to > decide if we'll keep VIDIOC_G_AUDIO_OLD behavior, or honor the API change > stated on 2003. No matter what decision we'll take, this will have impacts at > userspace applications. > > 6) Since VIDIOC_ENUMAUDIO is mandatory when V4L2_CAP_AUDIO, several drivers are > violating the API, since this ioctl exists only at cx18, gspca, hdpvr, ivtv, > pvrusb2 and sn9c102. > > So, or we should change V4L2 API to state this as optional ioctl or deprecate > it (this would make sense if we decide that VIDIOC_G_AUDIO should use the old > behavior), or we should implement it on almost all drivers. > > As VIDIOC_ENUMAUDIO and VIDIOC_G_AUDIO are very close, in terms of > implementation, maybe we can merge those two at v4l2-ioctl, to simplify a > little bit the driver's code. > > Cheers, > Mauro Hi, I go seemingly off topic. First of all, it is a pleasure to see Alex here again, he helped a lot in the past. Now going off topic. We have no rules about NDAs. Luckily we have folks obtaining them on OEM manufactures levels, on the one hand, but all known shows, that OEMs happily ignore what chip manufactures do intend to do. Starts with having no eeprom at all and never ends for eeprom content. This is all over the m$ stuff, how could we ever become better? I try to remember, that Hartmut had the recommended conditions about how to use eeprom content on Philips stuff, Nickolay tried to get it out of him ... But he was such aware already, that many OEMs are already breaking any such rules, that he did not want to have this on his neck on linux. I did fully understand this, hopefully. We should at least have an instance for chip manufactures, to propose what they recommend to identify their devices. That can't be any OEMs, for what I can see. Cheers, Hermann -- 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
