All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Pioneer devices: engage implicit feedback sync for playback
@ 2021-03-22  4:16 Geraldo
  2021-03-27  0:42 ` Geraldo
  0 siblings, 1 reply; 4+ messages in thread
From: Geraldo @ 2021-03-22  4:16 UTC (permalink / raw)
  To: alsa-devel

Hello everybody,

This is a highly experimental patch on top of 5.12-rc4 that's supposed to
engage implicit feedback sync on the playback for Pioneer devices. Without
implicit feedback sync the inputs started glitching due to clock drift in
about an hour in my Pioneer DJ DDJ-SR2.

If you own a Pioneer device please test this patch and verify in the dyndbg
logs if your capture(input) EP is being opened twice and if implicit_fb = 1
for the playback EP. I can't ask of you all to test this for hours and
hours but if you wanted to have an excuse to digitize vinyl or spin up DVS
this is your perfect one. Help open source!

I'd like to thank Takashi Iwai for his tireless support in the Pioneer
regression and beyond. Hopefully this highly experimental feature's code
can be polished and mainlined eventually.

--- implicit.c.git      2021-03-21 22:40:48.363417245 -0300
+++ implicit.c  2021-03-22 01:03:05.726729481 -0300
@@ -177,30 +177,31 @@
                                       ifnum, alts);
 }

-/* Playback and capture EPs on Pioneer devices share the same iface/altset,
- * but they don't seem working with the implicit fb mode well, hence we
- * just return as if the sync were already set up.
+/* Pioneer devices: playback and capture streams sharing the same
iface/altset
  */
-static int skip_pioneer_sync_ep(struct snd_usb_audio *chip,
-                               struct audioformat *fmt,
-                               struct usb_host_interface *alts)
-{
-       struct usb_endpoint_descriptor *epd;
-
-       if (alts->desc.bNumEndpoints != 2)
-               return 0;
-
-       epd = get_endpoint(alts, 1);
-       if (!usb_endpoint_is_isoc_in(epd) ||
-           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
USB_ENDPOINT_SYNC_ASYNC ||
-           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
-            USB_ENDPOINT_USAGE_DATA &&
-            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
-            USB_ENDPOINT_USAGE_IMPLICIT_FB))
-               return 0;
-       return 1; /* don't handle with the implicit fb, just skip sync EP */
+static int add_pioneer_implicit_fb(struct snd_usb_audio *chip,
+                                  struct audioformat *fmt,
+                                  struct usb_host_interface *alts)
+{
+       struct usb_endpoint_descriptor *epd;
+
+       if (alts->desc.bNumEndpoints != 2)
+               return 0;
+
+       epd = get_endpoint(alts, 1);
+
+       if (!usb_endpoint_is_isoc_in(epd) ||
+           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
USB_ENDPOINT_SYNC_ASYNC ||
+           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
+            USB_ENDPOINT_USAGE_DATA &&
+            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
+            USB_ENDPOINT_USAGE_IMPLICIT_FB))
+               return 0;
+       return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, 1,
+                                      alts->desc.bInterfaceNumber, alts);
 }

+
 static int __add_generic_implicit_fb(struct snd_usb_audio *chip,
                                     struct audioformat *fmt,
                                     int iface, int altset)
@@ -306,7 +307,7 @@
            alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */)) {
-               if (skip_pioneer_sync_ep(chip, fmt, alts))
+               if (add_pioneer_implicit_fb(chip, fmt, alts))
                        return 1;
        }

@@ -339,8 +340,20 @@
                                    struct audioformat *fmt,
                                    struct usb_host_interface *alts)
 {
-       if (fmt->endpoint & USB_DIR_IN)
-               return audioformat_capture_quirk(chip, fmt, alts);
+        bool isPioneer;
+
+        if (alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
+            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
+             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */))
+            isPioneer = true;
+
+       if (fmt->endpoint & USB_DIR_IN) {
+                if (isPioneer == true)
+                    return 1;
+                else
+                   return audioformat_capture_quirk(chip, fmt, alts);
+        }
+
        else
                return audioformat_implicit_fb_quirk(chip, fmt, alts);
 }


--- endpoint.c.git      2021-03-21 22:40:54.800084101 -0300
+++ endpoint.c  2021-03-21 22:45:41.543425855 -0300
@@ -688,7 +688,7 @@
                goto unlock;
        }

-       if (!ep->opened) {
+       if (ep->opened < 2) {
                if (is_sync_ep) {
                        ep->iface = fp->sync_iface;
                        ep->altsetting = fp->sync_altsetting;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Pioneer devices: engage implicit feedback sync for playback
  2021-03-22  4:16 [PATCH] Pioneer devices: engage implicit feedback sync for playback Geraldo
@ 2021-03-27  0:42 ` Geraldo
  2021-03-27  1:01   ` Geraldo
  0 siblings, 1 reply; 4+ messages in thread
From: Geraldo @ 2021-03-27  0:42 UTC (permalink / raw)
  To: alsa-devel

Sorry, I'm reposting because apparently I didn't get diff -up right...
---
Hello everybody,

This is a highly experimental patch on top of 5.12-rc4 that's supposed to
engage implicit feedback sync on the playback for Pioneer devices. Without
implicit feedback sync the inputs started glitching due to clock drift in
about an hour in my Pioneer DJ DDJ-SR2.

If you own a Pioneer device please test this patch and verify in the dyndbg
logs if your capture(input) EP is being opened twice and if implicit_fb = 1
for the playback EP. I can't ask of you all to test this for hours and
hours but if you wanted to have an excuse to digitize vinyl or spin up DVS
this is your perfect one. Help open source!

I'd like to thank Takashi Iwai for his tireless support in the Pioneer
regression and beyond. Hopefully this highly experimental feature's code
can be polished and mainlined eventually.

--- endpoint.c.git      2021-03-21 22:40:54.800084101 -0300
+++ endpoint.c  2021-03-25 21:24:45.253396350 -0300
@@ -688,7 +688,8 @@ snd_usb_endpoint_open(struct snd_usb_aud
                goto unlock;
        }

-       if (!ep->opened) {
+       //if (!ep->opened) {
+       if (ep->opened < 2) {
                if (is_sync_ep) {
                        ep->iface = fp->sync_iface;
                        ep->altsetting = fp->sync_altsetting;

--- implicit.c.git      2021-03-21 22:40:48.363417245 -0300
+++ implicit.c  2021-03-25 20:44:31.826678422 -0300
@@ -50,7 +50,7 @@ static const struct snd_usb_implicit_fb_

        /* Fixed EP */
        /* FIXME: check the availability of generic matching */
-       IMPLICIT_FB_FIXED_DEV(0x1397, 0x0001, 0x81, 1), /* Behringer
UFX1604 */
+       //IMPLICIT_FB_FIXED_DEV(0x1397, 0x0001, 0x81, 1), /* Behringer
UFX1604 */
        IMPLICIT_FB_FIXED_DEV(0x1397, 0x0002, 0x81, 1), /* Behringer
UFX1204 */
        IMPLICIT_FB_FIXED_DEV(0x2466, 0x8010, 0x81, 2), /* Fractal Audio
Axe-Fx III */
        IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0001, 0x81, 2), /* Solid State
Logic SSL2 */
@@ -177,30 +177,31 @@ static int add_roland_implicit_fb(struct
                                       ifnum, alts);
 }

-/* Playback and capture EPs on Pioneer devices share the same iface/altset,
- * but they don't seem working with the implicit fb mode well, hence we
- * just return as if the sync were already set up.
+/* Pioneer devices: playback and capture streams sharing the same
iface/altset
  */
-static int skip_pioneer_sync_ep(struct snd_usb_audio *chip,
-                               struct audioformat *fmt,
-                               struct usb_host_interface *alts)
+static int add_pioneer_implicit_fb(struct snd_usb_audio *chip,
+                                  struct audioformat *fmt,
+                                  struct usb_host_interface *alts)
 {
-       struct usb_endpoint_descriptor *epd;
+       struct usb_endpoint_descriptor *epd;

-       if (alts->desc.bNumEndpoints != 2)
-               return 0;
+       if (alts->desc.bNumEndpoints != 2)
+               return 0;

-       epd = get_endpoint(alts, 1);
-       if (!usb_endpoint_is_isoc_in(epd) ||
-           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
USB_ENDPOINT_SYNC_ASYNC ||
-           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
-            USB_ENDPOINT_USAGE_DATA &&
-            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
-            USB_ENDPOINT_USAGE_IMPLICIT_FB))
-               return 0;
-       return 1; /* don't handle with the implicit fb, just skip sync EP */
+       epd = get_endpoint(alts, 1);
+
+       if (!usb_endpoint_is_isoc_in(epd) ||
+           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
USB_ENDPOINT_SYNC_ASYNC ||
+           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
+            USB_ENDPOINT_USAGE_DATA &&
+            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
+            USB_ENDPOINT_USAGE_IMPLICIT_FB))
+               return 0;
+       return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, 1,
+                                      alts->desc.bInterfaceNumber, alts);
 }

+
 static int __add_generic_implicit_fb(struct snd_usb_audio *chip,
                                     struct audioformat *fmt,
                                     int iface, int altset)
@@ -306,7 +307,7 @@ static int audioformat_implicit_fb_quirk
            alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */)) {
-               if (skip_pioneer_sync_ep(chip, fmt, alts))
+               if (add_pioneer_implicit_fb(chip, fmt, alts))
                        return 1;
        }

@@ -329,6 +330,7 @@ static int audioformat_capture_quirk(str
        if (p && p->type == IMPLICIT_FB_FIXED)
                return add_implicit_fb_sync_ep(chip, fmt, p->ep_num, 0,
                                               p->iface, NULL);
+       //return 0;
        return 0;
 }

@@ -339,8 +341,20 @@ int snd_usb_parse_implicit_fb_quirk(stru
                                    struct audioformat *fmt,
                                    struct usb_host_interface *alts)
 {
-       if (fmt->endpoint & USB_DIR_IN)
-               return audioformat_capture_quirk(chip, fmt, alts);
+        bool isPioneer;
+
+        if (alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
+            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
+             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */))
+            isPioneer = true;
+
+       if (fmt->endpoint & USB_DIR_IN) {
+                if (isPioneer == true)
+                    return 1;
+                else
+                   return audioformat_capture_quirk(chip, fmt, alts);
+        }
+
        else
                return audioformat_implicit_fb_quirk(chip, fmt, alts);
 }

On Mon, Mar 22, 2021 at 1:16 AM Geraldo <geraldogabriel@gmail.com> wrote:

> Hello everybody,
>
> This is a highly experimental patch on top of 5.12-rc4 that's supposed to
> engage implicit feedback sync on the playback for Pioneer devices. Without
> implicit feedback sync the inputs started glitching due to clock drift in
> about an hour in my Pioneer DJ DDJ-SR2.
>
> If you own a Pioneer device please test this patch and verify in the
> dyndbg logs if your capture(input) EP is being opened twice and if
> implicit_fb = 1 for the playback EP. I can't ask of you all to test this
> for hours and hours but if you wanted to have an excuse to digitize vinyl
> or spin up DVS this is your perfect one. Help open source!
>
> I'd like to thank Takashi Iwai for his tireless support in the Pioneer
> regression and beyond. Hopefully this highly experimental feature's code
> can be polished and mainlined eventually.
>
> --- implicit.c.git      2021-03-21 22:40:48.363417245 -0300
> +++ implicit.c  2021-03-22 01:03:05.726729481 -0300
> @@ -177,30 +177,31 @@
>                                        ifnum, alts);
>  }
>
> -/* Playback and capture EPs on Pioneer devices share the same
> iface/altset,
> - * but they don't seem working with the implicit fb mode well, hence we
> - * just return as if the sync were already set up.
> +/* Pioneer devices: playback and capture streams sharing the same
> iface/altset
>   */
> -static int skip_pioneer_sync_ep(struct snd_usb_audio *chip,
> -                               struct audioformat *fmt,
> -                               struct usb_host_interface *alts)
> -{
> -       struct usb_endpoint_descriptor *epd;
> -
> -       if (alts->desc.bNumEndpoints != 2)
> -               return 0;
> -
> -       epd = get_endpoint(alts, 1);
> -       if (!usb_endpoint_is_isoc_in(epd) ||
> -           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
> USB_ENDPOINT_SYNC_ASYNC ||
> -           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> -            USB_ENDPOINT_USAGE_DATA &&
> -            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> -            USB_ENDPOINT_USAGE_IMPLICIT_FB))
> -               return 0;
> -       return 1; /* don't handle with the implicit fb, just skip sync EP
> */
> +static int add_pioneer_implicit_fb(struct snd_usb_audio *chip,
> +                                  struct audioformat *fmt,
> +                                  struct usb_host_interface *alts)
> +{
> +       struct usb_endpoint_descriptor *epd;
> +
> +       if (alts->desc.bNumEndpoints != 2)
> +               return 0;
> +
> +       epd = get_endpoint(alts, 1);
> +
> +       if (!usb_endpoint_is_isoc_in(epd) ||
> +           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
> USB_ENDPOINT_SYNC_ASYNC ||
> +           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> +            USB_ENDPOINT_USAGE_DATA &&
> +            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> +            USB_ENDPOINT_USAGE_IMPLICIT_FB))
> +               return 0;
> +       return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, 1,
> +                                      alts->desc.bInterfaceNumber, alts);
>  }
>
> +
>  static int __add_generic_implicit_fb(struct snd_usb_audio *chip,
>                                      struct audioformat *fmt,
>                                      int iface, int altset)
> @@ -306,7 +307,7 @@
>             alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
>             (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
>              USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */)) {
> -               if (skip_pioneer_sync_ep(chip, fmt, alts))
> +               if (add_pioneer_implicit_fb(chip, fmt, alts))
>                         return 1;
>         }
>
> @@ -339,8 +340,20 @@
>                                     struct audioformat *fmt,
>                                     struct usb_host_interface *alts)
>  {
> -       if (fmt->endpoint & USB_DIR_IN)
> -               return audioformat_capture_quirk(chip, fmt, alts);
> +        bool isPioneer;
> +
> +        if (alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
> +            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
> +             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */))
> +            isPioneer = true;
> +
> +       if (fmt->endpoint & USB_DIR_IN) {
> +                if (isPioneer == true)
> +                    return 1;
> +                else
> +                   return audioformat_capture_quirk(chip, fmt, alts);
> +        }
> +
>         else
>                 return audioformat_implicit_fb_quirk(chip, fmt, alts);
>  }
>
>
> --- endpoint.c.git      2021-03-21 22:40:54.800084101 -0300
> +++ endpoint.c  2021-03-21 22:45:41.543425855 -0300
> @@ -688,7 +688,7 @@
>                 goto unlock;
>         }
>
> -       if (!ep->opened) {
> +       if (ep->opened < 2) {
>                 if (is_sync_ep) {
>                         ep->iface = fp->sync_iface;
>                         ep->altsetting = fp->sync_altsetting;
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Pioneer devices: engage implicit feedback sync for playback
  2021-03-27  0:42 ` Geraldo
