Re: [PATCH - JACK IO plug 1/1] pcm: ioplug: Provide avail helper function for plugins

From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: twischer@de.adit-jv.com, patch@alsa-project.org, tiwai@suse.de
Cc: alsa-devel@alsa-project.org
Subject: Re: [PATCH - JACK IO plug 1/1] pcm: ioplug: Provide avail helper function for plugins
Date: Wed, 4 Jul 2018 09:34:47 +0900	[thread overview]
Message-ID: <6496aaee-6158-d880-f683-29dc1d926887@sakamocchi.jp> (raw)
In-Reply-To: <1530626363-8808-2-git-send-email-twischer@de.adit-jv.com>

Iwai-san,

On Jul 3 2018 22:59, twischer@de.adit-jv.com wrote:
> From: Timo Wischer <twischer@de.adit-jv.com>
> 
> This function can be called without calling snd_pcm_avail_update().
> 
> The call to snd_pcm_avail_update() can take some time.
> Therefore some developers would not like to call it from a real-time
> context (e.g. from JACK client context).
> 
> Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
> 
> diff --git a/include/pcm_ioplug.h b/include/pcm_ioplug.h
> index c1310e3..b16fc8b 100644
> --- a/include/pcm_ioplug.h
> +++ b/include/pcm_ioplug.h
> @@ -235,6 +235,9 @@ int snd_pcm_ioplug_set_param_list(snd_pcm_ioplug_t *io, int type, unsigned int n
>   int snd_pcm_ioplug_set_state(snd_pcm_ioplug_t *ioplug, snd_pcm_state_t state);
>   
>   /* calucalte the available frames */
> +snd_pcm_uframes_t snd_pcm_ioplug_avail(const snd_pcm_ioplug_t * const ioplug,
> +				       const snd_pcm_uframes_t hw_ptr,
> +				       const snd_pcm_uframes_t appl_ptr);
>   snd_pcm_uframes_t snd_pcm_ioplug_hw_avail(const snd_pcm_ioplug_t * const ioplug,
>   					  const snd_pcm_uframes_t hw_ptr,
>   					  const snd_pcm_uframes_t appl_ptr);
> diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c
> index 4d44ae2..6d52c27 100644
> --- a/src/pcm/pcm_ioplug.c
> +++ b/src/pcm/pcm_ioplug.c
> @@ -1221,6 +1221,21 @@ int snd_pcm_ioplug_set_state(snd_pcm_ioplug_t *ioplug, snd_pcm_state_t state)
>    * \param ioplug the ioplug handle
>    * \param hw_ptr hardware pointer in frames
>    * \param appl_ptr application pointer in frames
> + * \return available frames for the application
> + */
> +snd_pcm_uframes_t snd_pcm_ioplug_avail(const snd_pcm_ioplug_t * const ioplug,
> +				       const snd_pcm_uframes_t hw_ptr,
> +				       const snd_pcm_uframes_t appl_ptr)
> +{
> +	return __snd_pcm_avail(ioplug->pcm, hw_ptr, appl_ptr);
> +}
> +
> +/**
> + * \brief Get the available frames. This function can be used to calculate the
> + * the available frames before calling #snd_pcm_avail_update()
> + * \param ioplug the ioplug handle
> + * \param hw_ptr hardware pointer in frames
> + * \param appl_ptr application pointer in frames
>    * \return available frames for the hardware
>    */
>   snd_pcm_uframes_t snd_pcm_ioplug_hw_avail(const snd_pcm_ioplug_t * const ioplug,
> @@ -1230,8 +1245,9 @@ snd_pcm_uframes_t snd_pcm_ioplug_hw_avail(const snd_pcm_ioplug_t * const ioplug,
>   	/* available data/space which can be transferred by the user
>   	 * application
>   	 */
> -	const snd_pcm_uframes_t user_avail = __snd_pcm_avail(ioplug->pcm,
> -							     hw_ptr, appl_ptr);
> +	const snd_pcm_uframes_t user_avail = snd_pcm_ioplug_avail(ioplug,
> +								  hw_ptr,
> +								  appl_ptr);
>   
>   	if (user_avail > ioplug->pcm->buffer_size) {
>   		/* there was an Xrun */

I have a question about maintenance policy of this library to update 
version script for GNU Linker (ld) when introducing new symbols. The 
version script is not updated since node name as 'ALSA_0.9.7'. The newly
intduced symbol, 'snd_pcm_ioplug_hw_avail' is in node name of
'ALSA_0.9'.

```
$ readelf -s src/.libs/libasound.so.2.0.0 | grep snd_pcm_ioplug_hw_avail
   1254: 000000000009bfe0    40 FUNC    GLOBAL DEFAULT   13 
snd_pcm_ioplug_hw_avail@@ALSA_0.9
   3840: 000000000009bfe0    40 FUNC    GLOBAL DEFAULT   13 
snd_pcm_ioplug_hw_avail
```

The last node name is 'ALSA_1.1.6':

```
$ tail -n 10 src/Versions.in
   global:

     @SYMBOL_PREFIX@alsa_lisp_*;
} ALSA_0.9.5;

ALSA_1.1.6 {
   global:

     @SYMBOL_PREFIX@snd_dlopen;
} ALSA_0.9.7;
```

Practically this brings no issue, but theoretically the newly introduced 
symbol should be in a new node name next to ALSA_1.1.6. I'm not so 
strict in this point, but it's better to decide maintenance policy 
(because I've added some APIs in recent years).

Regards

Takashi Sakamoto