pcm: clarify documentation of poll descriptor usage

[z-s-e]

This is based on my understanding of the intended behavior and the test/pcm.c example code.

There needs to be more clarifiaction regarding the exact semantics of the value of the revents output parameter of snd_pcm_poll_descriptors_revents, since there are events that do not necessarily correspond to POLLIN or POLLOUT (such as period events), but I believe this is a lot less obvious and needs confirmation first.

[borine]

I also have an interest in this: I would would like to see clarification of the permitted pfd array that may be passed to snd_pcm_poll_descriptors_revents(). Specifically, must it be the same array (ie the same number of fds in the same order) as was returned by snd_pcm_poll_descriptors(), or can the application pass any arbirary subset or re-ordering of the array? At present it is open to interpretation by the application author and the plugin author, with occasional unfortunate inconsistencies.

[perexg]

It is not allowed to change the returned pfds array. The snd_pcm_poll_descriptors is just like an "allocator" and snd_pcm_poll_descriptors_revents is demangling the result from poll().

Which plugins do have bad interpretation?

[perexg]

To clarify: The pfds pointer may change, but the file descriptor order (fd and events members from struct pollfd) must follow the order returned from snd_pcm_poll_descriptors. E.g. you can copy those structures directly to bigger struct pollfd array or point to a fixed location in this bigger array for snd_pcm_poll_descriptors.

[perexg]

The ordering is visible in the hw plugin: https://github.com/search?q=repo%3Aalsa-project%2Falsa-lib%20%20snd_pcm_hw_poll_revents&type=code

[z-s-e]

The pipewire alsa plugin initially interpreted the snd_pcm_poll_descriptors usage differently - I reported it and it got fixed, but I believe this demonstrated the need for official clarification. They had a point when they assumed you would have to do the snd_pcm_poll_descriptors call before every poll to make sure nothing changed.

Wrt the snd_pcm_poll_descriptors_revents argument discussion from borine, I agree that this is another point that would deserve to be clarified in the documentation.

[borine]

Which plugins do have bad interpretation?

This point arose quite a long time ago now, when trying to debug an issue using the BlueALSA plugin (https://github.com/arkq/bluez-alsa) with MPD (https://github.com/MusicPlayerDaemon/MPD). It turned out that MPD breaks up the pfd array in order to use epoll() rather than poll(), but then constructs a new array from the epoll() result which contains only those fds on which an event is ready, and in a different order. This is of no consequence with the hw plugin which has a pfd array size of 1, but completely messed up the bluealsa plugin which has multiple fds in its pfd array, On the MPD side, the author maintained that the ALSA documentation does not forbid this so it must be the plugin's responsibility to deal with it, whereas the BlueALSA author maintained that the bluealsa plugin should expect to receive back the same array, just updated by poll(). I just think that such situations would be avoided if the ALSA documentation was more explicit about what the actual constraints are. (Sorry to steal this PR for my own concerns, just thought it might be an opportunity to get some extra text into the docs while they are being changed anyway).

[perexg]

Actually, those descriptors won't change after snd_pcm_open call, but it would be good to recommend to fetch them when all stream parameters are set (after snd_pcm_hw_params / snd_pcm_prepare) - stream is in SND_PCM_STATE_PREPARED state.

[z-s-e]

I don't quite understand why you say "recommend". Shouldn't the documentation clearly state what the user must do for correct operation? So either it is valid to fetch the descriptors once for a pcm device (then it makes no sense to recommend anything else) or you need to update them whenever the hardware configuration changes, but then it is not a recommendation but a requirement for correct behavior.

[z-s-e]

Hm, I suppose what you mean is "when you don't follow the recommendation, it may or may not work correctly for your hardware/plugin", so it is just your manner of speaking and I misunderstood that. Though I believe it is sufficient that the API simply describes its safe correct usage, as it is obvious that any deviation from it may run into issues. In my eyes a "recommendation" can be interpreted as not really binding when it comes to being clear about what is a bug on the user's side vs a bug in the driver/plugin.

[perexg]

It is a recommendation for possible future extensions. The current requirement is to fetch file descriptors at any time from the valid PCM handle. They won't be changed in the current plugins (but I cannot speak for the plugins outside ALSA repos). It was most probably the reason why we don't write much about this when the APIs were designed.

We should not probably complicate things so much - if we have users or requirements in future, we can probably update APIs. So drop this idea (for prepare state).

[z-s-e]

Agreed. If you decide to rework the APIs in the future, I think it is better to add new methods and keep the old the same to keep backwards compatibility and not break old application code. Anyway, for now just documenting correct usage should be fine.

[perexg]

The revents function description should be extended that the caller should KEEP the file descriptor order in array as returned poll_descriptors as discussed.

[z-s-e]

Just to confirm, is it just the order that must be kept for the array? Say, snd_pcm_poll_descriptors returned an array of 3 pfds {pfd1, pfd2, pfd3}, is it allowed to pass snd_pcm_poll_descriptors_revents a subset array of only {pfd1, pfd3} (given that their relative order is kept)? Or must the array argument be a continuous sub-range (or even a full copy) of what snd_pcm_poll_descriptors returned?

[perexg]

No order, count or index changes are allowed in the descriptor array.