@ 2021-03-27  1:01   ` Geraldo
  2021-04-04 21:12     ` Fwd: " Geraldo
  0 siblings, 1 reply; 4+ messages in thread
From: Geraldo @ 2021-03-27  1:01 UTC (permalink / raw)
  To: alsa-devel

And now I got diff -up right but forgot to edit out UFX1604 related
modifications.

I'm so sorry, reposting once again and hopefully for the last time

This wouldn't be happening if I had set up different git branches...
---
Hello everybody,

This is a highly experimental patch on top of 5.12-rc4 that's supposed to
engage implicit feedback sync on the playback for Pioneer devices. Without
implicit feedback sync the inputs started glitching due to clock drift in
about an hour in my Pioneer DJ DDJ-SR2.

If you own a Pioneer device please test this patch and verify in the dyndbg
logs if your capture(input) EP is being opened twice and if implicit_fb = 1
for the playback EP. I can't ask of you all to test this for hours and
hours but if you wanted to have an excuse to digitize vinyl or spin up DVS
this is your perfect one. Help open source!

I'd like to thank Takashi Iwai for his tireless support in the Pioneer
regression and beyond. Hopefully this highly experimental feature's code
can be polished and mainlined eventually.

--- endpoint.c.git      2021-03-21 22:40:54.800084101 -0300
+++ endpoint.c  2021-03-25 21:24:45.253396350 -0300
@@ -688,7 +688,8 @@ snd_usb_endpoint_open(struct snd_usb_aud
                goto unlock;
        }

