Re: [PATCH v5 1/2] ALSA: pcm: reinvent the stream synchronization ID API

[Jaroslav Kysela <perex@xxxxxxxx>]

On 24. 06. 24 15:38, Amadeusz Sławiński wrote:
> On 6/24/2024 3:07 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.
> >
> > Note that the contents of ID is not supposed to be used for direct
> > comparison with a specific byte sequence. The "empty" case is when
> > all bytes are zero (driver does not offer this information)
> > and all other cases must be only used for equal comparison among
> > PCM streams (including different sound cards) if they are using
> > identical hardware clock.
> >
> > Cc: Takashi Sakamoto <takaswie@xxxxxxxxxx>
> > Signed-off-by: Jaroslav Kysela <perex@xxxxxxxx>
> > ---
>
> (...)
>
> >    
> > @@ -420,7 +420,8 @@ struct snd_pcm_hw_params {
> >    	unsigned int rate_num;		/* R: rate numerator */
> >    	unsigned int rate_den;		/* R: rate denominator */
> >    	snd_pcm_uframes_t fifo_size;	/* R: chip FIFO size in frames */
> > -	unsigned char reserved[64];	/* reserved for future */
> > +	unsigned char sync[16];		/* R: synchronization ID (perfect sync - one clock source) */
>
> If it is introduced as new API, can't this be done better? Maybe like:
> struct sync {
> 	char cardnum[4];
> 	char id[12];
> };
> or maybe just:
> struct sync {
> 	u32 cardnum;
> 	char id[12];
> };
> or something like that? It is bit hard to follow in next patch all this
> params->sync + 4 and memset/strncpy. And having named fields would help.0

The ID may be not related to one sound card. Multiple cards can share one 
clock source. It's internal kernel ID generation scheme which may be changed 
later when there's another demand in future. The applications should handle 
this as 16-byte blob which is used only for equal comparison among multiple 
PCM streams.

					Jaroslav

--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.