diff mbox

Provide card number / PID via sequencer client info

Message ID 20160215214454.GA11052@mail.zuhause (mailing list archive)
State New, archived
Headers show

Commit Message

Martin Koegler Feb. 15, 2016, 9:44 p.m. UTC
On Mon, Feb 15, 2016 at 08:08:12PM +0100, Takashi Iwai wrote:
> On Mon, 15 Feb 2016 19:32:43 +0100,
> Martin Koegler wrote:
> > 
> > On Mon, Feb 15, 2016 at 11:30:15AM +0100, Takashi Iwai wrote:
> > > The idea looks good.  One remaining question is whether only providing
> > > the card number or pid is sufficient for all cases.
> > 
> > I only care about the kernel client. 
> 
> Well, the question is whether only card number is enough.  What if
> cards provide multiple rawmidi devices?

Aren't they currently ports?
seq_midi.c creates just one sequencer device per card number.

Regards,
Martin

Comments

Takashi Iwai Feb. 15, 2016, 10:34 p.m. UTC | #1
On Mon, 15 Feb 2016 22:44:54 +0100,
Martin Koegler wrote:
> 
> On Mon, Feb 15, 2016 at 08:08:12PM +0100, Takashi Iwai wrote:
> > On Mon, 15 Feb 2016 19:32:43 +0100,
> > Martin Koegler wrote:
> > > 
> > > On Mon, Feb 15, 2016 at 11:30:15AM +0100, Takashi Iwai wrote:
> > > > The idea looks good.  One remaining question is whether only providing
> > > > the card number or pid is sufficient for all cases.
> > > 
> > > I only care about the kernel client. 
> > 
> > Well, the question is whether only card number is enough.  What if
> > cards provide multiple rawmidi devices?
> 
> Aren't they currently ports?
> seq_midi.c creates just one sequencer device per card number.

Look at cards with synth support, e.g. emu10k1 or OPL3/OPL4.


Takashi
Martin Koegler Feb. 16, 2016, 8:03 a.m. UTC | #2
On Mon, Feb 15, 2016 at 11:34:09PM +0100, Takashi Iwai wrote:
> > On Mon, Feb 15, 2016 at 08:08:12PM +0100, Takashi Iwai wrote:
> > > On Mon, 15 Feb 2016 19:32:43 +0100,
> > > Martin Koegler wrote:
> > > > 
> > > > On Mon, Feb 15, 2016 at 11:30:15AM +0100, Takashi Iwai wrote:
> > > > > The idea looks good.  One remaining question is whether only providing
> > > > > the card number or pid is sufficient for all cases.
> > > > 
> > > > I only care about the kernel client. 
> > > 
> > > Well, the question is whether only card number is enough.  What if
> > > cards provide multiple rawmidi devices?
> > 
> > Aren't they currently ports?
> > seq_midi.c creates just one sequencer device per card number.
> 
> Look at cards with synth support, e.g. emu10k1 or OPL3/OPL4.

Should I export the client_index parameter of snd_seq_create_kernel_client too?

Regards,
Martin
Takashi Iwai Feb. 16, 2016, 8:41 a.m. UTC | #3
On Tue, 16 Feb 2016 09:03:34 +0100,
Martin Koegler wrote:
> 
> On Mon, Feb 15, 2016 at 11:34:09PM +0100, Takashi Iwai wrote:
> > > On Mon, Feb 15, 2016 at 08:08:12PM +0100, Takashi Iwai wrote:
> > > > On Mon, 15 Feb 2016 19:32:43 +0100,
> > > > Martin Koegler wrote:
> > > > > 
> > > > > On Mon, Feb 15, 2016 at 11:30:15AM +0100, Takashi Iwai wrote:
> > > > > > The idea looks good.  One remaining question is whether only providing
> > > > > > the card number or pid is sufficient for all cases.
> > > > > 
> > > > > I only care about the kernel client. 
> > > > 
> > > > Well, the question is whether only card number is enough.  What if
> > > > cards provide multiple rawmidi devices?
> > > 
> > > Aren't they currently ports?
> > > seq_midi.c creates just one sequencer device per card number.
> > 
> > Look at cards with synth support, e.g. emu10k1 or OPL3/OPL4.
> 
> Should I export the client_index parameter of snd_seq_create_kernel_client too?

Well, I'd rather ask what are the requirements -- in wide ranges.

As you already noticed, multiple rawmidi devices and rawmidi
subdevices are mapped into a single client with various ports.  But we
have no exact mapping information there, too.  So, if you want to have
an exact mapping between sequencer and rawmidi, wouldn't you need the
similar information in snd_seq_port_info, too?

And, as mentioned, some cards provide indeed multiple clients.  How do
you identify which client is for what?  So far, it's identified only
from the name string and the type bits of each port.  Isn't it enough?
If not, what have to be provided?


Takashi
Clemens Ladisch Feb. 16, 2016, 8:59 a.m. UTC | #4
Takashi Iwai wrote:
> Martin Koegler wrote:
>> On Mon, Feb 15, 2016 at 11:34:09PM +0100, Takashi Iwai wrote:
>>>> On Mon, Feb 15, 2016 at 08:08:12PM +0100, Takashi Iwai wrote:
>>>>> Well, the question is whether only card number is enough.  What if
>>>>> cards provide multiple rawmidi devices?
>>>>
>>>> Aren't they currently ports?
>>>> seq_midi.c creates just one sequencer device per card number.
>>>
>>> Look at cards with synth support, e.g. emu10k1 or OPL3/OPL4.

Or snd-virmidi.

>> Should I export the client_index parameter of snd_seq_create_kernel_client too?
>
> Well, I'd rather ask what are the requirements -- in wide ranges.

The original requirement was just the ability to get the card name.

> As you already noticed, multiple rawmidi devices and rawmidi
> subdevices are mapped into a single client with various ports.

Actually, multiple devices get multiple clients.

> And, as mentioned, some cards provide indeed multiple clients.  How do
> you identify which client is for what?  So far, it's identified only
> from the name string and the type bits of each port.  Isn't it enough?
> If not, what have to be provided?

This patch is about the card.  If we really need a method to identify
the device, we can still add it later -- this patch does not obstruct
such an extension.


Regards,
Clemens
Takashi Iwai Feb. 16, 2016, 9:27 a.m. UTC | #5
On Tue, 16 Feb 2016 09:59:37 +0100,
Clemens Ladisch wrote:
> 
> Takashi Iwai wrote:
> > Martin Koegler wrote:
> >> On Mon, Feb 15, 2016 at 11:34:09PM +0100, Takashi Iwai wrote:
> >>>> On Mon, Feb 15, 2016 at 08:08:12PM +0100, Takashi Iwai wrote:
> >>>>> Well, the question is whether only card number is enough.  What if
> >>>>> cards provide multiple rawmidi devices?
> >>>>
> >>>> Aren't they currently ports?
> >>>> seq_midi.c creates just one sequencer device per card number.
> >>>
> >>> Look at cards with synth support, e.g. emu10k1 or OPL3/OPL4.
> 
> Or snd-virmidi.
> 
> >> Should I export the client_index parameter of snd_seq_create_kernel_client too?
> >
> > Well, I'd rather ask what are the requirements -- in wide ranges.
> 
> The original requirement was just the ability to get the card name.

Yep.  And it's the assumption of only rawmidi, I suppose.  How would
it differentiate from synth?

> > As you already noticed, multiple rawmidi devices and rawmidi
> > subdevices are mapped into a single client with various ports.
> 
> Actually, multiple devices get multiple clients.

I thought of so, but reading back seq_midi.c, it looks different...

> > And, as mentioned, some cards provide indeed multiple clients.  How do
> > you identify which client is for what?  So far, it's identified only
> > from the name string and the type bits of each port.  Isn't it enough?
> > If not, what have to be provided?
> 
> This patch is about the card.  If we really need a method to identify
> the device, we can still add it later -- this patch does not obstruct
> such an extension.

Sure, it won't conflict.  But, the question is (back to square)
whether it's enough.

If we know that we shall change the API again, it's better to consider
the design well from the beginning.  A short-living API version bump
is the thing we should avoid as much as possible.  It's nothing but a
needless stress for maintainers :)

That said, I'm not particularly against the currently proposed change.
My only concern is whether we need any further change in (near)
future.


Takashi
Martin Koegler Feb. 16, 2016, 6:21 p.m. UTC | #6
On Tue, Feb 16, 2016 at 10:27:20AM +0100, Takashi Iwai wrote:
> On Tue, 16 Feb 2016 09:59:37 +0100,
> > >>>> Aren't they currently ports?
> > >>>> seq_midi.c creates just one sequencer device per card number.
> > >>>
> > >>> Look at cards with synth support, e.g. emu10k1 or OPL3/OPL4.
> > 
> > Or snd-virmidi.
> > 
> > >> Should I export the client_index parameter of snd_seq_create_kernel_client too?
> > >
> > > Well, I'd rather ask what are the requirements -- in wide ranges.
> > 
> > The original requirement was just the ability to get the card name.
> 
> Yep.  And it's the assumption of only rawmidi, I suppose.  How would
> it differentiate from synth?

My users are using a few identical USB MIDI keyboards [or USB MIDI interfaces] and
I need to recognize the devices even after reboot. 

=> I want a stable, unique (composite) device ID for kernel sequencer clients+ports.

My current ID is: client name, sysfs-path and port number.

The sound card number allows to find the device in sysfs and determine the used USB port as stable identifier.
I hope, that the registration order for the ports of one USB MIDI device [or sound card] will always be the same.

see my prototype: https://build.opensuse.org/package/view_file/home:e9925248:branches:openSUSE:Leap:42.1:Update/grandorgue/0001-Add-support-for-unique-device-IDs.patch?expand=1

OPL3/4 + emuXXX synth register with a different name, so they should be distinct. 

> > > And, as mentioned, some cards provide indeed multiple clients.  How do
> > > you identify which client is for what?  So far, it's identified only
> > > from the name string and the type bits of each port.  Isn't it enough?
> > > If not, what have to be provided?
> > 
> > This patch is about the card.  If we really need a method to identify
> > the device, we can still add it later -- this patch does not obstruct
> > such an extension.
> 
> Sure, it won't conflict.  But, the question is (back to square)
> whether it's enough.
> 
> If we know that we shall change the API again, it's better to consider
> the design well from the beginning.  A short-living API version bump
> is the thing we should avoid as much as possible.  It's nothing but a
> needless stress for maintainers :)
> 
> That said, I'm not particularly against the currently proposed change.
> My only concern is whether we need any further change in (near)
> future.

+1

I just ask myself, if the device index would provide any useful information.

I don't know, how to map it to a anything below the soundcard in sysfs. 
Is the device ID dymanic? [= Are they depending, if I first load the rawmidi module and then the synth module or the other way around].

Regards,
Martin
Takashi Iwai Feb. 17, 2016, 2:07 p.m. UTC | #7
On Tue, 16 Feb 2016 19:21:37 +0100,
Martin Koegler wrote:
> 
> On Tue, Feb 16, 2016 at 10:27:20AM +0100, Takashi Iwai wrote:
> > On Tue, 16 Feb 2016 09:59:37 +0100,
> > > >>>> Aren't they currently ports?
> > > >>>> seq_midi.c creates just one sequencer device per card number.
> > > >>>
> > > >>> Look at cards with synth support, e.g. emu10k1 or OPL3/OPL4.
> > > 
> > > Or snd-virmidi.
> > > 
> > > >> Should I export the client_index parameter of snd_seq_create_kernel_client too?
> > > >
> > > > Well, I'd rather ask what are the requirements -- in wide ranges.
> > > 
> > > The original requirement was just the ability to get the card name.
> > 
> > Yep.  And it's the assumption of only rawmidi, I suppose.  How would
> > it differentiate from synth?
> 
> My users are using a few identical USB MIDI keyboards [or USB MIDI interfaces] and
> I need to recognize the devices even after reboot. 
> 
> => I want a stable, unique (composite) device ID for kernel sequencer clients+ports.
> 
> My current ID is: client name, sysfs-path and port number.
> 
> The sound card number allows to find the device in sysfs and determine the used USB port as stable identifier.
> I hope, that the registration order for the ports of one USB MIDI device [or sound card] will always be the same.
> 
> see my prototype: https://build.opensuse.org/package/view_file/home:e9925248:branches:openSUSE:Leap:42.1:Update/grandorgue/0001-Add-support-for-unique-device-IDs.patch?expand=1
> 
> OPL3/4 + emuXXX synth register with a different name, so they should be distinct. 

Yeah, there is a difference, of course.  But the identifying by the
name string suffices?  In other words, how would user-space identify
the client at best?  Passing another attribute to client_info like
port type?


> > > > And, as mentioned, some cards provide indeed multiple clients.  How do
> > > > you identify which client is for what?  So far, it's identified only
> > > > from the name string and the type bits of each port.  Isn't it enough?
> > > > If not, what have to be provided?
> > > 
> > > This patch is about the card.  If we really need a method to identify
> > > the device, we can still add it later -- this patch does not obstruct
> > > such an extension.
> > 
> > Sure, it won't conflict.  But, the question is (back to square)
> > whether it's enough.
> > 
> > If we know that we shall change the API again, it's better to consider
> > the design well from the beginning.  A short-living API version bump
> > is the thing we should avoid as much as possible.  It's nothing but a
> > needless stress for maintainers :)
> > 
> > That said, I'm not particularly against the currently proposed change.
> > My only concern is whether we need any further change in (near)
> > future.
> 
> +1
> 
> I just ask myself, if the device index would provide any useful information.
 
Which "device index" do you mean?  The index passed at created
snd_seq_create_kerne_client()?

> I don't know, how to map it to a anything below the soundcard in sysfs. 
> Is the device ID dymanic? [= Are they depending, if I first load the rawmidi module and then the synth module or the other way around].

If you mean about the client index offset, then usually they are fixed
by each driver.  And even the client id number is fixed (up to 4
clients per card):
  client = 16 + card * 4 + index.

So, in most case, the card number can be calculated even without the
new information.  But, this is rather heuristic, and a clear mapping
would be better, of course.


Takashi
Martin Koegler Feb. 17, 2016, 9:30 p.m. UTC | #8
On Wed, Feb 17, 2016 at 03:07:25PM +0100, Takashi Iwai wrote:
> > My users are using a few identical USB MIDI keyboards [or USB MIDI interfaces] and
> > I need to recognize the devices even after reboot. 
> > 
> > => I want a stable, unique (composite) device ID for kernel sequencer clients+ports.
> > 
> > My current ID is: client name, sysfs-path and port number.
> > 
> > The sound card number allows to find the device in sysfs and determine the used USB port as stable identifier.
> > I hope, that the registration order for the ports of one USB MIDI device [or sound card] will always be the same.
> > 
> > see my prototype: https://build.opensuse.org/package/view_file/home:e9925248:branches:openSUSE:Leap:42.1:Update/grandorgue/0001-Add-support-for-unique-device-IDs.patch?expand=1
> > 
> > OPL3/4 + emuXXX synth register with a different name, so they should be distinct. 
> 
> Yeah, there is a difference, of course.  But the identifying by the
> name string suffices?  In other words, how would user-space identify
> the client at best?  Passing another attribute to client_info like
> port type?

port type is not property of struct snd_seq_client - so it is better reported via port info.
It already has SND_SEQ_PORT_TYPE_SYNTHESIZER and SND_SEQ_PORT_TYPE_PORT.

For my usecase, I don't care about the type.
Either the user select it from a drop-down or uses auto-detection: He triggers at request 
a MIDI event and the port with activity is selected - If you want to see it in action: https://www.youtube.com/watch?v=3g7H1W4x2cg [at 1:20]

> > I just ask myself, if the device index would provide any useful information.
>  
> Which "device index" do you mean?  The index passed at created
> snd_seq_create_kerne_client()?

Yes.

> > I don't know, how to map it to a anything below the soundcard in sysfs. 
> > Is the device ID dymanic? [= Are they depending, if I first load the rawmidi module and then the synth module or the other way around].
> 
> If you mean about the client index offset, then usually they are fixed
> by each driver.  And even the client id number is fixed (up to 4
> clients per card):
>   client = 16 + card * 4 + index.
> 
> So, in most case, the card number can be calculated even without the
> new information.  But, this is rather heuristic, and a clear mapping
> would be better, of course.

The key word is "usually". Its only suitable for an interface, if it is "always" fixed or can be used as an pointer on sysfs.

Regards,
Martin
diff mbox

Patch

From 133a8b3e059a09bf50ce62ae9960472dc09e71d2 Mon Sep 17 00:00:00 2001
From: Martin Koegler <martin.koegler@chello.at>
Date: Thu, 11 Feb 2016 20:07:20 +0100
Subject: [PATCH] Provide card number / PID via sequencer client info

