PCM API update

PCM API update

[Jaroslav Kysela]

Hi all,

	these enhancements are in CVS for PCM API:

- added snd_pcm_avail() function - this function returns count of 
  available frames for write or read operations, the position
  is read directly from hardware; the snd_pcm_avail_update() call
  is still mandatory before any I/O!!!
- updated behaviour of poll() and read()/write() functions as 
  heavy discussed on this list (wait when buffer is full, but not started)

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project  http://www.alsa-project.org
SuSE Linux    http://www.suse.com



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: PCM API update

[Abramo Bagnara]

Jaroslav Kysela wrote:
> 
> Hi all,
> 
>         these enhancements are in CVS for PCM API:
> 
> - added snd_pcm_avail() function - this function returns count of
>   available frames for write or read operations, the position
>   is read directly from hardware; the snd_pcm_avail_update() call
>   is still mandatory before any I/O!!!

I really don't understand why you've added another ioctl and another
function to replicate in all PCM classes.
snd_pcm_delay existence is enough to implement that.
I'm missing something?

-- 
Abramo Bagnara                       mailto:abramo.bagnara@libero.it

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: PCM API update

[Jaroslav Kysela]

On Fri, 11 Oct 2002, Abramo Bagnara wrote:

> Jaroslav Kysela wrote:
> > 
> > Hi all,
> > 
> >         these enhancements are in CVS for PCM API:
> > 
> > - added snd_pcm_avail() function - this function returns count of
> >   available frames for write or read operations, the position
> >   is read directly from hardware; the snd_pcm_avail_update() call
> >   is still mandatory before any I/O!!!
> 
> I really don't understand why you've added another ioctl and another
> function to replicate in all PCM classes.
> snd_pcm_delay existence is enough to implement that.
> I'm missing something?

