Re: [PATCH 1/2] pcm_pulse: do not trigger immediately at start

[David Henningsson <david.henningsson@canonical.com>]

Hi Remi, and thanks for your patch, this is indeed a thorny area where 
you change behaviour and fix one application and break another...that 
has at least been my experience with the alsa-pulse plugin...

On 11/14/2012 07:07 PM, Rémi Denis-Courmont wrote:
> There are no samples to play at that point so this is useless.

I'm not following this reasoning. The start callback corresponds to the 
_trigger callback in kernel, which wants the playback to start immediately.

For playback streams, you should not call the start callback (or any 
alsa-lib function that ends up calling it) from your application unless 
there are already samples in the buffer, or you will get an immediate 
underrun.

For recording streams, starting with an empty buffer is expected and not 
useless at all.

> The non-zero buffer_attr.prebuf ensures that the PulseAudio daemon will
> start streaming automatically (like ALSA sw_params start_threshold).

The start callback is for the cases where the application wants to 
explicitly start the stream before prebuf has been reached.

> ---
>   pulse/pcm_pulse.c |   12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/pulse/pcm_pulse.c b/pulse/pcm_pulse.c
> index 24fd4da..0165120 100644
> --- a/pulse/pcm_pulse.c
> +++ b/pulse/pcm_pulse.c
> @@ -204,8 +204,8 @@ static void stream_success_cb(pa_stream * p, int success, void *userdata)
>   static int pulse_start(snd_pcm_ioplug_t * io)
>   {
>   	snd_pcm_pulse_t *pcm = io->private_data;
> -	pa_operation *o, *u;
> -	int err = 0, err_o = 0, err_u = 0;
> +	pa_operation *o;
> +	int err = 0, err_o = 0;
>
>   	assert(pcm);
>
> @@ -224,18 +224,12 @@ static int pulse_start(snd_pcm_ioplug_t * io)
>   		goto finish;
>   	}
>
> -	u = pa_stream_trigger(pcm->stream, stream_success_cb, pcm);
> -
>   	pcm->underrun = 0;
>   	err_o = pulse_wait_operation(pcm->p, o);
> -	if (u)
> -		err_u = pulse_wait_operation(pcm->p, u);
>
>   	pa_operation_unref(o);
> -	if (u)
> -		pa_operation_unref(u);
>
> -	if (err_o < 0 || err_u < 0) {
> +	if (err_o < 0) {
>   		err = -EIO;
>   		goto finish;
>   	}
>



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic