* [PATCH] ALSA: pcm: Unify snd_pcm_delay() and snd_pcm_hwsync()
@ 2021-10-14 14:53 Takashi Iwai
2021-10-14 15:59 ` Pierre-Louis Bossart
0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2021-10-14 14:53 UTC (permalink / raw)
To: alsa-devel
Both snd_pcm_delay() and snd_pcm_hwsync() do the almost same thing.
The only difference is that the former calculate the delay, so unify
them as a code cleanup, and treat NULL delay argument only for hwsync
operation.
Also, the patch does a slight code refactoring in snd_pcm_delay().
The initialization of the delay value is done in the caller side now.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/core/pcm_native.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 46c643db18eb..627b201b6084 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2932,32 +2932,24 @@ static snd_pcm_sframes_t snd_pcm_forward(struct snd_pcm_substream *substream,
return ret;
}
-static int snd_pcm_hwsync(struct snd_pcm_substream *substream)
-{
- int err;
-
- snd_pcm_stream_lock_irq(substream);
- err = do_pcm_hwsync(substream);
- snd_pcm_stream_unlock_irq(substream);
- return err;
-}
-
static int snd_pcm_delay(struct snd_pcm_substream *substream,
snd_pcm_sframes_t *delay)
{
int err;
- snd_pcm_sframes_t n = 0;
snd_pcm_stream_lock_irq(substream);
err = do_pcm_hwsync(substream);
- if (!err)
- n = snd_pcm_calc_delay(substream);
+ if (delay && !err)
+ *delay = snd_pcm_calc_delay(substream);
snd_pcm_stream_unlock_irq(substream);
- if (!err)
- *delay = n;
return err;
}
+static inline int snd_pcm_hwsync(struct snd_pcm_substream *substream)
+{
+ return snd_pcm_delay(substream, NULL);
+}
+
static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream,
struct snd_pcm_sync_ptr __user *_sync_ptr)
{
@@ -3275,7 +3267,7 @@ static int snd_pcm_common_ioctl(struct file *file,
return snd_pcm_hwsync(substream);
case SNDRV_PCM_IOCTL_DELAY:
{
- snd_pcm_sframes_t delay;
+ snd_pcm_sframes_t delay = 0;
snd_pcm_sframes_t __user *res = arg;
int err;
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: pcm: Unify snd_pcm_delay() and snd_pcm_hwsync()
2021-10-14 14:53 [PATCH] ALSA: pcm: Unify snd_pcm_delay() and snd_pcm_hwsync() Takashi Iwai
@ 2021-10-14 15:59 ` Pierre-Louis Bossart
2021-10-14 16:08 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Pierre-Louis Bossart @ 2021-10-14 15:59 UTC (permalink / raw)
To: Takashi Iwai, alsa-devel
On 10/14/21 9:53 AM, Takashi Iwai wrote:
> Both snd_pcm_delay() and snd_pcm_hwsync() do the almost same thing.
> The only difference is that the former calculate the delay, so unify
> them as a code cleanup, and treat NULL delay argument only for hwsync
> operation.
>
> Also, the patch does a slight code refactoring in snd_pcm_delay().
> The initialization of the delay value is done in the caller side now.
I would have done the opposite change, i.e. keep snd_pcm_hwsync() but
add an optional delay argument/calculation.
'snd_pcm_delay' doesn't really hint at any hwsync operation.
Just a naming difference really.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> sound/core/pcm_native.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 46c643db18eb..627b201b6084 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -2932,32 +2932,24 @@ static snd_pcm_sframes_t snd_pcm_forward(struct snd_pcm_substream *substream,
> return ret;
> }
>
> -static int snd_pcm_hwsync(struct snd_pcm_substream *substream)
> -{
> - int err;
> -
> - snd_pcm_stream_lock_irq(substream);
> - err = do_pcm_hwsync(substream);
> - snd_pcm_stream_unlock_irq(substream);
> - return err;
> -}
> -
> static int snd_pcm_delay(struct snd_pcm_substream *substream,
> snd_pcm_sframes_t *delay)
> {
> int err;
> - snd_pcm_sframes_t n = 0;
>
> snd_pcm_stream_lock_irq(substream);
> err = do_pcm_hwsync(substream);
> - if (!err)
> - n = snd_pcm_calc_delay(substream);
> + if (delay && !err)
> + *delay = snd_pcm_calc_delay(substream);
> snd_pcm_stream_unlock_irq(substream);
> - if (!err)
> - *delay = n;
> return err;
> }
>
> +static inline int snd_pcm_hwsync(struct snd_pcm_substream *substream)
> +{
> + return snd_pcm_delay(substream, NULL);
> +}
> +
> static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream,
> struct snd_pcm_sync_ptr __user *_sync_ptr)
> {
> @@ -3275,7 +3267,7 @@ static int snd_pcm_common_ioctl(struct file *file,
> return snd_pcm_hwsync(substream);
> case SNDRV_PCM_IOCTL_DELAY:
> {
> - snd_pcm_sframes_t delay;
> + snd_pcm_sframes_t delay = 0;
> snd_pcm_sframes_t __user *res = arg;
> int err;
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: pcm: Unify snd_pcm_delay() and snd_pcm_hwsync()
2021-10-14 15:59 ` Pierre-Louis Bossart
@ 2021-10-14 16:08 ` Takashi Iwai
2021-10-14 16:19 ` Pierre-Louis Bossart
0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2021-10-14 16:08 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: alsa-devel
On Thu, 14 Oct 2021 17:59:21 +0200,
Pierre-Louis Bossart wrote:
>
>
>
> On 10/14/21 9:53 AM, Takashi Iwai wrote:
> > Both snd_pcm_delay() and snd_pcm_hwsync() do the almost same thing.
> > The only difference is that the former calculate the delay, so unify
> > them as a code cleanup, and treat NULL delay argument only for hwsync
> > operation.
> >
> > Also, the patch does a slight code refactoring in snd_pcm_delay().
> > The initialization of the delay value is done in the caller side now.
>
> I would have done the opposite change, i.e. keep snd_pcm_hwsync() but
> add an optional delay argument/calculation.
>
> 'snd_pcm_delay' doesn't really hint at any hwsync operation.
>
> Just a naming difference really.
Yes, but also the difference of the number of arguments. Changing
other way round would need to more modifications in the end.
Which calls what is rather an implementation detail :)
Takashi
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> > sound/core/pcm_native.c | 24 ++++++++----------------
> > 1 file changed, 8 insertions(+), 16 deletions(-)
> >
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index 46c643db18eb..627b201b6084 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -2932,32 +2932,24 @@ static snd_pcm_sframes_t snd_pcm_forward(struct snd_pcm_substream *substream,
> > return ret;
> > }
> >
> > -static int snd_pcm_hwsync(struct snd_pcm_substream *substream)
> > -{
> > - int err;
> > -
> > - snd_pcm_stream_lock_irq(substream);
> > - err = do_pcm_hwsync(substream);
> > - snd_pcm_stream_unlock_irq(substream);
> > - return err;
> > -}
> > -
> > static int snd_pcm_delay(struct snd_pcm_substream *substream,
> > snd_pcm_sframes_t *delay)
> > {
> > int err;
> > - snd_pcm_sframes_t n = 0;
> >
> > snd_pcm_stream_lock_irq(substream);
> > err = do_pcm_hwsync(substream);
> > - if (!err)
> > - n = snd_pcm_calc_delay(substream);
> > + if (delay && !err)
> > + *delay = snd_pcm_calc_delay(substream);
> > snd_pcm_stream_unlock_irq(substream);
> > - if (!err)
> > - *delay = n;
> > return err;
> > }
> >
> > +static inline int snd_pcm_hwsync(struct snd_pcm_substream *substream)
> > +{
> > + return snd_pcm_delay(substream, NULL);
> > +}
> > +
> > static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream,
> > struct snd_pcm_sync_ptr __user *_sync_ptr)
> > {
> > @@ -3275,7 +3267,7 @@ static int snd_pcm_common_ioctl(struct file *file,
> > return snd_pcm_hwsync(substream);
> > case SNDRV_PCM_IOCTL_DELAY:
> > {
> > - snd_pcm_sframes_t delay;
> > + snd_pcm_sframes_t delay = 0;
> > snd_pcm_sframes_t __user *res = arg;
> > int err;
> >
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: pcm: Unify snd_pcm_delay() and snd_pcm_hwsync()
2021-10-14 16:08 ` Takashi Iwai
@ 2021-10-14 16:19 ` Pierre-Louis Bossart
0 siblings, 0 replies; 4+ messages in thread
From: Pierre-Louis Bossart @ 2021-10-14 16:19 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On 10/14/21 11:08 AM, Takashi Iwai wrote:
> On Thu, 14 Oct 2021 17:59:21 +0200,
> Pierre-Louis Bossart wrote:
>>
>>
>>
>> On 10/14/21 9:53 AM, Takashi Iwai wrote:
>>> Both snd_pcm_delay() and snd_pcm_hwsync() do the almost same thing.
>>> The only difference is that the former calculate the delay, so unify
>>> them as a code cleanup, and treat NULL delay argument only for hwsync
>>> operation.
>>>
>>> Also, the patch does a slight code refactoring in snd_pcm_delay().
>>> The initialization of the delay value is done in the caller side now.
>>
>> I would have done the opposite change, i.e. keep snd_pcm_hwsync() but
>> add an optional delay argument/calculation.
>>
>> 'snd_pcm_delay' doesn't really hint at any hwsync operation.
>>
>> Just a naming difference really.
>
> Yes, but also the difference of the number of arguments. Changing
> other way round would need to more modifications in the end.
Ah yes, makes sense
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-14 16:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-14 14:53 [PATCH] ALSA: pcm: Unify snd_pcm_delay() and snd_pcm_hwsync() Takashi Iwai
2021-10-14 15:59 ` Pierre-Louis Bossart
2021-10-14 16:08 ` Takashi Iwai
2021-10-14 16:19 ` Pierre-Louis Bossart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.