Bug 76413 - arts does not follow ALSA API
Summary: arts does not follow ALSA API
Status: RESOLVED FIXED
Alias: None
Product: arts
Classification: Miscellaneous
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Stefan Westerfeld
URL:
Keywords:
: 76521 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-02-29 15:17 UTC by Jaroslav Kysela
Modified: 2004-04-02 14:37 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Kysela 2004-02-29 15:17:22 UTC
Version:           all (using KDE Devel)
Installed from:    Compiled sources
OS:          Linux

Subject: Arts audioio design is broken

Hi,

        I would like to notify you that arts AudioIO design regarding
the select/poll is broken for some ALSA's plugins, because write direction
does not imply POLLOUT (or writefds for select semantics) and versa vice
for read direction. Also, we might "watch" more filedescriptors than only
one in the future.

We have these three functions:

int snd_pcm_poll_descriptors_count(snd_pcm_t *pcm);
int snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds,
                             unsigned int space);
int snd_pcm_poll_descriptors_revents(snd_pcm_t *pcm, struct pollfd *pfds,
                                     unsigned int nfds, unsigned short *revents);

All three must be used for successfull usage of select() or poll() in case
of ALSA. Note that the revents function will return the correct revents
(poll semantics as you expect, but it's necessary to not look into pfds
contents directly).

Could you redesign your classes? Or will you (at least) accept our
patches? Unfortunately, I don't have so much time for this job and I
think, you will do these changes much more quickly.

Test case: Try use arts with 'plug:dmix' output device (ALSA). It hangs.

                                                Jaroslav
Comment 1 Tommi Tervo 2004-03-01 19:58:59 UTC
*** Bug 76521 has been marked as a duplicate of this bug. ***
Comment 2 Allan Sandfeld 2004-03-03 13:13:12 UTC
We would be happy to accept your patches. I've started to work on this bug, but I know neither ALSA nor much of aRts.

Currently the ALSA-plugin in aRts only calls snd_pcm_poll_descriptors, and uses the first returned fd there for later use in select. If I understand you correctly we need to be able to handle multiple file-descriptors, and can't be garantied that they have the right direction?

Now how do we tell which of the returned descriptors are writefds and which are readfds? The only solution I can think of right now is to checks the revents and hope all the writable ones are already in a POLLOUT state.

I am also worried about the ALSA-documentation which states the fds returned by snd_pcm_poll_descriotors might be bogus. 
Comment 3 Jaroslav Kysela 2004-03-03 13:58:23 UTC
Yes, the comment is not clear. I meant that the values are bugs from the application perspective (POLLIN does not means read direction and versa vice).

The values from snd_pcm_poll_descriptors() are supposed to be a direct input
for poll() syscall - but you may convert fd.events value to your layer, too.

Please, don't forget to "demangle" the revents with the snd_pcm_poll_descriptors_revents() function which returns the correct
mask (POLLIN = capture - read, POLLOUT = playback - write).

Comment 4 Allan Sandfeld 2004-03-19 14:50:04 UTC
Okay, having researched the problem. The reason that dmix doesnt work with aRts has nothing to do with how we use the alsa API because all our assumptions are still true. snd_pcm_poll_descriptors() returns exactly one descriptor, but calling select upon this descriptor no matter how it is done will _never_ return. In other words the reason it doesnt work is that dmix is broken!

I've looked a bit at how other projects use ALSA, and it seems we could make it work if we used another API and disregarded the broken descriptors alltogether, in other words if we rewrite the entire audio-subsystem of aRts to work-around a bug in ALSA (I have no intention of doing so).

Btw. I did make playback over dmix work, but the it required using timing events rather than IO-events and thus CPU-usage was unacceptable.
Comment 5 Matthieu Robin 2004-03-19 15:26:40 UTC
But don't you think that having arts outputing to dmix is a little stupid ? You get two mixers...

imho the best solution is to let kde apps directly output to alsa.

But there is definitely a big issue here : we finally have a real sound system that give us a working mix, whatever the soundcard is, and kde is the only app I know who can't use this system : I tried mplayer, xine, every sdl-based apps, and more, only arts fails :-( .

Currently, the best solution I found is to execute a "aplay /usr/share/sound/..." command for every knotify event... A little boring, isn't it ?
Comment 6 Allan Sandfeld 2004-03-19 16:05:27 UTC
CVS commit by carewolf: 

After venting my fury I calmed down and looked at the dmix-code. It seems 
it is really more borken than broken, and nothing we can't work around.

- Make aRts work with dmix. Two important changes:
  1. Dmix uses read-events to signify write-events, so we need to autodetect 
     what kind of crack ALSA is smoking today.
  2. Dmix needs to be started like a recorder even if this is contrary
     to the ALSA API documentation.

If everything works like expected I will backport and close bugs 76413 and 70802.

CCMAIL: 76413@bugs.kde.org
CCMAIL: 70802@bugs.kde.org


  M +110 -48   audioioalsa9.cc   1.6



Comment 7 Jaroslav Kysela 2004-03-19 19:53:12 UTC
Alan,

      my request was polite and you're trying to move this conversation to totaly another point. If you see my original request, we have covered in our API, how the select and poll calls must behave to follow ALSA API. Again, if you don't use our functions and add a hybrid code to your application, then other "future" plugins or ALSA code might not work. Even your assumption - that only one event descriptor is present - is not true. Yes, current ALSA code has only one descriptor, but I can imagine that several descriptor might be used in future. You're trying to simplify your work, but that's bad.
     Anyway, using arts as the common KDE sound API is bad in my eyes. A thin code should be designed which can redirect calls to any sound HAL. A daemon which eats about 6MB of memory is not thin at all.

                                            Jaroslav
Comment 8 Allan Sandfeld 2004-03-19 20:33:51 UTC
I was not trying to move the conversion, but I excuse for my use of expressions. I had spend quite some time following you guides and trying to fix aRts this way, but unsuccessfully, so when I finally narrowed the problem down to being an unresponse fd, due to dmix abusing the API, I was slightly pissed.

I know that the API allows multiple descriptors and so do my version of aRts now, it just doesnt make any difference when it comes to the dmix problem (or any problem). The issue with dmix is, that it uses a fake file-descriptor which is really a timer set to interrupt too often. This descriptor issues POLLIN events rather than POLLOUT. What needs to be described in your documentation, and what I asked about in my second question, is that the intentioned direction of the descriptors can be found in pfd->events. Something I had to look into dmix code to realize.

Now that I use the direction of the select correctly, the sound-system works.  Although on my system, aRts through dmix now uses close to 90% CPU-time. So I am working on adding even more workaround for dmix, because you not only had the crazy idea of using a timer as a fd, but sat it to interrupt _way_ too often (about 4000 times too often). Perhaps I can persuade you to add a TODO-item about moving dmix to use a limited buffer with a file-descriptor sometime in the future? Just so that it acts as one would expect from its API.

And finally we are all hoping for aRts to die soon, it just needs a proper replacement.
Comment 9 Jaroslav Kysela 2004-03-19 20:54:08 UTC
About the POLLIN problem: Even the function name (snd_pcm_poll_descriptors) tells you, that you can expect the input for the poll function (thus fds and events). If you have any better description, I will add it to our code. Note that the "demangling" of revents does the snd_pcm_poll_descriptors_revents which returns POLLOUT _correctly_ even for the dmix plugin. Yes, I know, you need to convert pfd->events to another layer and put back the result from this layer to pfd->revents, but if you use directly poll() then you don't need to care.

The CPU usage problem: You've hit probably a bug in our rate resampling code which was fixed a few days ago in our CVS (you may try to force dmix pcm to same rate as arts uses for your tests). The dmix's slave timer uses the master PCM stream as clock, thus it is clocked with the period time.

Thanks for taking care about this problem.
Comment 10 Allan Sandfeld 2004-04-02 13:25:32 UTC
CVS commit by carewolf: 

Follow the ALSA API to support future versions of ALSA.

Note by the way, that the returned file-descriptor in the case of Dmix does 
not work together with select() but only poll()...

CCMAIL: 76413-done@bugs.kde.org


  M +48 -29    audioioalsa9.cc   1.10



Comment 11 Jaroslav Kysela 2004-04-02 14:37:50 UTC
It's not quite correct. You may also use select() without problems.

You must translate the ufds to fd_set before select() call and when
the call is finished, translate back fd_set to ufds, so snd_pcm_poll_descriptors_revents() can be called.

See: alsa-oss package: alsa-oss/alsa/pcm.c - lib_oss_pcm_select_prepare() and lib_oss_pcm_select_result() for hints.

But select() is obsolete and should be replaced with poll() anyway (kernel always emulates the select syscall on top of the poll callbacks).