You're completely right, but in my eyes, the avail ioctl looks much better
for kernel space (it's a base value for computing delay, computing avail
from delay needs twice substractions which doesn't look very good).
Unfortunately, delay ioctl was first, and we should keep binary
compatibility for 2.5, so we should leave both ioctls there. In library,
we can remove the delay operation from chains and use avail to compute it 
as well to simplify code.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project  http://www.alsa-project.org
SuSE Linux    http://www.suse.com



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: PCM API update

[Abramo Bagnara]

Jaroslav Kysela wrote:
> 
> On Fri, 11 Oct 2002, Abramo Bagnara wrote:
> 
> > I really don't understand why you've added another ioctl and another
> > function to replicate in all PCM classes.
> > snd_pcm_delay existence is enough to implement that.
> > I'm missing something?
> 
> You're completely right, but in my eyes, the avail ioctl looks much better
> for kernel space (it's a base value for computing delay, computing avail
> from delay needs twice substractions which doesn't look very good).
> Unfortunately, delay ioctl was first, and we should keep binary
> compatibility for 2.5, so we should leave both ioctls there. In library,
> we can remove the delay operation from chains and use avail to compute it
> as well to simplify code.

No, you can't.
The semantic of snd_pcm_delay is:
"How many frames are current application read/write pointer far away
from 'now' (i.e. real world sound)?"

It's *not* simply equivalent to buffer_size - avail (playback) or avail
(capture). It's designed to take in account also olatencies introduced
by other things (network layer, hardware limits, etc.).

Although I don't understand why you think that snd_pcm_avail is needed
when we have the right (and safe) snd_pcm_avail_update, it can be
implemented calling snd_pcm_delay (so to update hw pointer) and then
computing the avail count using the mmap'ed counters.

Of course this can't work for chained PCM (and this is why we have
snd_pcm_avail_update).

I think you should revert your patch (and to force yourself to believe a
bit more in peer review before important changes although it seems I'm
beating a dead horse here).

-- 
Abramo Bagnara                       mailto:abramo.bagnara@libero.it

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: PCM API update

[Jaroslav Kysela]

On Sat, 12 Oct 2002, Abramo Bagnara wrote:

> Jaroslav Kysela wrote:
> > 
> > On Fri, 11 Oct 2002, Abramo Bagnara wrote:
> > 
> > > I really don't understand why you've added another ioctl and another
> > > function to replicate in all PCM classes.
> > > snd_pcm_delay existence is enough to implement that.
> > > I'm missing something?
> > 
> > You're completely right, but in my eyes, the avail ioctl looks much better
> > for kernel space (it's a base value for computing delay, computing avail
> > from delay needs twice substractions which doesn't look very good).
> > Unfortunately, delay ioctl was first, and we should keep binary
> > compatibility for 2.5, so we should leave both ioctls there. In library,
> > we can remove the delay operation from chains and use avail to compute it
> > as well to simplify code.
> 
> No, you can't.
> The semantic of snd_pcm_delay is:
> "How many frames are current application read/write pointer far away
> from 'now' (i.e. real world sound)?"
> 
> It's *not* simply equivalent to buffer_size - avail (playback) or avail
> (capture). It's designed to take in account also olatencies introduced
> by other things (network layer, hardware limits, etc.).
> 
> Although I don't understand why you think that snd_pcm_avail is needed
> when we have the right (and safe) snd_pcm_avail_update, it can be
> implemented calling snd_pcm_delay (so to update hw pointer) and then
> computing the avail count using the mmap'ed counters.
> 
> Of course this can't work for chained PCM (and this is why we have
> snd_pcm_avail_update).
> 
> I think you should revert your patch (and to force yourself to believe a
> bit more in peer review before important changes although it seems I'm
> beating a dead horse here).

In this case, I propose to change snd_pcm_avail() to snd_pcm_hwsync() 
function with description: "synchronize r/w pointers with hardware". 
Really, after some thinking, the return value from snd_pcm_avail() cannot 
be used for nothing serious. I simply don't like that delay() functions 
do more arithmetic than necessary. Overdesign has been criticized in this 
list, too.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project  http://www.alsa-project.org
SuSE Linux    http://www.suse.com



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: PCM API update

[Abramo Bagnara]

Jaroslav Kysela wrote:
> 
> 
> In this case, I propose to change snd_pcm_avail() to snd_pcm_hwsync()
> function with description: "synchronize r/w pointers with hardware".
> Really, after some thinking, the return value from snd_pcm_avail() cannot
> be used for nothing serious. I simply don't like that delay() functions
> do more arithmetic than necessary. Overdesign has been criticized in this
> list, too.

Let examine kernel level: snd_pcm_{plaback,capture}_delay do a
snd_pcm_update_hw_ptr and a snd_pcm_{playback_hw,capture}_avail (that's
only a wrap safe difference).

Are you sure you want to add another ioctl and another API function in
all the PCM classes just to avoid a subtraction?

-- 
Abramo Bagnara                       mailto:abramo.bagnara@libero.it

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: PCM API update

[Jaroslav Kysela]

On Sat, 12 Oct 2002, Abramo Bagnara wrote:

> Jaroslav Kysela wrote:
> > 
> > 
> > In this case, I propose to change snd_pcm_avail() to snd_pcm_hwsync()
> > function with description: "synchronize r/w pointers with hardware".
> > Really, after some thinking, the return value from snd_pcm_avail() cannot
> > be used for nothing serious. I simply don't like that delay() functions
> > do more arithmetic than necessary. Overdesign has been criticized in this
> > list, too.
> 
> Let examine kernel level: snd_pcm_{plaback,capture}_delay do a
> snd_pcm_update_hw_ptr and a snd_pcm_{playback_hw,capture}_avail (that's
> only a wrap safe difference).
> 
> Are you sure you want to add another ioctl and another API function in
> all the PCM classes just to avoid a subtraction?

You've already noted that delay() expression might be more complex, so the 
code path eats more CPU than needed for sync operation.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project  http://www.alsa-project.org
SuSE Linux    http://www.suse.com



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: PCM API update

[Abramo Bagnara]

Jaroslav Kysela wrote:
> 
> On Sat, 12 Oct 2002, Abramo Bagnara wrote:
> 
> > Jaroslav Kysela wrote:
> > >
> > >
> > > In this case, I propose to change snd_pcm_avail() to snd_pcm_hwsync()
> > > function with description: "synchronize r/w pointers with hardware".
> > > Really, after some thinking, the return value from snd_pcm_avail() cannot
> > > be used for nothing serious. I simply don't like that delay() functions
> > > do more arithmetic than necessary. Overdesign has been criticized in this
> > > list, too.
> >
> > Let examine kernel level: snd_pcm_{plaback,capture}_delay do a
> > snd_pcm_update_hw_ptr and a snd_pcm_{playback_hw,capture}_avail (that's
> > only a wrap safe difference).
> >
> > Are you sure you want to add another ioctl and another API function in
> > all the PCM classes just to avoid a subtraction?
> 
> You've already noted that delay() expression might be more complex, so the
> code path eats more CPU than needed for sync operation.

Actually this might be true in future, although it's hard to know if
your proposal will worth that change. If you believe so, I've no further
objections.

-- 
Abramo Bagnara                       mailto:abramo.bagnara@libero.it

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf