[alsa-devel] [PATCH] PulseAudio plugin: report XRUN state back to application
Hi!
Please consider merging this patch we ship in Fedora:
http://cvs.fedoraproject.org/viewcvs/*checkout*/devel/alsa-plugins/1.0.14-st...
It adds support to report back XRUN to the application if one happens. This is required to make some applications work on top of the pulse plugin. One being XMMS, which checks if a song finished to play by waiting for an XRUN (yes, I don't argue that XMMS shouldn't do that, but nonetheless it is a good thing if XRUNs are reported properly.)
Now, this patch is a bit problematic, since it touches pcm->io.state which is not supposed to be touched by ioplug-based plugins -- according to the comments in ioplug. However, I saw no other way to fix this, and from judging the ioplug code overwriting the variable should be safe. Thus I decided to simply touch the variable but documented it in a comment.
Alternatively, ioplug could be beefed up to properly handle XRUNs, but I thought this fix is simpler.
Also reported on the ALSA BTS:
https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3579
Lennart
At Sun, 25 Nov 2007 22:12:26 +0100, Lennart Poettering wrote:
Hi!
Please consider merging this patch we ship in Fedora:
http://cvs.fedoraproject.org/viewcvs/*checkout*/devel/alsa-plugins/1.0.14-st...
It adds support to report back XRUN to the application if one happens. This is required to make some applications work on top of the pulse plugin. One being XMMS, which checks if a song finished to play by waiting for an XRUN (yes, I don't argue that XMMS shouldn't do that, but nonetheless it is a good thing if XRUNs are reported properly.)
Now, this patch is a bit problematic, since it touches pcm->io.state which is not supposed to be touched by ioplug-based plugins -- according to the comments in ioplug. However, I saw no other way to fix this, and from judging the ioplug code overwriting the variable should be safe. Thus I decided to simply touch the variable but documented it in a comment.
Alternatively, ioplug could be beefed up to properly handle XRUNs, but I thought this fix is simpler.
But it's a layer violation :)
We need a proper state-notification function. It'd be simply setting the state in the end, but at least, it's clear and the ioplug can do any action or sanity-check if once needed.
A proposed patch is below. Comments?
Takashi
diff -r 672c5387645d include/pcm_ioplug.h --- a/include/pcm_ioplug.h Fri Nov 23 15:46:48 2007 +0100 +++ b/include/pcm_ioplug.h Mon Nov 26 11:32:10 2007 +0100 @@ -208,6 +208,9 @@ int snd_pcm_ioplug_set_param_minmax(snd_ int snd_pcm_ioplug_set_param_minmax(snd_pcm_ioplug_t *io, int type, unsigned int min, unsigned int max); int snd_pcm_ioplug_set_param_list(snd_pcm_ioplug_t *io, int type, unsigned int num_list, const unsigned int *list);
+/* change PCM status */ +int snd_pcm_ioplug_set_state(snd_pcm_ioplug_t *ioplug, snd_pcm_state_t state); + /** } */
#endif /* __ALSA_PCM_IOPLUG_H */ diff -r 672c5387645d src/pcm/pcm_ioplug.c --- a/src/pcm/pcm_ioplug.c Fri Nov 23 15:46:48 2007 +0100 +++ b/src/pcm/pcm_ioplug.c Mon Nov 26 11:32:10 2007 +0100 @@ -614,6 +614,8 @@ static snd_pcm_sframes_t snd_pcm_ioplug_ snd_pcm_uframes_t avail;
snd_pcm_ioplug_hw_ptr_update(pcm); + if (io->data->state == SNDRV_PCM_STATE_XRUN) + return -EPIPE; if (pcm->stream == SND_PCM_STREAM_CAPTURE && pcm->access != SND_PCM_ACCESS_RW_INTERLEAVED && pcm->access != SND_PCM_ACCESS_RW_NONINTERLEAVED) { @@ -1037,3 +1039,19 @@ const snd_pcm_channel_area_t *snd_pcm_io return snd_pcm_mmap_areas(ioplug->pcm); return NULL; } + +/** + * \brief Change the ioplug PCM status + * \param ioplug the ioplug handle + * \param state the PCM status + * \return zero if successful or a negative error code + * + * Changes the PCM status of the ioplug to the given value. + * This function can be used for external plugins to notify the status + * change, e.g. XRUN. + */ +int snd_pcm_ioplug_set_state(snd_pcm_ioplug_t *ioplug, snd_pcm_state_t state) +{ + ioplug->state = state; + return 0; +}
On Mon, 26.11.07 11:13, Takashi Iwai (tiwai@suse.de) wrote:
Alternatively, ioplug could be beefed up to properly handle XRUNs, but I thought this fix is simpler.
But it's a layer violation :)
We need a proper state-notification function. It'd be simply setting the state in the end, but at least, it's clear and the ioplug can do any action or sanity-check if once needed.
A proposed patch is below. Comments?
Looks good to me.
Lennart
participants (2)
-
Lennart Poettering
-
Takashi Iwai