Em 23-05-2011 19:27, Laurent Pinchart escreveu: > Hi Mauro, > > On Friday 20 May 2011 23:01:18 Mauro Carvalho Chehab wrote: >> Em 20-05-2011 16:47, Laurent Pinchart escreveu: >>> On Friday 20 May 2011 21:16:49 Mauro Carvalho Chehab wrote: >>>> Em 20-05-2011 12:49, Laurent Pinchart escreveu: >>>>> On Friday 20 May 2011 17:32:45 Mauro Carvalho Chehab wrote: >>>>>> Em 15-05-2011 04:48, Laurent Pinchart escreveu: >>>>>>> Hi Mauro, >>>>>>> >>>>>>> The following changes since commit >>>>> >>>>> f9b51477fe540fb4c65a05027fdd6f2ecce4db3b: >>>>>>> [media] DVB: return meaningful error codes in dvb_frontend >>>>>>> (2011-05-09 05:47:20 +0200) >>>>>>> >>>>>>> are available in the git repository at: >>>>>>> git://linuxtv.org/pinchartl/uvcvideo.git uvcvideo-next >>>>>>> >>>>>>> They replace the git pull request I've sent on Thursday with the same >>>>>>> subject. > > [snip] > >>>>>>> Laurent Pinchart (4): >>>>>>> v4l: Release module if subdev registration fails >>>>>>> uvcvideo: Register a v4l2_device >>>>>>> uvcvideo: Register subdevices for each entity >>>>>>> uvcvideo: Connect video devices to media entities >>>>>> >>>>>> We've discussed already about using the media controller for uvcvideo, >>>>>> but I can't remember anymore what where your aguments in favor of >>>>>> merging it (and, even if I've remembered it right now, the #irc >>>>>> channel log is not the proper way to document the rationale to apply >>>>>> a patch). >>>>>> >>>>>> The thing is: it is clear that SoC embedded devices need the media >>>>>> controller, as they have IP blocks that do weird things, and userspace >>>>>> may need to access those, as it is not possible to control such IP >>>>>> blocks using the V4L2 API. >>>>>> >>>>>> However, I have serious concerns about media_controller API usage on >>>>>> generic drivers, as it is required that all drivers should be fully >>>>>> configurable via V4L2 API alone, otherwise we'll have regressions, as >>>>>> no generic applications use the media_controller. >>>>>> >>>>>> In other words, if you have enough arguments about why we should add >>>>>> media_controller support at the uvcvideo, please clearly provide them >>>>>> at the patch descriptions, as this is not obvious. It would equally >>>>>> important do document, at the uvcvideo doc, what kind of information >>>>>> is provided via the media_controller and why an userspace application >>>>>> should care to use it. >>>>> >>>>> First of all, the uvcvideo driver doesn't require application to use >>>>> the media controller API to operate. All configuration is handled >>>>> through a V4L2 video device node, and these patches do not modify >>>>> that. No change is required to applications to use the uvcvideo >>>>> driver. >>>>> >>>>> There's however a reason why I want to add support for the media >>>>> controller API to the uvcvideo driver (I wouldn't have submitted the >>>>> patches otherwise >>>>> >>>>> :-)). UVC-compliant devices are modeled as a connected graph of >>>>> :entities >>>>> >>>>> (called terminals and units in the UVC world). The device topology can >>>>> be arbitrarily complex (or simple, which is fortunately often the >>>>> case) and is exported by the device to the host through USB >>>>> descriptors. The uvcvideo driver parses the descriptor, creates an >>>>> internal >>>>> representation of the device internal topology, and maps V4L2 >>>>> operations to the various entities that the device contains. >>>>> The UVC specification standardizes a couple of entities (camera >>>>> terminal, processing unit, ...) and allows device vendors to create >>>>> vendor-specific entities called extension units (XUs in short). Those >>>>> XUs are usually used to expose controls that are not standardized by >>>>> UVC to the host. These controls can be anything from an activity LED >>>>> to a firmware update system. The uvcvideo tries to map those XU >>>>> controls to V4L2 controls when it makes sense (and when the controls >>>>> are documented by the device manufacturer, which is unfortunately >>>>> often not the case). If an XU control can't be mapped to a V4L2 >>>>> control, it can be accessed through uvcvideo-specific (documented) >>>>> ioctls. >>>>> >>>>> In order to access those XU controls, device-specific applications >>>>> (such as a firmware update application for instance) need to know what >>>>> XUs are present in the device and possibly how they are connected. >>>>> That information can't be exported through V4L2. That's why I'm adding >>>>> media controller support to the uvcvideo driver. >>>> >>>> By allowing access to those undocumented XU controls an Evil >>>> Manufacturer(tm) could try to sell its own proprietary software that >>>> will work on Linux where all other software will deadly fail or will >>>> produce very bad results. We've seen that history before. >> >> As you're putting some names, let me be clearer. In the past we had bad >> stuff like this one: >> http://kerneltrap.org/node/3729 >> >> that ended by the need of somebody to rewrite the entire pwc driver, >> because the only way to get a decent image using a Philips camera, with >> the "open source" driver were to run a closed-source binary. > > I clearly remember that, and I think you know that I'm a strong advocate of > open-source drivers (right ? :-)). > >> undocumented controls that can do everything, as you said, can create a >> similar situation to what we had in the past. > > The situation with the pwc driver was very different. The open-source driver > was designed for the closed-source module. That's not at all what I'm > proposing for the uvcvideo driver. > > Furthermore, implementing proprietary closed-source drivers will always be > possible, and can already be done for UVC devices with libusb. We're thus not > talking about opening the door to possible (and, I believe, very unlikely) > proprietary Linux drivers for UVC devices. > > UVC compliance, using the Microsoft driver, is a requirement for webcams to > receive the "designed for Windows Vista/7" certification. Vendors are thus not > trying to push all kind of proprietary, non UVC-compliant features for their > devices (most of them don't have the necessary resources to implement a custom > UVC driver). If you look on how MCE remote controllers are implemented, you'll see that this is a really bad example. I was told by one of the MCE manufacturers that even them don't know the MCE protocol used there. Microsoft doesn't care about open specs. They only care about having the device working with their drivers. > Can you imagine a vendor looking at the Linux driver, seeing that UVC XUs can > be accessed directly, and then deciding to design their hardware based on that > ? I can't :-) XUs are accessible in Windows through a documented API. If > vendors want to design devices that expose XU controls, they will do it, > regardless of whether Linux implements support for that or not. It is to fragile to assume that. At Windows, vendors write their own drivers, and are allowed to do whatever they want on a closed source. >>> We have several alternatives. One of them, that is being shipped in some >>> systems, is a uvcvideo driver patched by the Evil Manufacturer(tm), >>> incompatible with the mainline version. Another one is a closed-source >>> userspace driver based on libusb shipped by the Evil Manufacturer(tm). >>> Yet another one is webcams that work on Windows only. Which one do you >>> prefer ? >> >> I prefer to ask the vendor about the XU controls that he needs and add a >> proper interface for them. > > And I would rather having Nvidia documenting their hardware, but that's not > the world we live in :-) > > Some XU controls are variable-size binary chunks of data. We can't expose that > as V4L2 controls, which is why I expose them using a documented UVC API. The V4L2 API allows string controls. >>>> That's why I'm concerned a lot about exposing such internal raw >>>> interfaces to userspace. >>>> >>>> It should be noticed that such XU-specific Linux applications will >>>> depend if the vendor is working on Linux or if somebody else did some >>>> reverse engineering and discovered (for example) how to upgrade a >>>> firmware for a certain camera. >>> >>> The only Evil Manufacturer(tm) I know of that really uses XUs is >>> Logitech. They've been quite supportive so far, and have documented at >>> least part of their XU controls. >> >> If they're quite supportive, they are not an Evil Manufacturer. We can ask >> them to document the XU controls they need and add a proper documented >> support for them. > > They won't document everything. LED control is documented. Pan/tilt is > documented. Firmware update isn't. > > Firmware update can even require a crypto handshake with the device. That's > understandable, as they want to avoid people breaking their devices and > sending them back to tech support. A Firmware update should require root access. Normal users/applications should not be allowed to touch at the firmware without root access, as otherwise some application may damage a device or put some weird stuff there. The XU controls (or whatever interface) for firmware upgrade should require CAP_SYS_ADMIN. I agree that the vendor should not be required to open the firmware upgrade protocol, but it can document what XU controls will be used for that, in order for the driver to require the proper security capability bits. >>>> In the first case, we should simply ask the vendor to document that XU >>>> control and export it as something else. >> >> s/something else/other controls/ >> >> (that line got mangled when I've removed another phase from the sentence >> while editing it) > > It's not just a matter of exposing controls, see my comment below about the > new UVC H.264 spec. > >>> Why "something else" ? The XU interface has been designed by the USB-IF >>> to be a kernelspace to userspace API. That's how Microsoft, and I think >>> Apple, implemented it (it might not be a reference though). >>> >>>> In the latter case, the developer that did the reverse engineering can >>>> just send us a patch adding the new V4L control/firmware upgrade >>>> logic/whatever to us. >>> >>> UVC is a class specification. I don't want to cripple the driver with >>> tons of device-specific code. We already have Apple iSight- and >>> Logitech-specific code and way too many quirks for my taste. >> >> Unfortunately, by being a generic driver for an USB class, and with vendors >> not quite following the specs, there's no way to avoid having >> device-specific stuff there. Other similar drivers like snd-usb-audio and >> sound hda driver has lots of quirks. In particular, the hda driver >> contains more lines to the patch-*.c drivers (with the device-specific >> stuff) than the driver core: >> >> $ wc -l sound/pci/hda/*.c >> 267 sound/pci/hda/hda_beep.c >> 5072 sound/pci/hda/hda_codec.c >> 637 sound/pci/hda/hda_eld.c >> 1084 sound/pci/hda/hda_generic.c >> 818 sound/pci/hda/hda_hwdep.c >> 2854 sound/pci/hda/hda_intel.c >> 727 sound/pci/hda/hda_proc.c >> 4988 sound/pci/hda/patch_analog.c >> 572 sound/pci/hda/patch_ca0110.c >> 1314 sound/pci/hda/patch_cirrus.c >> 776 sound/pci/hda/patch_cmedia.c >> 3905 sound/pci/hda/patch_conexant.c >> 1749 sound/pci/hda/patch_hdmi.c >> 20167 sound/pci/hda/patch_realtek.c >> 335 sound/pci/hda/patch_si3054.c >> 6434 sound/pci/hda/patch_sigmatel.c >> 6107 sound/pci/hda/patch_via.c >> 57806 total >> >> Yeah, device-specific stuff sucks, but sometimes there's no way to properly >> support a device. > > Luckily there are proper ways to support UVC XUs, by exposing them to > userspace :-) This is not the proper way. If you get a Realtek audio device (the biggest one from the list above), it will just works, as kernelspace will abstract the hardware for you. However, if you move patch_realtek to userspace, the driver would not work without a Realtek specific stuff in userspace, that could eventually be closed source. We should never allow such things, otherwise we'll be distrying the open source ecosystem. >>>> So, I'm yet not convinced ;) In fact, I think we should just deprecate >>>> the XU private ioctls. >>> >>> http://www.quickcamteam.net/uvc-h264/USB_Video_Payload_H.264_0.87.pdf >>> >>> That's a brain-dead proposal for a new H.264 payload format pushed by >>> Logitech and Microsoft. The document is a bit outdated, but the final >>> version will likely be close. It requires direct XU access from >>> applications. I don't like it either, and the alternative will be to not >>> support H.264 UVC cameras at all (something I might consider, by >>> blacklisting the product completely). Are you ready to refuse supporting >>> large classes of USB hardware ? >> >> What's the difference between: >> 1) exposing XU access to userspace and having no applications using it; >> 2) just blacklisting them. >> >> The end result is the same. > > Why would there be no applications using it ? The UVC H.264 XUs are documented > in the above spec, so application can use them. The Linux kernel were designed to abstract hardware differences. We should not move this task to userspace. >> The V4L2 API is capable of handling H.264 payload format. >> >> So, between the two above alternatives, I would choose 3: >> - to handle such XU access internally at the driver. > > You should read the above spec. It used MJPEG as a container format and embeds > YUV and H.264 payloads in the MJPEG headers, most of the time without any > actual MJPEG data after the header. This needs to be configured an > demultiplexed in software, and it's a mess. We don't want to implement that in > the kernel. To support this, the proper solution is 3: > > - implement a libv4l plugin that uses the UVC XU API. Yeah, libv4l can be used for handling the h.264 stuff. 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
