Re: [RFCv1] Media Token API needs - Was: Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-29 Thread Mauro Carvalho Chehab
Em Tue, 28 Oct 2014 22:00:51 -0200
Mauro Carvalho Chehab mche...@osg.samsung.com escreveu:

 Em Tue, 28 Oct 2014 21:42:50 -0200
 Mauro Carvalho Chehab mche...@osg.samsung.com escreveu:
 
  Before starting with the description, this is the simplified diagram of
  a media device (without IR, eeprom and other beasts):
 
 As reference, a more complete diagram would be:

An even more complete (but still simplified) diagram is shown at:
http://linuxtv.org/downloads/presentations/typical_hybrid_hardware.png

The dot lines represent the parts of the graph that are switched by
the tuner, DMA or input select.

Please notice that the DMA engines, together with the stuff needed to
control A/V switches is at one single chip. Changing the registers there
can affect the other streams, specially on most sophisticated devices like
cx231xx, where it even has a power management IP block that validades if
a device to be turned on/off won't exceed the maximum drain current of
500mA. That's basically why we need to do a temporary lock alsa, dvb, v4l
and IR drivers when doing certain changes.

Also, please notice that I2C buses that can be as slow as 10kbps 
are used to control for several devices, like:
- the tuner
- the Digital TV (DTV) demod
- Analog and/or Video demod (sometimes embedded at the main chip)
- DTV demux (sometimes embedded at the main chip)
- The remote controller (sometimes embedded at the main chip)

For some devices, after powered on, or when certain parameters change, a
new firmware (and sometimes a hardware reset) is required. The firmware
size can be about 64KB or even bigger.

Also, the A/V switch it is actually two independent switches (or one
switch for video and one audio mux for audio) that needs to be changed 
together when the source changes.

Regards,
Mauro

For those curious enough or that are using mutt/pine this is the graph,
in text mode, manually adjusted to fit into 80 cols, and with a link
added by hand, as graph-easy failed to represent everything on this
graph:

  +--+
  |IR|
  +--+
|
|
v
  +--+
  |  IR POLL or DMA  |
  +--+
|
|
v
  +--+
  |  devnode input8  |
  +--+

   ..
   ::
   :  +-+---+-+
   :  |  digital|   tuner   | analog  |
   :  +-+---+-+
   ::
   :: DTV IF
   :: off on A/V
   :v
   :  +--+
   :  |DTV demod |
   :  +--+
   :|
   :| MPEG-TS
   :| off on A/V
   :v
   :  +--+
   :  |  demux   |
   :  +--+
   :|
   :| MPEG-TS
   :| off on A/V
   :v
   :  +---+-+-+
   :  |dvb|   DMA   |analog   | +
   :  +---+-+-+  |
   ::  : |
   :: MPEG-TS Video stream :Video stream | 
   :: off on A/Voff on DTV :  off on DTV |
   :v  v |
   :  +--+   ++  |
ATV IF   

Re: [RFCv1] Media Token API needs - Was: Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-29 Thread Shuah Khan
On 10/28/2014 05:42 PM, Mauro Carvalho Chehab wrote:
 Hi Shuah,
 
 I'm understanding that you're collecting comments to write a RFC with the
 needs by the media token, right?
 
 I'm sending you my contributions to such text. See enclosed.
 
 I suggest to change the subject and submit this on a separate thread, after
 we finish the review of such document. Anyway, I'm changing the subject
 of this Thread to reflect that.
 
 Regards,
 Mauro
 
 Em Tue, 28 Oct 2014 15:15:43 -0600
 Shuah Khan shua...@osg.samsung.com escreveu:
 
 On 10/27/2014 06:52 AM, Mauro Carvalho Chehab wrote:
 Em Sun, 26 Oct 2014 09:27:40 +0100
 Takashi Iwai ti...@suse.de escreveu:



 Hmm... this is actually more complex than that. V4L2 driver doesn't
 know if ALSA is streaming or not, or even if ALSA device node is opened
 while he is touching at the hardware configuration or changing the
 state. I mean: it is not an error to set the hardware. The error only
 happens if ALSA and V4L2 tries to do it at the same time on an incompatible
 way.

 Also, this won't work for DVB, as on DVB this is really an exclusive
 lock that would prevent both ALSA and V4L2 drivers to stream while in
 DVB mode.

 Implementing it with a lock seems to be the best approach, at least on
 my eyes.

 That said, we should go back and start discussing the design goal at
 first.

 Surely.

 This is long, however, hoping it will describe the problem and
 solution that is being pursued in detail:
 
 Before starting with the description, this is the simplified diagram of
 a media device (without IR, eeprom and other beasts):
 
   
 +-+
   |   
   |
   |   ++ 
 +--+---++  |
   |   |  demod_video   | -- |  analog  | tuner | 
 digital|  |
   |   ++ 
 +--+---++  |
   | |  |   |  
   |
   | |  |   |  
   |
   v v  v   v  
   |
 +--+-+-+ +--+
 +---+  |
 | dvb  | DMA |  analog | |   demod_audio|| 
 digital_demux | -+
 +--+-+-+ +--+
 +---+
   | |  |
   | |  |
   v v  v
 +--+  ++ +--+
 | devnode dvr0 |  | devnode video0 | |audio DMA |
 +--+  ++ +--+
|
|
v
  +--+
  | devnode pcmC1D0c |
  +--+
 
 There are two components that are shared there between analog and digital:
 the tuner (where the signal is captured) and the DMA engine used to stream
 analog and Digital TV (dvb).
 
 PS.: the diagram is over-simplified, as the tuner is just one of the possible
 inputs for the analog part of the device. Other possible inputs are S-Video,
 composite, HDMI, etc.
 
 Sometimes, the audio DMA is also shared, e. g. just one stream comes from
 the hardware. It is up to the driver to split audio and video and send
 them to the V4L2 and ALSA APIs. This is the case of tm6000 driver.
 
 Those shared components can be used either at analog or digital mode,
 but not at the same time.
 
 Also, programming the V4L2 analog and audio DMA and demods should be done
 via V4L2 API, as this API allows the selection of the proper audio/video
 input (almost all devices have multiple analog inputs).
 
 Please notice that, if the tuner is on digital mode, the entire analog
 path is disabled, including ALSA output.
 
 If the tuner is on analog mode, both ALSA and V4L2 can work at the
 same time. However, during the period where the tuner firmware is
 loaded, and during the DMA configuration and input selection time,
 neither ALSA or V4L2 can stream. Such configuration/firmware load
 is commanded via V4L2 API, as ALSA knows nothing about tuner or
 input selection.
 

 At a higher level the problem description is:

 There are 3 different device files that get created to control
 tuner and audio functions on a media device. 3 drivers (dvb,
 v4l2, alsa), and 3 core apis (dvb-core, v4l-core, audio) that
 control the tuner and audio hardware and 

Re: [RFCv1] Media Token API needs - Was: Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-29 Thread Mauro Carvalho Chehab
Em Wed, 29 Oct 2014 10:06:51 -0600
Shuah Khan shua...@osg.samsung.com escreveu:

 On 10/28/2014 05:42 PM, Mauro Carvalho Chehab wrote:
  Hi Shuah,
  
  I'm understanding that you're collecting comments to write a RFC with the
  needs by the media token, right?
  
  I'm sending you my contributions to such text. See enclosed.
  
  I suggest to change the subject and submit this on a separate thread, after
  we finish the review of such document. Anyway, I'm changing the subject
  of this Thread to reflect that.
  
  Regards,
  Mauro
  
  Em Tue, 28 Oct 2014 15:15:43 -0600
  Shuah Khan shua...@osg.samsung.com escreveu:
  
  On 10/27/2014 06:52 AM, Mauro Carvalho Chehab wrote:
  Em Sun, 26 Oct 2014 09:27:40 +0100
  Takashi Iwai ti...@suse.de escreveu:
 
 
 
  Hmm... this is actually more complex than that. V4L2 driver doesn't
  know if ALSA is streaming or not, or even if ALSA device node is opened
  while he is touching at the hardware configuration or changing the
  state. I mean: it is not an error to set the hardware. The error only
  happens if ALSA and V4L2 tries to do it at the same time on an 
  incompatible
  way.
 
  Also, this won't work for DVB, as on DVB this is really an exclusive
  lock that would prevent both ALSA and V4L2 drivers to stream while in
  DVB mode.
 
  Implementing it with a lock seems to be the best approach, at least on
  my eyes.
 
  That said, we should go back and start discussing the design goal at
  first.
 
  Surely.
 
  This is long, however, hoping it will describe the problem and
  solution that is being pursued in detail:
  
  Before starting with the description, this is the simplified diagram of
  a media device (without IR, eeprom and other beasts):
  

  +-+
| 
  |
|   ++ 
  +--+---++  |
|   |  demod_video   | -- |  analog  | tuner |   
digital|  |
|   ++ 
  +--+---++  |
| |  |   
  ||
| |  |   
  ||
v v  v   
  v|
  +--+-+-+ +--+
  +---+  |
  | dvb  | DMA |  analog | |   demod_audio|| 
  digital_demux | -+
  +--+-+-+ +--+
  +---+
| |  |
| |  |
v v  v
  +--+  ++ +--+
  | devnode dvr0 |  | devnode video0 | |audio DMA |
  +--+  ++ +--+
 |
 |
 v
   +--+
   | devnode pcmC1D0c |
   +--+
  
  There are two components that are shared there between analog and digital:
  the tuner (where the signal is captured) and the DMA engine used to stream
  analog and Digital TV (dvb).
  
  PS.: the diagram is over-simplified, as the tuner is just one of the 
  possible
  inputs for the analog part of the device. Other possible inputs are S-Video,
  composite, HDMI, etc.
  
  Sometimes, the audio DMA is also shared, e. g. just one stream comes from
  the hardware. It is up to the driver to split audio and video and send
  them to the V4L2 and ALSA APIs. This is the case of tm6000 driver.
  
  Those shared components can be used either at analog or digital mode,
  but not at the same time.
  
  Also, programming the V4L2 analog and audio DMA and demods should be done
  via V4L2 API, as this API allows the selection of the proper audio/video
  input (almost all devices have multiple analog inputs).
  
  Please notice that, if the tuner is on digital mode, the entire analog
  path is disabled, including ALSA output.
  
  If the tuner is on analog mode, both ALSA and V4L2 can work at the
  same time. However, during the period where the tuner firmware is
  loaded, and during the DMA configuration and input selection time,
  neither ALSA or V4L2 can stream. Such configuration/firmware load
  is commanded via V4L2 API, as ALSA knows nothing about tuner or
  input selection.
  
 
  At a higher level the problem description is:
 
  There are 3 different 

Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-28 Thread Shuah Khan
On 10/27/2014 06:52 AM, Mauro Carvalho Chehab wrote:
 Em Sun, 26 Oct 2014 09:27:40 +0100
 Takashi Iwai ti...@suse.de escreveu:
 

 
 Hmm... this is actually more complex than that. V4L2 driver doesn't
 know if ALSA is streaming or not, or even if ALSA device node is opened
 while he is touching at the hardware configuration or changing the
 state. I mean: it is not an error to set the hardware. The error only
 happens if ALSA and V4L2 tries to do it at the same time on an incompatible
 way.
 
 Also, this won't work for DVB, as on DVB this is really an exclusive
 lock that would prevent both ALSA and V4L2 drivers to stream while in
 DVB mode.
 
 Implementing it with a lock seems to be the best approach, at least on
 my eyes.
 
 That said, we should go back and start discussing the design goal at
 first.
 
 Surely.

