Em Wed, 5 Aug 2009 11:18:24 -0400
Michael Krufky <mkrufky@xxxxxxxxxxxxxx> escreveu:
> On Tue, Aug 4, 2009 at 7:40 PM, Mauro Carvalho
> > 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.
>
>
> Thank you for the feedback, Mauro -- I see your point. We will have
> to take some time to come up with a better solution.
>
> I will have to assess how much work needs to be
> done in order to comply with everything that you stated above. Is
> there any middle ground that we can agree on, so that we can break
> this large problem up into several smaller problems?
With respect to your patch:
due to issue #1, please implement VIDIOC_S_AUDIO;
due to issue #2, just remove the comments mentioning mplayer;
due to issue #3, I agree with Trent. IMO you can just do strcpy().
due to issue #5, for now, the better seems to not check for index when implementing VIDIOC_G_AUDIO (the "new" behavior);
due to issue #6, you should also implement VIDIOC_ENUMAUDIO. The only difference, when comparing with VIDIOC_G_AUDIO is
that it should return -EINVAL if index > 0.
With respect to mplayer (issue #4):
We need to contact the V4L2/PVR driver maintainer and/or mailing list for they to fix mplayer.
With respect to a definitive solution for V4L2 compliance (issues #5 and #6):
We need to discuss a little bit what would be the better way to address. It
would be nice if userspace guys could give us a feedback about what
applications only work with the old or with the new behavior.
I'll start a new thread for discussing this issue.
Cheers,
Mauro
--
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
