[GIT PATCH FOR 2.6.40] uvcvideo patches

[GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

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.

Bob Liu (2):
      Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
      uvcvideo: Add support for NOMMU arch

Hans de Goede (2):
      v4l: Add M420 format definition
      uvcvideo: Add M420 format support

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                                                                                                    
                                                                                                                                                           
 Documentation/DocBook/media-entities.tmpl |    1 +                                                                                                        
 Documentation/DocBook/v4l/pixfmt-m420.xml |  147 +++++++++++++++++++++++++++++                                                                            
 Documentation/DocBook/v4l/pixfmt.xml      |    1 +                                                                                                        
 Documentation/DocBook/v4l/videodev2.h.xml |    1 +                                                                                                        
 drivers/media/video/uvc/Makefile          |    3 +                                                                                                        
 drivers/media/video/uvc/uvc_driver.c      |   71 +++++++++++++--                                                                                          
 drivers/media/video/uvc/uvc_entity.c      |  118 +++++++++++++++++++++++                                                                                  
 drivers/media/video/uvc/uvc_queue.c       |   34 +++++++-                                                                                                 
 drivers/media/video/uvc/uvc_v4l2.c        |   17 ++++                                                                                                     
 drivers/media/video/uvc/uvcvideo.h        |   27 +++++                                                                                                    
 drivers/media/video/v4l2-dev.c            |   18 ++++                                                                                                     
 drivers/media/video/v4l2-device.c         |    5 +-                                                                                                       
 include/linux/videodev2.h                 |    1 +                                                                                                        
 include/media/v4l2-dev.h                  |    2 +                                                                                                        
 14 files changed, 437 insertions(+), 9 deletions(-)                                                                                                       
 create mode 100644 Documentation/DocBook/v4l/pixfmt-m420.xml                                                                                              
 create mode 100644 drivers/media/video/uvc/uvc_entity.c

-- 
Regards,

Laurent Pinchart

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

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.
> 
> Bob Liu (2):
>       Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
>       uvcvideo: Add support for NOMMU arch

IMO, such fixes should happen inside the arch bits, and not on each driver. If this fix
is needed for uvc video, the same fix should probably needed to all other USB drivers, in
order to work on NOMMU arch.

For now, I'm accepting this as a workaround, but please work on a generic solution
for it.

> Hans de Goede (2):
>       v4l: Add M420 format definition
>       uvcvideo: Add M420 format support

OK.

> 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.

Thanks,
Mauro.

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

Hi Mauro,

Thanks for handling the pull request.

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.
> > 
> > Bob Liu (2):
> >       Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
> >       uvcvideo: Add support for NOMMU arch
> 
> IMO, such fixes should happen inside the arch bits, and not on each driver.
> If this fix is needed for uvc video, the same fix should probably needed
> to all other USB drivers, in order to work on NOMMU arch.
> 
> For now, I'm accepting this as a workaround, but please work on a generic
> solution for it.

A fix at the arch/ level isn't possible, as drivers need to implement the 
get_unmapped_area file operation in order to support NOMMU architectures. The 
proper fix is of course to port uvcvideo to videobuf2, and implement support 
for NOMMU in videobuf2. Modifications to individual drivers will still be 
needed to fill the get_unmapped_area operation pointer with a videobuf2 
handler though.

> > Hans de Goede (2):
> >       v4l: Add M420 format definition
> >       uvcvideo: Add M420 format support
> 
> OK.
> 
> > 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.

The media controller has been designed to export the device internal topology 
to userspace and to make it configurable. That makes it an ideal candidate for 
the task at hand, which is exporting the device internal topology to 
userspace. The uvcvideo driver doesn't allow applications to configure the 
device through the media controller API, so there will be no change for V4L2-
only applications.

-- 
Regards,

Laurent Pinchart

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Rémi Denis-Courmont]

Le vendredi 20 mai 2011 18:32:45 Mauro Carvalho Chehab, vous avez écrit :
> 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.

If VLC counts as a generic application, I'd be more than API to use the 
media_controller (or whatever else) if only to find which ALSA (sub)device, if 
any, corresponds to the V4L2 node of a given USB webcam or such.

I don't know any solution at the moment, and this is a major inconvenience on 
Linux desktop compared to DirectShow.

-- 
Rémi Denis-Courmont
http://www.remlab.info/
http://fi.linkedin.com/in/remidenis

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

Hi Rémi,

On Friday 20 May 2011 17:55:52 Rémi Denis-Courmont wrote:
> Le vendredi 20 mai 2011 18:32:45 Mauro Carvalho Chehab, vous avez écrit :
> > 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.
> 
> If VLC counts as a generic application, I'd be more than API to use the
> media_controller (or whatever else) if only to find which ALSA (sub)device,
> if any, corresponds to the V4L2 node of a given USB webcam or such.

That feature is not available yet, but it's definitely something I want to 
fix. It might be a bit tricky for USB devices (as uvcvideo and usbaudio don't 
know about eachother), but we need to find a proper solution.

> I don't know any solution at the moment, and this is a major inconvenience
> on Linux desktop compared to DirectShow.

-- 
Regards,

Laurent Pinchart

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

Em 20-05-2011 12:55, Rémi Denis-Courmont escreveu:
> Le vendredi 20 mai 2011 18:32:45 Mauro Carvalho Chehab, vous avez écrit :
>> 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.
> 
> If VLC counts as a generic application, I'd be more than API to use the 
> media_controller (or whatever else) if only to find which ALSA (sub)device, if 
> any, corresponds to the V4L2 node of a given USB webcam or such.
> 
> I don't know any solution at the moment, and this is a major inconvenience on 
> Linux desktop compared to DirectShow.

You don't need the media controller for it.

The proper solution for it is to use the sysfs to identify the alsa sub-device.

For example, I have this on one of my machines:

$ lspci |grep Multimedia
1c:00.0 Multimedia video controller: Conexant Systems, Inc. CX23885 PCI Video and Audio Decoder (rev 04)
37:09.0 Multimedia controller: Philips Semiconductors SAA7131/SAA7133/SAA7135 Video Broadcast Decoder (rev d1)

$ $ lsusb
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 006 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 007 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 008 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 001 Device 004: ID 0409:005a NEC Corp. HighSpeed Hub
Bus 001 Device 005: ID 0ac8:3330 Z-Star Microelectronics Corp. 
Bus 008 Device 002: ID 2101:020f ActionStar 
Bus 001 Device 003: ID 2040:4200 Hauppauge 
Bus 001 Device 006: ID 0d8c:0126 C-Media Electronics, Inc. 

