* [PATCH] [RFC] alsaloop: add feedback frequency control support for UAC2 gadgets
@ 2020-11-12 22:34 Ruslan Bilovol
2020-11-27 9:17 ` Jerome Brunet
0 siblings, 1 reply; 4+ messages in thread
From: Ruslan Bilovol @ 2020-11-12 22:34 UTC (permalink / raw)
To: perex; +Cc: alsa-devel, linux-usb, gschmottlach
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;
+ }
}
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)
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] [RFC] alsaloop: add feedback frequency control support for UAC2 gadgets
2020-11-12 22:34 [PATCH] [RFC] alsaloop: add feedback frequency control support for UAC2 gadgets Ruslan Bilovol
@ 2020-11-27 9:17 ` Jerome Brunet
2020-12-04 19:06 ` Ruslan Bilovol
0 siblings, 1 reply; 4+ messages in thread
From: Jerome Brunet @ 2020-11-27 9:17 UTC (permalink / raw)
To: Ruslan Bilovol, perex; +Cc: alsa-devel, linux-usb, gschmottlach
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)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [RFC] alsaloop: add feedback frequency control support for UAC2 gadgets
2020-11-27 9:17 ` Jerome Brunet
@ 2020-12-04 19:06 ` Ruslan Bilovol
0 siblings, 0 replies; 4+ messages in thread
From: Ruslan Bilovol @ 2020-12-04 19:06 UTC (permalink / raw)
To: Jerome Brunet; +Cc: Jaroslav Kysela, alsa-devel, Linux USB, Glenn Schmottlach
On Fri, Nov 27, 2020 at 11:17 AM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> 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.
I actually initially implemented "Rate Shift" control which simplified
brignup of UAC2 feedback support.
However, there is an issue with it.
The way how it's supposed to work is next - userspace calculates
real sampling frequency at which it consumes samples (usually it's
some internal clock frequency) then tells the real sampling frequency
to UAC2 gadget drivers which notifies the host which can adjust the
number of samples it sends to the gadget.
With "Rate Shift" control userspace has to convert measured absolute clock
frequency to relative 100000 format, then write this to UAC2 gadget which has
to convert it back from relative 100000 format to absolute frequency in Hz,
so it does a lot of unneeded conversions here.
Another issue with this approach is userspace can't know at which resolution
works feedback endpoint, so it continuously updates UAC's feedback frequency
"Rate Shift" control without any effect (because current feedback implementation
works with 1Hz resolution)
So what was designed to work with in-kernel alsaloop 'aloop' loopback, doesn't
fit well in the UAC2 case.
However, I want to improve resolution of feedback frequency, it can be better as
per USB spec, so this interface may be changed in next versions of
this patch set
Thanks,
Ruslan
>
> > }
> > 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)
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [RFC] alsaloop: add feedback frequency control support for UAC2 gadgets
@ 2020-12-04 19:06 ` Ruslan Bilovol
0 siblings, 0 replies; 4+ messages in thread
From: Ruslan Bilovol @ 2020-12-04 19:06 UTC (permalink / raw)
To: Jerome Brunet; +Cc: alsa-devel, Linux USB, Glenn Schmottlach
On Fri, Nov 27, 2020 at 11:17 AM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> 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.
I actually initially implemented "Rate Shift" control which simplified
brignup of UAC2 feedback support.
However, there is an issue with it.
The way how it's supposed to work is next - userspace calculates
real sampling frequency at which it consumes samples (usually it's
some internal clock frequency) then tells the real sampling frequency
to UAC2 gadget drivers which notifies the host which can adjust the
number of samples it sends to the gadget.
With "Rate Shift" control userspace has to convert measured absolute clock
frequency to relative 100000 format, then write this to UAC2 gadget which has
to convert it back from relative 100000 format to absolute frequency in Hz,
so it does a lot of unneeded conversions here.
Another issue with this approach is userspace can't know at which resolution
works feedback endpoint, so it continuously updates UAC's feedback frequency
"Rate Shift" control without any effect (because current feedback implementation
works with 1Hz resolution)
So what was designed to work with in-kernel alsaloop 'aloop' loopback, doesn't
fit well in the UAC2 case.
However, I want to improve resolution of feedback frequency, it can be better as
per USB spec, so this interface may be changed in next versions of
this patch set
Thanks,
Ruslan
>
> > }
> > 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)
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-04 19:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-12 22:34 [PATCH] [RFC] alsaloop: add feedback frequency control support for UAC2 gadgets Ruslan Bilovol
2020-11-27 9:17 ` Jerome Brunet
2020-12-04 19:06 ` Ruslan Bilovol
2020-12-04 19:06 ` Ruslan Bilovol
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.