[PATCH] ALSA: pcm: Unify snd_pcm_delay() and snd_pcm_hwsync()

[Takashi Iwai]

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 at 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;
> >  
> > 
>