I wrote an utility in the past that dig into the sysfs stuff to identify it, but
I'm not sure if it is still working fine, as some changes at sysfs might affect it,
as I never intended to maintain such utility.

I'll seek for some time to re-write it with another approach and add it as a 
library, inside v4l2-utils, eventually together with libv4l.

Basically, all V4L devices are under video4linux class:

$ tree /sys/class/video4linux/
/sys/class/video4linux/
├── vbi0 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/video4linux/vbi0
├── video0 -> ../../devices/pci0000:00/0000:00:1c.0/0000:1c:00.0/video4linux/video0
├── video1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.0/video4linux/video1
├── video2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/video4linux/video2
└── video3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.2/1-6.2:1.0/video4linux/video3

And all alsa devices are under sound class:

$ tree /sys/class/sound/
/sys/class/sound/
├── card0 -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0
├── card1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.1/sound/card1
├── card2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/sound/card2
├── card3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.3/1-6.3:1.0/sound/card3
├── controlC0 -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0/controlC0
├── controlC1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.1/sound/card1/controlC1
├── controlC2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/sound/card2/controlC2
├── controlC3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.3/1-6.3:1.0/sound/card3/controlC3
├── hwC0D0 -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0/hwC0D0
├── pcmC0D0c -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0/pcmC0D0c
├── pcmC0D0p -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0/pcmC0D0p
├── pcmC1D0c -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.1/sound/card1/pcmC1D0c
├── pcmC2D0c -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/sound/card2/pcmC2D0c
├── pcmC3D0p -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.3/1-6.3:1.0/sound/card3/pcmC3D0p
├── seq -> ../../devices/virtual/sound/seq
└── timer -> ../../devices/virtual/sound/timer

All that such library/utility/function needs do to is to parse the two above sysfs directories
and associate the devices based on the provided aliases.

For example, on the above, we have 4 V4L cards:

PCI card (in this case, a saa7134-based card)
├── vbi0 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/video4linux/vbi0
├── video2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/video4linux/video2
└── card2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/sound/card2

All tree are at the PCI device 0000:37:09.0.

USB card (in this example, an em28xx-based card, that uses snd-usb-audio for alsa)
├── video1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.0/video4linux/video1
└── card1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.1/sound/card1

All two are at the USB 1-5 device.

DVB-C/DVB-T PCI card (in this case cx23885, without any alsa-associated device)
└── video0 -> ../../devices/pci0000:00/0000:00:1c.0/0000:1c:00.0/video4linux/video0

This one is at PCI device 0000:1c:00.0.

UVC webcam (with audio provided by snd-usb-audio)
├── video3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.2/1-6.2:1.0/video4linux/video3
└── card3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.3/1-6.3:1.0/sound/card3

All two are at the USB 1-6 device.

So, it is easy to associate video and audio for each device. You can even associate the volume
controls and the PCM input/outputs using the above info.

Cheers,
Mauro.

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

Em 20-05-2011 12:49, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> Thanks for handling the pull request.
> 
> 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.
>>>
>>> Bob Liu (2):
>>>       Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
>>>       uvcvideo: Add support for NOMMU arch
>>
>> IMO, such fixes should happen inside the arch bits, and not on each driver.
>> If this fix is needed for uvc video, the same fix should probably needed
>> to all other USB drivers, in order to work on NOMMU arch.
>>
>> For now, I'm accepting this as a workaround, but please work on a generic
>> solution for it.
> 
> A fix at the arch/ level isn't possible, as drivers need to implement the 
> get_unmapped_area file operation in order to support NOMMU architectures. The 
> proper fix is of course to port uvcvideo to videobuf2, and implement support 
> for NOMMU in videobuf2. Modifications to individual drivers will still be 
> needed to fill the get_unmapped_area operation pointer with a videobuf2 
> handler though.

This doesn't sound nice, as most people test their drivers only on an specific
architecture. If the driver can work on more then one architecture (e. g. if
it is not part of the IP block of some SoC chip, but, instead, uses some generic
bus like USB or PCI), the driver itself shouldn't contain any arch-specific bits.
IMO, the proper fix should be either at the DMA stuff or somewhere inside the bus
driver implementation.

>>> Hans de Goede (2):
>>>       v4l: Add M420 format definition
>>>       uvcvideo: Add M420 format support
>>
>> OK.
>>
>>> 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.

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.

In the first case, we should simply ask the vendor to document that XU control and
export it as something else. 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.

So, I'm yet not convinced ;) In fact, I think we should just deprecate the XU
private ioctls.

> The media controller has been designed to export the device internal topology 
> to userspace and to make it configurable. That makes it an ideal candidate for 
> the task at hand, which is exporting the device internal topology to 
> userspace. The uvcvideo driver doesn't allow applications to configure the 
> device through the media controller API, so there will be no change for V4L2-
> only applications.
> 

Mauro

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

Hi Mauro,

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.
> >>> 
> >>> Bob Liu (2):
> >>>       Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
> >>>       uvcvideo: Add support for NOMMU arch
> >> 
> >> IMO, such fixes should happen inside the arch bits, and not on each
> >> driver. If this fix is needed for uvc video, the same fix should
> >> probably needed to all other USB drivers, in order to work on NOMMU
> >> arch.
> >> 
> >> For now, I'm accepting this as a workaround, but please work on a
> >> generic solution for it.
> > 
> > A fix at the arch/ level isn't possible, as drivers need to implement the
> > get_unmapped_area file operation in order to support NOMMU architectures.
> > The proper fix is of course to port uvcvideo to videobuf2, and implement
> > support for NOMMU in videobuf2. Modifications to individual drivers will
> > still be needed to fill the get_unmapped_area operation pointer with a
> > videobuf2 handler though.
> 
> This doesn't sound nice, as most people test their drivers only on an
> specific architecture. If the driver can work on more then one
> architecture (e. g. if it is not part of the IP block of some SoC chip,
> but, instead, uses some generic bus like USB or PCI), the driver itself
> shouldn't contain any arch-specific bits. IMO, the proper fix should be
> either at the DMA stuff or somewhere inside the bus driver implementation.

It might not sound nice, but that's how NOMMU works. It needs 
get_unmapped_area. If you can get rid of that requirement I'll be happy to 
remove NOMMU-specific support from the driver :-)

> >>> Hans de Goede (2):
> >>>       v4l: Add M420 format definition
> >>>       uvcvideo: Add M420 format support
> >> 
> >> OK.
> >> 
> >>> 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.

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 ?

> 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.

> In the first case, we should simply ask the vendor to document that XU
> control and export it as something else.

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.

> 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 ?

> > The media controller has been designed to export the device internal
> > topology to userspace and to make it configurable. That makes it an
> > ideal candidate for the task at hand, which is exporting the device
> > internal topology to userspace. The uvcvideo driver doesn't allow
> > applications to configure the device through the media controller API,
> > so there will be no change for V4L2-only applications.

-- 
Regards,

Laurent Pinchart

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

Em 20-05-2011 16:47, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> 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.
>>>>>
>>>>> Bob Liu (2):
>>>>>       Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
>>>>>       uvcvideo: Add support for NOMMU arch
>>>>
>>>> IMO, such fixes should happen inside the arch bits, and not on each
>>>> driver. If this fix is needed for uvc video, the same fix should
>>>> probably needed to all other USB drivers, in order to work on NOMMU
>>>> arch.
>>>>
>>>> For now, I'm accepting this as a workaround, but please work on a
>>>> generic solution for it.
>>>
>>> A fix at the arch/ level isn't possible, as drivers need to implement the
>>> get_unmapped_area file operation in order to support NOMMU architectures.
>>> The proper fix is of course to port uvcvideo to videobuf2, and implement
>>> support for NOMMU in videobuf2. Modifications to individual drivers will
>>> still be needed to fill the get_unmapped_area operation pointer with a
>>> videobuf2 handler though.
>>
>> This doesn't sound nice, as most people test their drivers only on an
>> specific architecture. If the driver can work on more then one
>> architecture (e. g. if it is not part of the IP block of some SoC chip,
>> but, instead, uses some generic bus like USB or PCI), the driver itself
>> shouldn't contain any arch-specific bits. IMO, the proper fix should be
>> either at the DMA stuff or somewhere inside the bus driver implementation.
> 
> It might not sound nice, but that's how NOMMU works. It needs 
> get_unmapped_area. If you can get rid of that requirement I'll be happy to 
> remove NOMMU-specific support from the driver :-)

As I said, the patches were added, but we need to work on a better solution
than polluting every driver with

#if CONFIG_NOMMU

just because arm arch is crappy.

> 
>>>>> Hans de Goede (2):
>>>>>       v4l: Add M420 format definition
>>>>>       uvcvideo: Add M420 format support
>>>>
>>>> OK.
>>>>
>>>>> 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.

undocumented controls that can do everything, as you said, can create a
similar situation to what we had in the past.

> 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.

>> 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.

>> 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)

> 
> 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.

>> 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.

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.

Mauro.

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

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.
> >>>>> 
> >>>>> Bob Liu (2):
> >>>>>       Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
> >>>>>       uvcvideo: Add support for NOMMU arch
> >>>> 
> >>>> IMO, such fixes should happen inside the arch bits, and not on each
> >>>> driver. If this fix is needed for uvc video, the same fix should
> >>>> probably needed to all other USB drivers, in order to work on NOMMU
> >>>> arch.
> >>>> 
> >>>> For now, I'm accepting this as a workaround, but please work on a
> >>>> generic solution for it.
> >>> 
> >>> A fix at the arch/ level isn't possible, as drivers need to implement
> >>> the get_unmapped_area file operation in order to support NOMMU
> >>> architectures. The proper fix is of course to port uvcvideo to
> >>> videobuf2, and implement support for NOMMU in videobuf2. Modifications
> >>> to individual drivers will still be needed to fill the
> >>> get_unmapped_area operation pointer with a videobuf2 handler though.
> >> 
> >> This doesn't sound nice, as most people test their drivers only on an
> >> specific architecture. If the driver can work on more then one
> >> architecture (e. g. if it is not part of the IP block of some SoC chip,
> >> but, instead, uses some generic bus like USB or PCI), the driver itself
> >> shouldn't contain any arch-specific bits. IMO, the proper fix should be
> >> either at the DMA stuff or somewhere inside the bus driver
> >> implementation.
> > 
> > It might not sound nice, but that's how NOMMU works. It needs
> > get_unmapped_area. If you can get rid of that requirement I'll be happy
> > to remove NOMMU-specific support from the driver :-)
> 
> As I said, the patches were added, but we need to work on a better solution
> than polluting every driver with
> 
> #if CONFIG_NOMMU
> 
> just because arm arch is crappy.

There might be some misunderstanding here (not that ARM doesn't bring its 
share of issues, we both agree on that :-)). NOMMU has nothing to do with ARM, 
it's for architectures that have no system MMU. Everything lives in the same 
address space, so some support is needed from drivers when "mapping" memory to 
"userspace".

I'll answer the MC part over the weekend, I need to sleep now :-)

-- 
Regards,

Laurent Pinchart

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

Em 20-05-2011 18:29, 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.
>>>>>>>
>>>>>>> Bob Liu (2):
>>>>>>>       Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
>>>>>>>       uvcvideo: Add support for NOMMU arch
>>>>>>
>>>>>> IMO, such fixes should happen inside the arch bits, and not on each
>>>>>> driver. If this fix is needed for uvc video, the same fix should
>>>>>> probably needed to all other USB drivers, in order to work on NOMMU
>>>>>> arch.
>>>>>>
>>>>>> For now, I'm accepting this as a workaround, but please work on a
>>>>>> generic solution for it.
>>>>>
>>>>> A fix at the arch/ level isn't possible, as drivers need to implement
>>>>> the get_unmapped_area file operation in order to support NOMMU
>>>>> architectures. The proper fix is of course to port uvcvideo to
>>>>> videobuf2, and implement support for NOMMU in videobuf2. Modifications
>>>>> to individual drivers will still be needed to fill the
>>>>> get_unmapped_area operation pointer with a videobuf2 handler though.
>>>>
>>>> This doesn't sound nice, as most people test their drivers only on an
>>>> specific architecture. If the driver can work on more then one
>>>> architecture (e. g. if it is not part of the IP block of some SoC chip,
>>>> but, instead, uses some generic bus like USB or PCI), the driver itself
>>>> shouldn't contain any arch-specific bits. IMO, the proper fix should be
>>>> either at the DMA stuff or somewhere inside the bus driver
>>>> implementation.
>>>
>>> It might not sound nice, but that's how NOMMU works. It needs
>>> get_unmapped_area. If you can get rid of that requirement I'll be happy
>>> to remove NOMMU-specific support from the driver :-)
>>
>> As I said, the patches were added, but we need to work on a better solution
>> than polluting every driver with
>>
>> #if CONFIG_NOMMU
>>
>> just because arm arch is crappy.
> 
> There might be some misunderstanding here (not that ARM doesn't bring its 
> share of issues, we both agree on that :-)). NOMMU has nothing to do with ARM, 
> it's for architectures that have no system MMU. Everything lives in the same 
> address space, so some support is needed from drivers when "mapping" memory to 
> "userspace".