-       if (!ep->opened) {
+       //if (!ep->opened) {
+       if (ep->opened < 2) {
                if (is_sync_ep) {
                        ep->iface = fp->sync_iface;
                        ep->altsetting = fp->sync_altsetting;

--- implicit.c.git      2021-03-21 22:40:48.363417245 -0300
+++ implicit.c  2021-03-25 20:44:31.826678422 -0300
@@ -177,30 +177,31 @@ static int add_roland_implicit_fb(struct
                                       ifnum, alts);
 }

-/* Playback and capture EPs on Pioneer devices share the same iface/altset,
- * but they don't seem working with the implicit fb mode well, hence we
- * just return as if the sync were already set up.
+/* Pioneer devices: playback and capture streams sharing the same
iface/altset
  */
-static int skip_pioneer_sync_ep(struct snd_usb_audio *chip,
-                               struct audioformat *fmt,
-                               struct usb_host_interface *alts)
+static int add_pioneer_implicit_fb(struct snd_usb_audio *chip,
+                                  struct audioformat *fmt,
+                                  struct usb_host_interface *alts)
 {
-       struct usb_endpoint_descriptor *epd;
+       struct usb_endpoint_descriptor *epd;

-       if (alts->desc.bNumEndpoints != 2)
-               return 0;
+       if (alts->desc.bNumEndpoints != 2)
+               return 0;

-       epd = get_endpoint(alts, 1);
-       if (!usb_endpoint_is_isoc_in(epd) ||
-           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
USB_ENDPOINT_SYNC_ASYNC ||
-           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
-            USB_ENDPOINT_USAGE_DATA &&
-            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
-            USB_ENDPOINT_USAGE_IMPLICIT_FB))
-               return 0;
-       return 1; /* don't handle with the implicit fb, just skip sync EP */
+       epd = get_endpoint(alts, 1);
+
+       if (!usb_endpoint_is_isoc_in(epd) ||
+           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
USB_ENDPOINT_SYNC_ASYNC ||
+           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
+            USB_ENDPOINT_USAGE_DATA &&
+            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
+            USB_ENDPOINT_USAGE_IMPLICIT_FB))
+               return 0;
+       return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, 1,
+                                      alts->desc.bInterfaceNumber, alts);
 }