This is long, however, hoping it will describe the problem and
solution that is being pursued in detail:

At a higher level the problem description is:

There are 3 different device files that get created to control
tuner and audio functions on a media device. 3 drivers (dvb,
v4l2, alsa), and 3 core apis (dvb-core, v4l-core, audio) that
control the tuner and audio hardware and provide user api to
these 3 device files.

User applications, drivers and the core have no knowledge of each
other. The only thing that is common across all these drivers is
the parent device for the main usb device which is controlled by
the main usb driver.

The premise for the main design idea in this series is creating
a common construct at the parent device structure that is visible
to all drivers to act as a master access control (lock). Let's call
this media token object with two sub-tokens one for tuner and another
for audio.

Each of the apis evolved separately, hence have their own backwards
compatibility to maintain. Starting with v4l2:

v4l2 case:
Multiple v4l2 applications are allowed to open /dev/video0 in
read/write mode with no restrictions as long as the tuner is in
analog mode. v4l2 core handles conflicting requests between v4l2
applications. It doesn't have the knowledge that the tuner is in
use by a dvb and/or audio is in use. As soon as a v4l2 application
starts, digital stream glitches and audio glitches.

dvb case:
Multiple dvb applications can open the dvb device in read only mode.
As soon an application open the device read/write mode a separate
kthread is kicked off to handle the request. Only one application
can open the device in read/write mode. Similar to v4l2 case,
dvb-core doesn't have any knowledge that the tuner is in use by
v4l2 and/or audio is in use. As soon as a dvb application starts v4l2
video glitches and audio glitches.

audio case:
Same scenario is applicable to audio application. When a v4l2 or dvb
application starts, audio application gets impacted.

Problems to address:

dvb owns tuner and audio: another dvb, v4l2 app and audio app should
  detect tuner/audio busy right away and exit.

v4l2 owns tuner and audio: another dvb and audio app should detect
   tuner/audio busy right away and exit.
   v4l2 app can continue to use it until it
   tries to change the tuner/audio state.


audio owns audio: dvb and v4l2 apps should detect audio busy and exit.

Special cases:

dvb apps. access tuner and audio in exclusive mode. i.e only one dvb app.
at a time is allowed to open the device read/write mode. As dvb apps.
create threads to handle audio and video, all threads in that group
should be allowed by the higher level construct to access the tuner and
audio. dvb application will have to hold tuner and audio tokens so v4l2
and audio apps. know they are in use.

audio apps. access audio in exclusive mode. i.e only one audio app. at
a time is allowed to open the device in read/write mode. Audio apps.
create threads and thread closes and re-opens the audio device. Threads
can do this and hence something that higher level construct has to allow.
audio app. has to hold audio token so dvb and v4l2 know that it is in use.
(Note: I am not sure if I have the audio scenario right)

v4l2 apps. access tuner and audio in shared v4l2 mode. i.e several v4l2
processes and threads could use tuner and audio at the same time. The
higher level construct has to allow multiple v4l2 apps. to access and
disallow dvb and audio apps. access when they are in use by v4l2.

Adding to this, both dvb and v4l2 open audio device and make snd pcm
capture callbacks. There is no way to tell if dvb or v4l2 or audio
app is the one that is making this request. dvb app would like audio
in exclusive mode allowing only one process and its threads to access
it. v4l2 on the other hand would like audio in shared state accessible
to all v4l2 processes. If dvb-core and v4l2-core get tuner and audio
tokens at the same time, the window for having tuner token and not
getting audio token go down.

In dvb case when dvb device is opened in 

[RFCv1] Media Token API needs - Was: Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-28 Thread Mauro Carvalho Chehab
Hi Shuah,

I'm understanding that you're collecting comments to write a RFC with the
needs by the media token, right?

I'm sending you my contributions to such text. See enclosed.

I suggest to change the subject and submit this on a separate thread, after
we finish the review of such document. Anyway, I'm changing the subject
of this Thread to reflect that.

Regards,
Mauro

Em Tue, 28 Oct 2014 15:15:43 -0600
Shuah Khan shua...@osg.samsung.com escreveu:

 On 10/27/2014 06:52 AM, Mauro Carvalho Chehab wrote:
  Em Sun, 26 Oct 2014 09:27:40 +0100
  Takashi Iwai ti...@suse.de escreveu:
  
 
  
  Hmm... this is actually more complex than that. V4L2 driver doesn't
  know if ALSA is streaming or not, or even if ALSA device node is opened
  while he is touching at the hardware configuration or changing the
  state. I mean: it is not an error to set the hardware. The error only
  happens if ALSA and V4L2 tries to do it at the same time on an incompatible
  way.
  
  Also, this won't work for DVB, as on DVB this is really an exclusive
  lock that would prevent both ALSA and V4L2 drivers to stream while in
  DVB mode.
  
  Implementing it with a lock seems to be the best approach, at least on
  my eyes.
  
  That said, we should go back and start discussing the design goal at
  first.
  
  Surely.
 
 This is long, however, hoping it will describe the problem and
 solution that is being pursued in detail:

Before starting with the description, this is the simplified diagram of
a media device (without IR, eeprom and other beasts):

  
+-+
  | 
|
  |   ++ 
+--+---++  |
  |   |  demod_video   | -- |  analog  | tuner | 
digital|  |
  |   ++ 
+--+---++  |
  | |  |   |
|
  | |  |   |
|
  v v  v   v
|
+--+-+-+ +--+
+---+  |
| dvb  | DMA |  analog | |   demod_audio|| 
digital_demux | -+
+--+-+-+ +--+
+---+
  | |  |
  | |  |
  v v  v
+--+  ++ +--+
| devnode dvr0 |  | devnode video0 | |audio DMA |
+--+  ++ +--+
   |
   |
   v
 +--+
 | devnode pcmC1D0c |
 +--+

There are two components that are shared there between analog and digital:
the tuner (where the signal is captured) and the DMA engine used to stream
analog and Digital TV (dvb).

PS.: the diagram is over-simplified, as the tuner is just one of the possible
inputs for the analog part of the device. Other possible inputs are S-Video,
composite, HDMI, etc.

Sometimes, the audio DMA is also shared, e. g. just one stream comes from
the hardware. It is up to the driver to split audio and video and send
them to the V4L2 and ALSA APIs. This is the case of tm6000 driver.

Those shared components can be used either at analog or digital mode,
but not at the same time.

Also, programming the V4L2 analog and audio DMA and demods should be done
via V4L2 API, as this API allows the selection of the proper audio/video
input (almost all devices have multiple analog inputs).

Please notice that, if the tuner is on digital mode, the entire analog
path is disabled, including ALSA output.

If the tuner is on analog mode, both ALSA and V4L2 can work at the
same time. However, during the period where the tuner firmware is
loaded, and during the DMA configuration and input selection time,
neither ALSA or V4L2 can stream. Such configuration/firmware load
is commanded via V4L2 API, as ALSA knows nothing about tuner or
input selection.

 
 At a higher level the problem description is:
 
 There are 3 different device files that get created to control
 tuner and audio functions on a media device. 3 drivers (dvb,
 v4l2, alsa), and 3 core apis (dvb-core, v4l-core, audio) that
 control the tuner and audio hardware and provide user api to
 these 3 device files.


There's actually a 4th component for some drivers: the mceusb 

Re: [RFCv1] Media Token API needs - Was: Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-28 Thread Mauro Carvalho Chehab
Em Tue, 28 Oct 2014 21:42:50 -0200
Mauro Carvalho Chehab mche...@osg.samsung.com escreveu:

 Before starting with the description, this is the simplified diagram of
 a media device (without IR, eeprom and other beasts):

As reference, a more complete diagram would be:




+--+
|IR 
   |

+--+
  |
  |
  v

+--+
|  IR POLL or 
DMA  |

+--+
  |
  |
  v

+--+
|  devnode 
input8  |

+--+

 ++
 v|
   ++  
+---+---+---+
   |  demod_video   |  +- | tuner | A/V switch 
   | composite |
   ++  |   
+---+---+---+
 | |  |
 | |  |
 v |  v
 +--+-+-+  |
+--+
  +- | dvb  | DMA |  analog |  ||   demod_audio 
   |
  |  +--+-+-+  |
+--+
  |^ | |  |
  |+---+ | |  |
  || v |  v
  |  +--+  |   ++  |
+--+
  + | devnode dvr0 |  |   | devnode video0 |  ||audio DMA  
   |
 +--+  |   ++  |
+--+
   |   |  |
   |   |  |
   |   |  v
   |   |
+--+
   |   || devnode 
pcmC1D0c |
   |   |
+--+
   |   |
   |   |
   |   |
   | 
+++---+
   | | analog |  tuner |  digital   
   |
   | 
+++---+
   |  |
   |  |
   |  v
   |
+--+
   +--- |  
digital_demux   |

+--+

Regards,
Mauro

-

Dot file for the above diagram:

digraph media_hardware {
  node [shape=record]
  DMA1[label = dvb dvb | dma DMA | video analog]
  DMA2[label = audio DMA]
  DMA3[label = IR POLL or DMA]
  tuner[label = analog analog | tuner tuner | digital digital]
  input[label = tuner tuner | switch A/V switch | composite composite]

  DMA1:video - devnode video0
  DMA1:dvb - devnode dvr0

  digital_demux - DMA1:dvb
  demod_video - DMA1:video

  tuner:digital - digital_demux
  tuner:analog - input:tuner

  input:switch - demod_video
  input:switch - demod_audio

  demod_audio - DMA2
  DMA2 - devnode pcmC1D0c

  IR - DMA3
  DMA3 - devnode input8
}
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-27 Thread Mauro Carvalho Chehab
Em Sun, 26 Oct 2014 09:27:40 +0100
Takashi Iwai ti...@suse.de escreveu:

 At Sat, 25 Oct 2014 11:41:15 -0200,
 Mauro Carvalho Chehab wrote:

   +---+
   | start |
   +---+
 |
 |
 v
   ++
   |  idle  | +
   ++  |
 |^   ||
 ||   ||
 v|   ||
   +---+  |   ||
   | configuration |  |   ||
   +---+  |   ||
 ||   ||
 ||   ||
 v|   ||
   +---+  |   ||