I see. Well, having this inside videobuf2 will hide it from the drivers 
that use it so, it is better than having the same code duplicated into 
all drivers. Yet, we have several streaming stuff spread. Just naming the
most used non-legacy drivers, we have: gspca, videobuf1, videobuf2 and uvcvideo.
There are the other drivers that are currently unmaintained with their own
streaming implementations. If UVC is broken if no MMU, all the other drivers
are also broken.

Maybe we should just make all other drivers, for now, dependent of !NOMMU, 
to avoid showing them to the architectures were they won't work. Or, if the
solution given for UVC is generic enough, to just apply it to the other drivers.

Yet, the better is to have a NOMMU-dependent code inside the arch-specific bits 
(or at least having a default handler that would take care of it for NOMMU, 
that would work for most drivers), to avoid code duplication or having generic
drivers dependent of !NOMMU.

> 
> I'll answer the MC part over the weekend, I need to sleep now :-)

OK.

Mauro.

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

Em 20-05-2011 15:48, Mauro Carvalho Chehab escreveu:
> Em 20-05-2011 12:55, Rémi Denis-Courmont escreveu:
>> Le vendredi 20 mai 2011 18:32:45 Mauro Carvalho Chehab, vous avez écrit :
>>> 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.
>>
>> If VLC counts as a generic application, I'd be more than API to use the 
>> media_controller (or whatever else) if only to find which ALSA (sub)device, if 
>> any, corresponds to the V4L2 node of a given USB webcam or such.
>>
>> I don't know any solution at the moment, and this is a major inconvenience on 
>> Linux desktop compared to DirectShow.
> 
> You don't need the media controller for it.
> 
> The proper solution for it is to use the sysfs to identify the alsa sub-device.
> 
> For example, I have this on one of my machines:
> 
> $ lspci |grep Multimedia
> 1c:00.0 Multimedia video controller: Conexant Systems, Inc. CX23885 PCI Video and Audio Decoder (rev 04)
> 37:09.0 Multimedia controller: Philips Semiconductors SAA7131/SAA7133/SAA7135 Video Broadcast Decoder (rev d1)
> 
> $ $ lsusb
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 006 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 007 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 008 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 001 Device 004: ID 0409:005a NEC Corp. HighSpeed Hub
> Bus 001 Device 005: ID 0ac8:3330 Z-Star Microelectronics Corp. 
> Bus 008 Device 002: ID 2101:020f ActionStar 
> Bus 001 Device 003: ID 2040:4200 Hauppauge 
> Bus 001 Device 006: ID 0d8c:0126 C-Media Electronics, Inc. 
> 
> I wrote an utility in the past that dig into the sysfs stuff to identify it, but
> I'm not sure if it is still working fine, as some changes at sysfs might affect it,
> as I never intended to maintain such utility.
> 
> I'll seek for some time to re-write it with another approach and add it as a 
> library, inside v4l2-utils, eventually together with libv4l.
> 
> Basically, all V4L devices are under video4linux class:
> 
> $ tree /sys/class/video4linux/
> /sys/class/video4linux/
> ├── vbi0 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/video4linux/vbi0
> ├── video0 -> ../../devices/pci0000:00/0000:00:1c.0/0000:1c:00.0/video4linux/video0
> ├── video1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.0/video4linux/video1
> ├── video2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/video4linux/video2
> └── video3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.2/1-6.2:1.0/video4linux/video3
> 
> And all alsa devices are under sound class:
> 
> $ tree /sys/class/sound/
> /sys/class/sound/
> ├── card0 -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0
> ├── card1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.1/sound/card1
> ├── card2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/sound/card2
> ├── card3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.3/1-6.3:1.0/sound/card3
> ├── controlC0 -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0/controlC0
> ├── controlC1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.1/sound/card1/controlC1
> ├── controlC2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/sound/card2/controlC2
> ├── controlC3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.3/1-6.3:1.0/sound/card3/controlC3
> ├── hwC0D0 -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0/hwC0D0
> ├── pcmC0D0c -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0/pcmC0D0c
> ├── pcmC0D0p -> ../../devices/pci0000:00/0000:00:1b.0/sound/card0/pcmC0D0p
> ├── pcmC1D0c -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.1/sound/card1/pcmC1D0c
> ├── pcmC2D0c -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/sound/card2/pcmC2D0c
> ├── pcmC3D0p -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.3/1-6.3:1.0/sound/card3/pcmC3D0p
> ├── seq -> ../../devices/virtual/sound/seq
> └── timer -> ../../devices/virtual/sound/timer
> 
> All that such library/utility/function needs do to is to parse the two above sysfs directories
> and associate the devices based on the provided aliases.
> 
> For example, on the above, we have 4 V4L cards:
> 
> PCI card (in this case, a saa7134-based card)
> ├── vbi0 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/video4linux/vbi0
> ├── video2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/video4linux/video2
> └── card2 -> ../../devices/pci0000:00/0000:00:1e.0/0000:37:09.0/sound/card2
> 
> All tree are at the PCI device 0000:37:09.0.
> 
> USB card (in this example, an em28xx-based card, that uses snd-usb-audio for alsa)
> ├── video1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.0/video4linux/video1
> └── card1 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-5/1-5:1.1/sound/card1
> 
> All two are at the USB 1-5 device.
> 
> DVB-C/DVB-T PCI card (in this case cx23885, without any alsa-associated device)
> └── video0 -> ../../devices/pci0000:00/0000:00:1c.0/0000:1c:00.0/video4linux/video0
> 
> This one is at PCI device 0000:1c:00.0.
> 
> UVC webcam (with audio provided by snd-usb-audio)
> ├── video3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.2/1-6.2:1.0/video4linux/video3
> └── card3 -> ../../devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.3/1-6.3:1.0/sound/card3
> 
> All two are at the USB 1-6 device.
> 
> So, it is easy to associate video and audio for each device. You can even associate the volume
> controls and the PCM input/outputs using the above info.

Ok, I wrote a small library for handling it. I added it at the v4l-utils tree:
	http://git.linuxtv.org/v4l-utils.git?a=commitdiff;h=0ec88068b8f227798e5ecf9d7c375f008d98743a

For the same above devices, it parses the V4L, DVB and sound subsystem info at
/sys/class and outputs the relevant information about them:

Device pci0000:00/0000:00:1a.7/usb1/1-5:
	video1(video) card1(sound card) pcmC1D0c(pcm capture) controlC1(mixer) 
Device pci0000:00/0000:00:1a.7/usb1/1-6:
	video0(video) card2(sound card) pcmC2D0p(pcm output) controlC2(mixer) 
Device pci0000:00/0000:00:1b.0:
	card0(sound card) pcmC0D0c(pcm capture) pcmC0D0p(pcm output) controlC0(mixer) hwC0D0(sound hardware) 
Device pci0000:00/0000:00:1c.0/0000:1c:00.0:
	video2(video) dvb0.frontend0(dvb frontend) dvb0.frontend1(dvb frontend) dvb1.frontend0(dvb frontend) dvb1.frontend1(dvb frontend) dvb0.demux0(dvb demux) dvb0.demux1(dvb demux) dvb1.demux0(dvb demux) dvb1.demux1(dvb demux) dvb0.dvr0(dvb dvr) dvb0.dvr1(dvb dvr) dvb1.dvr0(dvb dvr) dvb1.dvr1(dvb dvr) dvb0.net0(dvb net) dvb0.net1(dvb net) dvb1.net0(dvb net) dvb1.net1(dvb net) dvb0.ca0(dvb conditional access) dvb1.ca0(dvb conditional access) 
Device pci0000:00/0000:00:1e.0/0000:37:09.0:
	video3(video) vbi0(vbi) dvb2.frontend0(dvb frontend) dvb2.demux0(dvb demux) dvb2.dvr0(dvb dvr) dvb2.net0(dvb net) card3(sound card) pcmC3D0c(pcm capture) controlC3(mixer) 

The library, for now, is very simple: it provides just 3 functions:

struct media_devices *discover_media_devices(unsigned int *md_size);
void free_media_devices(struct media_devices *md, unsigned int md_size);
void display_media_devices(struct media_devices *md, unsigned int size);

The discover_media_devices() provide a list of device names inside the subsystem,
the PCI/USB device path and device type.

The list is freed with free_media_devices(), and the above output is produced with
display_media_devices().

It probably makes sense to add more methods there, like, for example, a method to
find the device path at /dev, but I opted to make the initial version as simple as 
possible. Feel free to contribute.

Cheers,
Mauro.

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

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).

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.

> > 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.

> >> 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.

> >> 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 :-)

> >> 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 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.

-- 
Regards,

Laurent Pinchart

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Sakari Ailus]

Hi Mauro and Laurent,

On Fri, May 20, 2011 at 06:01:18PM -0300, Mauro Carvalho Chehab wrote:
> Em 20-05-2011 16:47, Laurent Pinchart escreveu:
[clip]
> >> 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.
> 
> undocumented controls that can do everything, as you said, can create a
> similar situation to what we had in the past.
> 
> > 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.

As the UVC standard allows implementing custom functionality in a standard
way (UVC), I wouldn't necessarily prevent using that. I definitely agree
that the XU functionality should be properly documented by vendors, and what
can be supported using a standardised interface must be.

[clip]

> > 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.

In this case the functionality the hardware provides is rather well defined
--- audio streams and a set of mixer controls. It's just the implementation
which varies, but that can still be made fit behind a standard interface
which provides standardised functionality for the applications.

In the case of the UVC hardware, there's a standard which is mostly followed
relatively well by vendors. What likely cannot be standardised is, for
example, the firmware update for Logitech webcams mentioned above, which the
UVC standard still allows. This will stay specific to Logitech devices
probably forever.

As far as I understand, this discussion isn't really even related to the
patchset. The XU access is still provided to user space without the
patchset.

> >> 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.

I think that in this case it wouldn't be a choice between the two, but 1)
being "exposing XU access to user space and requiring applications to
support that". This case is documented, unlike the firmware update, and does
not have anything to do with the XU ioctl interface --- the only connection
to XUs is that the user needs to know that an XU is there to interpret the
image data in a certain way. The "user" in this case could be libv4l, so
that the applications wouldn't need to care.

This is a class of UVC hardware as far as I understand, and Logitech being a
major vendor there will likely be a bunch of those devices. My wish is still
that a proper spec will be written before anyone starts manufacturing those
in large quantities. :-)

Regards,

-- 
Sakari Ailus
sakari dot ailus at iki dot fi

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

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.

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Sakari Ailus]

Hi Mauro and Laurent,

On Tue, May 24, 2011 at 11:13:01AM -0300, Mauro Carvalho Chehab wrote:
> Em 23-05-2011 19:27, Laurent Pinchart escreveu:
> > Hi Mauro,
> > 
> > On Friday 20 May 2011 23:01:18 Mauro Carvalho Chehab wrote:
[clip]
> >> 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.

String controls are zero terminated, aren't they?

XU controls may contain zeros in them; Laurent, please correct me if I'm
wrong.

[clip]
> >>> 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.

That's true; if the ioctl has a capability to e.g. render the device
unusable then requiring CAP_SYS_ADMIN is a good idea.

[clip]
> >>> 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.

I fully agree with this: the driver definitely needs to provide a high level
interface on V4L2 node when reasonably possible. I think it would make sense
to provide XU controls that could be used for firmware update and require
SYS_CAP_ADMIN. Then regular applications couldn't access the XU interface,
be it firmware update or something else.

For what it's worth, the Linux kernel supports register level access to I2C
devices from user space --- see Documentation/i2c/dev-interface. Thia
interface is used seldom (as far as I know) due to obvious advantages of
higher level interfaces. I think the UVC XU controls are both conceptually
and in purpose very similar to this.

Kind regards,
Sakari

-- 
Sakari Ailus
sakari dot ailus at iki dot fi

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

Hi Mauro,

Thanks for applying the patches. For the record, the compromise was to 
implement XU controls filtering to make sure that userspace applications won't 
have access to potentially dangerous controls, and to push vendors to properly 
document their XUs.

On Tuesday 24 May 2011 16:13:01 Mauro Carvalho Chehab wrote:
> Em 23-05-2011 19:27, Laurent Pinchart escreveu:
> > 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:

[snip]

> > 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.

Oh, for sure. That's why many UVC devices crash when you send them requests 
that are not used by the Windows UVC driver. Some vendors (namely Logitech) 
started testing their devices on Linux, hopefully that trend will catch up.

My point was that, as devices need to work with the Windows UVC drivers, many 
manufacturers will not add non-compliant, undocumented features to their 
devices as they don't have the resources to implement a custom UVC driver. 
Bigger vendors still do that though.

> > 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.

