| Message ID | 1530626363-8808-2-git-send-email-twischer@de.adit-jv.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
On Wed, 04 Jul 2018 02:34:47 +0200, Takashi Sakamoto wrote: > > 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). 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. thanks, Takashi
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@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). > > 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
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 */