+ |   streaming   | -+   ||
|  +---+  ||
||||
||||
|vv|
|  +---+---++  |
+- |   1   | suspended |  2 | -+
   +---+---++


The above diagram is actually simplified. There's an extra state
there that should be mentioned:

idle - DVB stream

(actually, DVB stream is a copy of the above, except that ALSA
won't have any business to do while the device is on DVB mode)

 So, judging from your description, the problem isn't about the
 exclusive lock, but rather implementation a kind of master/slave
 devices?  Then the proposed patch doesn't look like a correct
 implementation to me.

You might see it as a master/slave, except that it is not that simple.
It is really a hardware lock.

There are actually 3 (sub)drivers that may need to set the hardware:
- ALSA driver;
- V4L2 driver;
- DVB driver.

The goal of the lock is to prevent that more than one driver would try 
to use a common piece of the hardware that it is already in usage
by another driver.

So, for example, the ALSA driver should not reprogram the hardware while 
the V4L2 driver is doing that.

I agree that, for V4L2/ALSA driver's interaction PoV, it could resemble
a sort of master/slave control, in the sense that ALSA capture start only
makes sense while V4L2 is at streaming state. Yet, just opening the device
or (even start capture on ALSA, for the hardware with multiple DMA engines,
like the ones that use snd-usb-audio) won't cause any harm if the V4L2 driver
is not reconfiguring the audio registers at the same time.

Also, ALSA open/close should be supported any time, as otherwise it will 
break existing applications. 

However, when the device is streaming on DVB mode, it is not possible
to stream on V4L2 mode, as there's just one DMA for both and just one
tuner. Also, ALSA capture doesn't make much sense on such case. Still,
locking on open/close may eventually break existing applications. Also,
it doesn't really make any sense to block the device to move from analog
to digital mode just because the ALSA devnodes are opened.

 What I (and supposedly Pierre) opposed is the implementation of
 exclusive lock control in spontaneous callbacks.  Especially the
 trigger callback is a bad place since it's a callback that is supposed
 to just trigger atomically.  In general, the only good place for
 allowing user-space to *control* the exclusive lock is open/close,
 unless the finer lock control is exposed.

I see your point. Then perhaps we should expose callbacks from other
parts of the ALSA core, perhaps at the logic that calls the trigger
callback at read and poll syscalls, plus the corresponding logic that is
used when the device is using the mmap syscall.

 But, reading through the argument from you guys, the intention of the
 patch seems like just to raise the conflict from the hardware level to
 the software level. 

Yes.

 If so, I doubt whether such an exclusive lock is
 the best way.   For example, audio stream can simply receive an error
 at any time if something is wrong and reacts accordingly instead of
 keeping the lock while streaming. Then the master side (video) can
 set the error flag, let the audio stream stop in the driver (if
 running), and sync with it.

Hmm... this is actually more complex than that. V4L2 driver doesn't
know if ALSA is streaming or not, or even if ALSA device node is opened
while he is touching at the hardware configuration or changing the
state. I mean: it is not an error to set the hardware. The error only
happens if ALSA and V4L2 tries to do it at the same time on an incompatible
way.

Also, this won't work for DVB, as on DVB this is really an exclusive
lock that would prevent both ALSA and V4L2 drivers to stream while in
DVB mode.

Implementing it with a lock seems to be the best approach, at least on
my eyes.

 That said, we should go back and start discussing the design goal at
 first.

Surely.

 
 
 thanks,
 
 Takashi
 --
 To unsubscribe 

Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-27 Thread Shuah Khan
On 10/25/2014 04:57 AM, Mauro Carvalho Chehab wrote:
 Hi,
 
 Sorry to enter into this thread so late. Last week was a full week, due to the
 4 conferences I paticiapated, and last week I needed to fill lots of trip
 reports. Also, I have another trip to give two speeches.
 
 Em Tue, 21 Oct 2014 11:32:10 -0600
 Shuah Khan shua...@osg.samsung.com escreveu:
 
 Here is what I propose for patch v3:
 - make it a module under media
 - collapse tuner and audio tokens into media token
 
 I'm a little skeptical about this. Merging tuner and audio tokens seems
 weird on my eyes, as there are actually two different hardware resources
 we need to lock, and we may be locking them on different places.

I think the suggestion for collapsing came about because, in this
patch series, the dvb and v4l use-cases are such that tuner and
audio need to be held together. i.e when tuner is held, audio is
held as well. With an intent to simplify the usage, I decided to
have get_tuner interface to return audio token. It does simplify
the logic for callers - all the paces that get tuner will also
need to make a call to get audio and handle errors when audio is
not available. That made it even more confusing perhaps and raised
the question that why can't we collapse.

Having two tokens will allow the ability hold them independently.
Maybe the right approach for patch v3 is don't collapse, but
change dvb and v4l to get tuner followed by audio and handle the
errors paths themselves.

 
 - change names (get rid of abbreviated tkn stuff)
 - Make other changes Takashi/Lars pointed out in pcm
 - hold token in pcm open/close
 

-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-26 Thread Takashi Iwai
At Sat, 25 Oct 2014 11:41:15 -0200,
Mauro Carvalho Chehab wrote:
 
 (re-sending from my third e-mail - somehow, the two emails I have at
  Samsung didn't seem to be delivering to vger.kernel.org today)
 
 Em Wed, 22 Oct 2014 14:26:41 -0500
 Pierre-Louis Bossart pierre-louis.boss...@linux.intel.com escreveu:
 
  On 10/21/14, 11:08 AM, Devin Heitmueller wrote:
   Sorry, I'm not convinced by that.  If the device has to be controlled
   exclusively, the right position is the open/close.  Otherwise, the
   program cannot know when it becomes inaccessible out of sudden during
   its operation.
  
   I can say that I've definitely seen cases where if you configure a
   device as the default capture device in PulseAudio, then pulse will
   continue to capture from it even if you're not actively capturing the
   audio from pulse.  I only spotted this because I had a USB analyzer on
   the device and was dumbfounded when the ISOC packets kept arriving
   even after I had closed VLC.
  
  this seems like a feature, not a bug. PulseAudio starts streaming before 
  clients push any data and likewise keeps sources active even after for 
  some time after clients stop recording. Closing VLC in your example 
  doesn't immediately close the ALSA device. look for 
  module-suspend-on-idle in your default.pa config file.
 
 This could be a feature for an audio capture device that is standalone,
 but for sure it isn't a feature for an audio capture device where the
 audio is only available if video is also being streamed.
 
 A V4L device with ALSA capture is a different beast than a standalone
 capture port. In a simplified way, it will basically follow the following
 state machine:
 
  +---+
  | start |
  +---+
|
|
v
  ++
  |  idle  | +
  ++  |
|^   ||
||   ||
v|   ||
  +---+  |   ||
  | configuration |  |   ||
  +---+  |   ||
||   ||
||   ||
v|   ||
  +---+  |   ||
   + |   streaming   | -+   ||
   |  +---+  ||
   ||||
   ||||
   |vv|
   |  +---+---++  |
   +- |   1   | suspended |  2 | -+
  +---+---++
 
 
 1) start state
 
 This is when the V4L2 device gots probed. It checks if the hardware is
 present and initializes some vars/registers, turning off everything
 that can be powered down.
 
 The tuner on put in sleep mode, analog audio/video decoders and the
 dvb frontend and demux are also turned off.
 
 2) idle state
 
 As the device is powered off, audio won't produce anything. 
 
 Depending on the device, reading for audio may return a sequence of 
 zeros, or may even fail, as the DMA engine is not ready yet for
 streaming.
 
 Also, the audio mixer is muted, but the audio input switch is on a
 random state.
 
 2) configuration state
 
 When V4L2 node device is opened and configured, the audio mixer will
 be switched to input audio from the same source of the video stream.
 The corresponding audio input is also unmuted. Almost all devices have 
 at least two audio/video inputs: TV TUNER and COMPOSITE. Other devices
 may also have S-VIDEO, COMPOSITE 2, RADIO TUNER, etc.
 
 If the device is set on TUNER mode, on modern devices, a tuner firmware
 will be loaded. That may require a long time. Typically, most devices
 take 1/2 seconds to load a firmware, but some devices may take up to 30
 seconds. The firmware may depend on the TV standard that will be used,
 so this can't be loaded at driver warm up state. 
 
 Also, the power consumption of the tuners is high (it can be ~100-200 mW
 or more when powered, and ~16mW when just I2C is powered). We don't want
 to keep it powered when the device is not used, as this spends battery.
 Also, the life of the device reduces a lot if we keep it always powered.
 
 During this stage, if an ALSA call is issued, it may interfere at the
 device settings and/or firmware load, with can cause the audio to fail. 
 On such cases, applications might need to close the V4L2 node and re-open
 again.
 
 3) streaming state
 
 The change to this staging requires a V4L2 ioctl.
 
 Please notice, however, that some apps will open the audio device before
 the V4L2 node, while others will open it after that.
 
 In any case, audio will only start to produce data after the V4L2 ioctl
 at V4L2 that starts the DMA engine there.
 
 After that ioctl:
  - Audio PCM capture will work;
  - The mixers will be in a good state: unmuted, 

Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-25 Thread Mauro Carvalho Chehab
(re-sending from my third e-mail - somehow, the two emails I have at
 Samsung didn't seem to be delivering to vger.kernel.org today)

Em Wed, 22 Oct 2014 14:26:41 -0500
Pierre-Louis Bossart pierre-louis.boss...@linux.intel.com escreveu:

 On 10/21/14, 11:08 AM, Devin Heitmueller wrote:
  Sorry, I'm not convinced by that.  If the device has to be controlled
  exclusively, the right position is the open/close.  Otherwise, the
  program cannot know when it becomes inaccessible out of sudden during
  its operation.
 
  I can say that I've definitely seen cases where if you configure a
  device as the default capture device in PulseAudio, then pulse will
  continue to capture from it even if you're not actively capturing the
  audio from pulse.  I only spotted this because I had a USB analyzer on
  the device and was dumbfounded when the ISOC packets kept arriving
  even after I had closed VLC.
 
 this seems like a feature, not a bug. PulseAudio starts streaming before 
 clients push any data and likewise keeps sources active even after for 
 some time after clients stop recording. Closing VLC in your example 
 doesn't immediately close the ALSA device. look for 
 module-suspend-on-idle in your default.pa config file.

This could be a feature for an audio capture device that is standalone,
but for sure it isn't a feature for an audio capture device where the
audio is only available if video is also being streamed.

A V4L device with ALSA capture is a different beast than a standalone
capture port. In a simplified way, it will basically follow the following
state machine:

 +---+
 | start |
 +---+
   |
   |
   v
 ++
 |  idle  | +
 ++  |
   |^   ||
   ||   ||
   v|   ||
 +---+  |   ||
 | configuration |  |   ||
 +---+  |   ||
   ||   ||
   ||   ||
   v|   ||
 +---+  |   ||
  + |   streaming   | -+   ||
  |  +---+  ||
  ||||
  ||||
  |vv|
  |  +---+---++  |
  +- |   1   | suspended |  2 | -+
 +---+---++


1) start state