For proprietary protocols that what happens. For UVC the situation is a bit 
better, as the Microsoft UVC driver is widely used nowadays. The Windows 
driver model allows vendors to write filter drivers though, so I agree that 
they can add support for proprietary device features.

> >>> 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.

Hans was very much against using string controls to pass raw binary data.

[snip]

> >> 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:

[snip]

> >>>> 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.

I agree in principle, but we will have to rethink this at some point in the 
future. I don't think it will always be possible to handle all hardware 
abstractions in the kernel. Some hardware require floating point operations in 
their drivers for instance.

There's an industry trend there, and we need to think about solutions now 
otherwise we will be left without any way forward when too many devices will 
be impossible to support from kernelspace (OMAP4 is a good example there, some 
device drivers require communication with other cores, and the communication 
API is implemented in userspace).

[snip]

-- 
Regards,

Laurent Pinchart

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

Em 25-05-2011 20:20, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> Thanks for applying the patches. For the record, the compromise was to 
> implement XU controls filtering to make sure that userspace applications won't 
> have access to potentially dangerous controls, and to push vendors to properly 
> document their XUs.

Ok, thanks!

>>> 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.
> 
> Hans was very much against using string controls to pass raw binary data.

Pass raw binary data is bad when we know nothing about what's passing there.
A "firmware update" control-type however, is a different thing, as we really
don't care about what's there. Yet, I agree that this may not be the best
way of doing it.

>>> 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.
> 
> I agree in principle, but we will have to rethink this at some point in the 
> future. I don't think it will always be possible to handle all hardware 
> abstractions in the kernel. Some hardware require floating point operations in 
> their drivers for instance.

I talked with Linus some years ago about float point ops in Kernel. He said he was
not against that, but there are some issues, as float point processors are
arch-dependent, and kernel doesn't save FP registers. So, if a driver really needs
to use it, extra care should be taken. That's said, some drivers use fixed point
operations for some specific usages.

> There's an industry trend there, and we need to think about solutions now 
> otherwise we will be left without any way forward when too many devices will 
> be impossible to support from kernelspace (OMAP4 is a good example there, some 
> device drivers require communication with other cores, and the communication 
> API is implemented in userspace).
 
Needing to go to userspace to allow inter-core communication seems very bad.
I seriously doubt that this is a trend. It seems more like a broken-by-design
type of architecture.

Mauro.

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

Hi Mauro,

On Thursday 26 May 2011 01:34:10 Mauro Carvalho Chehab wrote:
> Em 25-05-2011 20:20, Laurent Pinchart escreveu:
> > Hi Mauro,
> > 
> > Thanks for applying the patches. For the record, the compromise was to
> > implement XU controls filtering to make sure that userspace applications
> > won't have access to potentially dangerous controls, and to push vendors
> > to properly document their XUs.
> 
> Ok, thanks!
> 
> >>> 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.
> > 
> > Hans was very much against using string controls to pass raw binary data.
> 
> Pass raw binary data is bad when we know nothing about what's passing
> there. A "firmware update" control-type however, is a different thing, as
> we really don't care about what's there. Yet, I agree that this may not be
> the best way of doing it.
> 
> >>> 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.
> > 
> > I agree in principle, but we will have to rethink this at some point in
> > the future. I don't think it will always be possible to handle all
> > hardware abstractions in the kernel. Some hardware require floating
> > point operations in their drivers for instance.
> 
> I talked with Linus some years ago about float point ops in Kernel. He said
> he was not against that, but there are some issues, as float point
> processors are arch-dependent, and kernel doesn't save FP registers. So,
> if a driver really needs to use it, extra care should be taken. That's
> said, some drivers use fixed point operations for some specific usages.

Issues arise when devices have floating point registers. And yes, that 
happens, I've learnt today about an I2C sensor with floating point registers 
(in this specific case it should probably be put in the broken design 
category, but it exists :-)).

> > There's an industry trend there, and we need to think about solutions now
> > otherwise we will be left without any way forward when too many devices
> > will be impossible to support from kernelspace (OMAP4 is a good example
> > there, some device drivers require communication with other cores, and
> > the communication API is implemented in userspace).
> 
> Needing to go to userspace to allow inter-core communication seems very
> bad. I seriously doubt that this is a trend. It seems more like a
> broken-by-design type of architecture.

I'm inclined to agree with you, but we should address these issues now, while 
we have relatively few devices impacted by them. I fear that ignoring the 
problem and hoping it will go away by itself will bring us to a difficult 
position in the future. We should show the industry in which direction we 
would like it to go.

-- 
Regards,

Laurent Pinchart

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

Em 25-05-2011 20:43, Laurent Pinchart escreveu:

> Issues arise when devices have floating point registers. And yes, that 
> happens, I've learnt today about an I2C sensor with floating point registers 
> (in this specific case it should probably be put in the broken design 
> category, but it exists :-)).

Huh! Yeah, an I2C sensor with FP registers sound weird. We need more details
in order to address those.

>>> There's an industry trend there, and we need to think about solutions now
>>> otherwise we will be left without any way forward when too many devices
>>> will be impossible to support from kernelspace (OMAP4 is a good example
>>> there, some device drivers require communication with other cores, and
>>> the communication API is implemented in userspace).
>>
>> Needing to go to userspace to allow inter-core communication seems very
>> bad. I seriously doubt that this is a trend. It seems more like a
>> broken-by-design type of architecture.
> 
> I'm inclined to agree with you, but we should address these issues now, while 
> we have relatively few devices impacted by them. I fear that ignoring the 
> problem and hoping it will go away by itself will bring us to a difficult 
> position in the future. We should show the industry in which direction we 
> would like it to go.

I'm all about showing the industry in with direction we would like it to go.
We want that all Linux-supported architectures/sub-architectures support 
inter-core communications in kernelspace, in a more efficient way
that it would happen if such communication would happen in userspace.

Thanks,
Mauro.

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

On Thursday 26 May 2011 01:50:07 Mauro Carvalho Chehab wrote:
> Em 25-05-2011 20:43, Laurent Pinchart escreveu:
> > Issues arise when devices have floating point registers. And yes, that
> > happens, I've learnt today about an I2C sensor with floating point
> > registers (in this specific case it should probably be put in the broken
> > design category, but it exists :-)).
> 
> Huh! Yeah, an I2C sensor with FP registers sound weird. We need more
> details in order to address those.

Fortunately for the sensor I'm talking about most of those registers are read-
only and contain large values that can be handled as integers, so all we need 
to do is convert the 32-bit IEEE float value into an integer. Other hardware 
might require more complex FP handling.

> >>> There's an industry trend there, and we need to think about solutions
> >>> now otherwise we will be left without any way forward when too many
> >>> devices will be impossible to support from kernelspace (OMAP4 is a
> >>> good example there, some device drivers require communication with
> >>> other cores, and the communication API is implemented in userspace).
> >> 
> >> Needing to go to userspace to allow inter-core communication seems very
> >> bad. I seriously doubt that this is a trend. It seems more like a
> >> broken-by-design type of architecture.
> > 
> > I'm inclined to agree with you, but we should address these issues now,
> > while we have relatively few devices impacted by them. I fear that
> > ignoring the problem and hoping it will go away by itself will bring us
> > to a difficult position in the future. We should show the industry in
> > which direction we would like it to go.
> 
> I'm all about showing the industry in with direction we would like it to
> go. We want that all Linux-supported architectures/sub-architectures
> support inter-core communications in kernelspace, in a more efficient way
> that it would happen if such communication would happen in userspace.

I agree with that. My concern is about things like

"Standardizing on the OpenMax media libraries and the GStreamer framework is 
the direction that Linaro is going." (David Rusling, Linaro CTO, quoted on 
lwn.net)

We need to address this now, otherwise it will be too late.

-- 
Regards,

Laurent Pinchart

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Arnd Bergmann]

On Thursday 26 May 2011, Laurent Pinchart wrote:
> On Thursday 26 May 2011 01:50:07 Mauro Carvalho Chehab wrote:
> > Em 25-05-2011 20:43, Laurent Pinchart escreveu:
> > > Issues arise when devices have floating point registers. And yes, that
> > > happens, I've learnt today about an I2C sensor with floating point
> > > registers (in this specific case it should probably be put in the broken
> > > design category, but it exists :-)).
> > 
> > Huh! Yeah, an I2C sensor with FP registers sound weird. We need more
> > details in order to address those.
> 
> Fortunately for the sensor I'm talking about most of those registers are read-
> only and contain large values that can be handled as integers, so all we need 
> to do is convert the 32-bit IEEE float value into an integer. Other hardware 
> might require more complex FP handling.

As an additional remark here, most architectures can handle float in the
kernel in some way, but they all do it differently, so it's basically
impossible to do in a cross-architecture device driver.
 
> > I'm all about showing the industry in with direction we would like it to
> > go. We want that all Linux-supported architectures/sub-architectures
> > support inter-core communications in kernelspace, in a more efficient way
> > that it would happen if such communication would happen in userspace.
> 
> I agree with that. My concern is about things like
> 
> "Standardizing on the OpenMax media libraries and the GStreamer framework is 
> the direction that Linaro is going." (David Rusling, Linaro CTO, quoted on 
> lwn.net)
> 
> We need to address this now, otherwise it will be too late.

Absolutely agreed. OpenMAX needs to die as an interface abstraction layer.

IIRC, the last time we discussed this in Linaro, the outcome was basically
that we want to have an OpenMAX compatible library on top of V4L, so that the
Linaro members can have a checkmark in their product specs that lists them
as compatible, but we wouldn't do anything hardware specific in there, or
advocate the use of OpenMAX over v4l2 or gstreamer.

	Arnd

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Mauro Carvalho Chehab]

Arnd Bergmann <arnd@arndb.de> wrote:

>On Thursday 26 May 2011, Laurent Pinchart wrote:
>> On Thursday 26 May 2011 01:50:07 Mauro Carvalho Chehab wrote:
>> > Em 25-05-2011 20:43, Laurent Pinchart escreveu:
>> > > Issues arise when devices have floating point registers. And yes,
>that
>> > > happens, I've learnt today about an I2C sensor with floating
>point
>> > > registers (in this specific case it should probably be put in the
>broken
>> > > design category, but it exists :-)).
>> > 
>> > Huh! Yeah, an I2C sensor with FP registers sound weird. We need
>more
>> > details in order to address those.
>> 
>> Fortunately for the sensor I'm talking about most of those registers
>are read-
>> only and contain large values that can be handled as integers, so all
>we need 
>> to do is convert the 32-bit IEEE float value into an integer. Other
>hardware 
>> might require more complex FP handling.
>
>As an additional remark here, most architectures can handle float in
>the
>kernel in some way, but they all do it differently, so it's basically
>impossible to do in a cross-architecture device driver.
> 
>> > I'm all about showing the industry in with direction we would like
>it to
>> > go. We want that all Linux-supported
>architectures/sub-architectures
>> > support inter-core communications in kernelspace, in a more
>efficient way
>> > that it would happen if such communication would happen in
>userspace.
>> 
>> I agree with that. My concern is about things like
>> 
>> "Standardizing on the OpenMax media libraries and the GStreamer
>framework is 
>> the direction that Linaro is going." (David Rusling, Linaro CTO,
>quoted on 
>> lwn.net)
>> 
>> We need to address this now, otherwise it will be too late.
>
>Absolutely agreed. OpenMAX needs to die as an interface abstraction
>layer.
>
>IIRC, the last time we discussed this in Linaro, the outcome was
>basically
>that we want to have an OpenMAX compatible library on top of V4L, so
>that the
>Linaro members can have a checkmark in their product specs that lists
>them
>as compatible, but we wouldn't do anything hardware specific in there,
>or
>advocate the use of OpenMAX over v4l2 or gstreamer.

That looks to be the right approach. 
OpenMax as an optional  userspace library is fine, but implementing it as a replacement to v4l2 would be a huge mistake.
>
>	Arnd


Cheers,
Mauro 

-- 
Sent from my phone. Please excuse my brevity.

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Sakari Ailus]

Hi Arnd and others,

On Thu, May 26, 2011 at 11:20:41AM +0200, Arnd Bergmann wrote:
> On Thursday 26 May 2011, Laurent Pinchart wrote:
> > On Thursday 26 May 2011 01:50:07 Mauro Carvalho Chehab wrote:
> > > Em 25-05-2011 20:43, Laurent Pinchart escreveu:
> > > > Issues arise when devices have floating point registers. And yes, that
> > > > happens, I've learnt today about an I2C sensor with floating point
> > > > registers (in this specific case it should probably be put in the broken
> > > > design category, but it exists :-)).
> > > 
> > > Huh! Yeah, an I2C sensor with FP registers sound weird. We need more
> > > details in order to address those.
> > 
> > Fortunately for the sensor I'm talking about most of those registers are read-
> > only and contain large values that can be handled as integers, so all we need 
> > to do is convert the 32-bit IEEE float value into an integer. Other hardware 
> > might require more complex FP handling.
> 
> As an additional remark here, most architectures can handle float in the
> kernel in some way, but they all do it differently, so it's basically
> impossible to do in a cross-architecture device driver.

