Re: [RFC - AAF PCM plugin 3/5] aaf: Implement Playback mode support

["Guedes, Andre" <andre.guedes@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3860 bytes --]

Hi Pierre,

On Tue, 2018-08-21 at 17:51 -0500, Pierre-Louis Bossart wrote:
> > > > +static int aaf_mclk_start_playback(snd_pcm_aaf_t *aaf)
> > > > +{
> > > > +	int res;
> > > > +	struct timespec now;
> > > > +	struct itimerspec itspec;
> > > > +	snd_pcm_ioplug_t *io = &aaf->io;
> > > > +
> > > > +	res = clock_gettime(CLOCK_REF, &now);
> > > > +	if (res < 0) {
> > > > +		SNDERR("Failed to get time from clock");
> > > > +		return -errno;
> > > > +	}
> > > > +
> > > > +	aaf->mclk_period = (NSEC_PER_SEC * aaf-
> > > > >frames_per_pkt) /
> > > > io->rate;
> > > 
> > > is this always an integer? If not, don't you have a systematic
> > > arithmetic error?
> > 
> > NSEC_PER_SEC is 64-bit so I don't see an arithmetic error during
> > calculation (e.g. integer overflow). Not sure this was your
> > concern,
> > though. Let me know otherwise.
> 
> No, I was talking about the fractional part, e.g with 256 frames
> with 
> 44.1kHz you have a period of 5804988.662131519274376 - so your math
> adds 
> a truncation. same with 48khz, the fractional part is .333
> 
> I burned a number of my remaining neurons chasing a <100 ppb error
> which 
> led to underruns after 10 hours, so careful now with truncation...

Thanks for clarifying.

Yes, we can end up having a fractional period which is truncated. Note
that both 'frames' and 'rate' are configured by the user. The user
should set 'frames' as multiple of 'rate' whenever possible to avoid
inaccuracy.

From the plugin perspective, I'm not sure what we could do. Truncating
might lead to underruns as you said, but I'm afraid that rounding up
might lead to overruns, theoretically.

> > 
> > > > +static int aaf_poll_revents(snd_pcm_ioplug_t *io, struct
> > > > pollfd
> > > > *pfd,
> > > > +			    unsigned int nfds, unsigned short
> > > > *revents)
> > > > +{
> > > > +	int res;
> > > > +	snd_pcm_aaf_t *aaf = io->private_data;
> > > > +
> > > > +	if (nfds != FD_COUNT_PLAYBACK)
> > > > +		return -EINVAL;
> > > > +
> > > > +	if (pfd[0].revents & POLLIN) {
> > > > +		res = aaf_mclk_timeout_playback(aaf);
> > > > +		if (res < 0)
> > > > +			return res;
> > > > +
> > > > +		*revents = POLLIN;
> > > > +	}
> > > 
> > > I couldn't figure out how you use playback events and your timer.
> > 
> > Every time aaf->timer_fd expires, the audio buffer is consumed by
> > the
> > plugin, making some room available on the buffer. So here a POLLIN
> > event is returned so alsa-lib layer can copy more data into the
> > audio
> > buffer.
> > 
> > > When there are two audio clock sources or timers that's usually
> > > where
> > > the fun begins.
> > 
> > Regarding scenarios with two audio clock sources or timers, the
> > plugin
> > doesn't support them at the moment. This is something we should
> > work on
> > once the basic functionality is pushed upstream.
> 
> I was talking about adjusting the relationship between your 
> CLOCK_REALTIME timer and the media/network clock. I don't quite get
> how 
> this happens, I vaguely recall there should be a daemon which tracks
> the 
> difference between local and media/network clock, and I don't see it
> here.

Oh okay, I thought you were talking about something else :)

I believe you are referring to the gptp daemon from Openavnu [1]. The
AAF plugin doesn't use it. Instead, it uses linuxptp [2] which is
distributed by several Linux distros.

Linuxptp provides the phc2sys daemon that synchronizes both system
clock (i.e. CLOCK_REALTIME) and network clock (i.e. PTP clock). The
daemon disciplines the clocks instead of providing the time difference
to applications. So we don't need to do any cross-timestamping at the
plugin.

Regards,

Andre

[1] https://github.com/AVnu/OpenAvnu/tree/master/daemons/gptp
[2] http://linuxptp.sourceforge.net/

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]