+
 static int __add_generic_implicit_fb(struct snd_usb_audio *chip,
                                     struct audioformat *fmt,
                                     int iface, int altset)
@@ -306,7 +307,7 @@ static int audioformat_implicit_fb_quirk
            alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */)) {
-               if (skip_pioneer_sync_ep(chip, fmt, alts))
+               if (add_pioneer_implicit_fb(chip, fmt, alts))
                        return 1;
        }

@@ -329,6 +330,7 @@ static int audioformat_capture_quirk(str
        if (p && p->type == IMPLICIT_FB_FIXED)
                return add_implicit_fb_sync_ep(chip, fmt, p->ep_num, 0,
                                               p->iface, NULL);
+       //return 0;
        return 0;
 }

@@ -339,8 +341,20 @@ int snd_usb_parse_implicit_fb_quirk(stru
                                    struct audioformat *fmt,
                                    struct usb_host_interface *alts)
 {
-       if (fmt->endpoint & USB_DIR_IN)
-               return audioformat_capture_quirk(chip, fmt, alts);
+        bool isPioneer;
+
+        if (alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
+            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
+             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */))
+            isPioneer = true;
+
+       if (fmt->endpoint & USB_DIR_IN) {
+                if (isPioneer == true)
+                    return 1;
+                else
+                   return audioformat_capture_quirk(chip, fmt, alts);
+        }
+
        else
                return audioformat_implicit_fb_quirk(chip, fmt, alts);
 }