Yeah, I noticed this also. Luckily, usually no other operations are needed
on floating point numbers than converting them to integers.

> > > I'm all about showing the industry in with direction we would like it to
> > > go. We want that all Linux-supported architectures/sub-architectures
> > > support inter-core communications in kernelspace, in a more efficient way
> > > that it would happen if such communication would happen in userspace.
> > 
> > I agree with that. My concern is about things like
> > 
> > "Standardizing on the OpenMax media libraries and the GStreamer framework is 
> > the direction that Linaro is going." (David Rusling, Linaro CTO, quoted on 
> > lwn.net)
> > 
> > We need to address this now, otherwise it will be too late.
> 
> Absolutely agreed. OpenMAX needs to die as an interface abstraction layer.
> 
> IIRC, the last time we discussed this in Linaro, the outcome was basically
> that we want to have an OpenMAX compatible library on top of V4L, so that the
> Linaro members can have a checkmark in their product specs that lists them
> as compatible, but we wouldn't do anything hardware specific in there, or
> advocate the use of OpenMAX over v4l2 or gstreamer.

I strongly favour GStreamer below OpenMAX rather than V4L2. Naturally the
GStreamer source plugins do use V4L2 where applicable.

Much of the high level functionality in cameras that applications are
interested in (for example) is best implemented in GStreamer rather than
V4L2 which is quite low level interface in some cases. While some closed
source components will likely remain, the software stack is still primarily
Open Source software. The closed components are well isolated and
replaceable where they exist; essentially this means individual GStreamer
plugins.

Using GStreamer also would have the benefit that in practice most of the
code using V4L2 would be Open Source as well, not to mention fostering
development as people work on common software components rather than
everyone having their own, as in various OpenMAX implementations.

I wonder if vendors really are looking to provide new designs supporting
OpenMAX while NOT using V4L2, with the possible exception of the OMAP 5.
But, there's a project to develop a V4L2 driver for the OMAP 4 ISS which
hopefully would support OMAP 5 as well. Of course, this direction of
development must be supported where we can.

I think the goal should be that OpenMAX provides no useful functionality at
all. It should be just a legacy interface layer for applications dependent
on it. All the functionality should be implemented in V4L2 drivers and
GStreamer below OpenMAX.

Just my 5 euro cents.

Cheers,

-- 
Sakari Ailus
sakari dot ailus at iki dot fi

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Arnd Bergmann]

On Thursday 26 May 2011, Sakari Ailus wrote:
> I strongly favour GStreamer below OpenMAX rather than V4L2. Naturally the
> GStreamer source plugins do use V4L2 where applicable.

Interesting point, yes. Note that this is probably the opposite of
what David had in mind when talking about GStreamer and OpenMAX,
as I believe we have people working on the gstreamer-on-openmax
plugin, but not on openmax-on-gstreamer.

> Much of the high level functionality in cameras that applications are
> interested in (for example) is best implemented in GStreamer rather than
> V4L2 which is quite low level interface in some cases. While some closed
> source components will likely remain, the software stack is still primarily
> Open Source software. The closed components are well isolated and
> replaceable where they exist; essentially this means individual GStreamer
> plugins.

Right. Also since Linaro is not interested in closed-source components
(the individual members might be, but not Linaro as a group), it's
also good to isolate any closed source code as much as possible and
to make sure everthing else works without it.

> I think the goal should be that OpenMAX provides no useful functionality at
> all. It should be just a legacy interface layer for applications dependent
> on it.

Absolutely.

> All the functionality should be implemented in V4L2 drivers and
> GStreamer below OpenMAX.

Maybe. I'm not sure what the Linaro MM working group plans for this are,
but please bring up your arguments for that with them.

	Arnd

Re: [GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

Hi Mauro,

On Thursday 12 May 2011 17:30:36 Laurent Pinchart wrote:
> 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
> 
> Bob Liu (2):
>       Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
>       uvcvideo: Add support for NOMMU arch
> 
> Hans de Goede (2):
>       v4l: Add M420 format definition

I forgot to add documentation for the new format. Please ignore the pull 
request, I'll resubmit it tomorrow.

>       uvcvideo: Add M420 format support
> 
> 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
> 
>  drivers/media/video/uvc/Makefile     |    3 +
>  drivers/media/video/uvc/uvc_driver.c |   71 ++++++++++++++++++--
>  drivers/media/video/uvc/uvc_entity.c |  118
> ++++++++++++++++++++++++++++++++++
>  drivers/media/video/uvc/uvc_queue.c  |   34 ++++++++++-
>  drivers/media/video/uvc/uvc_v4l2.c   |   17 +++++
>  drivers/media/video/uvc/uvcvideo.h   |   27 ++++++++
>  drivers/media/video/v4l2-dev.c       |   18 +++++
>  drivers/media/video/v4l2-device.c    |    5 +-
>  include/linux/videodev2.h            |    1 +
>  include/media/v4l2-dev.h             |    2 +
>  10 files changed, 287 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/media/video/uvc/uvc_entity.c

-- 
Regards,

Laurent Pinchart

[GIT PATCH FOR 2.6.40] uvcvideo patches

[Laurent Pinchart]

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

Bob Liu (2):
      Revert "V4L/DVB: v4l2-dev: remove get_unmapped_area"
      uvcvideo: Add support for NOMMU arch

Hans de Goede (2):
      v4l: Add M420 format definition
      uvcvideo: Add M420 format support

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

 drivers/media/video/uvc/Makefile     |    3 +
 drivers/media/video/uvc/uvc_driver.c |   71 ++++++++++++++++++--
 drivers/media/video/uvc/uvc_entity.c |  118 
++++++++++++++++++++++++++++++++++
 drivers/media/video/uvc/uvc_queue.c  |   34 ++++++++++-
 drivers/media/video/uvc/uvc_v4l2.c   |   17 +++++
 drivers/media/video/uvc/uvcvideo.h   |   27 ++++++++
 drivers/media/video/v4l2-dev.c       |   18 +++++
 drivers/media/video/v4l2-device.c    |    5 +-
 include/linux/videodev2.h            |    1 +
 include/media/v4l2-dev.h             |    2 +
 10 files changed, 287 insertions(+), 9 deletions(-)
 create mode 100644 drivers/media/video/uvc/uvc_entity.c

-- 
Regards,

Laurent Pinchart