This is when the V4L2 device gots probed. It checks if the hardware is
present and initializes some vars/registers, turning off everything
that can be powered down.

The tuner on put in sleep mode, analog audio/video decoders and the
dvb frontend and demux are also turned off.

2) idle state

As the device is powered off, audio won't produce anything. 

Depending on the device, reading for audio may return a sequence of 
zeros, or may even fail, as the DMA engine is not ready yet for
streaming.

Also, the audio mixer is muted, but the audio input switch is on a
random state.

2) configuration state

When V4L2 node device is opened and configured, the audio mixer will
be switched to input audio from the same source of the video stream.
The corresponding audio input is also unmuted. Almost all devices have 
at least two audio/video inputs: TV TUNER and COMPOSITE. Other devices
may also have S-VIDEO, COMPOSITE 2, RADIO TUNER, etc.

If the device is set on TUNER mode, on modern devices, a tuner firmware
will be loaded. That may require a long time. Typically, most devices
take 1/2 seconds to load a firmware, but some devices may take up to 30
seconds. The firmware may depend on the TV standard that will be used,
so this can't be loaded at driver warm up state. 

Also, the power consumption of the tuners is high (it can be ~100-200 mW
or more when powered, and ~16mW when just I2C is powered). We don't want
to keep it powered when the device is not used, as this spends battery.
Also, the life of the device reduces a lot if we keep it always powered.

During this stage, if an ALSA call is issued, it may interfere at the
device settings and/or firmware load, with can cause the audio to fail. 
On such cases, applications might need to close the V4L2 node and re-open
again.

3) streaming state

The change to this staging requires a V4L2 ioctl.

Please notice, however, that some apps will open the audio device before
the V4L2 node, while others will open it after that.

In any case, audio will only start to produce data after the V4L2 ioctl
at V4L2 that starts the DMA engine there.

After that ioctl:
 - Audio PCM capture will work;
 - The mixers will be in a good state: unmuted, and switched to the
   corresponding input as the video stream.

If the user wants to do something unusual, like mixing the composite audio
input with the tuner audio input, it can use the 

Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-25 Thread Mauro Carvalho Chehab
(re-sending from my other e-mail - somehow, the email I sent via
 m.che...@samsung.com doesn't seem to be delivered at vger.kernel.org)

Em Wed, 22 Oct 2014 14:26:41 -0500
Pierre-Louis Bossart pierre-louis.boss...@linux.intel.com escreveu:

 On 10/21/14, 11:08 AM, Devin Heitmueller wrote:
  Sorry, I'm not convinced by that.  If the device has to be controlled
  exclusively, the right position is the open/close.  Otherwise, the
  program cannot know when it becomes inaccessible out of sudden during
  its operation.
 
  I can say that I've definitely seen cases where if you configure a
  device as the default capture device in PulseAudio, then pulse will
  continue to capture from it even if you're not actively capturing the
  audio from pulse.  I only spotted this because I had a USB analyzer on
  the device and was dumbfounded when the ISOC packets kept arriving
  even after I had closed VLC.
 
 this seems like a feature, not a bug. PulseAudio starts streaming before 
 clients push any data and likewise keeps sources active even after for 
 some time after clients stop recording. Closing VLC in your example 
 doesn't immediately close the ALSA device. look for 
 module-suspend-on-idle in your default.pa config file.

This could be a feature for an audio capture device that is standalone,
but for sure it isn't a feature for an audio capture device where the
audio is only available if video is also being streamed.

A V4L device with ALSA capture is a different beast than a standalone
capture port. In a simplified way, it will basically follow the following
state machine:

 +---+
 | start |
 +---+
   |
   |
   v
 ++
 |  idle  | +
 ++  |
   |^   ||
   ||   ||
   v|   ||
 +---+  |   ||
 | configuration |  |   ||
 +---+  |   ||
   ||   ||
   ||   ||
   v|   ||
 +---+  |   ||
  + |   streaming   | -+   ||
  |  +---+  ||
  ||||
  ||||
  |vv|
  |  +---+---++  |
  +- |   1   | suspended |  2 | -+
 +---+---++


1) start state

This is when the V4L2 device gots probed. It checks if the hardware is
present and initializes some vars/registers, turning off everything
that can be powered down.

The tuner on put in sleep mode, analog audio/video decoders and the
dvb frontend and demux are also turned off.

2) idle state

As the device is powered off, audio won't produce anything. 

Depending on the device, reading for audio may return a sequence of 
zeros, or may even fail, as the DMA engine is not ready yet for
streaming.

Also, the audio mixer is muted, but the audio input switch is on a
random state.

2) configuration state

When V4L2 node device is opened and configured, the audio mixer will
be switched to input audio from the same source of the video stream.
The corresponding audio input is also unmuted. Almost all devices have 
at least two audio/video inputs: TV TUNER and COMPOSITE. Other devices
may also have S-VIDEO, COMPOSITE 2, RADIO TUNER, etc.

If the device is set on TUNER mode, on modern devices, a tuner firmware
will be loaded. That may require a long time. Typically, most devices
take 1/2 seconds to load a firmware, but some devices may take up to 30
seconds. The firmware may depend on the TV standard that will be used,
so this can't be loaded at driver warm up state. 

Also, the power consumption of the tuners is high (it can be ~100-200 mW
or more when powered, and ~16mW when just I2C is powered). We don't want
to keep it powered when the device is not used, as this spends battery.
Also, the life of the device reduces a lot if we keep it always powered.

During this stage, if an ALSA call is issued, it may interfere at the
device settings and/or firmware load, with can cause the audio to fail. 
On such cases, applications might need to close the V4L2 node and re-open
again.

3) streaming state

The change to this staging requires a V4L2 ioctl.

Please notice, however, that some apps will open the audio device before
the V4L2 node, while others will open it after that.

In any case, audio will only start to produce data after the V4L2 ioctl
at V4L2 that starts the DMA engine there.

After that ioctl:
 - Audio PCM capture will work;
 - The mixers will be in a good state: unmuted, and switched to the
   corresponding input as the video stream.

If the user wants to do something unusual, like mixing the composite audio
input with the tuner audio input, it can use 

Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-25 Thread Mauro Carvalho Chehab
Em Wed, 22 Oct 2014 14:26:41 -0500
Pierre-Louis Bossart pierre-louis.boss...@linux.intel.com escreveu:

 On 10/21/14, 11:08 AM, Devin Heitmueller wrote:
  Sorry, I'm not convinced by that.  If the device has to be controlled
  exclusively, the right position is the open/close.  Otherwise, the
  program cannot know when it becomes inaccessible out of sudden during
  its operation.
 
  I can say that I've definitely seen cases where if you configure a
  device as the default capture device in PulseAudio, then pulse will
  continue to capture from it even if you're not actively capturing the
  audio from pulse.  I only spotted this because I had a USB analyzer on
  the device and was dumbfounded when the ISOC packets kept arriving
  even after I had closed VLC.
 
 this seems like a feature, not a bug. PulseAudio starts streaming before 
 clients push any data and likewise keeps sources active even after for 
 some time after clients stop recording. Closing VLC in your example 
 doesn't immediately close the ALSA device. look for 
 module-suspend-on-idle in your default.pa config file.

This could be a feature for an audio capture device that is standalone,
but for sure it isn't a feature for an audio capture device where the
audio is only available if video is also being streamed.

A V4L device with ALSA capture is a different beast than a standalone
capture port. In a simplified way, it will basically follow the following
state machine:

 +---+
 | start |
 +---+
   |
   |
   v
 ++
 |  idle  | +
 ++  |
   |^   ||
   ||   ||
   v|   ||
 +---+  |   ||
 | configuration |  |   ||
 +---+  |   ||
   ||   ||
   ||   ||
   v|   ||
 +---+  |   ||
  + |   streaming   | -+   ||
  |  +---+  ||
  ||||
  ||||
  |vv|
  |  +---+---++  |
  +- |   1   | suspended |  2 | -+
 +---+---++


1) start state

This is when the V4L2 device gots probed. It checks if the hardware is
present and initializes some vars/registers, turning off everything
that can be powered down.

The tuner on put in sleep mode, analog audio/video decoders and the
dvb frontend and demux are also turned off.

2) idle state

As the device is powered off, audio won't produce anything. 

Depending on the device, reading for audio may return a sequence of 
zeros, or may even fail, as the DMA engine is not ready yet for
streaming.

Also, the audio mixer is muted, but the audio input switch is on a
random state.

2) configuration state

When V4L2 node device is opened and configured, the audio mixer will
be switched to input audio from the same source of the video stream.
The corresponding audio input is also unmuted. Almost all devices have 
at least two audio/video inputs: TV TUNER and COMPOSITE. Other devices
may also have S-VIDEO, COMPOSITE 2, RADIO TUNER, etc.

If the device is set on TUNER mode, on modern devices, a tuner firmware
will be loaded. That may require a long time. Typically, most devices
take 1/2 seconds to load a firmware, but some devices may take up to 30
seconds. The firmware may depend on the TV standard that will be used,
so this can't be loaded at driver warm up state. 

Also, the power consumption of the tuners is high (it can be ~100-200 mW
or more when powered, and ~16mW when just I2C is powered). We don't want
to keep it powered when the device is not used, as this spends battery.
Also, the life of the device reduces a lot if we keep it always powered.

During this stage, if an ALSA call is issued, it may interfere at the
device settings and/or firmware load, with can cause the audio to fail. 
On such cases, applications might need to close the V4L2 node and re-open
again.

3) streaming state

The change to this staging requires a V4L2 ioctl.

Please notice, however, that some apps will open the audio device before
the V4L2 node, while others will open it after that.

In any case, audio will only start to produce data after the V4L2 ioctl
at V4L2 that starts the DMA engine there.