On Fri, Mar 26, 2021 at 9:42 PM Geraldo <geraldogabriel@gmail.com> wrote:

> Sorry, I'm reposting because apparently I didn't get diff -up right...
> ---
> Hello everybody,
>
> This is a highly experimental patch on top of 5.12-rc4 that's supposed to
> engage implicit feedback sync on the playback for Pioneer devices. Without
> implicit feedback sync the inputs started glitching due to clock drift in
> about an hour in my Pioneer DJ DDJ-SR2.
>
> If you own a Pioneer device please test this patch and verify in the
> dyndbg logs if your capture(input) EP is being opened twice and if
> implicit_fb = 1 for the playback EP. I can't ask of you all to test this
> for hours and hours but if you wanted to have an excuse to digitize vinyl
> or spin up DVS this is your perfect one. Help open source!
>
> I'd like to thank Takashi Iwai for his tireless support in the Pioneer
> regression and beyond. Hopefully this highly experimental feature's code
> can be polished and mainlined eventually.
>
> --- endpoint.c.git      2021-03-21 22:40:54.800084101 -0300
> +++ endpoint.c  2021-03-25 21:24:45.253396350 -0300
> @@ -688,7 +688,8 @@ snd_usb_endpoint_open(struct snd_usb_aud
>                 goto unlock;
>         }
>
> -       if (!ep->opened) {
> +       //if (!ep->opened) {
> +       if (ep->opened < 2) {
>                 if (is_sync_ep) {
>                         ep->iface = fp->sync_iface;
>                         ep->altsetting = fp->sync_altsetting;
>
> --- implicit.c.git      2021-03-21 22:40:48.363417245 -0300
> +++ implicit.c  2021-03-25 20:44:31.826678422 -0300
> @@ -50,7 +50,7 @@ static const struct snd_usb_implicit_fb_
>
>         /* Fixed EP */
>         /* FIXME: check the availability of generic matching */
> -       IMPLICIT_FB_FIXED_DEV(0x1397, 0x0001, 0x81, 1), /* Behringer
> UFX1604 */
> +       //IMPLICIT_FB_FIXED_DEV(0x1397, 0x0001, 0x81, 1), /* Behringer
> UFX1604 */
>         IMPLICIT_FB_FIXED_DEV(0x1397, 0x0002, 0x81, 1), /* Behringer
> UFX1204 */
>         IMPLICIT_FB_FIXED_DEV(0x2466, 0x8010, 0x81, 2), /* Fractal Audio
> Axe-Fx III */
>         IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0001, 0x81, 2), /* Solid State
> Logic SSL2 */
> @@ -177,30 +177,31 @@ static int add_roland_implicit_fb(struct
>                                        ifnum, alts);
>  }
>
> -/* Playback and capture EPs on Pioneer devices share the same
> iface/altset,
> - * but they don't seem working with the implicit fb mode well, hence we
> - * just return as if the sync were already set up.
> +/* Pioneer devices: playback and capture streams sharing the same
> iface/altset
>   */
> -static int skip_pioneer_sync_ep(struct snd_usb_audio *chip,
> -                               struct audioformat *fmt,
> -                               struct usb_host_interface *alts)
> +static int add_pioneer_implicit_fb(struct snd_usb_audio *chip,
> +                                  struct audioformat *fmt,
> +                                  struct usb_host_interface *alts)
>  {
> -       struct usb_endpoint_descriptor *epd;
> +       struct usb_endpoint_descriptor *epd;
>
> -       if (alts->desc.bNumEndpoints != 2)
> -               return 0;
> +       if (alts->desc.bNumEndpoints != 2)
> +               return 0;
>
> -       epd = get_endpoint(alts, 1);
> -       if (!usb_endpoint_is_isoc_in(epd) ||
> -           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
> USB_ENDPOINT_SYNC_ASYNC ||
> -           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> -            USB_ENDPOINT_USAGE_DATA &&
> -            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> -            USB_ENDPOINT_USAGE_IMPLICIT_FB))
> -               return 0;
> -       return 1; /* don't handle with the implicit fb, just skip sync EP
> */
> +       epd = get_endpoint(alts, 1);
> +
> +       if (!usb_endpoint_is_isoc_in(epd) ||
> +           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
> USB_ENDPOINT_SYNC_ASYNC ||
> +           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> +            USB_ENDPOINT_USAGE_DATA &&
> +            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> +            USB_ENDPOINT_USAGE_IMPLICIT_FB))
> +               return 0;
> +       return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, 1,
> +                                      alts->desc.bInterfaceNumber, alts);
>  }
>
> +
>  static int __add_generic_implicit_fb(struct snd_usb_audio *chip,
>                                      struct audioformat *fmt,
>                                      int iface, int altset)
> @@ -306,7 +307,7 @@ static int audioformat_implicit_fb_quirk
>             alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
>             (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
>              USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */)) {
> -               if (skip_pioneer_sync_ep(chip, fmt, alts))
> +               if (add_pioneer_implicit_fb(chip, fmt, alts))
>                         return 1;
>         }
>
> @@ -329,6 +330,7 @@ static int audioformat_capture_quirk(str
>         if (p && p->type == IMPLICIT_FB_FIXED)
>                 return add_implicit_fb_sync_ep(chip, fmt, p->ep_num, 0,
>                                                p->iface, NULL);
> +       //return 0;
>         return 0;
>  }
>
> @@ -339,8 +341,20 @@ int snd_usb_parse_implicit_fb_quirk(stru
>                                     struct audioformat *fmt,
>                                     struct usb_host_interface *alts)
>  {
> -       if (fmt->endpoint & USB_DIR_IN)
> -               return audioformat_capture_quirk(chip, fmt, alts);
> +        bool isPioneer;
> +
> +        if (alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
> +            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
> +             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */))
> +            isPioneer = true;
> +
> +       if (fmt->endpoint & USB_DIR_IN) {
> +                if (isPioneer == true)
> +                    return 1;
> +                else
> +                   return audioformat_capture_quirk(chip, fmt, alts);
> +        }
> +
>         else
>                 return audioformat_implicit_fb_quirk(chip, fmt, alts);
>  }
>
> On Mon, Mar 22, 2021 at 1:16 AM Geraldo <geraldogabriel@gmail.com> wrote:
>
>> Hello everybody,
>>
>> This is a highly experimental patch on top of 5.12-rc4 that's supposed to
>> engage implicit feedback sync on the playback for Pioneer devices. Without
>> implicit feedback sync the inputs started glitching due to clock drift in
>> about an hour in my Pioneer DJ DDJ-SR2.
>>
>> If you own a Pioneer device please test this patch and verify in the
>> dyndbg logs if your capture(input) EP is being opened twice and if
>> implicit_fb = 1 for the playback EP. I can't ask of you all to test this
>> for hours and hours but if you wanted to have an excuse to digitize vinyl
>> or spin up DVS this is your perfect one. Help open source!
>>
>> I'd like to thank Takashi Iwai for his tireless support in the Pioneer
>> regression and beyond. Hopefully this highly experimental feature's code
>> can be polished and mainlined eventually.
>>
>> --- implicit.c.git      2021-03-21 22:40:48.363417245 -0300
>> +++ implicit.c  2021-03-22 01:03:05.726729481 -0300
>> @@ -177,30 +177,31 @@
>>                                        ifnum, alts);
>>  }
>>
>> -/* Playback and capture EPs on Pioneer devices share the same
>> iface/altset,
>> - * but they don't seem working with the implicit fb mode well, hence we
>> - * just return as if the sync were already set up.
>> +/* Pioneer devices: playback and capture streams sharing the same
>> iface/altset
>>   */
>> -static int skip_pioneer_sync_ep(struct snd_usb_audio *chip,
>> -                               struct audioformat *fmt,
>> -                               struct usb_host_interface *alts)
>> -{
>> -       struct usb_endpoint_descriptor *epd;
>> -
>> -       if (alts->desc.bNumEndpoints != 2)
>> -               return 0;
>> -
>> -       epd = get_endpoint(alts, 1);
>> -       if (!usb_endpoint_is_isoc_in(epd) ||
>> -           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
>> USB_ENDPOINT_SYNC_ASYNC ||
>> -           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
>> -            USB_ENDPOINT_USAGE_DATA &&
>> -            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
>> -            USB_ENDPOINT_USAGE_IMPLICIT_FB))
>> -               return 0;
>> -       return 1; /* don't handle with the implicit fb, just skip sync EP
>> */
>> +static int add_pioneer_implicit_fb(struct snd_usb_audio *chip,
>> +                                  struct audioformat *fmt,
>> +                                  struct usb_host_interface *alts)
>> +{
>> +       struct usb_endpoint_descriptor *epd;
>> +
>> +       if (alts->desc.bNumEndpoints != 2)
>> +               return 0;
>> +
>> +       epd = get_endpoint(alts, 1);
>> +
>> +       if (!usb_endpoint_is_isoc_in(epd) ||
>> +           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
>> USB_ENDPOINT_SYNC_ASYNC ||
>> +           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
>> +            USB_ENDPOINT_USAGE_DATA &&
>> +            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
>> +            USB_ENDPOINT_USAGE_IMPLICIT_FB))
>> +               return 0;
>> +       return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress,
>> 1,
>> +                                      alts->desc.bInterfaceNumber, alts);
>>  }
>>
>> +
>>  static int __add_generic_implicit_fb(struct snd_usb_audio *chip,
>>                                      struct audioformat *fmt,
>>                                      int iface, int altset)
>> @@ -306,7 +307,7 @@
>>             alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
>>             (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
>>              USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */)) {
>> -               if (skip_pioneer_sync_ep(chip, fmt, alts))
>> +               if (add_pioneer_implicit_fb(chip, fmt, alts))
>>                         return 1;
>>         }
>>
>> @@ -339,8 +340,20 @@
>>                                     struct audioformat *fmt,
>>                                     struct usb_host_interface *alts)
>>  {
>> -       if (fmt->endpoint & USB_DIR_IN)
>> -               return audioformat_capture_quirk(chip, fmt, alts);
>> +        bool isPioneer;
>> +
>> +        if (alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
>> +            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
>> +             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */))
>> +            isPioneer = true;
>> +
>> +       if (fmt->endpoint & USB_DIR_IN) {
>> +                if (isPioneer == true)
>> +                    return 1;
>> +                else
>> +                   return audioformat_capture_quirk(chip, fmt, alts);
>> +        }
>> +
>>         else
>>                 return audioformat_implicit_fb_quirk(chip, fmt, alts);
>>  }
>>
>>
>> --- endpoint.c.git      2021-03-21 22:40:54.800084101 -0300
>> +++ endpoint.c  2021-03-21 22:45:41.543425855 -0300
>> @@ -688,7 +688,7 @@
>>                 goto unlock;
>>         }
>>
>> -       if (!ep->opened) {
>> +       if (ep->opened < 2) {
>>                 if (is_sync_ep) {
>>                         ep->iface = fp->sync_iface;
>>                         ep->altsetting = fp->sync_altsetting;
>>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Fwd: [PATCH] Pioneer devices: engage implicit feedback sync for playback
  2021-03-27  1:01   ` Geraldo
@ 2021-04-04 21:12     ` Geraldo
  0 siblings, 0 replies; 4+ messages in thread
From: Geraldo @ 2021-04-04 21:12 UTC (permalink / raw)
  To: fabian, livvy, dmanlfc, marcan, dmitry, franta-linux, ard
  Cc: Takashi Iwai, alsa-devel

Hello, Linux users of Pioneer gear!

I was recently able to engage implicit feedback sync on the playback for my
Pioneer DJ DDJ-SR2.

Without this workaround you can all expect the sound quality of your inputs
to degrade in about an hour, give it or take it, due to clock drift.

Please give this patch a good reading and a good test and let us know on
alsa-devel how it performed.

---

Hello everybody,

This is a highly experimental patch on top of 5.12-rc4 that's supposed to
engage implicit feedback sync on the playback for Pioneer devices. Without
implicit feedback sync the inputs started glitching due to clock drift in
about an hour in my Pioneer DJ DDJ-SR2.

If you own a Pioneer device please test this patch and verify in the dyndbg
logs if your capture(input) EP is being opened twice and if implicit_fb = 1
for the playback EP. I can't ask of you all to test this for hours and
hours but if you wanted to have an excuse to digitize vinyl or spin up DVS
this is your perfect one. Help open source!

I'd like to thank Takashi Iwai for his tireless support in the Pioneer
regression and beyond. Hopefully this highly experimental feature's code
can be polished and mainlined eventually.

--- endpoint.c.git      2021-03-21 22:40:54.800084101 -0300
+++ endpoint.c  2021-03-25 21:24:45.253396350 -0300
@@ -688,7 +688,8 @@ snd_usb_endpoint_open(struct snd_usb_aud
                goto unlock;
        }

-       if (!ep->opened) {
+       //if (!ep->opened) {
+       if (ep->opened < 2) {
                if (is_sync_ep) {
                        ep->iface = fp->sync_iface;
                        ep->altsetting = fp->sync_altsetting;

--- implicit.c.git      2021-03-21 22:40:48.363417245 -0300
+++ implicit.c  2021-03-25 20:44:31.826678422 -0300
@@ -177,30 +177,31 @@ static int add_roland_implicit_fb(struct
                                       ifnum, alts);
 }

-/* Playback and capture EPs on Pioneer devices share the same iface/altset,
- * but they don't seem working with the implicit fb mode well, hence we
- * just return as if the sync were already set up.
+/* Pioneer devices: playback and capture streams sharing the same
iface/altset
  */
-static int skip_pioneer_sync_ep(struct snd_usb_audio *chip,
-                               struct audioformat *fmt,
-                               struct usb_host_interface *alts)
+static int add_pioneer_implicit_fb(struct snd_usb_audio *chip,
+                                  struct audioformat *fmt,
+                                  struct usb_host_interface *alts)
 {
-       struct usb_endpoint_descriptor *epd;
+       struct usb_endpoint_descriptor *epd;

-       if (alts->desc.bNumEndpoints != 2)
-               return 0;
+       if (alts->desc.bNumEndpoints != 2)
+               return 0;

-       epd = get_endpoint(alts, 1);
-       if (!usb_endpoint_is_isoc_in(epd) ||
-           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
USB_ENDPOINT_SYNC_ASYNC ||
-           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
-            USB_ENDPOINT_USAGE_DATA &&
-            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
-            USB_ENDPOINT_USAGE_IMPLICIT_FB))
-               return 0;
-       return 1; /* don't handle with the implicit fb, just skip sync EP */
+       epd = get_endpoint(alts, 1);
+
+       if (!usb_endpoint_is_isoc_in(epd) ||
+           (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
USB_ENDPOINT_SYNC_ASYNC ||
+           ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
+            USB_ENDPOINT_USAGE_DATA &&
+            (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
+            USB_ENDPOINT_USAGE_IMPLICIT_FB))
+               return 0;
+       return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, 1,
+                                      alts->desc.bInterfaceNumber, alts);
 }

+
 static int __add_generic_implicit_fb(struct snd_usb_audio *chip,
                                     struct audioformat *fmt,
                                     int iface, int altset)
@@ -306,7 +307,7 @@ static int audioformat_implicit_fb_quirk
            alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */)) {
-               if (skip_pioneer_sync_ep(chip, fmt, alts))
+               if (add_pioneer_implicit_fb(chip, fmt, alts))
                        return 1;
        }

@@ -329,6 +330,7 @@ static int audioformat_capture_quirk(str
        if (p && p->type == IMPLICIT_FB_FIXED)
                return add_implicit_fb_sync_ep(chip, fmt, p->ep_num, 0,
                                               p->iface, NULL);
+       //return 0;
        return 0;
 }

@@ -339,8 +341,20 @@ int snd_usb_parse_implicit_fb_quirk(stru
                                    struct audioformat *fmt,
                                    struct usb_host_interface *alts)
 {
-       if (fmt->endpoint & USB_DIR_IN)
-               return audioformat_capture_quirk(chip, fmt, alts);
+        bool isPioneer;
+
+        if (alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
+            (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
+             USB_ID_VENDOR(chip->usb_id) == 0x08e4    /* Pioneer */))
+            isPioneer = true;
+
+       if (fmt->endpoint & USB_DIR_IN) {
+                if (isPioneer == true)
+                    return 1;
+                else
+                   return audioformat_capture_quirk(chip, fmt, alts);
+        }
+
        else
                return audioformat_implicit_fb_quirk(chip, fmt, alts);
 }

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-04-04 21:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22  4:16 [PATCH] Pioneer devices: engage implicit feedback sync for playback Geraldo
2021-03-27  0:42 ` Geraldo
2021-03-27  1:01   ` Geraldo
2021-04-04 21:12     ` Fwd: " Geraldo

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.