rawmidi devices expose the card number via IOCTLs, which allows to
find the corresponding device in sysfs.

The sequencer provides no identifing data. Chromium works around this
issue by scanning rawmidi as well as sequencer devices and matching
them by using assumtions, how the kernel register sequencer devices.

This changes adds support for exposing the card number for kernel clients
as well as the PID for user client.

The minor of the API version is changed to distinguish between the zero
initialised reserved field and card number 0.

Signed-off-by: Martin Koegler <martin.koegler@chello.at>
---
 include/uapi/sound/asequencer.h |  6 ++++--
 sound/core/seq/seq_clientmgr.c  | 14 ++++++++++++++
 sound/core/seq/seq_clientmgr.h  |  2 ++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/include/uapi/sound/asequencer.h b/include/uapi/sound/asequencer.h
index 5a5fa49..8c7da5a 100644
--- a/include/uapi/sound/asequencer.h
+++ b/include/uapi/sound/asequencer.h
@@ -25,7 +25,7 @@ 
 #include <sound/asound.h>
 
 /** version of the sequencer */
-#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION (1, 0, 1)
+#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION (1, 0, 2)
 
 /**
  * definition of sequencer event types
@@ -357,7 +357,9 @@  struct snd_seq_client_info {
 	unsigned char event_filter[32];	/* event filter bitmap */
 	int num_ports;			/* RO: number of ports */
 	int event_lost;			/* number of lost events */
-	char reserved[64];		/* for future use */
+	int card;			/* RO: card number[kernel] */
+	int pid;			/* RO: pid[user] */
+	char reserved[56];		/* for future use */
 };
 
 
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 58e79e0..d6d9419 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -364,6 +364,7 @@  static int snd_seq_open(struct inode *inode, struct file *file)
 	/* fill client data */
 	user->file = file;
 	sprintf(client->name, "Client-%d", c);
+	client->data.user.owner = get_pid(task_pid(current));
 
 	/* make others aware this new client */
 	snd_seq_system_client_ev_client_start(c);
@@ -380,6 +381,7 @@  static int snd_seq_release(struct inode *inode, struct file *file)
 		seq_free_client(client);
 		if (client->data.user.fifo)
 			snd_seq_fifo_delete(&client->data.user.fifo);
+		put_pid(client->data.user.owner);
 		kfree(client);
 	}
 
@@ -1197,6 +1199,17 @@  static void get_client_info(struct snd_seq_client *cptr,
 	info->event_lost = cptr->event_lost;
 	memcpy(info->event_filter, cptr->event_filter, 32);
 	info->num_ports = cptr->num_ports;
+
+	if (cptr->type == USER_CLIENT)
+		info->pid = pid_vnr(cptr->data.user.owner);
+	else
+		info->pid = -1;
+
+	if (cptr->type == KERNEL_CLIENT)
+		info->card = cptr->data.kernel.card ? cptr->data.kernel.card->number : -1;
+	else
+		info->card = -1;
+
 	memset(info->reserved, 0, sizeof(info->reserved));
 }
 
@@ -2271,6 +2284,7 @@  int snd_seq_create_kernel_client(struct snd_card *card, int client_index,
 
 	client->accept_input = 1;
 	client->accept_output = 1;
+	client->data.kernel.card = card;
 		
 	va_start(args, name_fmt);
 	vsnprintf(client->name, sizeof(client->name), name_fmt, args);
diff --git a/sound/core/seq/seq_clientmgr.h b/sound/core/seq/seq_clientmgr.h
index 20f0a72..031462e 100644
--- a/sound/core/seq/seq_clientmgr.h
+++ b/sound/core/seq/seq_clientmgr.h
@@ -33,6 +33,7 @@ 
 struct snd_seq_user_client {
 	struct file *file;	/* file struct of client */
 	/* ... */
+	struct pid* owner;
 	
 	/* fifo */
 	struct snd_seq_fifo *fifo;	/* queue for incoming events */
@@ -41,6 +42,7 @@  struct snd_seq_user_client {
 
 struct snd_seq_kernel_client {
 	/* ... */
+	struct snd_card* card;
 };
 
 
-- 
2.1.4