ALSA USB MIDI port names are poorly constructed and truncate iJACK names

ALSA USB MIDI port names are poorly constructed and truncate iJACK names

[Paul Davis]

ALSA's USB MIDI support appears to construct MIDI port names by
concatenating the iProduct name and the iJACK name. The problem with
this is that there is a fixed length to the port name in ALSA, and for
an increasing number of devices, this behavior truncates critical
information from the end of the iJACK name.

For example, culled from lsusb output:

Bus 005 Device 098: ID 1235:0145 Focusrite-Novation Launchkey MK4 49
  iProduct                2 Launchkey MK4 49
    iConfiguration          4 Launchkey MK4 49
      iInterface              5 Launchkey MK4 49 MIDI
      iInterface              6 Launchkey MK4 49 MIDI Int
        iJack                   7 Launchkey MK4 49 MIDI In
        iJack                   8 Launchkey MK4 49 MIDI Out
        iJack                   9 Launchkey MK4 49 DAW In

gives these port names:

44:0    Launchkey MK4 49                 Launchkey MK4 49 Launchkey MK4
 44:1    Launchkey MK4 49                 Launchkey MK4 49 Launchkey MK4

which is useless since it fails to include the "DAW" and "MIDI Out"
parts of the iJACK names.

The correct fix, I think is to only prepend the iProduct name if that
name is not present at the start of the iJACK name. This will cover
devices where the manufacturer does *not* include the device name in
the iJACK name, but will fix this situation for devices where they do
(Novation and Arturia and many others do this).

If agreed, I can construct a patch for this.

Re: ALSA USB MIDI port names are poorly constructed and truncate iJACK names

[Takashi Iwai]

On Tue, 22 Apr 2025 16:31:50 +0200,
Paul Davis wrote:
> 
> ALSA's USB MIDI support appears to construct MIDI port names by
> concatenating the iProduct name and the iJACK name. The problem with
> this is that there is a fixed length to the port name in ALSA, and for
> an increasing number of devices, this behavior truncates critical
> information from the end of the iJACK name.
> 
> For example, culled from lsusb output:
> 
> Bus 005 Device 098: ID 1235:0145 Focusrite-Novation Launchkey MK4 49
>   iProduct                2 Launchkey MK4 49
>     iConfiguration          4 Launchkey MK4 49
>       iInterface              5 Launchkey MK4 49 MIDI
>       iInterface              6 Launchkey MK4 49 MIDI Int
>         iJack                   7 Launchkey MK4 49 MIDI In
>         iJack                   8 Launchkey MK4 49 MIDI Out
>         iJack                   9 Launchkey MK4 49 DAW In
> 
> gives these port names:
> 
> 44:0    Launchkey MK4 49                 Launchkey MK4 49 Launchkey MK4
>  44:1    Launchkey MK4 49                 Launchkey MK4 49 Launchkey MK4
> 
> which is useless since it fails to include the "DAW" and "MIDI Out"
> parts of the iJACK names.
> 
> The correct fix, I think is to only prepend the iProduct name if that
> name is not present at the start of the iJACK name. This will cover
> devices where the manufacturer does *not* include the device name in
> the iJACK name, but will fix this situation for devices where they do
> (Novation and Arturia and many others do this).
> 
> If agreed, I can construct a patch for this.

So you're suggesting to remove the substring matched with iProduct at
copying from iJack name?  That'll make sense and go ahead.


thanks,

Takashi

Re: ALSA USB MIDI port names are poorly constructed and truncate iJACK names

[Paul Davis]

This patch was developed using the Debian 6.12.12 kernel source, but I
believe it works without changes for current git.

The approach taken is to always look for the iProduct name
(umidi->card->shortname) in the iJACK name (jack_name). If not found,
the code path is unchanged. If found, a different string format is
used to create the port name, using only the jack name. Note that
since finding the product name in the jack name implies the jack name
is not default_jack_name, there is never any reason to use "number" in
constructing the port name.

This generates the expected port names for several USB MIDI devices
here, some of which have "product name in jack name" and some which do
not:

% aplaymidi -l
 Port    Client name                      Port name
 14:0    Midi Through                     Midi Through Port-0
 16:0    Scarlett 18i20 USB               Scarlett 18i20 USB MIDI 1
 36:0    Keystation 88                    Keystation 88 MIDI 1
 40:0    Launchkey MK4 49                 Launchkey MK4 49 MIDI In
 40:1    Launchkey MK4 49                 Launchkey MK4 49 DAW In
 44:0    MidiSport 2x2                    MidiSport 2x2 MIDI 1
 44:1    MidiSport 2x2                    MidiSport 2x2 MIDI 2


Signed-off-by: Paul Davis <paul@linuxaudiosystems.com>

--- midi.c.orig    2025-04-22 11:20:30.192338737 -0600
+++ midi.c    2025-04-22 11:20:47.373155820 -0600
@@ -1817,10 +1817,18 @@ static void snd_usbmidi_init_substream(s
     }

     port_info = find_port_info(umidi, number);
