[alsa-devel] [PATCH] Provide sequencer sound card number / PID via alsa-lib
From: Martin Koegler martin.koegler@chello.at
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.
It supports kernels with and without the required support.
Signed-off-by: Martin Koegler martin.koegler@chello.at --- include/seq.h | 1 + include/sound/asequencer.h | 5 +++-- src/seq/seq.c | 11 +++++++++++ src/seq/seq_hw.c | 8 +++++++- 4 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/include/seq.h b/include/seq.h index 9576822..77996e5 100644 --- a/include/seq.h +++ b/include/seq.h @@ -143,6 +143,7 @@ snd_seq_client_type_t snd_seq_client_info_get_type(const snd_seq_client_info_t * const char *snd_seq_client_info_get_name(snd_seq_client_info_t *info); int snd_seq_client_info_get_broadcast_filter(const snd_seq_client_info_t *info); int snd_seq_client_info_get_error_bounce(const snd_seq_client_info_t *info); +int snd_seq_client_info_get_owner(const snd_seq_client_info_t *info); const unsigned char *snd_seq_client_info_get_event_filter(const snd_seq_client_info_t *info); int snd_seq_client_info_get_num_ports(const snd_seq_client_info_t *info); int snd_seq_client_info_get_event_lost(const snd_seq_client_info_t *info); diff --git a/include/sound/asequencer.h b/include/sound/asequencer.h index 09c8a00..7ebf7fd 100644 --- a/include/sound/asequencer.h +++ b/include/sound/asequencer.h @@ -24,7 +24,7 @@
/** 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 @@ -356,7 +356,8 @@ 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 owner; /* RO: card number[kernel] / PID[user] */ + char reserved[64 - sizeof(int)]; /* for future use */ };
diff --git a/src/seq/seq.c b/src/seq/seq.c index 620ca3f..2505293 100644 --- a/src/seq/seq.c +++ b/src/seq/seq.c @@ -1522,6 +1522,17 @@ int snd_seq_client_info_get_error_bounce(const snd_seq_client_info_t *info) }
/** + * \brief Get the sound card number (kernel) or owning PID + * \param info client_info container + * \return card number/PID/-1 if value is not available. + */ +int snd_seq_client_info_get_owner(const snd_seq_client_info_t *info) +{ + assert(info); + return info->owner; +} + +/** * \brief (DEPRECATED) Get the event filter bitmap of a client_info container * \param info client_info container * \return NULL if no event filter, or pointer to event filter bitmap diff --git a/src/seq/seq_hw.c b/src/seq/seq_hw.c index d033367..a4c083a 100644 --- a/src/seq/seq_hw.c +++ b/src/seq/seq_hw.c @@ -32,10 +32,11 @@ const char *_snd_module_seq_hw = ""; #ifndef DOC_HIDDEN #define SNDRV_FILE_SEQ ALSA_DEVICE_DIRECTORY "seq" #define SNDRV_FILE_ALOADSEQ ALOAD_DEVICE_DIRECTORY "aloadSEQ" -#define SNDRV_SEQ_VERSION_MAX SNDRV_PROTOCOL_VERSION(1, 0, 1) +#define SNDRV_SEQ_VERSION_MAX SNDRV_PROTOCOL_VERSION(1, 0, 2)
typedef struct { int fd; + int micro_version; } snd_seq_hw_t; #endif /* DOC_HIDDEN */
@@ -100,6 +101,8 @@ static int snd_seq_hw_get_client_info(snd_seq_t *seq, snd_seq_client_info_t * in /*SYSERR("SNDRV_SEQ_IOCTL_GET_CLIENT_INFO failed");*/ return -errno; } + if (hw->micro_version < SNDRV_PROTOCOL_MICRO(SNDRV_SEQ_VERSION_MAX)) + info->owner = -1; return 0; }
@@ -368,6 +371,8 @@ static int snd_seq_hw_query_next_client(snd_seq_t *seq, snd_seq_client_info_t *i /*SYSERR("SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT failed");*/ return -errno; } + if (hw->micro_version < SNDRV_PROTOCOL_MICRO(SNDRV_SEQ_VERSION_MAX)) + info->owner = -1; return 0; }
@@ -480,6 +485,7 @@ int snd_seq_hw_open(snd_seq_t **handle, const char *name, int streams, int mode) return -ENOMEM; } hw->fd = fd; + hw->micro_version = SNDRV_PROTOCOL_MICRO(ver); if (streams & SND_SEQ_OPEN_OUTPUT) { seq->obuf = (char *) malloc(seq->obufsize = SND_SEQ_OBUF_SIZE); if (!seq->obuf) {
From: Martin Koegler martin.koegler@chello.at
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 patch adds support for displaying the sound card number/PID to aconnect.
Signed-off-by: Martin Koegler martin.koegler@chello.at --- seq/aconnect/aconnect.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/seq/aconnect/aconnect.c b/seq/aconnect/aconnect.c index 8d6cebb..3508e1b 100644 --- a/seq/aconnect/aconnect.c +++ b/seq/aconnect/aconnect.c @@ -166,11 +166,19 @@ static void print_port(snd_seq_t *seq, snd_seq_client_info_t *cinfo, snd_seq_port_info_t *pinfo, int count) { if (! count) { - printf(_("client %d: '%s' [type=%s]\n"), + printf(_("client %d: '%s' [type=%s"), snd_seq_client_info_get_client(cinfo), snd_seq_client_info_get_name(cinfo), (snd_seq_client_info_get_type(cinfo) == SND_SEQ_USER_CLIENT ? _("user") : _("kernel"))); + int owner = snd_seq_client_info_get_owner(cinfo); + if (owner != -1) { + if (snd_seq_client_info_get_type(cinfo) == SND_SEQ_USER_CLIENT) + printf(",pid=%d", owner); + else + printf(",card=%d", owner); + } + printf("]\n"); } printf(" %3d '%-16s'\n", snd_seq_port_info_get_port(pinfo),
From: Martin Koegler martin.koegler@chello.at
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 | 5 +++-- sound/core/seq/seq_clientmgr.c | 18 ++++++++++++++++++ sound/core/seq/seq_clientmgr.h | 2 ++ 3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/include/uapi/sound/asequencer.h b/include/uapi/sound/asequencer.h index 5a5fa49..4726579 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,8 @@ 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 owner; /* RO: card number[kernel] / PID[user] */ + char reserved[64 - sizeof(int)]; /* for future use */ };
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 58e79e0..ac07ab7 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,21 @@ 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; + + switch (cptr->type) { + case USER_CLIENT: + info->owner = pid_vnr(cptr->data.user.owner); + break; + + case KERNEL_CLIENT: + info->owner = cptr->data.kernel.card ? cptr->data.kernel.card->number : -1; + break; + + default: + info->owner = -1; + break; + } + memset(info->reserved, 0, sizeof(info->reserved)); }
@@ -2271,6 +2288,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; };
On Sat, 13 Feb 2016 14:42:17 +0100, Martin Koegler wrote:
From: Martin Koegler martin.koegler@chello.at
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
The idea looks good. One remaining question is whether only providing the card number or pid is sufficient for all cases.
Some review comments below:
include/uapi/sound/asequencer.h | 5 +++-- sound/core/seq/seq_clientmgr.c | 18 ++++++++++++++++++ sound/core/seq/seq_clientmgr.h | 2 ++ 3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/include/uapi/sound/asequencer.h b/include/uapi/sound/asequencer.h index 5a5fa49..4726579 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,8 @@ 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 owner; /* RO: card number[kernel] / PID[user] */
- char reserved[64 - sizeof(int)]; /* for future use */
The sizeof(int) is always 4. So let's make it explicit.
But, please make sure that this change won't lead to any incompatibility. Some architectures might align with 64bit long, although the 32bit int and the rest char[] should be fine on all known architectures, AFAIK.
};
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 58e79e0..ac07ab7 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);
Shouldn't the counterpart (put_pid()) be called anywhere else?
thanks,
Takashi
Takashi Iwai wrote:
Martin Koegler wrote:
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
+++ b/include/uapi/sound/asequencer.h @@ -357,7 +357,8 @@ struct snd_seq_client_info {
- char reserved[64]; /* for future use */
- int owner; /* RO: card number[kernel] / PID[user] */
- char reserved[64 - sizeof(int)]; /* for future use */
The sizeof(int) is always 4. So let's make it explicit.
But, please make sure that this change won't lead to any incompatibility. Some architectures might align with 64bit long, although the 32bit int and the rest char[] should be fine on all known architectures, AFAIK.
Or just make it two fields, card and pid. The two values are currently exclusive, but there is no technical reason for this.
Regards, Clemens
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.
@@ -357,7 +357,8 @@ 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 owner; /* RO: card number[kernel] / PID[user] */
- char reserved[64 - sizeof(int)]; /* for future use */
The sizeof(int) is always 4. So let's make it explicit.
But, please make sure that this change won't lead to any incompatibility. Some architectures might align with 64bit long, although the 32bit int and the rest char[] should be fine on all known architectures, AFAIK.
Sorry, I don't have access to the various non x86-architectures, so I can't test.
I will change to int card; int pid; char reserve[56];
More safety would just provide a much more complex (ugly?) structure:
struct _int_snd_seq_client_info_old { int client; /* client number to inquire */ snd_seq_client_type_t type; /* client type */ char name[64]; /* client name */ unsigned int filter; /* filter flags */ unsigned char multicast_filter[8]; /* multicast filter bitmap */ 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 */ };
struct _int_snd_seq_client_info_new { int client; /* client number to inquire */ snd_seq_client_type_t type; /* client type */ char name[64]; /* client name */ unsigned int filter; /* filter flags */ unsigned char multicast_filter[8]; /* multicast filter bitmap */ unsigned char event_filter[32]; /* event filter bitmap */ int num_ports; /* RO: number of ports */ int event_lost; /* number of lost events */ int card; int pid; char reserved[64]; /* for future use */ };
struct snd_seq_client_info { int client; /* client number to inquire */ snd_seq_client_type_t type; /* client type */ char name[64]; /* client name */ unsigned int filter; /* filter flags */ unsigned char multicast_filter[8]; /* multicast filter bitmap */ unsigned char event_filter[32]; /* event filter bitmap */ int num_ports; /* RO: number of ports */ int event_lost; /* number of lost events */ int card; int pid; char reserved[64 - sizeof(struct _int_snd_seq_client_info_new) + sizeof(_int_snd_seq_client_info_old)]; /* for future use */ };
This should handle all alignment and type size issues automatically.
If you really want this, I can provide the apropriate patches.
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 58e79e0..ac07ab7 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);Shouldn't the counterpart (put_pid()) be called anywhere else?
The only way to free a client structure is seq_free_client1 (static). This is called by snd_seq_open before get_pid [=> irelevant] and seq_free_client (static). seq_free_client is called by snd_seq_release, which calls put_pid and snd_seq_delete_kernel_client, which destroys a kernel client.
Are I'm missing a code path?
Regards, Martin
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?
@@ -357,7 +357,8 @@ 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 owner; /* RO: card number[kernel] / PID[user] */
- char reserved[64 - sizeof(int)]; /* for future use */
The sizeof(int) is always 4. So let's make it explicit.
But, please make sure that this change won't lead to any incompatibility. Some architectures might align with 64bit long, although the 32bit int and the rest char[] should be fine on all known architectures, AFAIK.
Sorry, I don't have access to the various non x86-architectures, so I can't test.
Usually it's tested by cross-compiling, so everyone can do it.
I will change to int card; int pid; char reserve[56];
Yes, this would be better.
More safety would just provide a much more complex (ugly?) structure:
struct _int_snd_seq_client_info_old { int client; /* client number to inquire */ snd_seq_client_type_t type; /* client type */ char name[64]; /* client name */ unsigned int filter; /* filter flags */ unsigned char multicast_filter[8]; /* multicast filter bitmap */ 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 */ };
struct _int_snd_seq_client_info_new { int client; /* client number to inquire */ snd_seq_client_type_t type; /* client type */ char name[64]; /* client name */ unsigned int filter; /* filter flags */ unsigned char multicast_filter[8]; /* multicast filter bitmap */ unsigned char event_filter[32]; /* event filter bitmap */ int num_ports; /* RO: number of ports */ int event_lost; /* number of lost events */ int card; int pid; char reserved[64]; /* for future use */ };
struct snd_seq_client_info { int client; /* client number to inquire */ snd_seq_client_type_t type; /* client type */ char name[64]; /* client name */ unsigned int filter; /* filter flags */ unsigned char multicast_filter[8]; /* multicast filter bitmap */ unsigned char event_filter[32]; /* event filter bitmap */ int num_ports; /* RO: number of ports */ int event_lost; /* number of lost events */ int card; int pid; char reserved[64 - sizeof(struct _int_snd_seq_client_info_new) + sizeof(_int_snd_seq_client_info_old)]; /* for future use */ };
This should handle all alignment and type size issues automatically.
You'd need to define old and new structs without reserved field. The calculation still works but it's bogus.
If you really want this, I can provide the apropriate patches.
No, I never want such a thing :)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 58e79e0..ac07ab7 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);Shouldn't the counterpart (put_pid()) be called anywhere else?
The only way to free a client structure is seq_free_client1 (static). This is called by snd_seq_open before get_pid [=> irelevant] and seq_free_client (static). seq_free_client is called by snd_seq_release, which calls put_pid and snd_seq_delete_kernel_client, which destroys a kernel client.
OK, I misread the patch where to place the code. Then this should be fine.
Takashi
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
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
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
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
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
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
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...
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
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...
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
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...
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
Martin Koegler wrote:
+++ b/src/seq/seq.c /**
- \brief Get the sound card number (kernel) or owning PID
- \param info client_info container
- \return card number/PID/-1 if value is not available.
- */
Please document that card/pid are valid only for clients of type SND_SEQ_KERNEL/USER_CLIENT.
And I think two functions for card and pid are more appropriate.
Regards, Clemens
participants (3)
-
Clemens Ladisch -
Martin Koegler -
Takashi Iwai