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

[Takashi Sakamoto]

Hi,

On Jul 4 2018 15:21, Takashi Iwai wrote:
> On Wed, 04 Jul 2018 02:34:47 +0200,
> Takashi Sakamoto wrote:
>>
>> Iwai-san,
>>
>> On Jul 3 2018 22:59, twischer at de.adit-jv.com wrote:
>>> From: Timo Wischer <twischer at 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 at 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 at alsa_lisp_*;
>> } ALSA_0.9.5;
>>
>> ALSA_1.1.6 {
>>    global:
>>
>>      @SYMBOL_PREFIX at 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).
> 
> Yes, that's an open question.
> 
> I myself am no big fan of the versioned symbols.  This has been a PITA
> for many applications.  The versioned smybols is useful only if the
> function signature may change.  But what's the difference from
> renaming the function, then?
> 
> So we've stopped putting the new symbols into the new versioned
> section; the snd_dlopen was an exception because it really changed the
> signature.

Nowadays I have no positive reason to use versioned symbols as you said.
I'm OK and thanks for your explanation.


Takashi Sakamoto