-    name_format = port_info ? port_info->name :
-        (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
-    snprintf(substream->name, sizeof(substream->name),
-         name_format, umidi->card->shortname, jack_name, number + 1);
+
+    if (strncmp (umidi->card->shortname, jack_name, strlen
(umidi->card->shortname)) != 0) {
+        name_format = port_info ? port_info->name :
+            (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
+        snprintf(substream->name, sizeof(substream->name),
+                 name_format, umidi->card->shortname, jack_name, number + 1);
+    } else {
+        /* The manufacturer included the iProduct name in the jack
name, do not use both */
+        name_format = port_info ? port_info->name : "%s";
+        snprintf(substream->name, sizeof(substream->name),
name_format, jack_name);
+    }
+

     *rsubstream = substream;
 }


On Tue, Apr 22, 2025 at 8:46 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Tue, 22 Apr 2025 16:31:50 +0200,
> Paul Davis wrote:
> >
> > ALSA's USB MIDI support appears to construct MIDI port names by
> > concatenating the iProduct name and the iJACK name. The problem with
> > this is that there is a fixed length to the port name in ALSA, and for
> > an increasing number of devices, this behavior truncates critical
> > information from the end of the iJACK name.
> >
> > For example, culled from lsusb output:
> >
> > Bus 005 Device 098: ID 1235:0145 Focusrite-Novation Launchkey MK4 49
> >   iProduct                2 Launchkey MK4 49
> >     iConfiguration          4 Launchkey MK4 49
> >       iInterface              5 Launchkey MK4 49 MIDI
> >       iInterface              6 Launchkey MK4 49 MIDI Int
> >         iJack                   7 Launchkey MK4 49 MIDI In
> >         iJack                   8 Launchkey MK4 49 MIDI Out
> >         iJack                   9 Launchkey MK4 49 DAW In
> >
> > gives these port names:
> >
> > 44:0    Launchkey MK4 49                 Launchkey MK4 49 Launchkey MK4
> >  44:1    Launchkey MK4 49                 Launchkey MK4 49 Launchkey MK4
> >
> > which is useless since it fails to include the "DAW" and "MIDI Out"
> > parts of the iJACK names.
> >
> > The correct fix, I think is to only prepend the iProduct name if that
> > name is not present at the start of the iJACK name. This will cover
> > devices where the manufacturer does *not* include the device name in
> > the iJACK name, but will fix this situation for devices where they do
> > (Novation and Arturia and many others do this).
> >
> > If agreed, I can construct a patch for this.
>
> So you're suggesting to remove the substring matched with iProduct at
> copying from iJack name?  That'll make sense and go ahead.
>
>
> thanks,
>
> Takashi

Re: ALSA USB MIDI port names are poorly constructed and truncate iJACK names

[Takashi Iwai]

On Tue, 22 Apr 2025 19:31:59 +0200,
Paul Davis wrote:
> 
> This patch was developed using the Debian 6.12.12 kernel source, but I
> believe it works without changes for current git.
> 
> The approach taken is to always look for the iProduct name
> (umidi->card->shortname) in the iJACK name (jack_name). If not found,
> the code path is unchanged. If found, a different string format is
> used to create the port name, using only the jack name. Note that
> since finding the product name in the jack name implies the jack name
> is not default_jack_name, there is never any reason to use "number" in
> constructing the port name.
> 
> This generates the expected port names for several USB MIDI devices
> here, some of which have "product name in jack name" and some which do
> not:
> 
> % aplaymidi -l
>  Port    Client name                      Port name
>  14:0    Midi Through                     Midi Through Port-0
>  16:0    Scarlett 18i20 USB               Scarlett 18i20 USB MIDI 1
>  36:0    Keystation 88                    Keystation 88 MIDI 1
>  40:0    Launchkey MK4 49                 Launchkey MK4 49 MIDI In
>  40:1    Launchkey MK4 49                 Launchkey MK4 49 DAW In
>  44:0    MidiSport 2x2                    MidiSport 2x2 MIDI 1
>  44:1    MidiSport 2x2                    MidiSport 2x2 MIDI 2
> 
> 
> Signed-off-by: Paul Davis <paul@linuxaudiosystems.com>
> 
> --- midi.c.orig    2025-04-22 11:20:30.192338737 -0600
> +++ midi.c    2025-04-22 11:20:47.373155820 -0600
> @@ -1817,10 +1817,18 @@ static void snd_usbmidi_init_substream(s
>      }
> 
>      port_info = find_port_info(umidi, number);
> -    name_format = port_info ? port_info->name :
> -        (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> -    snprintf(substream->name, sizeof(substream->name),
> -         name_format, umidi->card->shortname, jack_name, number + 1);
> +
> +    if (strncmp (umidi->card->shortname, jack_name, strlen
> (umidi->card->shortname)) != 0) {
> +        name_format = port_info ? port_info->name :
> +            (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> +        snprintf(substream->name, sizeof(substream->name),
> +                 name_format, umidi->card->shortname, jack_name, number + 1);
> +    } else {
> +        /* The manufacturer included the iProduct name in the jack
> name, do not use both */
> +        name_format = port_info ? port_info->name : "%s";
> +        snprintf(substream->name, sizeof(substream->name),
> name_format, jack_name);
> +    }

Thanks for the patch.

However, dropping the number suffix will break the uniqueness.
For example, in your case above, MidiSport 2x2 will have two identical
port names, I suppose?

IMO, we should keep adding the suffix number unless the uniqueness is
guaranteed instead of blindly dropping.  Admittedly the current code
isn't in the best form and this also needs more modification, too.

Also, the pre-defined port names defined in snd_usb_midi_port_info[]
have the format like "%s Synth", too, and those should be adjusted, as
well.

Last but not least, after we get a final form, could you try to submit
a patch in the proper format as defined in
  Documentation/process/submitting-patches.rst?


thanks,

Takashi

Re: ALSA USB MIDI port names are poorly constructed and truncate iJACK names

[Paul Davis]

No, this is not correct.

The number is only used when constructing a "default jack name" as in
"SHORT_DEVICE_NAME MIDI NUMBER". It is never used when the jack name
is given by the manufacturer.

In the case of the Midisport 2x2, for example, the manufacturer does
not provide jack names, and so we create "Midisport 2x2 MIDI 1" and
"Midisport 2x2 MIDI 2". You can see this in the example output I
provided.

If the manufacturer has provided the jack name, the code already
assumed that the jack names will be unique (because otherwise things
would just break on Windows and macOS, and because they bothered to
provide names). And if the jack name includes the product name, even
more reason to continue with this assumption.

I realized after I sent the patch that I had run diff -uRN at the
wrong level of the source tree, but I did read the doc and other than
that, I think there were no other errors? I run a standard debian
kernel and so am not working with git.

On Wed, Apr 23, 2025 at 12:45 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Tue, 22 Apr 2025 19:31:59 +0200,
> Paul Davis wrote:
> >
> > This patch was developed using the Debian 6.12.12 kernel source, but I
> > believe it works without changes for current git.
> >
> > The approach taken is to always look for the iProduct name
> > (umidi->card->shortname) in the iJACK name (jack_name). If not found,
> > the code path is unchanged. If found, a different string format is
> > used to create the port name, using only the jack name. Note that
> > since finding the product name in the jack name implies the jack name
> > is not default_jack_name, there is never any reason to use "number" in
> > constructing the port name.
> >
> > This generates the expected port names for several USB MIDI devices
> > here, some of which have "product name in jack name" and some which do
> > not:
> >
> > % aplaymidi -l
> >  Port    Client name                      Port name
> >  14:0    Midi Through                     Midi Through Port-0
> >  16:0    Scarlett 18i20 USB               Scarlett 18i20 USB MIDI 1
> >  36:0    Keystation 88                    Keystation 88 MIDI 1
> >  40:0    Launchkey MK4 49                 Launchkey MK4 49 MIDI In
> >  40:1    Launchkey MK4 49                 Launchkey MK4 49 DAW In
> >  44:0    MidiSport 2x2                    MidiSport 2x2 MIDI 1
> >  44:1    MidiSport 2x2                    MidiSport 2x2 MIDI 2
> >
> >
> > Signed-off-by: Paul Davis <paul@linuxaudiosystems.com>
> >
> > --- midi.c.orig    2025-04-22 11:20:30.192338737 -0600
> > +++ midi.c    2025-04-22 11:20:47.373155820 -0600
> > @@ -1817,10 +1817,18 @@ static void snd_usbmidi_init_substream(s
> >      }
> >
> >      port_info = find_port_info(umidi, number);
> > -    name_format = port_info ? port_info->name :
> > -        (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > -    snprintf(substream->name, sizeof(substream->name),
> > -         name_format, umidi->card->shortname, jack_name, number + 1);
> > +
> > +    if (strncmp (umidi->card->shortname, jack_name, strlen
> > (umidi->card->shortname)) != 0) {
> > +        name_format = port_info ? port_info->name :
> > +            (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > +        snprintf(substream->name, sizeof(substream->name),
> > +                 name_format, umidi->card->shortname, jack_name, number + 1);
> > +    } else {
> > +        /* The manufacturer included the iProduct name in the jack
> > name, do not use both */
> > +        name_format = port_info ? port_info->name : "%s";
> > +        snprintf(substream->name, sizeof(substream->name),
> > name_format, jack_name);
> > +    }
>
> Thanks for the patch.
>
> However, dropping the number suffix will break the uniqueness.
> For example, in your case above, MidiSport 2x2 will have two identical
> port names, I suppose?
>
> IMO, we should keep adding the suffix number unless the uniqueness is
> guaranteed instead of blindly dropping.  Admittedly the current code
> isn't in the best form and this also needs more modification, too.
>
> Also, the pre-defined port names defined in snd_usb_midi_port_info[]
> have the format like "%s Synth", too, and those should be adjusted, as
> well.
>
> Last but not least, after we get a final form, could you try to submit
> a patch in the proper format as defined in
>   Documentation/process/submitting-patches.rst?
>
>
> thanks,
>
> Takashi

Re: ALSA USB MIDI port names are poorly constructed and truncate iJACK names

[Takashi Iwai]

On Fri, 25 Apr 2025 16:39:59 +0200,
Paul Davis wrote:
> 
> No, this is not correct.
> 
> The number is only used when constructing a "default jack name" as in
> "SHORT_DEVICE_NAME MIDI NUMBER". It is never used when the jack name
> is given by the manufacturer.
> 
> In the case of the Midisport 2x2, for example, the manufacturer does
> not provide jack names, and so we create "Midisport 2x2 MIDI 1" and
> "Midisport 2x2 MIDI 2". You can see this in the example output I
> provided.
> 
> If the manufacturer has provided the jack name, the code already
> assumed that the jack names will be unique (because otherwise things
> would just break on Windows and macOS, and because they bothered to
> provide names). And if the jack name includes the product name, even
> more reason to continue with this assumption.

Ah OK, now it's clearer.  In anyway, if you give the result after your
patch for comparison, it'll give a better demonstration.

But another point about the predefined port_info is still valid, I
suppose.  snd_usbmidi_port_info[] provides the own format strings, and
they are all like "%s Tone Generator", where "%s" should be filled
with the shortname, and not jack_name.

So, when port_info is defined, we should skip the duplicated word
check.  That being said, the change can be something like:

-- 8< --
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -1885,10 +1885,18 @@ static void snd_usbmidi_init_substream(struct snd_usb_midi *umidi,
 	}
 
 	port_info = find_port_info(umidi, number);
-	name_format = port_info ? port_info->name :
-		(jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
-	snprintf(substream->name, sizeof(substream->name),
-		 name_format, umidi->card->shortname, jack_name, number + 1);
+	if (port_info || jack_name == default_jack_name ||
+	    strncmp(umidi->card->shortname, jack_name, strlen(umidi->card->shortname)) != 0) {
+		name_format = port_info ? port_info->name :
+			(jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
+		snprintf(substream->name, sizeof(substream->name),
+			 name_format, umidi->card->shortname, jack_name, number + 1);
+	} else {
+		/* The manufacturer included the iProduct name in the jack
+		 * name, do not use both
+		 */
+		strscpy(substream->name, jack_name);
+	}
 
 	*rsubstream = substream;
 }
-- 8< --

> I realized after I sent the patch that I had run diff -uRN at the
> wrong level of the source tree, but I did read the doc and other than
> that, I think there were no other errors? I run a standard debian
> kernel and so am not working with git.

Indeed, the patch level isn't right, but the biggest problem is that
tabs/spaces are broken (possibly due to your mailer), so it can't be
applied as is, unfortunately.


thanks,

Takashi

> On Wed, Apr 23, 2025 at 12:45 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > On Tue, 22 Apr 2025 19:31:59 +0200,
> > Paul Davis wrote:
> > >
> > > This patch was developed using the Debian 6.12.12 kernel source, but I
> > > believe it works without changes for current git.
> > >
> > > The approach taken is to always look for the iProduct name
> > > (umidi->card->shortname) in the iJACK name (jack_name). If not found,
> > > the code path is unchanged. If found, a different string format is
> > > used to create the port name, using only the jack name. Note that
> > > since finding the product name in the jack name implies the jack name
> > > is not default_jack_name, there is never any reason to use "number" in
> > > constructing the port name.
> > >
> > > This generates the expected port names for several USB MIDI devices
> > > here, some of which have "product name in jack name" and some which do
> > > not:
> > >
> > > % aplaymidi -l
> > >  Port    Client name                      Port name
> > >  14:0    Midi Through                     Midi Through Port-0
> > >  16:0    Scarlett 18i20 USB               Scarlett 18i20 USB MIDI 1
> > >  36:0    Keystation 88                    Keystation 88 MIDI 1
> > >  40:0    Launchkey MK4 49                 Launchkey MK4 49 MIDI In
> > >  40:1    Launchkey MK4 49                 Launchkey MK4 49 DAW In
> > >  44:0    MidiSport 2x2                    MidiSport 2x2 MIDI 1
> > >  44:1    MidiSport 2x2                    MidiSport 2x2 MIDI 2
> > >
> > >
> > > Signed-off-by: Paul Davis <paul@linuxaudiosystems.com>
> > >
> > > --- midi.c.orig    2025-04-22 11:20:30.192338737 -0600
> > > +++ midi.c    2025-04-22 11:20:47.373155820 -0600
> > > @@ -1817,10 +1817,18 @@ static void snd_usbmidi_init_substream(s
> > >      }
> > >
> > >      port_info = find_port_info(umidi, number);
> > > -    name_format = port_info ? port_info->name :
> > > -        (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > > -    snprintf(substream->name, sizeof(substream->name),
> > > -         name_format, umidi->card->shortname, jack_name, number + 1);
> > > +
> > > +    if (strncmp (umidi->card->shortname, jack_name, strlen
> > > (umidi->card->shortname)) != 0) {
> > > +        name_format = port_info ? port_info->name :
> > > +            (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > > +        snprintf(substream->name, sizeof(substream->name),
> > > +                 name_format, umidi->card->shortname, jack_name, number + 1);
> > > +    } else {
> > > +        /* The manufacturer included the iProduct name in the jack
> > > name, do not use both */
> > > +        name_format = port_info ? port_info->name : "%s";
> > > +        snprintf(substream->name, sizeof(substream->name),
> > > name_format, jack_name);
> > > +    }
> >
> > Thanks for the patch.
> >
> > However, dropping the number suffix will break the uniqueness.
> > For example, in your case above, MidiSport 2x2 will have two identical
> > port names, I suppose?
> >
> > IMO, we should keep adding the suffix number unless the uniqueness is
> > guaranteed instead of blindly dropping.  Admittedly the current code
> > isn't in the best form and this also needs more modification, too.
> >
> > Also, the pre-defined port names defined in snd_usb_midi_port_info[]
> > have the format like "%s Synth", too, and those should be adjusted, as
> > well.
> >
> > Last but not least, after we get a final form, could you try to submit
> > a patch in the proper format as defined in
> >   Documentation/process/submitting-patches.rst?
> >
> >
> > thanks,
> >
> > Takashi

Re: ALSA USB MIDI port names are poorly constructed and truncate iJACK names

[Paul Davis]

I already included the examples from before and after upthread.

The tabs/spaces is my editor, not my mailer. I'm not I can be bothered
to fix it at this point (I'm set up to edit Ardour, not the kernel).
especially since you want the substreams renamed too. I should leave
it with you.

On Fri, Apr 25, 2025 at 9:10 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Fri, 25 Apr 2025 16:39:59 +0200,
> Paul Davis wrote:
> >
> > No, this is not correct.
> >
> > The number is only used when constructing a "default jack name" as in
> > "SHORT_DEVICE_NAME MIDI NUMBER". It is never used when the jack name
> > is given by the manufacturer.
> >
> > In the case of the Midisport 2x2, for example, the manufacturer does
> > not provide jack names, and so we create "Midisport 2x2 MIDI 1" and
> > "Midisport 2x2 MIDI 2". You can see this in the example output I
> > provided.
> >
> > If the manufacturer has provided the jack name, the code already
> > assumed that the jack names will be unique (because otherwise things
> > would just break on Windows and macOS, and because they bothered to
> > provide names). And if the jack name includes the product name, even
> > more reason to continue with this assumption.
>
> Ah OK, now it's clearer.  In anyway, if you give the result after your
> patch for comparison, it'll give a better demonstration.
>
> But another point about the predefined port_info is still valid, I
> suppose.  snd_usbmidi_port_info[] provides the own format strings, and
> they are all like "%s Tone Generator", where "%s" should be filled
> with the shortname, and not jack_name.
>
> So, when port_info is defined, we should skip the duplicated word
> check.  That being said, the change can be something like:
>
> -- 8< --
> --- a/sound/usb/midi.c
> +++ b/sound/usb/midi.c
> @@ -1885,10 +1885,18 @@ static void snd_usbmidi_init_substream(struct snd_usb_midi *umidi,
>         }
>
>         port_info = find_port_info(umidi, number);
> -       name_format = port_info ? port_info->name :
> -               (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> -       snprintf(substream->name, sizeof(substream->name),
> -                name_format, umidi->card->shortname, jack_name, number + 1);
> +       if (port_info || jack_name == default_jack_name ||
> +           strncmp(umidi->card->shortname, jack_name, strlen(umidi->card->shortname)) != 0) {
> +               name_format = port_info ? port_info->name :
> +                       (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> +               snprintf(substream->name, sizeof(substream->name),
> +                        name_format, umidi->card->shortname, jack_name, number + 1);
> +       } else {
> +               /* The manufacturer included the iProduct name in the jack
> +                * name, do not use both
> +                */
> +               strscpy(substream->name, jack_name);
> +       }
>
>         *rsubstream = substream;
>  }
> -- 8< --
>
> > I realized after I sent the patch that I had run diff -uRN at the
> > wrong level of the source tree, but I did read the doc and other than
> > that, I think there were no other errors? I run a standard debian
> > kernel and so am not working with git.
>
> Indeed, the patch level isn't right, but the biggest problem is that
> tabs/spaces are broken (possibly due to your mailer), so it can't be
> applied as is, unfortunately.
>
>
> thanks,
>
> Takashi
>
> > On Wed, Apr 23, 2025 at 12:45 AM Takashi Iwai <tiwai@suse.de> wrote:
> > >
> > > On Tue, 22 Apr 2025 19:31:59 +0200,
> > > Paul Davis wrote:
> > > >
> > > > This patch was developed using the Debian 6.12.12 kernel source, but I
> > > > believe it works without changes for current git.
> > > >
> > > > The approach taken is to always look for the iProduct name
> > > > (umidi->card->shortname) in the iJACK name (jack_name). If not found,
> > > > the code path is unchanged. If found, a different string format is
> > > > used to create the port name, using only the jack name. Note that
> > > > since finding the product name in the jack name implies the jack name
> > > > is not default_jack_name, there is never any reason to use "number" in
> > > > constructing the port name.
> > > >
> > > > This generates the expected port names for several USB MIDI devices
> > > > here, some of which have "product name in jack name" and some which do
> > > > not:
> > > >
> > > > % aplaymidi -l
> > > >  Port    Client name                      Port name
> > > >  14:0    Midi Through                     Midi Through Port-0
> > > >  16:0    Scarlett 18i20 USB               Scarlett 18i20 USB MIDI 1
> > > >  36:0    Keystation 88                    Keystation 88 MIDI 1
> > > >  40:0    Launchkey MK4 49                 Launchkey MK4 49 MIDI In
> > > >  40:1    Launchkey MK4 49                 Launchkey MK4 49 DAW In
> > > >  44:0    MidiSport 2x2                    MidiSport 2x2 MIDI 1
> > > >  44:1    MidiSport 2x2                    MidiSport 2x2 MIDI 2
> > > >
> > > >
> > > > Signed-off-by: Paul Davis <paul@linuxaudiosystems.com>
> > > >
> > > > --- midi.c.orig    2025-04-22 11:20:30.192338737 -0600
> > > > +++ midi.c    2025-04-22 11:20:47.373155820 -0600
> > > > @@ -1817,10 +1817,18 @@ static void snd_usbmidi_init_substream(s
> > > >      }
> > > >
> > > >      port_info = find_port_info(umidi, number);
> > > > -    name_format = port_info ? port_info->name :
> > > > -        (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > > > -    snprintf(substream->name, sizeof(substream->name),
> > > > -         name_format, umidi->card->shortname, jack_name, number + 1);
> > > > +
> > > > +    if (strncmp (umidi->card->shortname, jack_name, strlen
> > > > (umidi->card->shortname)) != 0) {
> > > > +        name_format = port_info ? port_info->name :
> > > > +            (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > > > +        snprintf(substream->name, sizeof(substream->name),
> > > > +                 name_format, umidi->card->shortname, jack_name, number + 1);
> > > > +    } else {
> > > > +        /* The manufacturer included the iProduct name in the jack
> > > > name, do not use both */
> > > > +        name_format = port_info ? port_info->name : "%s";
> > > > +        snprintf(substream->name, sizeof(substream->name),
> > > > name_format, jack_name);
> > > > +    }
> > >
> > > Thanks for the patch.
> > >
> > > However, dropping the number suffix will break the uniqueness.
> > > For example, in your case above, MidiSport 2x2 will have two identical
> > > port names, I suppose?
> > >
> > > IMO, we should keep adding the suffix number unless the uniqueness is
> > > guaranteed instead of blindly dropping.  Admittedly the current code
> > > isn't in the best form and this also needs more modification, too.
> > >
> > > Also, the pre-defined port names defined in snd_usb_midi_port_info[]
> > > have the format like "%s Synth", too, and those should be adjusted, as
> > > well.
> > >
> > > Last but not least, after we get a final form, could you try to submit
> > > a patch in the proper format as defined in
> > >   Documentation/process/submitting-patches.rst?
> > >
> > >
> > > thanks,
> > >
> > > Takashi

Re: ALSA USB MIDI port names are poorly constructed and truncate iJACK names

[Paul Davis]

I can, however, confirm that the patch as you reformulated works as intended.

On Fri, Apr 25, 2025 at 9:10 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Fri, 25 Apr 2025 16:39:59 +0200,
> Paul Davis wrote:
> >
> > No, this is not correct.
> >
> > The number is only used when constructing a "default jack name" as in
> > "SHORT_DEVICE_NAME MIDI NUMBER". It is never used when the jack name
> > is given by the manufacturer.
> >
> > In the case of the Midisport 2x2, for example, the manufacturer does
> > not provide jack names, and so we create "Midisport 2x2 MIDI 1" and
> > "Midisport 2x2 MIDI 2". You can see this in the example output I
> > provided.
> >
> > If the manufacturer has provided the jack name, the code already
> > assumed that the jack names will be unique (because otherwise things
> > would just break on Windows and macOS, and because they bothered to
> > provide names). And if the jack name includes the product name, even
> > more reason to continue with this assumption.
>
> Ah OK, now it's clearer.  In anyway, if you give the result after your
> patch for comparison, it'll give a better demonstration.
>
> But another point about the predefined port_info is still valid, I
> suppose.  snd_usbmidi_port_info[] provides the own format strings, and
> they are all like "%s Tone Generator", where "%s" should be filled
> with the shortname, and not jack_name.
>
> So, when port_info is defined, we should skip the duplicated word
> check.  That being said, the change can be something like:
>
> -- 8< --
> --- a/sound/usb/midi.c
> +++ b/sound/usb/midi.c
> @@ -1885,10 +1885,18 @@ static void snd_usbmidi_init_substream(struct snd_usb_midi *umidi,
>         }
>
>         port_info = find_port_info(umidi, number);
> -       name_format = port_info ? port_info->name :
> -               (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> -       snprintf(substream->name, sizeof(substream->name),
> -                name_format, umidi->card->shortname, jack_name, number + 1);
> +       if (port_info || jack_name == default_jack_name ||
> +           strncmp(umidi->card->shortname, jack_name, strlen(umidi->card->shortname)) != 0) {
> +               name_format = port_info ? port_info->name :
> +                       (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> +               snprintf(substream->name, sizeof(substream->name),
> +                        name_format, umidi->card->shortname, jack_name, number + 1);
> +       } else {
> +               /* The manufacturer included the iProduct name in the jack
> +                * name, do not use both
> +                */
> +               strscpy(substream->name, jack_name);
> +       }
>
>         *rsubstream = substream;
>  }
> -- 8< --
>
> > I realized after I sent the patch that I had run diff -uRN at the
> > wrong level of the source tree, but I did read the doc and other than
> > that, I think there were no other errors? I run a standard debian
> > kernel and so am not working with git.
>
> Indeed, the patch level isn't right, but the biggest problem is that
> tabs/spaces are broken (possibly due to your mailer), so it can't be
> applied as is, unfortunately.
>
>
> thanks,
>
> Takashi
>
> > On Wed, Apr 23, 2025 at 12:45 AM Takashi Iwai <tiwai@suse.de> wrote:
> > >
> > > On Tue, 22 Apr 2025 19:31:59 +0200,
> > > Paul Davis wrote:
> > > >
> > > > This patch was developed using the Debian 6.12.12 kernel source, but I
> > > > believe it works without changes for current git.
> > > >
> > > > The approach taken is to always look for the iProduct name
> > > > (umidi->card->shortname) in the iJACK name (jack_name). If not found,
> > > > the code path is unchanged. If found, a different string format is
> > > > used to create the port name, using only the jack name. Note that
> > > > since finding the product name in the jack name implies the jack name
> > > > is not default_jack_name, there is never any reason to use "number" in
> > > > constructing the port name.
> > > >
> > > > This generates the expected port names for several USB MIDI devices
> > > > here, some of which have "product name in jack name" and some which do
> > > > not:
> > > >
> > > > % aplaymidi -l
> > > >  Port    Client name                      Port name
> > > >  14:0    Midi Through                     Midi Through Port-0
> > > >  16:0    Scarlett 18i20 USB               Scarlett 18i20 USB MIDI 1
> > > >  36:0    Keystation 88                    Keystation 88 MIDI 1
> > > >  40:0    Launchkey MK4 49                 Launchkey MK4 49 MIDI In
> > > >  40:1    Launchkey MK4 49                 Launchkey MK4 49 DAW In
> > > >  44:0    MidiSport 2x2                    MidiSport 2x2 MIDI 1
> > > >  44:1    MidiSport 2x2                    MidiSport 2x2 MIDI 2
> > > >
> > > >
> > > > Signed-off-by: Paul Davis <paul@linuxaudiosystems.com>
> > > >
> > > > --- midi.c.orig    2025-04-22 11:20:30.192338737 -0600
> > > > +++ midi.c    2025-04-22 11:20:47.373155820 -0600
> > > > @@ -1817,10 +1817,18 @@ static void snd_usbmidi_init_substream(s
> > > >      }
> > > >
> > > >      port_info = find_port_info(umidi, number);
> > > > -    name_format = port_info ? port_info->name :
> > > > -        (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > > > -    snprintf(substream->name, sizeof(substream->name),
> > > > -         name_format, umidi->card->shortname, jack_name, number + 1);
> > > > +
> > > > +    if (strncmp (umidi->card->shortname, jack_name, strlen
> > > > (umidi->card->shortname)) != 0) {
> > > > +        name_format = port_info ? port_info->name :
> > > > +            (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > > > +        snprintf(substream->name, sizeof(substream->name),
> > > > +                 name_format, umidi->card->shortname, jack_name, number + 1);
> > > > +    } else {
> > > > +        /* The manufacturer included the iProduct name in the jack
> > > > name, do not use both */
> > > > +        name_format = port_info ? port_info->name : "%s";
> > > > +        snprintf(substream->name, sizeof(substream->name),
> > > > name_format, jack_name);
> > > > +    }
> > >
> > > Thanks for the patch.
> > >
> > > However, dropping the number suffix will break the uniqueness.
> > > For example, in your case above, MidiSport 2x2 will have two identical
> > > port names, I suppose?
> > >
> > > IMO, we should keep adding the suffix number unless the uniqueness is
> > > guaranteed instead of blindly dropping.  Admittedly the current code
> > > isn't in the best form and this also needs more modification, too.
> > >
> > > Also, the pre-defined port names defined in snd_usb_midi_port_info[]
> > > have the format like "%s Synth", too, and those should be adjusted, as
> > > well.
> > >
> > > Last but not least, after we get a final form, could you try to submit
> > > a patch in the proper format as defined in
> > >   Documentation/process/submitting-patches.rst?
> > >
> > >
> > > thanks,
> > >
> > > Takashi

Re: ALSA USB MIDI port names are poorly constructed and truncate iJACK names

[Takashi Iwai]

On Mon, 28 Apr 2025 22:36:30 +0200,
Paul Davis wrote:
> 
> I already included the examples from before and after upthread.

You're right, I didn't check from the beginning of the thread
carefully enough.

> The tabs/spaces is my editor, not my mailer. I'm not I can be bothered
> to fix it at this point (I'm set up to edit Ardour, not the kernel).
> especially since you want the substreams renamed too. I should leave
> it with you.

OK, let me write up and submit the patch.


thanks,

Takashi

> 
> On Fri, Apr 25, 2025 at 9:10 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > On Fri, 25 Apr 2025 16:39:59 +0200,
> > Paul Davis wrote:
> > >
> > > No, this is not correct.
> > >
> > > The number is only used when constructing a "default jack name" as in
> > > "SHORT_DEVICE_NAME MIDI NUMBER". It is never used when the jack name
> > > is given by the manufacturer.
> > >
> > > In the case of the Midisport 2x2, for example, the manufacturer does
> > > not provide jack names, and so we create "Midisport 2x2 MIDI 1" and
> > > "Midisport 2x2 MIDI 2". You can see this in the example output I
> > > provided.
> > >
> > > If the manufacturer has provided the jack name, the code already
> > > assumed that the jack names will be unique (because otherwise things
> > > would just break on Windows and macOS, and because they bothered to
> > > provide names). And if the jack name includes the product name, even
> > > more reason to continue with this assumption.
> >
> > Ah OK, now it's clearer.  In anyway, if you give the result after your
> > patch for comparison, it'll give a better demonstration.
> >
> > But another point about the predefined port_info is still valid, I
> > suppose.  snd_usbmidi_port_info[] provides the own format strings, and
> > they are all like "%s Tone Generator", where "%s" should be filled
> > with the shortname, and not jack_name.
> >
> > So, when port_info is defined, we should skip the duplicated word
> > check.  That being said, the change can be something like:
> >
> > -- 8< --
> > --- a/sound/usb/midi.c
> > +++ b/sound/usb/midi.c
> > @@ -1885,10 +1885,18 @@ static void snd_usbmidi_init_substream(struct snd_usb_midi *umidi,
> >         }
> >
> >         port_info = find_port_info(umidi, number);
> > -       name_format = port_info ? port_info->name :
> > -               (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > -       snprintf(substream->name, sizeof(substream->name),
> > -                name_format, umidi->card->shortname, jack_name, number + 1);
> > +       if (port_info || jack_name == default_jack_name ||
> > +           strncmp(umidi->card->shortname, jack_name, strlen(umidi->card->shortname)) != 0) {
> > +               name_format = port_info ? port_info->name :
> > +                       (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > +               snprintf(substream->name, sizeof(substream->name),
> > +                        name_format, umidi->card->shortname, jack_name, number + 1);
> > +       } else {
> > +               /* The manufacturer included the iProduct name in the jack
> > +                * name, do not use both
> > +                */
> > +               strscpy(substream->name, jack_name);
> > +       }
> >
> >         *rsubstream = substream;
> >  }
> > -- 8< --
> >
> > > I realized after I sent the patch that I had run diff -uRN at the
> > > wrong level of the source tree, but I did read the doc and other than
> > > that, I think there were no other errors? I run a standard debian
> > > kernel and so am not working with git.
> >
> > Indeed, the patch level isn't right, but the biggest problem is that
> > tabs/spaces are broken (possibly due to your mailer), so it can't be
> > applied as is, unfortunately.
> >
> >
> > thanks,
> >
> > Takashi
> >
> > > On Wed, Apr 23, 2025 at 12:45 AM Takashi Iwai <tiwai@suse.de> wrote:
> > > >
> > > > On Tue, 22 Apr 2025 19:31:59 +0200,
> > > > Paul Davis wrote:
> > > > >
> > > > > This patch was developed using the Debian 6.12.12 kernel source, but I
> > > > > believe it works without changes for current git.
> > > > >
> > > > > The approach taken is to always look for the iProduct name
> > > > > (umidi->card->shortname) in the iJACK name (jack_name). If not found,
> > > > > the code path is unchanged. If found, a different string format is
> > > > > used to create the port name, using only the jack name. Note that
> > > > > since finding the product name in the jack name implies the jack name
> > > > > is not default_jack_name, there is never any reason to use "number" in
> > > > > constructing the port name.
> > > > >
> > > > > This generates the expected port names for several USB MIDI devices
> > > > > here, some of which have "product name in jack name" and some which do
> > > > > not:
> > > > >
> > > > > % aplaymidi -l
> > > > >  Port    Client name                      Port name
> > > > >  14:0    Midi Through                     Midi Through Port-0
> > > > >  16:0    Scarlett 18i20 USB               Scarlett 18i20 USB MIDI 1
> > > > >  36:0    Keystation 88                    Keystation 88 MIDI 1
> > > > >  40:0    Launchkey MK4 49                 Launchkey MK4 49 MIDI In
> > > > >  40:1    Launchkey MK4 49                 Launchkey MK4 49 DAW In
> > > > >  44:0    MidiSport 2x2                    MidiSport 2x2 MIDI 1
> > > > >  44:1    MidiSport 2x2                    MidiSport 2x2 MIDI 2
> > > > >
> > > > >
> > > > > Signed-off-by: Paul Davis <paul@linuxaudiosystems.com>
> > > > >
> > > > > --- midi.c.orig    2025-04-22 11:20:30.192338737 -0600
> > > > > +++ midi.c    2025-04-22 11:20:47.373155820 -0600
> > > > > @@ -1817,10 +1817,18 @@ static void snd_usbmidi_init_substream(s
> > > > >      }
> > > > >
> > > > >      port_info = find_port_info(umidi, number);
> > > > > -    name_format = port_info ? port_info->name :
> > > > > -        (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > > > > -    snprintf(substream->name, sizeof(substream->name),
> > > > > -         name_format, umidi->card->shortname, jack_name, number + 1);
> > > > > +
> > > > > +    if (strncmp (umidi->card->shortname, jack_name, strlen
> > > > > (umidi->card->shortname)) != 0) {
> > > > > +        name_format = port_info ? port_info->name :
> > > > > +            (jack_name != default_jack_name  ? "%s %s" : "%s %s %d");
> > > > > +        snprintf(substream->name, sizeof(substream->name),
> > > > > +                 name_format, umidi->card->shortname, jack_name, number + 1);
> > > > > +    } else {
> > > > > +        /* The manufacturer included the iProduct name in the jack
> > > > > name, do not use both */
> > > > > +        name_format = port_info ? port_info->name : "%s";
> > > > > +        snprintf(substream->name, sizeof(substream->name),
> > > > > name_format, jack_name);
> > > > > +    }
> > > >
> > > > Thanks for the patch.
> > > >
> > > > However, dropping the number suffix will break the uniqueness.
> > > > For example, in your case above, MidiSport 2x2 will have two identical
> > > > port names, I suppose?
> > > >
> > > > IMO, we should keep adding the suffix number unless the uniqueness is
> > > > guaranteed instead of blindly dropping.  Admittedly the current code
> > > > isn't in the best form and this also needs more modification, too.
> > > >
> > > > Also, the pre-defined port names defined in snd_usb_midi_port_info[]
> > > > have the format like "%s Synth", too, and those should be adjusted, as
> > > > well.
> > > >
> > > > Last but not least, after we get a final form, could you try to submit
> > > > a patch in the proper format as defined in
> > > >   Documentation/process/submitting-patches.rst?
> > > >
> > > >
> > > > thanks,
> > > >
> > > > Takashi
>