Re: [PATCH] [RFC] alsaloop: add feedback frequency control support for UAC2 gadgets

On Thu 12 Nov 2020 at 23:34, Ruslan Bilovol ruslan.bilovol@gmail.com wrote:

Add support of special "PCM Feedback Frequency Hz" UAC2 Gadget mixer control that is designed to notify host about real sampling frequency of the gadget so it can adjust number of samples that hosts sends to the gadget.

This is useful if both host and gadget has its own internal freerunning clock, so host can adjust number of samples sent, preventing overrun/underrun conditions.

This patch reuses logic of the "PCM Rate Shift 100000" control used in case of in-kernel ALSA loopback driver. The only difference is alsaloop reports not rate shift but frequency in Hz

Signed-off-by: Ruslan Bilovol ruslan.bilovol@gmail.com

alsaloop/alsaloop.h | 1 + alsaloop/pcmjob.c | 35 +++++++++++++++++++++++++---------- 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/alsaloop/alsaloop.h b/alsaloop/alsaloop.h index c4aa618..9a50a42 100644 --- a/alsaloop/alsaloop.h +++ b/alsaloop/alsaloop.h @@ -122,6 +122,7 @@ struct loopback_handle { unsigned int ctl_pollfd_count; snd_ctl_elem_value_t *ctl_notify; snd_ctl_elem_value_t *ctl_rate_shift;

• snd_ctl_elem_value_t *ctl_fback_freq; snd_ctl_elem_value_t *ctl_active; snd_ctl_elem_value_t *ctl_format; snd_ctl_elem_value_t *ctl_rate;

diff --git a/alsaloop/pcmjob.c b/alsaloop/pcmjob.c index 6a9aff4..b3802a8 100644 --- a/alsaloop/pcmjob.c +++ b/alsaloop/pcmjob.c @@ -1058,15 +1058,22 @@ static int set_notify(struct loopback_handle *lhandle, int enable)

static int set_rate_shift(struct loopback_handle *lhandle, double pitch) {

• int err;

• int err = 0;

• if (lhandle->ctl_rate_shift == NULL)

• return 0;
  

• snd_ctl_elem_value_set_integer(lhandle->ctl_rate_shift, 0, pitch * 100000);
• err = snd_ctl_elem_write(lhandle->ctl, lhandle->ctl_rate_shift);
• if (err < 0) {

• logit(LOG_CRIT, "Cannot set PCM Rate Shift element for %s: %s\n", lhandle->id, snd_strerror(err));
  

• return err;
  

• if (lhandle->ctl_rate_shift) {

• snd_ctl_elem_value_set_integer(lhandle->ctl_rate_shift, 0, pitch * 100000);
  

• err = snd_ctl_elem_write(lhandle->ctl, lhandle->ctl_rate_shift);
  

• if (err < 0) {
  

• 	logit(LOG_CRIT, "Cannot set PCM Rate Shift element for %s: %s\n", lhandle->id, snd_strerror(err));
  

• 	return err;
  

• }
  

• } else if (lhandle->ctl_fback_freq) {

• snd_ctl_elem_value_set_integer(lhandle->ctl_fback_freq, 0, lhandle->rate * (2.0 - pitch));
  

• err = snd_ctl_elem_write(lhandle->ctl, lhandle->ctl_fback_freq);
  

• if (err < 0) {
  

• 	logit(LOG_CRIT, "Cannot set PCM Feedback Frequency element for %s: %s\n", lhandle->id, snd_strerror(err));
  

• 	return err;
  

• }
  

Hi Ruslan,

I wonder why bother adding a control of another type for the audio gadget ? Why not give the gadget a "Rate Shift" control, instead "Feedback Frequency" and let the driver deal with shift as necessary ?

It would be easier for the applications to re-use the same logic.

} return 0; } @@ -1195,6 +1202,7 @@ static int openctl(struct loopback_handle *lhandle, int device, int subdevice) int err;

lhandle->ctl_rate_shift = NULL;

• lhandle->ctl_fback_freq = NULL; if (lhandle->loopback->play == lhandle) { if (lhandle->loopback->controls) goto __events;

@@ -1204,6 +1212,8 @@ static int openctl(struct loopback_handle *lhandle, int device, int subdevice) &lhandle->ctl_notify); openctl_elem(lhandle, device, subdevice, "PCM Rate Shift 100000", &lhandle->ctl_rate_shift);

• openctl_elem(lhandle, device, subdevice, "PCM Feedback Frequency Hz",

• 	&lhandle->ctl_fback_freq);
  

  set_rate_shift(lhandle, 1); openctl_elem(lhandle, device, subdevice, "PCM Slave Active", &lhandle->ctl_active);

@@ -1289,6 +1299,9 @@ static int closeit(struct loopback_handle *lhandle) if (lhandle->ctl_rate_shift) snd_ctl_elem_value_free(lhandle->ctl_rate_shift); lhandle->ctl_rate_shift = NULL;

• if (lhandle->ctl_fback_freq)

• snd_ctl_elem_value_free(lhandle->ctl_fback_freq);
  

• lhandle->ctl_fback_freq = NULL; if (lhandle->ctl) err = snd_ctl_close(lhandle->ctl); lhandle->ctl = NULL;

@@ -1334,9 +1347,11 @@ int pcmjob_init(struct loopback *loop) snprintf(id, sizeof(id), "%s/%s", loop->play->id, loop->capt->id); id[sizeof(id)-1] = '\0'; loop->id = strdup(id);

• if (loop->sync == SYNC_TYPE_AUTO && loop->capt->ctl_rate_shift)

• if (loop->sync == SYNC_TYPE_AUTO && (loop->capt->ctl_rate_shift ||

• 	loop->capt->ctl_fback_freq))
  

  loop->sync = SYNC_TYPE_CAPTRATESHIFT;

• if (loop->sync == SYNC_TYPE_AUTO && loop->play->ctl_rate_shift)

• if (loop->sync == SYNC_TYPE_AUTO && (loop->play->ctl_rate_shift ||

• 	loop->play->ctl_fback_freq))
  

  loop->sync = SYNC_TYPE_PLAYRATESHIFT;

#ifdef USE_SAMPLERATE if (loop->sync == SYNC_TYPE_AUTO && loop->src_enable)