After that ioctl:
 - Audio PCM capture will work;
 - The mixers will be in a good state: unmuted, and switched to the
   corresponding input as the video stream.

If the user wants to do something unusual, like mixing the composite audio
input with the tuner audio input, it can use the ALSA mixer for doing
that. Otherwise, the only part of the ALSA device that will be used is
the PCM engine.

4) streaming-stop 

Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-24 Thread Shuah Khan
On 10/22/2014 01:45 PM, Devin Heitmueller wrote:
 this seems like a feature, not a bug. PulseAudio starts streaming before
 clients push any data and likewise keeps sources active even after for some
 time after clients stop recording. Closing VLC in your example doesn't
 immediately close the ALSA device. look for module-suspend-on-idle in your
 default.pa config file.
 
 The ALSA userland emulation in PulseAudio is supposed to faithfully emulate
 the behavior of the ALSA kernel ABI... except when it doesn't, then it's not
 a bug but rather a feature.  :-)
 
 I also agree that the open/close of the alsa device is the only way to
 control exclusion.
 
 I was also a proponent that we should have fairly coarse locking done
 at open/close for the various device nodes (ALSA/V4L/DVB).  The challenge here
 is that we have a large installed based of existing applications that
 rely on kernel
 behavior that isn't formally specified in any specification.  Hence
 we're forced to try
 to come up with a solution that minimizes the risk of ABI breakage.
 
 If we were doing this from scratch then we could lay down some hard/fast rules
 about things apps aren't supposed to do and how apps are supposed to respond
 to those exception cases.  Unfortunately we don't have that luxury here.
 

Sounds like we don't have a clear direction on open/close or capture
start/stop. What I am hearing is open/close isn't acceptable for
media maintainers and capture trigger start/stop isn't acceptable
to sound maintainers. :) Fork in the road, which way do we go?

Implementation wise, supporting capture trigger start/stop approach
will be harder to maintain in longterm. It adds more variables to
the mix. Applications open sounds device from the main thread and
then create a new thread to handle streams. I can see that based
on the token hold requests that come in. So the token hold logic
will have to take that into account, leading into potential
unbalanced lock/unlock scenarios. It is not impossible to solve,
that's what I did in this patch series, but it does get complex.

What I am looking for is some consensus on let's go with an approach
and try. Doesn't matter which way we go, and how much testing I do,
I am bound to miss something and this work needs to soak for a bit in
the media experimental branch.

thanks,
-- Shuah



-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-22 Thread Hans Verkuil
Hi Shuah,

Some notes below...

On 10/21/2014 07:32 PM, Shuah Khan wrote:
 On 10/21/2014 10:05 AM, Takashi Iwai wrote:
 At Tue, 21 Oct 2014 17:42:51 +0200,
 Hans Verkuil wrote:
 

 Quite often media apps open the alsa device at the start and then switch
 between TV, radio or DVB mode. If the alsa device would claim the tuner
 just by being opened (as opposed to actually using the tuner, which happens
 when you start streaming),

 What about parameter changes?  The sound devices have to be configured
 before using.  Don't they influence on others at all, i.e. you can
 change the PCM sample rate etc during TV, radio or DVB is running?
 
 Yes. kaffeine uses  snd_usb_capture_ops ioctl - snd_pcm_lib_ioctl
 
 Other v4l and vlc (dvb) uses open/close as well as trigger start and
 stop. trigger start/stop is done by a special audio thread in some
 cases. open/close happens from the main thread.
 

 then that would make it impossible for the
 application to switch tuner mode. In general you want to avoid that open()
 will start configuring hardware since that can quite often be slow. Tuner
 configuration in particular can be slow since several common tuners need
 to load firmware over i2c. You only want to do that when it is really 
 needed,
 and not when some application (udev!) opens the device just to examine what
 sort of device it is.

 But most apps close the device soon after that, no?
 Which programs keep the PCM device (not the control) opened without
 actually using?

 So claiming the tuner in the trigger seems to be the right place. If
 returning EBUSY is a poor error code for alsa, then we can use something 
 else
 for that. EACCES perhaps?

 Sorry, I'm not convinced by that.  If the device has to be controlled
 exclusively, the right position is the open/close.  Otherwise, the
 program cannot know when it becomes inaccessible out of sudden during
 its operation.


 
 Let me share my test matrix for this patch series. Hans pointed out
 one test case I didn't know about as a result missed testing. Please
 see if any of the tests miss use-cases or break them: you can scroll
 down to the proposal at the end, if this is too much detail :)
 
 Digital active and analog starting testing:
 kaffeine running
 - v4l2-ctl --all - works

I would recommend using v4l2-ctl --all --list-inputs, this enumerates
inputs as well (which includes reading the tuner status). This would
have failed with all these tests with this patch series.

 - Changing channels works with the same token hold, even when
   frequency changes. Tested changing channels that force freq
   change.
 - vlc resource is busy with no disruption to kaffeine
 - xawtv - tuner busy when it tries to do ioctls that change
   tuner settings - snd_usb_pcm_open detects device is busy
   ( pcm open called from the same thread, trigger gets called
 from another thread )
 - tvtime - tuner busy when it tries to do ioctls that change
   tuner settings with no disruption to kaffeine
   ( pcm open called from the same thread, trigger gets called
 from another thread )
 - vlc - audio capture on WinTV HVR-950 - device is busy
   start vlc with no channels for this test
 - arecord to capture on WinTV HVR-950 - device busy
 
 vlc running
 vlc -v channels.xspf
 - v4l2-ctl --all - works
 - Changing channels works with the same token hold, even when
   frequency changes. Tested changing channels that force freq
   change.
 - kaffeine resource is busy with no disruption to vlc
 - xawtv - tuner busy when it tries to do ioctls that change
   tuner settings - snd_usb_pcm_open detects device is busy
   ( pcm open called from the same thread, trigger gets called
 from another thread )
 - tvtime - tuner busy when it tries to do ioctls that change
   tuner settings with no disruption to kaffeine
   ( pcm open called from the same thread, trigger gets called
 from another thread )
 - vlc - audio capture on WinTV HVR-950 - device is busy
 - arecord to capture on WinTV HVR-950 - device busy
 
 Analog active and start digital testing:
 xawtv -noalsa -c /dev/video1
 - v4l2-ctl --all - works
 - start kaffeine - fails with device busy and no disruption
 - start vlc - fails with device busy and no disruption
 - tvtime - tuner busy when it tries to do ioctls that change
   tuner settings with no disruption to kaffeine
 - vlc - audio capture on WinTV HVR-950 - device is busy
 - arecord to capture on WinTV HVR-950 - device busy
 
 tvtime
 - v4l2-ctl --all - works
 - start kaffeine - fails with device busy and no disruption
 - start vlc - fails with device busy and no disruption
 - xawtv - tuner busy when it tries to do ioctls that change
   tuner settings with no disruption to kaffeine
 - vlc - audio capture on WinTV HVR-950 - device is busy
 - arecord to capture on WinTV HVR-950 - device busy
 
 The following audio/video start/stop combination tests:
 ( used arecord as well to test these cases, arecord )
 
 - tvtime start/vlc start/vlc stop/tvtime stop
   no disruption to 

Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-22 Thread Pierre-Louis Bossart

On 10/21/14, 11:08 AM, Devin Heitmueller wrote:

Sorry, I'm not convinced by that.  If the device has to be controlled
exclusively, the right position is the open/close.  Otherwise, the
program cannot know when it becomes inaccessible out of sudden during
its operation.


I can say that I've definitely seen cases where if you configure a
device as the default capture device in PulseAudio, then pulse will
continue to capture from it even if you're not actively capturing the
audio from pulse.  I only spotted this because I had a USB analyzer on
the device and was dumbfounded when the ISOC packets kept arriving
even after I had closed VLC.


this seems like a feature, not a bug. PulseAudio starts streaming before 
clients push any data and likewise keeps sources active even after for 
some time after clients stop recording. Closing VLC in your example 
doesn't immediately close the ALSA device. look for 
module-suspend-on-idle in your default.pa config file.
I also agree that the open/close of the alsa device is the only way to 
control exclusion.

-Pierre

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-22 Thread Devin Heitmueller
 this seems like a feature, not a bug. PulseAudio starts streaming before
 clients push any data and likewise keeps sources active even after for some
 time after clients stop recording. Closing VLC in your example doesn't
 immediately close the ALSA device. look for module-suspend-on-idle in your
 default.pa config file.

The ALSA userland emulation in PulseAudio is supposed to faithfully emulate
the behavior of the ALSA kernel ABI... except when it doesn't, then it's not
a bug but rather a feature.  :-)

 I also agree that the open/close of the alsa device is the only way to
 control exclusion.

I was also a proponent that we should have fairly coarse locking done
at open/close for the various device nodes (ALSA/V4L/DVB).  The challenge here
is that we have a large installed based of existing applications that
rely on kernel
behavior that isn't formally specified in any specification.  Hence
we're forced to try
to come up with a solution that minimizes the risk of ABI breakage.

If we were doing this from scratch then we could lay down some hard/fast rules
about things apps aren't supposed to do and how apps are supposed to respond
to those exception cases.  Unfortunately we don't have that luxury here.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-21 Thread Hans Verkuil



On 10/16/2014 04:48 PM, Takashi Iwai wrote:

At Thu, 16 Oct 2014 08:39:14 -0600,
Shuah Khan wrote:


On 10/16/2014 08:16 AM, Takashi Iwai wrote:

At Thu, 16 Oct 2014 08:10:52 -0600,
Shuah Khan wrote:


On 10/16/2014 08:01 AM, Takashi Iwai wrote:

At Thu, 16 Oct 2014 07:10:37 -0600,
Shuah Khan wrote:


On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:

On 10/14/2014 04:58 PM, Shuah Khan wrote:
[...]

   switch (cmd) {
   case SNDRV_PCM_TRIGGER_START:
+err = media_get_audio_tkn(subs-dev-dev);
+if (err == -EBUSY) {
+dev_info(subs-dev-dev, %s device is busy\n,
+__func__);


In my opinion this should not dev_info() as this is out of band error
signaling and also as the potential to spam the log. The userspace
application is already properly notified by the return code.



Yes it has the potential to flood the dmesg especially with alsa,
I will remove the dev_info().


Yes.  And, I think doing this in the trigger isn't the best.
Why not in open  close?


My first cut of this change was in open and close. I saw pulseaudio
application go into this loop trying to open the device. To avoid
such problems, I went with trigger stat and stop. That made all the
pulseaudio continues attempts to open problems go away.


But now starting the stream gives the error, and PA would loop it
again, no?



Also, is this token restriction needed only for PCM?  No mixer or
other controls?


snd_pcm_ops are the only ones media drivers implement and use. So
I don't think mixer is needed. If it is needed, it is not to hard
to add for that case.


Well, then I wonder what resource does actually conflict with
usb-audio and media drivers at all...?



audio for dvb/v4l apps gets disrupted when audio app starts. For
example, dvb or v4l app tuned to a channel, and when an audio app
starts. audio path needs protected to avoid conflicts between
digital and analog applications as well.


OK, then concentrating on only PCM is fine.

But, I'm still not convinced about doing the token management in the
trigger.  The reason -EBUSY doesn't work is that it's the very same
error code when a PCM device is blocked by other processes.  And
-EAGAIN is interpreted by PCM core to -EBUSY for historical reasons.

How applications (e.g. PA) should behave if the token get fails?
Shouldn't it retry or totally give up?


Quite often media apps open the alsa device at the start and then switch
between TV, radio or DVB mode. If the alsa device would claim the tuner
just by being opened (as opposed to actually using the tuner, which happens
when you start streaming), then that would make it impossible for the
application to switch tuner mode. In general you want to avoid that open()
will start configuring hardware since that can quite often be slow. Tuner
configuration in particular can be slow since several common tuners need
to load firmware over i2c. You only want to do that when it is really needed,
and not when some application (udev!) opens the device just to examine what
sort of device it is.

So claiming the tuner in the trigger seems to be the right place. If
returning EBUSY is a poor error code for alsa, then we can use something else
for that. EACCES perhaps?

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-21 Thread Takashi Iwai
At Tue, 21 Oct 2014 17:42:51 +0200,
Hans Verkuil wrote:
 
 
 
 On 10/16/2014 04:48 PM, Takashi Iwai wrote:
  At Thu, 16 Oct 2014 08:39:14 -0600,
  Shuah Khan wrote:
 
  On 10/16/2014 08:16 AM, Takashi Iwai wrote:
  At Thu, 16 Oct 2014 08:10:52 -0600,
  Shuah Khan wrote:
 
  On 10/16/2014 08:01 AM, Takashi Iwai wrote:
  At Thu, 16 Oct 2014 07:10:37 -0600,
  Shuah Khan wrote:
 
  On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
  On 10/14/2014 04:58 PM, Shuah Khan wrote:
  [...]
 switch (cmd) {
 case SNDRV_PCM_TRIGGER_START:
  +err = media_get_audio_tkn(subs-dev-dev);
  +if (err == -EBUSY) {
  +dev_info(subs-dev-dev, %s device is busy\n,
  +__func__);
 
  In my opinion this should not dev_info() as this is out of band error
  signaling and also as the potential to spam the log. The userspace
  application is already properly notified by the return code.
 
 
  Yes it has the potential to flood the dmesg especially with alsa,
  I will remove the dev_info().
 
  Yes.  And, I think doing this in the trigger isn't the best.
  Why not in open  close?
 
  My first cut of this change was in open and close. I saw pulseaudio
  application go into this loop trying to open the device. To avoid
  such problems, I went with trigger stat and stop. That made all the
  pulseaudio continues attempts to open problems go away.
 
  But now starting the stream gives the error, and PA would loop it
  again, no?
 
 
  Also, is this token restriction needed only for PCM?  No mixer or
  other controls?
 
  snd_pcm_ops are the only ones media drivers implement and use. So
  I don't think mixer is needed. If it is needed, it is not to hard
  to add for that case.
 
  Well, then I wonder what resource does actually conflict with
  usb-audio and media drivers at all...?
 
 
  audio for dvb/v4l apps gets disrupted when audio app starts. For
  example, dvb or v4l app tuned to a channel, and when an audio app
  starts. audio path needs protected to avoid conflicts between
  digital and analog applications as well.
 
  OK, then concentrating on only PCM is fine.
 
  But, I'm still not convinced about doing the token management in the
  trigger.  The reason -EBUSY doesn't work is that it's the very same
  error code when a PCM device is blocked by other processes.  And
  -EAGAIN is interpreted by PCM core to -EBUSY for historical reasons.
 
  How applications (e.g. PA) should behave if the token get fails?
  Shouldn't it retry or totally give up?
 
 Quite often media apps open the alsa device at the start and then switch
 between TV, radio or DVB mode. If the alsa device would claim the tuner
 just by being opened (as opposed to actually using the tuner, which happens
 when you start streaming),

What about parameter changes?  The sound devices have to be configured
before using.  Don't they influence on others at all, i.e. you can
change the PCM sample rate etc during TV, radio or DVB is running?

 then that would make it impossible for the
 application to switch tuner mode. In general you want to avoid that open()
 will start configuring hardware since that can quite often be slow. Tuner
 configuration in particular can be slow since several common tuners need
 to load firmware over i2c. You only want to do that when it is really needed,
 and not when some application (udev!) opens the device just to examine what
 sort of device it is.

But most apps close the device soon after that, no?
Which programs keep the PCM device (not the control) opened without
actually using?

 So claiming the tuner in the trigger seems to be the right place. If
 returning EBUSY is a poor error code for alsa, then we can use something else
 for that. EACCES perhaps?

Sorry, I'm not convinced by that.  If the device has to be controlled
exclusively, the right position is the open/close.  Otherwise, the
program cannot know when it becomes inaccessible out of sudden during
its operation.


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-21 Thread Devin Heitmueller
 Sorry, I'm not convinced by that.  If the device has to be controlled
 exclusively, the right position is the open/close.  Otherwise, the
 program cannot know when it becomes inaccessible out of sudden during
 its operation.

I can say that I've definitely seen cases where if you configure a
device as the default capture device in PulseAudio, then pulse will
continue to capture from it even if you're not actively capturing the
audio from pulse.  I only spotted this because I had a USB analyzer on
the device and was dumbfounded when the ISOC packets kept arriving
even after I had closed VLC.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-21 Thread Takashi Iwai
At Tue, 21 Oct 2014 12:08:59 -0400,
Devin Heitmueller wrote:
 
  Sorry, I'm not convinced by that.  If the device has to be controlled
  exclusively, the right position is the open/close.  Otherwise, the
  program cannot know when it becomes inaccessible out of sudden during
  its operation.
 
 I can say that I've definitely seen cases where if you configure a
 device as the default capture device in PulseAudio, then pulse will
 continue to capture from it even if you're not actively capturing the
 audio from pulse.  I only spotted this because I had a USB analyzer on
 the device and was dumbfounded when the ISOC packets kept arriving
 even after I had closed VLC.

You might have had an input monitor active in some PA apps?
PA shouldn't do it as default.  If it does unintentionally, you should
report it to PA guys.


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-21 Thread Shuah Khan
On 10/21/2014 10:05 AM, Takashi Iwai wrote:
 At Tue, 21 Oct 2014 17:42:51 +0200,
 Hans Verkuil wrote:


 Quite often media apps open the alsa device at the start and then switch
 between TV, radio or DVB mode. If the alsa device would claim the tuner
 just by being opened (as opposed to actually using the tuner, which happens
 when you start streaming),
 
 What about parameter changes?  The sound devices have to be configured
 before using.  Don't they influence on others at all, i.e. you can
 change the PCM sample rate etc during TV, radio or DVB is running?

Yes. kaffeine uses  snd_usb_capture_ops ioctl - snd_pcm_lib_ioctl

Other v4l and vlc (dvb) uses open/close as well as trigger start and
stop. trigger start/stop is done by a special audio thread in some
cases. open/close happens from the main thread.

 
 then that would make it impossible for the
 application to switch tuner mode. In general you want to avoid that open()
 will start configuring hardware since that can quite often be slow. Tuner
 configuration in particular can be slow since several common tuners need
 to load firmware over i2c. You only want to do that when it is really needed,
 and not when some application (udev!) opens the device just to examine what
 sort of device it is.
 
 But most apps close the device soon after that, no?
 Which programs keep the PCM device (not the control) opened without
 actually using?
 
 So claiming the tuner in the trigger seems to be the right place. If
 returning EBUSY is a poor error code for alsa, then we can use something else
 for that. EACCES perhaps?
 
 Sorry, I'm not convinced by that.  If the device has to be controlled
 exclusively, the right position is the open/close.  Otherwise, the
 program cannot know when it becomes inaccessible out of sudden during
 its operation.
 
 

Let me share my test matrix for this patch series. Hans pointed out
one test case I didn't know about as a result missed testing. Please
see if any of the tests miss use-cases or break them: you can scroll
down to the proposal at the end, if this is too much detail :)

Digital active and analog starting testing:
kaffeine running
- v4l2-ctl --all - works
- Changing channels works with the same token hold, even when
  frequency changes. Tested changing channels that force freq
  change.
- vlc resource is busy with no disruption to kaffeine
- xawtv - tuner busy when it tries to do ioctls that change
  tuner settings - snd_usb_pcm_open detects device is busy
  ( pcm open called from the same thread, trigger gets called
from another thread )
- tvtime - tuner busy when it tries to do ioctls that change
  tuner settings with no disruption to kaffeine
  ( pcm open called from the same thread, trigger gets called
from another thread )
- vlc - audio capture on WinTV HVR-950 - device is busy
  start vlc with no channels for this test
- arecord to capture on WinTV HVR-950 - device busy

vlc running
vlc -v channels.xspf
- v4l2-ctl --all - works
- Changing channels works with the same token hold, even when
  frequency changes. Tested changing channels that force freq
  change.
- kaffeine resource is busy with no disruption to vlc
- xawtv - tuner busy when it tries to do ioctls that change
  tuner settings - snd_usb_pcm_open detects device is busy
  ( pcm open called from the same thread, trigger gets called
from another thread )
- tvtime - tuner busy when it tries to do ioctls that change
  tuner settings with no disruption to kaffeine
  ( pcm open called from the same thread, trigger gets called
from another thread )
- vlc - audio capture on WinTV HVR-950 - device is busy
- arecord to capture on WinTV HVR-950 - device busy

Analog active and start digital testing:
xawtv -noalsa -c /dev/video1
- v4l2-ctl --all - works
- start kaffeine - fails with device busy and no disruption
- start vlc - fails with device busy and no disruption
- tvtime - tuner busy when it tries to do ioctls that change
  tuner settings with no disruption to kaffeine
- vlc - audio capture on WinTV HVR-950 - device is busy
- arecord to capture on WinTV HVR-950 - device busy

tvtime
- v4l2-ctl --all - works
- start kaffeine - fails with device busy and no disruption
- start vlc - fails with device busy and no disruption
- xawtv - tuner busy when it tries to do ioctls that change
  tuner settings with no disruption to kaffeine
- vlc - audio capture on WinTV HVR-950 - device is busy
- arecord to capture on WinTV HVR-950 - device busy

The following audio/video start/stop combination tests:
( used arecord as well to test these cases, arecord )

- tvtime start/vlc start/vlc stop/tvtime stop
  no disruption to tvtime
- tvtime start/vlc start/tvtie stop/vlc stop
  One tvtime stops, could trigger capture manually
- vlc start/tvtime start/tvtime stop/vlc stop
  vlc audio capture continues, tvtime detect tuner busy
- vlc start/tvtime start/vlc stop/tvtime start
  when vlc stops, tvtime could open the tuner device

Repeated the above with kaffeine and vlc and 

Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-19 Thread Takashi Iwai
At Sat, 18 Oct 2014 20:49:58 +0200,
Mauro Carvalho Chehab wrote:
 
 Em Thu, 16 Oct 2014 08:59:29 -0600
 Shuah Khan shua...@osg.samsung.com escreveu:
 
  On 10/16/2014 08:48 AM, Takashi Iwai wrote:
   At Thu, 16 Oct 2014 08:39:14 -0600,
   Shuah Khan wrote:
  
   On 10/16/2014 08:16 AM, Takashi Iwai wrote:
   At Thu, 16 Oct 2014 08:10:52 -0600,
   Shuah Khan wrote:
  
   On 10/16/2014 08:01 AM, Takashi Iwai wrote:
   At Thu, 16 Oct 2014 07:10:37 -0600,
   Shuah Khan wrote:
  
   On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
   On 10/14/2014 04:58 PM, Shuah Khan wrote:
   [...]
 switch (cmd) {
 case SNDRV_PCM_TRIGGER_START:
   +err = media_get_audio_tkn(subs-dev-dev);
   +if (err == -EBUSY) {
   +dev_info(subs-dev-dev, %s device is busy\n,
   +__func__);
  
   In my opinion this should not dev_info() as this is out of band 
   error
   signaling and also as the potential to spam the log. The userspace
   application is already properly notified by the return code.
  
  
   Yes it has the potential to flood the dmesg especially with alsa,
   I will remove the dev_info().
  
   Yes.  And, I think doing this in the trigger isn't the best.
   Why not in open  close?
  
   My first cut of this change was in open and close. I saw pulseaudio
   application go into this loop trying to open the device. To avoid
   such problems, I went with trigger stat and stop. That made all the
   pulseaudio continues attempts to open problems go away.
  
   But now starting the stream gives the error, and PA would loop it
   again, no?
  
  
   Also, is this token restriction needed only for PCM?  No mixer or
   other controls?
  
   snd_pcm_ops are the only ones media drivers implement and use. So
   I don't think mixer is needed. If it is needed, it is not to hard
   to add for that case.
  
   Well, then I wonder what resource does actually conflict with
   usb-audio and media drivers at all...?
  
  
   audio for dvb/v4l apps gets disrupted when audio app starts. For
   example, dvb or v4l app tuned to a channel, and when an audio app
   starts. audio path needs protected to avoid conflicts between
   digital and analog applications as well.
   
   OK, then concentrating on only PCM is fine.
   
   But, I'm still not convinced about doing the token management in the
   trigger.  The reason -EBUSY doesn't work is that it's the very same
   error code when a PCM device is blocked by other processes.  And
   -EAGAIN is interpreted by PCM core to -EBUSY for historical reasons.
  
  ah. ok your recommendation is to go with open and close.
  Mauro has some reservations with holding at open when I discussed
  my observations with pulseaudio when I was holding token in open
  instead of trigger start. Maybe he can chime with his concerns.
  I think his concern was breaking applications if token is held in
  open().
 
 Yes. My concern is that PA has weird behaviors, and it tries to open and
 keep opened all audio devices.

PA usually closes the PCM devices when unused.  If it doesn't, it must
be a bug.


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-18 Thread Mauro Carvalho Chehab
Em Thu, 16 Oct 2014 08:59:29 -0600
Shuah Khan shua...@osg.samsung.com escreveu:

 On 10/16/2014 08:48 AM, Takashi Iwai wrote:
  At Thu, 16 Oct 2014 08:39:14 -0600,
  Shuah Khan wrote:
 
  On 10/16/2014 08:16 AM, Takashi Iwai wrote:
  At Thu, 16 Oct 2014 08:10:52 -0600,
  Shuah Khan wrote:
 
  On 10/16/2014 08:01 AM, Takashi Iwai wrote:
  At Thu, 16 Oct 2014 07:10:37 -0600,
  Shuah Khan wrote:
 
  On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
  On 10/14/2014 04:58 PM, Shuah Khan wrote:
  [...]
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
  +err = media_get_audio_tkn(subs-dev-dev);
  +if (err == -EBUSY) {
  +dev_info(subs-dev-dev, %s device is busy\n,
  +__func__);
 
  In my opinion this should not dev_info() as this is out of band error
  signaling and also as the potential to spam the log. The userspace
  application is already properly notified by the return code.
 
 
  Yes it has the potential to flood the dmesg especially with alsa,
  I will remove the dev_info().
 
  Yes.  And, I think doing this in the trigger isn't the best.
  Why not in open  close?
 
  My first cut of this change was in open and close. I saw pulseaudio
  application go into this loop trying to open the device. To avoid
  such problems, I went with trigger stat and stop. That made all the
  pulseaudio continues attempts to open problems go away.
 
  But now starting the stream gives the error, and PA would loop it
  again, no?
 
 
  Also, is this token restriction needed only for PCM?  No mixer or
  other controls?
 
  snd_pcm_ops are the only ones media drivers implement and use. So
  I don't think mixer is needed. If it is needed, it is not to hard
  to add for that case.
 
  Well, then I wonder what resource does actually conflict with
  usb-audio and media drivers at all...?
 
 
  audio for dvb/v4l apps gets disrupted when audio app starts. For
  example, dvb or v4l app tuned to a channel, and when an audio app
  starts. audio path needs protected to avoid conflicts between
  digital and analog applications as well.
  
  OK, then concentrating on only PCM is fine.
  
  But, I'm still not convinced about doing the token management in the
  trigger.  The reason -EBUSY doesn't work is that it's the very same
  error code when a PCM device is blocked by other processes.  And
  -EAGAIN is interpreted by PCM core to -EBUSY for historical reasons.
 
 ah. ok your recommendation is to go with open and close.
 Mauro has some reservations with holding at open when I discussed
 my observations with pulseaudio when I was holding token in open
 instead of trigger start. Maybe he can chime with his concerns.
 I think his concern was breaking applications if token is held in
 open().

Yes. My concern is that PA has weird behaviors, and it tries to open and
keep opened all audio devices. Is there a way for avoiding it to keep doing
it for V4L devices?

 
 Based on what you are seeing trigger could be worse.
 
  
  How applications (e.g. PA) should behave if the token get fails?
  Shouldn't it retry or totally give up?
  
 
 It would be up to the application I would think. I see that arecord
 quits right away when it finds the device busy. pluseaudio on the other
 hand appears to retry. I downloaded pulseaudio sources to understand
 what it is doing, however I didn't get too far. The way it does audio
 handling is complex for me to follow without spending a lot of time.
 
 thanks,
 -- Shuah
 


-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-16 Thread Lars-Peter Clausen

On 10/14/2014 04:58 PM, Shuah Khan wrote:
[...]

switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
+   err = media_get_audio_tkn(subs-dev-dev);
+   if (err == -EBUSY) {
+   dev_info(subs-dev-dev, %s device is busy\n,
+   __func__);


In my opinion this should not dev_info() as this is out of band error 
signaling and also as the potential to spam the log. The userspace 
application is already properly notified by the return code.



+   return err;
+   }
err = start_endpoints(subs, false);
if (err  0)
return err;


--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-16 Thread Shuah Khan
On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
 On 10/14/2014 04:58 PM, Shuah Khan wrote:
 [...]
   switch (cmd) {
   case SNDRV_PCM_TRIGGER_START:
 +err = media_get_audio_tkn(subs-dev-dev);
 +if (err == -EBUSY) {
 +dev_info(subs-dev-dev, %s device is busy\n,
 +__func__);
 
 In my opinion this should not dev_info() as this is out of band error
 signaling and also as the potential to spam the log. The userspace
 application is already properly notified by the return code.
 

Yes it has the potential to flood the dmesg especially with alsa,
I will remove the dev_info().

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-16 Thread Takashi Iwai
At Thu, 16 Oct 2014 07:10:37 -0600,
Shuah Khan wrote:
 
 On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
  On 10/14/2014 04:58 PM, Shuah Khan wrote:
  [...]
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
  +err = media_get_audio_tkn(subs-dev-dev);
  +if (err == -EBUSY) {
  +dev_info(subs-dev-dev, %s device is busy\n,
  +__func__);
  
  In my opinion this should not dev_info() as this is out of band error
  signaling and also as the potential to spam the log. The userspace
  application is already properly notified by the return code.
  
 
 Yes it has the potential to flood the dmesg especially with alsa,
 I will remove the dev_info().

Yes.  And, I think doing this in the trigger isn't the best.
Why not in open  close?

Also, is this token restriction needed only for PCM?  No mixer or
other controls?


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-16 Thread Shuah Khan
On 10/16/2014 08:01 AM, Takashi Iwai wrote:
 At Thu, 16 Oct 2014 07:10:37 -0600,
 Shuah Khan wrote:

 On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
 On 10/14/2014 04:58 PM, Shuah Khan wrote:
 [...]
   switch (cmd) {
   case SNDRV_PCM_TRIGGER_START:
 +err = media_get_audio_tkn(subs-dev-dev);
 +if (err == -EBUSY) {
 +dev_info(subs-dev-dev, %s device is busy\n,
 +__func__);

 In my opinion this should not dev_info() as this is out of band error
 signaling and also as the potential to spam the log. The userspace
 application is already properly notified by the return code.


 Yes it has the potential to flood the dmesg especially with alsa,
 I will remove the dev_info().
 
 Yes.  And, I think doing this in the trigger isn't the best.
 Why not in open  close?

My first cut of this change was in open and close. I saw pulseaudio
application go into this loop trying to open the device. To avoid
such problems, I went with trigger stat and stop. That made all the
pulseaudio continues attempts to open problems go away.

 
 Also, is this token restriction needed only for PCM?  No mixer or
 other controls?

snd_pcm_ops are the only ones media drivers implement and use. So
I don't think mixer is needed. If it is needed, it is not to hard
to add for that case.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-16 Thread Takashi Iwai
At Thu, 16 Oct 2014 08:10:52 -0600,
Shuah Khan wrote:
 
 On 10/16/2014 08:01 AM, Takashi Iwai wrote:
  At Thu, 16 Oct 2014 07:10:37 -0600,
  Shuah Khan wrote:
 
  On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
  On 10/14/2014 04:58 PM, Shuah Khan wrote:
  [...]
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
  +err = media_get_audio_tkn(subs-dev-dev);
  +if (err == -EBUSY) {
  +dev_info(subs-dev-dev, %s device is busy\n,
  +__func__);
 
  In my opinion this should not dev_info() as this is out of band error
  signaling and also as the potential to spam the log. The userspace
  application is already properly notified by the return code.
 
 
  Yes it has the potential to flood the dmesg especially with alsa,
  I will remove the dev_info().
  
  Yes.  And, I think doing this in the trigger isn't the best.
  Why not in open  close?
 
 My first cut of this change was in open and close. I saw pulseaudio
 application go into this loop trying to open the device. To avoid
 such problems, I went with trigger stat and stop. That made all the
 pulseaudio continues attempts to open problems go away.

But now starting the stream gives the error, and PA would loop it
again, no?


  Also, is this token restriction needed only for PCM?  No mixer or
  other controls?
 
 snd_pcm_ops are the only ones media drivers implement and use. So
 I don't think mixer is needed. If it is needed, it is not to hard
 to add for that case.

Well, then I wonder what resource does actually conflict with
usb-audio and media drivers at all...?


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-16 Thread Shuah Khan
On 10/16/2014 08:16 AM, Takashi Iwai wrote:
 At Thu, 16 Oct 2014 08:10:52 -0600,
 Shuah Khan wrote:

 On 10/16/2014 08:01 AM, Takashi Iwai wrote:
 At Thu, 16 Oct 2014 07:10:37 -0600,
 Shuah Khan wrote:

 On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
 On 10/14/2014 04:58 PM, Shuah Khan wrote:
 [...]
   switch (cmd) {
   case SNDRV_PCM_TRIGGER_START:
 +err = media_get_audio_tkn(subs-dev-dev);
 +if (err == -EBUSY) {
 +dev_info(subs-dev-dev, %s device is busy\n,
 +__func__);

 In my opinion this should not dev_info() as this is out of band error
 signaling and also as the potential to spam the log. The userspace
 application is already properly notified by the return code.


 Yes it has the potential to flood the dmesg especially with alsa,
 I will remove the dev_info().

 Yes.  And, I think doing this in the trigger isn't the best.
 Why not in open  close?

 My first cut of this change was in open and close. I saw pulseaudio
 application go into this loop trying to open the device. To avoid
 such problems, I went with trigger stat and stop. That made all the
 pulseaudio continues attempts to open problems go away.
 
 But now starting the stream gives the error, and PA would loop it
 again, no?
 
 
 Also, is this token restriction needed only for PCM?  No mixer or
 other controls?

 snd_pcm_ops are the only ones media drivers implement and use. So
 I don't think mixer is needed. If it is needed, it is not to hard
 to add for that case.
 
 Well, then I wonder what resource does actually conflict with
 usb-audio and media drivers at all...?
 

audio for dvb/v4l apps gets disrupted when audio app starts. For
example, dvb or v4l app tuned to a channel, and when an audio app
starts. audio path needs protected to avoid conflicts between
digital and analog applications as well.

-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-16 Thread Takashi Iwai
At Thu, 16 Oct 2014 08:39:14 -0600,
Shuah Khan wrote:
 
 On 10/16/2014 08:16 AM, Takashi Iwai wrote:
  At Thu, 16 Oct 2014 08:10:52 -0600,
  Shuah Khan wrote:
 
  On 10/16/2014 08:01 AM, Takashi Iwai wrote:
  At Thu, 16 Oct 2014 07:10:37 -0600,
  Shuah Khan wrote:
 
  On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
  On 10/14/2014 04:58 PM, Shuah Khan wrote:
  [...]
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
  +err = media_get_audio_tkn(subs-dev-dev);
  +if (err == -EBUSY) {
  +dev_info(subs-dev-dev, %s device is busy\n,
  +__func__);
 
  In my opinion this should not dev_info() as this is out of band error
  signaling and also as the potential to spam the log. The userspace
  application is already properly notified by the return code.
 
 
  Yes it has the potential to flood the dmesg especially with alsa,
  I will remove the dev_info().
 
  Yes.  And, I think doing this in the trigger isn't the best.
  Why not in open  close?
 
  My first cut of this change was in open and close. I saw pulseaudio
  application go into this loop trying to open the device. To avoid
  such problems, I went with trigger stat and stop. That made all the
  pulseaudio continues attempts to open problems go away.
  
  But now starting the stream gives the error, and PA would loop it
  again, no?
  
  
  Also, is this token restriction needed only for PCM?  No mixer or
  other controls?
 
  snd_pcm_ops are the only ones media drivers implement and use. So
  I don't think mixer is needed. If it is needed, it is not to hard
  to add for that case.
  
  Well, then I wonder what resource does actually conflict with
  usb-audio and media drivers at all...?
  
 
 audio for dvb/v4l apps gets disrupted when audio app starts. For
 example, dvb or v4l app tuned to a channel, and when an audio app
 starts. audio path needs protected to avoid conflicts between
 digital and analog applications as well.

OK, then concentrating on only PCM is fine.

But, I'm still not convinced about doing the token management in the
trigger.  The reason -EBUSY doesn't work is that it's the very same
error code when a PCM device is blocked by other processes.  And
-EAGAIN is interpreted by PCM core to -EBUSY for historical reasons.

How applications (e.g. PA) should behave if the token get fails?
Shouldn't it retry or totally give up?


Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-16 Thread Shuah Khan
On 10/16/2014 08:48 AM, Takashi Iwai wrote:
 At Thu, 16 Oct 2014 08:39:14 -0600,
 Shuah Khan wrote:

 On 10/16/2014 08:16 AM, Takashi Iwai wrote:
 At Thu, 16 Oct 2014 08:10:52 -0600,
 Shuah Khan wrote:

 On 10/16/2014 08:01 AM, Takashi Iwai wrote:
 At Thu, 16 Oct 2014 07:10:37 -0600,
 Shuah Khan wrote:

 On 10/16/2014 06:00 AM, Lars-Peter Clausen wrote:
 On 10/14/2014 04:58 PM, Shuah Khan wrote:
 [...]
   switch (cmd) {
   case SNDRV_PCM_TRIGGER_START:
 +err = media_get_audio_tkn(subs-dev-dev);
 +if (err == -EBUSY) {
 +dev_info(subs-dev-dev, %s device is busy\n,
 +__func__);

 In my opinion this should not dev_info() as this is out of band error
 signaling and also as the potential to spam the log. The userspace
 application is already properly notified by the return code.


 Yes it has the potential to flood the dmesg especially with alsa,
 I will remove the dev_info().

 Yes.  And, I think doing this in the trigger isn't the best.
 Why not in open  close?

 My first cut of this change was in open and close. I saw pulseaudio
 application go into this loop trying to open the device. To avoid
 such problems, I went with trigger stat and stop. That made all the
 pulseaudio continues attempts to open problems go away.

 But now starting the stream gives the error, and PA would loop it
 again, no?


 Also, is this token restriction needed only for PCM?  No mixer or
 other controls?

 snd_pcm_ops are the only ones media drivers implement and use. So
 I don't think mixer is needed. If it is needed, it is not to hard
 to add for that case.

 Well, then I wonder what resource does actually conflict with
 usb-audio and media drivers at all...?


 audio for dvb/v4l apps gets disrupted when audio app starts. For
 example, dvb or v4l app tuned to a channel, and when an audio app
 starts. audio path needs protected to avoid conflicts between
 digital and analog applications as well.
 
 OK, then concentrating on only PCM is fine.
 
 But, I'm still not convinced about doing the token management in the
 trigger.  The reason -EBUSY doesn't work is that it's the very same
 error code when a PCM device is blocked by other processes.  And
 -EAGAIN is interpreted by PCM core to -EBUSY for historical reasons.

ah. ok your recommendation is to go with open and close.
Mauro has some reservations with holding at open when I discussed
my observations with pulseaudio when I was holding token in open
instead of trigger start. Maybe he can chime with his concerns.
I think his concern was breaking applications if token is held in
open().

Based on what you are seeing trigger could be worse.

 
 How applications (e.g. PA) should behave if the token get fails?
 Shouldn't it retry or totally give up?
 

It would be up to the application I would think. I see that arecord
quits right away when it finds the device busy. pluseaudio on the other
hand appears to retry. I downloaded pulseaudio sources to understand
what it is doing, however I didn't get too far. The way it does audio
handling is complex for me to follow without spending a lot of time.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 5/6] sound/usb: pcm changes to use media token api

2014-10-14 Thread Shuah Khan
Change snd_usb_capture_ops trigger to hold audio token prior
starting endpoints for SNDRV_PCM_TRIGGER_START request and
release after stopping endpoints for SNDRV_PCM_TRIGGER_STOP
request. Audio token is released from snd_usb_capture_ops
close interface to cover the case where an application exits
without stopping capture. Audio token get request will succeed
if it is free or when a media application with the same task
pid or task gid makes the request. This covers the cases when
a media application first hold the tuner nd audio token and
then requests SNDRV_PCM_TRIGGER_START either from the same
thread or a another thread in the same process group.

Signed-off-by: Shuah Khan shua...@osg.samsung.com
---
 sound/usb/pcm.c |9 +
 1 file changed, 9 insertions(+)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index c62a165..d23abeb 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -21,6 +21,7 @@
 #include linux/usb.h
 #include linux/usb/audio.h
 #include linux/usb/audio-v2.h
+#include linux/media_tknres.h
 
 #include sound/core.h
 #include sound/pcm.h
@@ -1220,6 +1221,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream 
*substream, int direction)
 
subs-pcm_substream = NULL;
snd_usb_autosuspend(subs-stream-chip);
+   media_put_audio_tkn(subs-dev-dev);
 
return 0;
 }
@@ -1573,6 +1575,12 @@ static int snd_usb_substream_capture_trigger(struct 
snd_pcm_substream *substream
 
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
+   err = media_get_audio_tkn(subs-dev-dev);
+   if (err == -EBUSY) {
+   dev_info(subs-dev-dev, %s device is busy\n,
+   __func__);
+   return err;
+   }
err = start_endpoints(subs, false);
if (err  0)
return err;
@@ -1583,6 +1591,7 @@ static int snd_usb_substream_capture_trigger(struct 
snd_pcm_substream *substream
case SNDRV_PCM_TRIGGER_STOP:
stop_endpoints(subs, false);
subs-running = 0;
+   media_put_audio_tkn(subs-dev-dev);
return 0;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
subs-data_endpoint-retire_data_urb = NULL;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html