Re: [PATCH] ALSA: pcm: reinvent the stream synchronization ID API

[Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>]

On 4/30/2024 6:10 PM, Jaroslav Kysela wrote:
> Until the commit e11f0f90a626 ("ALSA: pcm: remove SNDRV_PCM_IOCTL1_INFO
> internal command"), there was a possibility to pass information
> about the synchronized streams to the user space. The mentioned
> commit removed blindly the appropriate code with an irrelevant comment.
>
> The revert may be appropriate, but since this API was lost for several
> years without any complains, it's time to improve it. The hardware
> parameters may change the used stream clock source (e.g. USB hardware)
> so move this synchronization ID to hw_params as read-only field.
>
> It seems that pipewire can benefit from this API (disable adaptive
> resampling for perfectly synchronized PCM streams) now.
>
> Cc: Takashi Sakamoto <takaswie@xxxxxxxxxx>
> Signed-off-by: Jaroslav Kysela <perex@xxxxxxxx>
> ---
>   include/sound/pcm.h         |  9 +++++++++
>   include/uapi/sound/asound.h |  8 +++++---
>   sound/core/pcm_lib.c        | 13 +++++++++++++
>   sound/core/pcm_native.c     |  6 ++++++
>   4 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 210096f124ee..f716e0d0d174 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -93,6 +93,7 @@ struct snd_pcm_ops {
>   #define SNDRV_PCM_IOCTL1_CHANNEL_INFO	2
>   /* 3 is absent slot. */
>   #define SNDRV_PCM_IOCTL1_FIFO_SIZE	4
> +#define SNDRV_PCM_IOCTL1_SYNC_ID	5
>   
>   #define SNDRV_PCM_TRIGGER_STOP		0
>   #define SNDRV_PCM_TRIGGER_START		1
> @@ -1565,6 +1566,14 @@ static inline u64 pcm_format_to_bits(snd_pcm_format_t pcm_format)
>   	     (__force int)(f) <= (__force int)SNDRV_PCM_FORMAT_LAST;	\
>   	     (f) = (__force snd_pcm_format_t)((__force int)(f) + 1))
>   
> +/**
> + * is sync id (clock id) empty?
> + */
> +static inline bool pcm_sync_empty(union snd_pcm_sync_id *sync)
> +{
> +	return sync->id64[0] == 0 && sync->id64[1] == 0;

Can you use SNDRV_PCM_STREAM_PLAYBACK and SNDRV_PCM_STREAM_CAPTURE here, 
for someone not familiar with ALSA code above will be confusing.

> +}
> +
>   /* printk helpers */
>   #define pcm_err(pcm, fmt, args...) \
>   	dev_err((pcm)->card->dev, fmt, ##args)
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index 628d46a0da92..13f8a08f3f33 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -142,7 +142,7 @@ struct snd_hwdep_dsp_image {
>    *                                                                           *
>    *****************************************************************************/
>   
> -#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 17)
> +#define SNDRV_PCM_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 18)
>   
>   typedef unsigned long snd_pcm_uframes_t;
>   typedef signed long snd_pcm_sframes_t;
> @@ -334,6 +334,7 @@ union snd_pcm_sync_id {
>   	unsigned char id[16];
>   	unsigned short id16[8];
>   	unsigned int id32[4];
> +	__u64 id64[2];

Similarly here, can SNDRV_PCM_STREAM_LAST be used?