From alsa-devel-owner@alsa.jcu.cz  Thu Feb 25 20:49:37 1999
Received: from post.mail.demon.net (finch-post-11.mail.demon.net [194.217.242.39])
	by marvin.jcu.cz (8.9.1a/8.9.1) with ESMTP id UAA27967
	for <alsa-devel@alsa.jcu.cz>; Thu, 25 Feb 1999 20:49:30 +0100
Received: from [212.228.182.246] (helo=ariel.sr.home)
	by post.mail.demon.net with esmtp (Exim 2.12 #1)
	id 10G6mj-000G8i-00
	for alsa-devel@alsa.jcu.cz; Thu, 25 Feb 1999 19:49:25 +0000
Received: (from steve@localhost)
	by ariel.sr.home (8.8.7/8.8.7) id TAA14346
	for alsa-devel@alsa.jcu.cz; Thu, 25 Feb 1999 19:32:57 GMT
Date: Thu, 25 Feb 1999 19:32:57 GMT
From: Steve Ratcliffe <steve@parabola.demon.co.uk>
Message-Id: <199902251932.TAA14346@ariel.sr.home>
To: alsa-devel@alsa.jcu.cz
Subject: Re: Instrument abstract layer
Reply-To: alsa-devel@alsa.jcu.cz
Sender: alsa-devel-owner@alsa.jcu.cz
Precedence: list


I've looked over the proposal and have some comments.  I like the idea
of having the DIRECT priority to avoid copying of large events, while
still being able to use queuing when appropriate.

First some general comments.

1. I was going to suggest that a much stronger separation should be made
between messages that would be used to communicate from an app to a
patch manager and from a patch manager to a lowlevel driver.
A low level driver would seldom want to know the patch
name for example.

Of course, a separate patch manager would not be necessary, but the
distinction should still be made, especially as I thought that it was
a design goal of the sequencer that this could be done.

Conceptually there are two operations here (1) selecting the instrument,
(2) doing whatever is needed to load that instrument (sysex, load
patch/samples or whatever) The current ALLOC is a bit of a mixture of
the two, and by separating them out, we could achieve one interface
at the app level that maps on to different but standard interfaces to
different kinds of equipment.

I've just seen the mail from Michael Ashton and I need
to ask him some questions about this before saying any more.

2. I believe that the events should be separated out into request and
reply events with their own argument types.  At present the arguments
have input and output parameters mixed together.  Code would be clearer
to have this separated. For example BEGIN:

SND_SEQ_EVENT_INSTR_BEGIN
 typedef struct {
 	long timeout;		/* zero = forever, otherwise timeout in ms */
 } snd_seq_instrument_begin_t;

SND_SEQ_EVENT_INSTR_REPLY
 /* Generic reply for when command has no data to return. */
 typedef struct {
 	int  result;
 } snd_seq_instrument_reply_t;

For more complex cases you would have pairs such as
	SND_SEQ_EVENT_INSTR_LIST / SND_SEQ_EVENT_INSTR_LIST_REPLY.
	snd_seq_instrument_xxx_t and snd_seq_instrument_xxx_reply_t

3. The GET event uses a data pointer, while the ALLOC and LIST events
have the data within the event itself.  Why not use the same method
for all three.  I think that I like using the pointer.  It seems
that this would work better in the app-->app case where shared
memory is being used, but I haven't really thought much about it.

Minor comments and corrections:

> SND_SEQ_EVENT_INSTR_ALLOC
>   - download instrument to synthesizer's memory

ALLOC is not really descriptive of what this does.  How about
SND_SEQ_EVENT_INSTR_LOAD or SND_SEQ_EVENT_INSTR_PUT?

> FREE
> ----
>
> The pointer to the instrument and total length of header + data will be
> stored to snd_seq_ev_ext structure.

Is there really extra data in this event?

> STATUS
> ------
>
> typedef struct {
 ...
> } snd_seq_instrument_info_t;

Should be snd_seq_instrument_status_t

From your later email:
> CLUSTER
> -------
>
> typedef struct {
> 		int result;                     /* operation result */
> 		char name[32];                  /* cluster name, '\0' => get */
> 		int priority;                   /* priority, -1 = get */
> 		char reserved[64];
> } snd_seq_instrument_cluster_t;

Is this instead of : typedef unsigned long snd_seq_instrument_cluster_t;
that was defined in the original proposal or is it a name clash?

Regards,

..Steve

