[-,alsa-utils,5/5] The amidicat program itself, better late than never
diff mbox

Message ID 1404118503-22921-6-git-send-email-alsa@krellan.com
State Accepted
Delegated to: Takashi Iwai
Headers show

Commit Message

Josh Lehan June 30, 2014, 8:55 a.m. UTC
Signed-off-by: Josh Lehan <alsa@krellan.com>

Comments

Clemens Ladisch June 30, 2014, 11:33 a.m. UTC | #1
Josh Lehan wrote:
> +++ b/amidicat/amidicat.c
> +#define MAX_HEX_DIGITS		(2)

Is this symbol defined for future compatibility with larger bytes?  ;-)

> +	int		is_hex;

For booleans, use "bool" from <stdbool.h>.

> +prettyprint_capmask(unsigned int capmask)
> +{
> +	/* The r/w letters indicate read/write capability */
> +	/* Capital R/W letters indicate subscription */

Would anybody understand the difference?

Just restrict the listing to ports that you can handle.

> +			port_name = strdup(
> +				snd_seq_port_info_get_name(port_info));

Please note that checkpatch is just a tool to find problems; introducing
new problems like this just to shut it up is counterproductive.

(Overlong lines could be a sign that the nesting is too deep, but the
ALSA function names do not help ...)

> +str_to_cli_port(snd_seq_t *handle, char *str, int *outcli, int *outport)

Why not use snd_seq_parse_address()?

> +	r = snd_seq_set_client_name(handle, cli_name);
> +	if (r < 0) {
> +		/* This early in the program, it's not threaded,
> +		 * errno is OK to use */
> +		fprintf(stderr, "Unable to set ALSA client name: %s\n",
> +			strerror(errno));

The error code is in r.  Use snd_strerror().

(And errno _is_ thread safe.)

> +	/* FUTURE: Do we need any more type bits here? */
> +	port_id = snd_seq_create_simple_port(handle, cli_name, caps,
> +		SND_SEQ_PORT_TYPE_MIDI_GENERIC);

You should set all appropriate TYPE bits.  You must set CAP bits if you
want to allow others to connect from/to this port.

> +		r = snd_seq_event_output_direct(handle, ev);
> +		if (r < 0) {
> +			/* Return if a real error happened */
> +			if (-EAGAIN != r)

EAGAIN can never happen if the device has not been configured to be
non-blocking.

> +		r = snd_seq_drain_output(handle);

snd_seq_event_output_direct() bypasses the output buffer, so this does
not make sense.

> +		r = snd_seq_sync_output_queue(handle);

You have not scheduled any events to be delivered later, so this does
not make sense.

> +		/* FUTURE: Why does snd_seq_sync_output_queue()
> +		 * always return 1, not 0 as it should? */

Because the documentation is wrong.

> +			r = printf("%02X%s", ui,
> +				((size_left > 1) ? " " : "\n"));

Why not use %c?

> +		/* Standard C whitespace */
> +		/* Avoid usage of isspace()
> +		 * because that would introduce locale variations */

Where does this program change the locale to be different from "C"?
(And what would it matter if a Klingon space were to be ignored? :-)

> +		/* Send a dummy message to ourself,
> +		 * so ALSA gets unblocked in other thread */
> +		snd_seq_ev_clear(&ev);

This results in a SND_SEQ_EVENT_SYSTEM event.  You might want to use
one of the USR events ...

> +		/* Check global flag, under lock,
> +		 * see if other thread is telling us to exit */
> +		pthread_mutex_lock(&g_mutex);
> +		is_endinput = g_is_endinput;
> +		pthread_mutex_unlock(&g_mutex);

... and if you check for that event (and its source address) here, you
don't need a separate mutex.

> +	/* Apologies for the awkward line breaks,
> +	 * the mandate of the checkpatch script left no alternative */

checkpatch looks for printk().  Ignore it.

> +	pthread_cond_init(&g_cond, NULL);

This is not used.


Regards,
Clemens
Josh Lehan July 1, 2014, 8:10 a.m. UTC | #2
On 06/30/2014 04:33 AM, Clemens Ladisch wrote:
> Josh Lehan wrote:
>> +++ b/amidicat/amidicat.c
>> +#define MAX_HEX_DIGITS		(2)
> 
> Is this symbol defined for future compatibility with larger bytes?  ;-)

No, it's just to avoid a magic number in the parser later on, makes it
somewhat easier to read.

>> +	int		is_hex;
> 
> For booleans, use "bool" from <stdbool.h>.

Good suggestion.

>> +prettyprint_capmask(unsigned int capmask)
>> +{
>> +	/* The r/w letters indicate read/write capability */
>> +	/* Capital R/W letters indicate subscription */
> 
> Would anybody understand the difference?
> 
> Just restrict the listing to ports that you can handle.

I wanted to give a complete listing, because it's useful for diagnostics
and troubleshooting.  It can handle both direct and subscription
connections.

It's surprising how different the various permissions are, for the
default devices on my system, and so it's useful to see this.

>> +			port_name = strdup(
>> +				snd_seq_port_info_get_name(port_info));
> 
> Please note that checkpatch is just a tool to find problems; introducing
> new problems like this just to shut it up is counterproductive.

Didn't think this was a problem, to copy this string locally.  It's
correctly freed later.

>> +str_to_cli_port(snd_seq_t *handle, char *str, int *outcli, int *outport)
> 
> Why not use snd_seq_parse_address()?

Nice, missed that function!  Will do.  Was looking for some helpful
utility function like that.

>> +	r = snd_seq_set_client_name(handle, cli_name);
>> +	if (r < 0) {
>> +		/* This early in the program, it's not threaded,
>> +		 * errno is OK to use */
>> +		fprintf(stderr, "Unable to set ALSA client name: %s\n",
>> +			strerror(errno));
> 
> The error code is in r.  Use snd_strerror().

Good suggestion, thanks.

>> +	/* FUTURE: Do we need any more type bits here? */
>> +	port_id = snd_seq_create_simple_port(handle, cli_name, caps,
>> +		SND_SEQ_PORT_TYPE_MIDI_GENERIC);
> 
> You should set all appropriate TYPE bits.  You must set CAP bits if you
> want to allow others to connect from/to this port.

I think I'm setting the appropriate CAP bits (including DUPLEX if both
read and write are enabled).

As for the TYPE bits, that is a point of confusion for me.  What would a
program like this be classified as?  Probably need to add at least
SND_SEQ_PORT_TYPE_SOFTWARE.  Are the TYPE bits used for any
decision-making within the ALSA sequencer, or are they merely
informational for users to see?

>> +		r = snd_seq_event_output_direct(handle, ev);
>> +		if (r < 0) {
>> +			/* Return if a real error happened */
>> +			if (-EAGAIN != r)
> 
> EAGAIN can never happen if the device has not been configured to be
> non-blocking.

I actually see EAGAIN quite commonly under high load.  This one-liner
demonstrates it, there's a high number of "output retries" seen, and the
counter for this comes from here.

cat /dev/urandom | ./amidicat --port "Midi Through" --verbose > /dev/null

>> +		r = snd_seq_drain_output(handle);
> 
> snd_seq_event_output_direct() bypasses the output buffer, so this does
> not make sense.
> 
>> +		r = snd_seq_sync_output_queue(handle);
> 
> You have not scheduled any events to be delivered later, so this does
> not make sense.

Interesting, perhaps I can remove this section entirely and it will
still work!

>> +		/* FUTURE: Why does snd_seq_sync_output_queue()
>> +		 * always return 1, not 0 as it should? */
> 
> Because the documentation is wrong.

Yikes!

>> +			r = printf("%02X%s", ui,
>> +				((size_left > 1) ? " " : "\n"));
> 
> Why not use %c?

I didn't want to print the byte as a raw byte here, but as a
human-readable hex number.  In the mode where raw bytes are output,
write() is used instead, not printf().

>> +		/* Standard C whitespace */
>> +		/* Avoid usage of isspace()
>> +		 * because that would introduce locale variations */
> 
> Where does this program change the locale to be different from "C"?
> (And what would it matter if a Klingon space were to be ignored? :-)

It doesn't change the locale.  I wanted it to run the same no matter
what locale the user had set, however.  Nothing else in the program
depends on locale, and I didn't want to bring that in just for this.

>> +		/* Send a dummy message to ourself,
>> +		 * so ALSA gets unblocked in other thread */
>> +		snd_seq_ev_clear(&ev);
> 
> This results in a SND_SEQ_EVENT_SYSTEM event.  You might want to use
> one of the USR events ...

Good suggestion.

>> +		/* Check global flag, under lock,
>> +		 * see if other thread is telling us to exit */
>> +		pthread_mutex_lock(&g_mutex);
>> +		is_endinput = g_is_endinput;
>> +		pthread_mutex_unlock(&g_mutex);
> 
> ... and if you check for that event (and its source address) here, you
> don't need a separate mutex.

Thanks, but it seems easier/cleaner just to check the global under a
mutex here, as I get the same result with much less code.

>> +	/* Apologies for the awkward line breaks,
>> +	 * the mandate of the checkpatch script left no alternative */
> 
> checkpatch looks for printk().  Ignore it.

This is userspace code, can't use printk().

I learned that I'm granted an indulgence from checkpatch, if the
offending long line contains only a string literal (and optional leading
whitespace).

>> +	pthread_cond_init(&g_cond, NULL);
> 
> This is not used.

Good catch!  It was a leftover from an earlier draft.

> Regards,
> Clemens

Thanks for reviewing.  Other than these points above, what do you think
of the program?  Did you try running it?  I'll submit a revised patch soon.

Josh
Sergey July 8, 2014, 11:55 p.m. UTC | #3
Jun 30 2014, Josh Lehan wrote:
> Signed-off-by: Josh Lehan < alsa@krellan.com >
> 
> diff --git a/amidicat/amidicat.c b/amidicat/amidicat.c

This is kind of a bugreport for amidicat.

I tested amidicat. The first use case I thought was:
[remotehost]$ timidity -iA -Os -B2,8 &
[remotehost]$ nc -l -p 12345 | amidicat --port 129:0 --noread --verbose --hex
[localhost]$ vkeybd &
[localhost]$ amidicat --port 128:0 --nowrite --verbose --hex | nc remotehost 12345

But that have not worked from the beginning.

First, I noticed that "amidicat" is not listed in `aconnect -iol` output. Why?
$ timidity -iA -Os -B2,8 &
$ nc -l -p 12345 | amidicat --port 128:0 --noread --verbose --hex
Then in another terminal:
$ aconnect -iol
client 0: 'System' [type=kernel]
    0 'Timer           '
    1 'Announce        '
client 14: 'Midi Through' [type=kernel]
    0 'Midi Through Port-0'
client 128: 'TiMidity' [type=user]
    0 'TiMidity port 0 '
Connected From: 129:0
    1 'TiMidity port 1 '
    2 'TiMidity port 2 '
    3 'TiMidity port 3 '
Notice the "Connected From: 129:0" line, but no "client 129". However:
$ amidicat --list
 Port    Client name                      Port name                        rwRW
  0:0    System                           Timer                            rwR-
  0:1    System                           Announce                         r-R-
 14:0    Midi Through                     Midi Through Port-0              rwRW
128:0    TiMidity                         TiMidity port 0                  -w-W
128:1    TiMidity                         TiMidity port 1                  -w-W
128:2    TiMidity                         TiMidity port 2                  -w-W
128:3    TiMidity                         TiMidity port 3                  -w-W
129:0    amidicat                         amidicat                         -w--
130:0    amidicat                         amidicat                         rwRW

Also, it prints nothing in the following case:
$ vkeybd &
$ amidicat --port 128:0 --nowrite --verbose --hex
except initial "Connected to ALSA sequencer on port 130:0" line
which itself looks suspicious, since I asked it to connect on port 128:0.

Maybe that line could be like:
  Created ALSA sequencer port 130:0
or even
  Created ALSA sequencer port 130:0, connected to 128:0

However `aseqdump` works like that:
$ vkeybd &
$ aseqdump -p 128:0

Even more interesting test: run them all.
[console1]$ vkeybd &
[console1]$ aseqdump -p 128:0
press a key, as expected I see:
128:0 Note on 0, note 60, velocity 127
128:0 Note off 0, note 60, velocity 0
while `aseqdump` is still running in another terminal I start `amidicat`:
[console2]$ amidicat --port 128:0 --nowrite --verbose --hex
then press a few keys and in aseqdump terminal I see:
128:0 Note on 0, note 60, velocity 127
128:0 Note on 0, note 60, velocity 127
128:0 Note on 0, note 60, velocity 127
128:0 Note on 0, note 60, velocity 127
128:0 Note on 0, note 60, velocity 127
128:0 Note on 0, note 60, velocity 127
128:0 Note on 0, note 60, velocity 127
128:0 Note on 0, note 60, velocity 127
how could that be? I pressed different keys, why same "Note on"?
then I interrupt `amidicat` with Ctrl+C and instantly see:
128:0 Note on 0, note 60, velocity 127
128:0 Note off 0, note 60, velocity 0
128:0 Note on 0, note 62, velocity 127
128:0 Note off 0, note 62, velocity 0
128:0 Note on 0, note 64, velocity 127
128:0 Note off 0, note 64, velocity 0
128:0 Note on 0, note 65, velocity 127
128:0 Note off 0, note 65, velocity 0
128:0 Note on 0, note 67, velocity 127
128:0 Note off 0, note 67, velocity 0
amidicat terminal was still empty, nothing except initial "Connected to ..." line.

Expected results:
  amidicat prints events from vkeybd
  does not affect other apps, e.g. aseqdump
  and is listed in `aconnect -iol` output

Note: I tested that on debian oldstable with alsa 1.0.23, could that be the reason?
Can you reproduce that on your system?

Thank you for an interesting tool.
-- 
  Sergey
Josh Lehan July 9, 2014, 5:26 a.m. UTC | #4
On 07/08/2014 04:55 PM, Sergey wrote:
> Jun 30 2014, Josh Lehan wrote:
>> Signed-off-by: Josh Lehan < alsa@krellan.com >
>>
>> diff --git a/amidicat/amidicat.c b/amidicat/amidicat.c
> 
> This is kind of a bugreport for amidicat.
> 
> I tested amidicat. The first use case I thought was:
> [remotehost]$ timidity -iA -Os -B2,8 &
> [remotehost]$ nc -l -p 12345 | amidicat --port 129:0 --noread --verbose --hex
> [localhost]$ vkeybd &
> [localhost]$ amidicat --port 128:0 --nowrite --verbose --hex | nc remotehost 12345
> 
> But that have not worked from the beginning.

I tried that myself, and ran into problems, you're right.

1) Subscribers vs. direct

There seems to be a limitation in vkeybd, or a bug in how amidicat
connects to vkeybd (but I can't find where the bug might be in amidicat,
it seems to be calling ALSA correctly).

The limitation is this: When vkeybd is ran without the --addr argument
(in "subscribers" mode), vkeybd won't send data.  You hit the keys and
nothing is sent.

This doesn't happen on a real synthesizer keyboard hooked up to ALSA, I
believe (this needs more testing, though).

However, when vkeybd is ran with --addr, explicitly connecting to
another ALSA port (not in "subscribers" mode), it works just fine then!
 To do this, though, you have to start amidicat first, note the ALSA
port that it has received, then start vkeybd after that, using that as
the --addr argument to vkeybd.

Somehow, the aseqdump program overcomes this limitation, though!  I need
to figure out how it does that.  I think it has something to do with how
the connection is made: simultaneously acting as both a direct
connection and a subscription connection at the same time.

2) Unwanted buffering with --hex

My bug here, good catch.  I'm not flushing streams often enough, so
unwanted buffering takes place.  In the default mode (binary output),
amidicat uses write(), so output is entirely untouched by stdio.
However, when --hex is used, I'm using printf() instead of write()!
Easy fix, just haven't done it yet.

3) Confusing options

Assuming bug above is fixed, I'm thinking of making --hex the default,
and adding the --binary option to get the previous default of raw binary
output.  Similarly, make --verbose the default, with corresponding
--quiet option.

Also, some recalcitrant programs (like timidity and vkeybd) are very
picky about ALSA permissions when trying to make a connection.  The
--noread and --nowrite options have to be used way too often, and they
are annoying and restrictive.

I'm thinking of having amidicat automatically probe for permissions by
default, so the user doesn't need to manually apply the --noread and
--nowrite constraints.  Upon encountering permissions failure, it would
give up the direction of I/O through ALSA that failed, either input or
output, as if the user had specified --noread or --nowrite, thus
reducing required permissions automatically, but otherwise carry on as
normal.

However, if this is to become the default, the user should have a way of
requiring that a successful read or write connection be made, such as
--requireread or --requirewrite.  The reason is that silently ignoring
permissions failures might lead to unwanted/surprising behavior, and
user might want to guard against this.

An example of surprising behavior is that when input is closed, you have
to provide the --wait option to amidicat, otherwise it doesn't know how
long to keep running for.  Usually, closure of input is what notifies
the program to stop running.  Reason for choosing to do it this way was
that I didn't want the program to simply block forever if it has nothing
more to do, as that would cause any callers to also block.  The
--nowrite option defeats this, though, causing the program to run
forever until manually stopped.  Should simplify this as well, calling
it --linger instead of --wait would make more sense.

4) Permissions backwards

Also, what about ALSA permissions that amidicat itself advertises?  To
make a long story short, I think I have this backwards.  No wonder the
above options are confusing, they don't work as originally intended!

Try this workflow, it should work:

[remotehost]$ timidity -iA -Os -B2,8 &
[remotehost]$ nc -l -p 12345 | amidicat --port "TiMidity" --noread --verbose
[localhost]$ amidicat --verbose --wait 300 < /dev/null | nc remotehost
12345 &
[localhost]$ vkeybd --addr 999:0

On localhost, replace 999:0 with the actual ALSA port you see on your
terminal (amidicat will show it to you by writing to standard error)
before starting vkeybd.

I'm referencing TiMidity by name instead of by number, which is a safer
behavior to do, since numbers often change.

I'm using binary, not hex, to work around the blocking bug for now (this
will shortly not be necessary).

On localhost, I had to give "< /dev/null" because otherwise I wouldn't
be able to disassociate standard input from the terminal, a requirement
for successfully running in the background.  This necessitated the use
of the --wait option, which I'm not happy about, because it imposes a
time limit.  I wouldn't have needed it if I could have given the
--nowrite option, however, if I did this, it wouldn't advertise ALSA
write permission to other programs.  The vkeybd program requires write
permission.

> First, I noticed that "amidicat" is not listed in `aconnect -iol` output. Why?

That's strange, it's showing up just fine for me.  Here's my output:

client 0: 'System' [type=kernel]
    0 'Timer           '
    1 'Announce        '
	Connecting To: 15:0
client 14: 'Midi Through' [type=kernel]
    0 'Midi Through Port-0'
client 128: 'TiMidity' [type=user]
    0 'TiMidity port 0 '
    1 'TiMidity port 1 '
    2 'TiMidity port 2 '
    3 'TiMidity port 3 '
client 129: 'amidicat' [type=user]
    0 'amidicat        '

> Also, it prints nothing in the following case:
> $ vkeybd &
> $ amidicat --port 128:0 --nowrite --verbose --hex
> except initial "Connected to ALSA sequencer on port 130:0" line
> which itself looks suspicious, since I asked it to connect on port 128:0.

That's a point of confusion.  There's two ALSA ports in the above
example: port 128 (I assume this is vkeybd), and port 130 (amidicat).

When it starts up, the amidicat program is assigned port 130 by ALSA,
and this port number is not knowable until runtime, which is why it's
printed at runtime.  It will still make an outbound connection to the
port you specified, which is port 128, though.  It's just like a TCP
connection on the Internet: in order to communicate, you must have both
a source port number and a destination port number.

I probably should add a --localport option, to make this more explicit
(and give you the opportunity to request a known local port number
instead of having to take whatever ALSA randomly assigns).

> However `aseqdump` works like that:
> $ vkeybd &
> $ aseqdump -p 128:0

The --port option of amidicat should be equivalent to the -p option of
aseqdump.

> Even more interesting test: run them all.
> [console1]$ vkeybd &
> [console1]$ aseqdump -p 128:0
> press a key, as expected I see:
> 128:0 Note on 0, note 60, velocity 127
> 128:0 Note off 0, note 60, velocity 0
> while `aseqdump` is still running in another terminal I start `amidicat`:
> [console2]$ amidicat --port 128:0 --nowrite --verbose --hex
> then press a few keys and in aseqdump terminal I see:
> 128:0 Note on 0, note 60, velocity 127
> 128:0 Note on 0, note 60, velocity 127
> 128:0 Note on 0, note 60, velocity 127
> 128:0 Note on 0, note 60, velocity 127
> 128:0 Note on 0, note 60, velocity 127
> 128:0 Note on 0, note 60, velocity 127
> 128:0 Note on 0, note 60, velocity 127
> 128:0 Note on 0, note 60, velocity 127
> how could that be? I pressed different keys, why same "Note on"?
> then I interrupt `amidicat` with Ctrl+C and instantly see:
> 128:0 Note on 0, note 60, velocity 127
> 128:0 Note off 0, note 60, velocity 0
> 128:0 Note on 0, note 62, velocity 127
> 128:0 Note off 0, note 62, velocity 0
> 128:0 Note on 0, note 64, velocity 127
> 128:0 Note off 0, note 64, velocity 0
> 128:0 Note on 0, note 65, velocity 127
> 128:0 Note off 0, note 65, velocity 0
> 128:0 Note on 0, note 67, velocity 127
> 128:0 Note off 0, note 67, velocity 0
> amidicat terminal was still empty, nothing except initial "Connected to ..." line.

I'm surprised about that as well.  Multiple copies of aseqdump can be
ran at the same time, and everything still works (ALSA correctly
multiplexes the output of vkeybd to all interested subscribers).

However, this doesn't work for amidicat, and that's a bug.  When
amidicat is running, ALSA blocks delivery of vkeybd events to the
aseqdump clients.  When amidicat exits, this blockage clears, and all
the blocked events are suddenly delivered to aseqdump (that's why you
get the flood of output in aseqdump when amidicat exits).  Obviously,
amidicat is doing something wrong, and confusing ALSA.

> Expected results:
>   amidicat prints events from vkeybd
>   does not affect other apps, e.g. aseqdump
>   and is listed in `aconnect -iol` output

Agreed.

> Note: I tested that on debian oldstable with alsa 1.0.23, could that be the reason?
> Can you reproduce that on your system?

As above, I can reproduce the vkeybd and aseqdump bugs/limitations.

However, I can't reproduce "aconnect -iol", it works fine for me, I see
amidicat correctly listed in the output.

Also, try "amidicat --list", which will give you output similar to
"aplaymidi -l" but include more devices (unlike aplaymidi, amidicat does
no filtering, it shows you everything, even including itself in the list).

The list of devices should be the same for both aconnect and amidicat,
however, since amidicat includes itself in the listing, you will see an
additional entry for that as well.

> Thank you for an interesting tool.

You're welcome!

I'm proud of the program, it fills a useful niche.

Thanks for testing.  You've exposed some bugs, and use cases I hadn't
thought about, so it's back to the drawing board for me, and I hope to
have an updated version finished soon.

Josh Lehan
Clemens Ladisch July 9, 2014, 7:54 a.m. UTC | #5
Josh Lehan wrote:
> Somehow, the aseqdump program overcomes this limitation, though!  I need
> to figure out how it does that.  I think it has something to do with how
> the connection is made: simultaneously acting as both a direct
> connection and a subscription connection at the same time.

Have a look at how aseqdump decides what PORT_CAP bits to set.

> Also, what about ALSA permissions that amidicat itself advertises?  To
> make a long story short, I think I have this backwards.

These bits specify what _other_ clients can do with the port.

> Multiple copies of aseqdump can be
> ran at the same time, and everything still works (ALSA correctly
> multiplexes the output of vkeybd to all interested subscribers).
>
> However, this doesn't work for amidicat, and that's a bug.  When
> amidicat is running, ALSA blocks delivery of vkeybd events to the
> aseqdump clients.  When amidicat exits, this blockage clears, and all
> the blocked events are suddenly delivered to aseqdump (that's why you
> get the flood of output in aseqdump when amidicat exits).  Obviously,
> amidicat is doing something wrong, and confusing ALSA.

Does the thread actually read the delivered events from the kernel
buffer?

> Also, try "amidicat --list", which will give you output similar to
> "aplaymidi -l" but include more devices (unlike aplaymidi, amidicat does
> no filtering, it shows you everything, even including itself in the list).

It should list only those ports it can use, i.e., connect from/to.


Regards,
Clemens
Josh Lehan July 9, 2014, 11:36 p.m. UTC | #6
On 07/09/2014 12:54 AM, Clemens Ladisch wrote:
> Have a look at how aseqdump decides what PORT_CAP bits to set.

Thanks, will do.

>> Also, what about ALSA permissions that amidicat itself advertises?  To
>> make a long story short, I think I have this backwards.
> 
> These bits specify what _other_ clients can do with the port.

Makes sense to me.

> Does the thread actually read the delivered events from the kernel
> buffer?

Should be, I'm calling snd_seq_event_input() in a tight loop.  Hoping
this is the appropriate function to be calling, and that all the various
structures around it are initialized correctly.

>> Also, try "amidicat --list", which will give you output similar to
>> "aplaymidi -l" but include more devices (unlike aplaymidi, amidicat does
>> no filtering, it shows you everything, even including itself in the list).
> 
> It should list only those ports it can use, i.e., connect from/to.

It already does, in a way.  Anything that has read/write permission,
direct and/or subscription, is usable.  I like showing everything, it's
useful for diagnostics/troubleshooting.  If user wants filtered output
they can apply that later (or perhaps I'll add it when adding the
--quiet flag).  I still think the most complete/useful output should be
the default.

Josh
Clemens Ladisch July 10, 2014, 6:53 a.m. UTC | #7
Josh Lehan wrote:
> On 07/09/2014 12:54 AM, Clemens Ladisch wrote:
>>> (unlike aplaymidi, amidicat does no filtering, it shows you
>>> everything, even including itself in the list).
>>
>> It should list only those ports it can use, i.e., connect from/to.
>
> It already does, in a way.  Anything that has read/write permission,
> direct and/or subscription, is usable.  I like showing everything, it's
> useful for diagnostics/troubleshooting.

Showing unusable ports _will_ introduce user errors.

If you are doing troubleshooting for something that does not involve
amidicat, the complete port list is already available in
/proc/asound/seq/clients.


Regards,
Clemens

Patch
diff mbox

diff --git a/amidicat/amidicat.c b/amidicat/amidicat.c
new file mode 100644
index 0000000..30d6356
--- /dev/null
+++ b/amidicat/amidicat.c
@@ -0,0 +1,1708 @@ 
+/*
+ *	amidicat
+ *
+ *	Copyright © 2010-2014 Joshua Lehan <alsa@krellan.com>
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+/*
+ *	After searching for a while, it seems there is no program
+ *	to simply send MIDI data into the ALSA MIDI system,
+ *	with a minimum of fuss.
+ *
+ *	The program aplaymidi(1) will play .MID files,
+ *	but not "live" MIDI data from standard input.
+ *
+ *	The program amidi(1) can send live data, but only works for
+ *	devices in the ALSA "RawMIDI" namespace, not the overall
+ *	MIDI system, so it can't be used to drive software
+ *	MIDI synthesizers such as TiMidity.
+ *
+ *	Creating stub .MID files on-the-fly and rapidly playing them
+ *	is a common workaround, but this is undesirable for
+ *	all but the most trivial of uses.
+ *
+ *	This program attempts to rectify these limitations,
+ *	similarly to the purpose of netcat(1).
+ *
+ *	What's more, this program also works in both directions
+ *	simultaneously: MIDI data can also be read out from
+ *	the ALSA MIDI system, and made available as standard
+ *	output.
+ *
+ *	This program is an ALSA sequencer client.
+ *
+ *	Much code was borrowed from vkeybd(1).
+ */
+
+
+#include <pthread.h>
+#include <stdio.h>
+#include <getopt.h>
+#include <alsa/asoundlib.h>
+#include <errno.h>
+#include <time.h>
+
+#include "version.h"
+
+
+/* Constants */
+#define DEFAULT_SEQ_NAME	("default")
+#define DEFAULT_CLI_NAME	("amidicat")
+#define ERR_OPEN		(10)
+#define ERR_FINDPORT		(11)
+#define ERR_CONNPORT		(12)
+#define ERR_PARAM		(13)
+#define ERR_THREAD		(14)
+#define DEFAULT_BUFSIZE		(65536)
+#define MAX_WAIT		(1000)
+#define MAX_DELAY		(1000)
+#define FILE_STDIN		("-")
+#define MAX_HEX_DIGITS		(2)
+
+
+/* Globals */
+pthread_mutex_t	g_mutex;
+pthread_cond_t	g_cond;
+int		g_is_endinput;
+
+
+/* Structures */
+struct in_args_t {
+	char		**args_ptr;
+	snd_seq_t	*handle;
+	int		is_hex;
+	int		cli_id;
+	int		port_id;
+	int		target_cli;
+	int		target_port;
+	int		spacing_us;
+	int		wait_sec;
+	int		is_read;
+};
+
+struct out_args_t {
+	snd_seq_t	*handle;
+	int		is_hex;
+};
+
+
+/*
+ * Better atoi() that returns -1, not 0, if error
+ * and checks entire string, not just first part, for validity
+ */
+int
+better_atoi(const char *nptr)
+{
+	char	*endptr;
+	long	num;
+	int	ret;
+
+	num = strtol(nptr, &endptr, 0);
+	if (NULL != nptr && '\0' == *endptr) {
+		/* String is valid */
+		ret = num;
+
+		/* Guard against size mismatch by comparing int and long */
+		if (ret == num)
+			return ret;
+	}
+
+	return -1;
+}
+
+
+/*
+ * Better usleep() that does not use signals,
+ * so is safe to use along with threads
+ */
+void
+microsleep(long usec)
+{
+	struct timespec	tv;
+
+	/* No need to sleep if time is not positive */
+	if (usec > 0) {
+		tv.tv_sec = usec / 1000000;
+		tv.tv_nsec = (usec % 1000000) * 1000;
+
+		/* Ignore return value, waking up early is not a problem */
+		nanosleep(&tv, NULL);
+	}
+}
+
+
+/* Prettyprints capabilities mask */
+void
+prettyprint_capmask(unsigned int capmask)
+{
+	/* The r/w letters indicate read/write capability */
+	/* Capital R/W letters indicate subscription */
+	printf("%s%s%s%s"
+		, ((capmask & SND_SEQ_PORT_CAP_READ)       ? "r" : "-")
+		, ((capmask & SND_SEQ_PORT_CAP_WRITE)      ? "w" : "-")
+		, ((capmask & SND_SEQ_PORT_CAP_SUBS_READ)  ? "R" : "-")
+		, ((capmask & SND_SEQ_PORT_CAP_SUBS_WRITE) ? "W" : "-")
+	);
+}
+
+
+/*
+ * Iterates through list of all active ALSA sequencer ports
+ * handle = ALSA sequencer handle
+ * str = String to search for a match for, or NULL to ignore
+ * is_print = Set to 1 if you want to see output
+ * outcli = Receives found client ID, if searching
+ * outport = Receives found port ID, if searching
+ * Returns 0 if a match was found, -1 if no matches were found
+ * The search is by exact string match (no wildcards or
+ * substrings yet).  The column of port names will be
+ * searched first, in top-down order, then the column of
+ * client names will be similarly searched.  First match wins.
+ */
+int
+discover_ports(snd_seq_t *handle, char *str, int is_print,
+int *outcli, int *outport)
+{
+	snd_seq_client_info_t	*cli_info;
+	snd_seq_port_info_t	*port_info;
+	char			*cli_name;
+	char			*port_name;
+	unsigned int		port_capmask;
+	unsigned int		port_typemask;
+	int			r;
+	int			cli_id;
+	int			cli_num_ports;
+	int			port_id;
+	int			lowest_port;
+	int			match_bycli_cli_id = -1;
+	int			match_bycli_port_id = -1;
+	int			match_byport_cli_id = -1;
+	int			match_byport_port_id = -1;
+	int			is_first = 1;
+
+	snd_seq_client_info_malloc(&cli_info);
+	snd_seq_port_info_malloc(&port_info);
+
+	/* Iterate through all clients */
+	snd_seq_client_info_set_client(cli_info, -1);
+	for (;;) {
+		r = snd_seq_query_next_client(handle, cli_info);
+		if (r < 0)
+			break;
+
+		/* Got a client */
+		cli_id = snd_seq_client_info_get_client(cli_info);
+		cli_name = strdup(snd_seq_client_info_get_name(cli_info));
+		cli_num_ports = snd_seq_client_info_get_num_ports(cli_info);
+
+		/* Iterate through all ports on this client */
+		snd_seq_port_info_set_client(port_info, cli_id);
+		lowest_port = -1;
+		snd_seq_port_info_set_port(port_info, lowest_port);
+		for (;;) {
+			r = snd_seq_query_next_port(handle, port_info);
+			if (r < 0)
+				break;
+
+			/* Got a port */
+			port_id =
+				snd_seq_port_info_get_port(port_info);
+			port_name = strdup(
+				snd_seq_port_info_get_name(port_info));
+			port_typemask =
+				snd_seq_port_info_get_type(port_info);
+			port_capmask =
+				snd_seq_port_info_get_capability(port_info);
+
+			/* Arbitrarily use lowest port number on this client
+			 * when matching by name */
+			if (-1 == lowest_port)
+				lowest_port = port_id;
+
+			/* Print header */
+			if (is_first) {
+				/* The field lengths of strings
+				 * are taken from "aplaymidi -l" */
+				if (is_print)
+					printf(" Port    %-32s %-32s rwRW\n"
+						, "Client name"
+						, "Port name"
+					);
+
+				is_first = 0;
+			}
+
+			/* Print line */
+			/* FUTURE: Perhaps also print type bits if relevant */
+			if (is_print) {
+				printf("%3d:%-3d  %-32s %-32s "
+					, cli_id
+					, port_id
+					, cli_name
+					, port_name
+				);
+				prettyprint_capmask(port_capmask);
+				printf("\n");
+			}
+
+			/* FUTURE: Perhaps also make use of this information */
+			/* Suppress warning message */
+			(void)cli_num_ports;
+			(void)port_typemask;
+
+			/* Test for match, if not already matched by port */
+			if (NULL != str && -1 == match_byport_cli_id) {
+				if (0 == strcasecmp(str, port_name)) {
+					match_byport_cli_id = cli_id;
+					match_byport_port_id = port_id;
+				}
+			}
+
+			free(port_name);
+		}
+
+		/* Test for match, if not already matched by client */
+		if (NULL != str && -1 == match_bycli_cli_id) {
+			if (0 == strcasecmp(str, cli_name)) {
+				if (-1 != lowest_port) {
+					match_bycli_cli_id = cli_id;
+					match_bycli_port_id = lowest_port;
+				}
+			}
+		}
+
+		free(cli_name);
+	}
+
+	snd_seq_port_info_free(port_info);
+	snd_seq_client_info_free(cli_info);
+
+	/* If both matched, prefer port match (most specific)
+	 * over client match */
+	if (-1 != match_byport_cli_id) {
+		*outcli = match_byport_cli_id;
+		*outport = match_byport_port_id;
+		return 0;
+	}
+	if (-1 != match_bycli_cli_id) {
+		*outcli = match_bycli_cli_id;
+		*outport = match_bycli_port_id;
+		return 0;
+	}
+
+	/* No match found, or no string given to match by */
+	return 1;
+}
+
+
+/*
+ * Parses string into ALSA client:port numbers
+ * String can be 2 numbers separated by ":" or "." or "-" delimiters,
+ * or the name of a client or port can be given
+ * and a lookup will be done in order to learn the numbers.
+ * Results stored in "outcli" and "outport".
+ * Returns 0 if successful, or -1 if unparseable or not found.
+ */
+int
+str_to_cli_port(snd_seq_t *handle, char *str, int *outcli, int *outport)
+{
+	char	*delim;
+	char	*nextstr;
+	int	cli_id = -1;
+	int	port_id = -1;
+	int	r;
+	char	savedch;
+
+	/* Fairly generous in choice of delimiters */
+	delim = strpbrk(str, ":.-");
+	if (delim) {
+		/* Look at string immediately following the delimiter */
+		nextstr = delim + 1;
+
+		/* Perform string fission */
+		savedch = *delim;
+		*delim  = '\0';
+
+		cli_id  = better_atoi(str);
+		port_id = better_atoi(nextstr);
+
+		*delim = savedch;
+	}
+
+	/* If not parseable as numbers, try string match */
+	if (cli_id < 0 || port_id < 0) {
+		r = discover_ports(handle, str, 0, &cli_id, &port_id);
+
+		/* Return error, regardless of ID, if discovery failed */
+		if (r < 0)
+			return -1;
+	}
+
+	/* Not found or not parseable */
+	if (cli_id < 0 || port_id < 0)
+		return -1;
+
+	/* Both are good results */
+	*outcli  = cli_id;
+	*outport = port_id;
+	return 0;
+}
+
+
+/*
+ * Opens a new connection to the ALSA sequencer
+ * Can be opened for input (read), output (write), or both
+ * Returns handle or NULL if error, prints message if error
+ */
+snd_seq_t *
+seq_open(int is_read, int is_write)
+{
+	snd_seq_t	*handle;
+	int		err;
+	int		mode;
+
+	/* Default to duplex unless told otherwise */
+	mode = SND_SEQ_OPEN_DUPLEX;
+
+	/* Read only */
+	if (is_read && !is_write)
+		mode = SND_SEQ_OPEN_INPUT;
+
+	/* Write only */
+	if (is_write && !is_read)
+		mode = SND_SEQ_OPEN_OUTPUT;
+
+	/* Always use the default "sequencer name" here,
+	 * this is not the client name */
+	err = snd_seq_open(&handle, DEFAULT_SEQ_NAME, mode, 0);
+	if (err < 0) {
+		fprintf(stderr, "Unable to open ALSA sequencer: %s\n",
+			strerror(errno));
+		return NULL;
+	}
+	return handle;
+}
+
+
+/*
+ * Closes the connection to the ALSA sequencer
+ */
+void
+seq_close(snd_seq_t *handle)
+{
+	snd_seq_close(handle);
+}
+
+
+/*
+ * Sets up the ALSA sequencer for use
+ * Sets our client name
+ * Allocates our port
+ * Connects to remote client and port,
+ * unless they are -1 then use ALSA subscription mechanism instead
+ * Learns our own ID's and saves them in "outcli" and "outport"
+ * Returns 0 if all went well, or -1 if error, prints message if error
+ */
+int
+seq_setup(snd_seq_t *handle, char *cli_name, int target_cli, int target_port,
+int is_read, int is_write, int *outcli, int *outport)
+{
+	int	cli_id;
+	int	port_id;
+	int	r;
+	int	caps = 0;
+	int	is_subs = 0;
+
+	/* Get our client ID */
+	cli_id = snd_seq_client_id(handle);
+	*outcli = cli_id;
+
+	/* Set our name */
+	r = snd_seq_set_client_name(handle, cli_name);
+	if (r < 0) {
+		/* This early in the program, it's not threaded,
+		 * errno is OK to use */
+		fprintf(stderr, "Unable to set ALSA client name: %s\n",
+			strerror(errno));
+		return -1;
+	}
+
+	/* Enable reading and/or writing */
+	if (is_read)
+		caps |= SND_SEQ_PORT_CAP_READ;
+	if (is_write)
+		caps |= SND_SEQ_PORT_CAP_WRITE;
+	if (is_read && is_write)
+		caps |= SND_SEQ_PORT_CAP_DUPLEX;
+
+	if (target_cli < 0 || target_port < 0) {
+		/* Use ALSA subscription mechanism if target not given */
+		/* FUTURE: Might want both subscription and target at once */
+		if (is_read)
+			caps |= SND_SEQ_PORT_CAP_SUBS_READ;
+		if (is_write)
+			caps |= SND_SEQ_PORT_CAP_SUBS_WRITE;
+
+		/* There is no corresponding SUBS_DUPLEX flag */
+		is_subs = 1;
+	}
+
+	/* Open origin port */
+	/* FUTURE: Do we need any more type bits here? */
+	port_id = snd_seq_create_simple_port(handle, cli_name, caps,
+		SND_SEQ_PORT_TYPE_MIDI_GENERIC);
+	if (port_id < 0) {
+		fprintf(stderr, "Unable to open ALSA sequencer port: %s\n",
+			strerror(errno));
+		return -1;
+	}
+
+	/* Connect both to and from target port, if not using subscription */
+	if (!is_subs) {
+		if (is_write) {
+			r = snd_seq_connect_to(handle, port_id,
+					       target_cli, target_port);
+			if (r < 0) {
+				fprintf(stderr,
+				"Unable to connect to ALSA port %d:%d: %s\n"
+					, target_cli
+					, target_port
+					, strerror(errno)
+				);
+				return -1;
+			}
+		}
+		if (is_read) {
+			r = snd_seq_connect_from(handle, port_id,
+						 target_cli, target_port);
+			if (r < 0) {
+				fprintf(stderr,
+				"Unable to connect from ALSA port %d:%d: %s\n"
+					, target_cli
+					, target_port
+					, strerror(errno)
+				);
+				return -1;
+			}
+		}
+	}
+
+	/* Should be all good to go now */
+	*outcli  = cli_id;
+	*outport = port_id;
+	return 0;
+}
+
+
+/*
+ * Writes an event into ALSA
+ * Blocks/retries until ALSA has received and delivered
+ * the event (as best as we can verify)
+ * Tries its best to avoid flooding the ALSA input queue
+ * ev = The event to be sent into ALSA
+ * port_id = Our port ID
+ * target_cli, target_port = The target's client and port ID,
+ * or -1 to just send to "subscribers"
+ * spacing_us = Spacing time, in microseconds,
+ * to be used when busywaiting some loops
+ * Returns negative error code if error,
+ * or the number of retries required if successful
+ */
+int
+write_event(snd_seq_t *handle, snd_seq_event_t *ev, int port_id,
+	    int target_cli, int target_port, int spacing_us)
+{
+	int	r;
+	int	ct_output = 0;
+	int	ct_drain = 0;
+	int	ct_sync = 0;
+	int	is_draingood;
+	int	is_syncgood;
+	int	total;
+
+	/* Fill in event data structure, these are macros and never fail */
+	snd_seq_ev_set_source(ev, port_id);
+
+	if (target_cli < 0 || target_port < 0)
+		/* Send to all subscribers,
+		 * possibly playing to an empty house */
+		snd_seq_ev_set_subs(ev);
+	else
+		snd_seq_ev_set_dest(ev, target_cli, target_port);
+
+	snd_seq_ev_set_direct(ev);
+
+	/* Fire event */
+	for (;;) {
+		/* FUTURE: Maybe have option to let user choose
+		 * between output and output_direct? */
+		r = snd_seq_event_output_direct(handle, ev);
+		if (r < 0) {
+			/* Return if a real error happened */
+			if (-EAGAIN != r)
+				return r;
+
+			/* Error was "Try again", wait and do just that */
+			microsleep(spacing_us);
+			ct_output++;
+			continue;
+		}
+
+		/* Event sent into ALSA, do NOT try again */
+		break;
+	}
+
+	/* Loop until output is fully pushed into ALSA */
+	/* FUTURE: Even though this loop works,
+	 * it's still too easy to flood the other end and overrun,
+	 * perhaps a queuing bug internally within ALSA? */
+	for (;;) {
+		is_draingood = 0;
+		is_syncgood  = 0;
+
+		r = snd_seq_drain_output(handle);
+		if (r < 0) {
+			/* Return if a real error happened */
+			if (-EAGAIN != r)
+				return r;
+		}
+
+		/* Stop retrying only when there are no more
+		 * events left to be drained */
+		if (0 == r)
+			is_draingood = 1;
+		else
+			ct_drain++;
+
+		r = snd_seq_sync_output_queue(handle);
+		if (r < 0) {
+			/* Return if a real error happened */
+			if (-EAGAIN != r)
+				return r;
+		}
+
+		/* FUTURE: Why does snd_seq_sync_output_queue()
+		 * always return 1, not 0 as it should? */
+		if (0 == r || 1 == r)
+			is_syncgood = 1;
+		else
+			ct_sync++;
+
+		/* Only return if both drain and sync are clear */
+		if (is_draingood && is_syncgood)
+			break;
+
+		/* Throttle CPU when busywaiting */
+		microsleep(spacing_us);
+	}
+
+	total = ct_output + ct_drain + ct_sync;
+	/* FUTURE: Suppress this text if verbose flag
+	 * was given (it becomes redundant) */
+	if (total > 0) {
+		fprintf(stderr, "Incoming congestion");
+		if (ct_output > 0)
+			fprintf(stderr, ", %d output retries", ct_output);
+		if (ct_drain > 0)
+			fprintf(stderr, ", %d drain retries", ct_drain);
+		if (ct_sync > 0)
+			fprintf(stderr, ", %d sync retries", ct_sync);
+		fprintf(stderr, "\n");
+	}
+
+	return total;
+}
+
+
+/*
+ * Writes a buffer to stdout
+ * Returns 0 if successful or nonzero if error
+ * Writes either as binary bytes or hex digits
+ */
+int
+write_stdout(unsigned char *buf, long bufsize, int is_hex)
+{
+	unsigned char	*bufptr;
+	long		size_left;
+	long		size_written;
+	unsigned int	ui;
+	int		r;
+	unsigned char	uc;
+
+	bufptr = buf;
+	size_left = bufsize;
+	if (is_hex) {
+		/* Print hex bytes, e.g. 90 3C 7F */
+		while (size_left > 0) {
+			uc = *bufptr;
+			ui = uc;
+
+			/* Separate by spaces,
+			 * unless it's the last one which gets newline */
+			r = printf("%02X%s", ui,
+				((size_left > 1) ? " " : "\n"));
+			if (r < 0)
+				return -1;
+
+			bufptr++;
+			size_left--;
+		}
+	} else {
+		/* Full write to stdout */
+		while (size_left > 0) {
+			size_written = write(STDOUT_FILENO,
+					     bufptr,
+					     size_left);
+			if (size_written < 0)
+				return -1;
+
+			bufptr += size_written;
+			size_left -= size_written;
+		}
+	}
+
+	return 0;
+}
+
+
+/*
+ * Parses an incoming unsigned byte of ASCII text, representing a
+ * hex digit, eventually building up and returning complete hex numbers
+ * Uses static variables to keep state across calls
+ * Hex digits are separated by whitespace, or if enough hex digits
+ * have been read and maximum size has been reached, no separator
+ * is necessary (so digits can all be ran together)
+ * If not whitespace, each byte of text must be in range [0-9A-Fa-f]
+ * Returns unsigned hex number if successful
+ * Returns -1 if error (unrecognized byte of ASCII text)
+ * Returns -2 if the complete hex number is not available yet (still good)
+ * As a special case, pass in a value of -2 when at an input boundary,
+ * this will reset the state and return the hex number that was in
+ * progress (if any)
+ */
+int
+parse_hex_byte(int ch)
+{
+	static int	hex_value;
+	static int	read_nybbles;
+
+	int		ret;
+	int		nybble;
+
+	/* Parse human-readable digit into nybble value */
+	nybble = -1;
+	if (ch >= '0' && ch <= '9')
+		nybble = ch - '0';
+	if (ch >= 'A' && ch <= 'F')
+		nybble = 10 + (ch - 'A');
+	if (ch >= 'a' && ch <= 'f')
+		nybble = 10 + (ch - 'a');
+
+	if (-1 == nybble) {
+		switch (ch) {
+		/* Special case accept -2 as a zero-length reset request */
+		case -2:
+			/* Fall through */
+
+		/* Standard C whitespace */
+		/* Avoid usage of isspace()
+		 * because that would introduce locale variations */
+		case ' ':
+		case '\f':
+		case '\n':
+		case '\r':
+		case '\t':
+		case '\v':
+			/* Clear state, and return -2
+			 * if there was nothing to begin with */
+			ret = -2;
+			if (read_nybbles > 0)
+				ret = hex_value;
+
+			hex_value = 0;
+			read_nybbles = 0;
+			return ret;
+
+		/* No default */
+		}
+
+		/* Unrecognized character */
+		return -1;
+	}
+
+	/* Digit is valid, build up a number with it */
+	hex_value <<= 4;
+	hex_value += nybble;
+	read_nybbles++;
+
+	/* Force number to be finished, if maximum digit count reached */
+	if (read_nybbles >= MAX_HEX_DIGITS) {
+		ret = hex_value;
+		hex_value = 0;
+		read_nybbles = 0;
+		return ret;
+	}
+
+	/* Successfully stored digit, but nothing to return yet */
+	return -2;
+}
+
+
+/*
+ * Reads a byte of input from various sources
+ * Fills in byte_ptr with the byte that was read
+ * Uses static variables to keep state across calls
+ * Pass in the argc array from the command line,
+ * after all options have been removed.
+ * Each string in the array should be a filename, and will
+ * be opened and read from.
+ * The filename "-" is special, and means standard input.
+ * Passing in a pointer that is valid but points to NULL,
+ * indicating an empty command line,
+ * is also special, and means standard input.
+ * If is_hex is true, will read human-readable hex digits,
+ * assemble them into bytes, and return the bytes.
+ * Returns 1 if successful, -1 if error,
+ * or 0 if clean EOF after finishing all files.
+ */
+int
+input_byte(char **args_ptr, int is_hex, char *byte_ptr)
+{
+	/* Static variables for holding state */
+	static char	**args_iter;
+	static char	*file_name;
+	static int	file_fd = -1;
+	static int	need_open;
+	static int	is_inited;
+	static int	is_delayed_eof;
+
+	unsigned char	byte_buf;
+	int		ret;
+	int		val;
+	int		r;
+
+	/* Only do initialization once */
+	if (!is_inited) {
+		args_iter = args_ptr;
+		need_open = 1;
+
+		is_inited = 1;
+	}
+
+	/* Keep looping around, opening next files as necessary,
+	 * until we have something to return */
+	for (;;) {
+		/* This gets set if previous file reached EOF,
+		 * or during init */
+		if (need_open) {
+			/* Open next file pointed to */
+			file_name = *args_iter;
+			if (NULL != file_name) {
+				/* Filename given */
+				if (0 == strcmp(FILE_STDIN, file_name)) {
+					/* Special case
+					 * filename "-" is stdin */
+					file_fd = STDIN_FILENO;
+				} else {
+					/* Open the given filename */
+					if (-1 != file_fd) {
+						fprintf(stderr,
+							"Internal error, failed to close previous file\n"
+						);
+						return -1;
+					}
+					file_fd = open(file_name, O_RDONLY);
+					if (-1 == file_fd) {
+						/* FUTURE: Should it
+						 * auto-advance to next file,
+						 * instead of erroring out? */
+						fprintf(stderr,
+							"Unable to open file %s: %s\n"
+							, file_name
+							, strerror(errno)
+						);
+						return file_fd;
+					}
+				}
+			} else {
+				/* No files at all on command line,
+				 * special case read from stdin */
+				file_name = "standard input";
+				file_fd = STDIN_FILENO;
+			}
+
+			/* Successfully at beginning of next file */
+			if (is_hex) {
+				/* Init hex state, better already be empty
+				 * from previous file */
+				val = parse_hex_byte(-2);
+				if (-2 != val) {
+					fprintf(stderr,
+						"Internal error, failed to clear hex state across files\n"
+					);
+					return -1;
+				}
+			}
+			need_open = 0;
+		}
+
+		/* Should have a valid file_fd at this point */
+		if (is_delayed_eof) {
+			/* No new reading of data during this pass,
+			 * just held over EOF from last time */
+			ret = 0;
+			is_delayed_eof = 0;
+		} else {
+			/* Read a byte of raw input (might be a hex digit) */
+			ret = read(file_fd, &byte_buf, 1);
+		}
+
+		if (-1 == ret) {
+			/* Ignore harmless errors and retry */
+			if (EINTR == errno || EAGAIN == errno)
+				continue;
+
+			fprintf(stderr,
+				"Problem reading from file %s: %s\n",
+				file_name, strerror(errno));
+			return ret;
+		}
+
+		/* Advance to next file, if EOF detected in this file */
+		if (0 == ret) {
+			/* The file might have ended
+			 * in the middle of a hex digit */
+			if (is_hex) {
+				val = parse_hex_byte(-2);
+				if (-2 != val) {
+					/* Poor man's coroutine: delay EOF by
+					 * one pass, insert this final byte */
+					is_delayed_eof = 1;
+
+					/* Return final byte */
+					*byte_ptr = (char)val;
+					return 1;
+				}
+			}
+
+			if (-1 != file_fd) {
+				/* Don't close stdin, we may need it again if
+				 * user gives "-" more than once
+				 * on command line */
+				if (file_fd != STDIN_FILENO) {
+					r = close(file_fd);
+					if (0 != r) {
+						/* This error is nonfatal,
+						 * don't return here */
+						fprintf(stderr,
+							"Problem closing file %s: %s\n"
+							, file_name
+							, strerror(errno)
+						);
+					}
+				}
+				file_fd = -1;
+			}
+
+			/* Take another look at command line */
+			file_name = *args_iter;
+			if (NULL != file_name) {
+				/* Advance to next file in sequence */
+				args_iter++;
+
+				file_name = *args_iter;
+				if (NULL != file_name) {
+					/* Next file will be opened
+					 * after we loop around */
+					need_open = 1;
+					continue;
+				} else {
+					/* Finished with all files
+					 * on command line */
+					return 0;
+				}
+			} else {
+				/* No files at all on command line,
+				 * EOF means end of stdin */
+				return 0;
+			}
+		}
+
+		/* Successfully read a byte of data */
+		if (1 == ret) {
+			/* Piece hex number together */
+			if (is_hex) {
+				val = parse_hex_byte(byte_buf);
+
+				/* Successfully read a byte,
+				 * but hex number not complete yet */
+				if (-2 == val)
+					continue;
+
+				if (-1 == val) {
+					fprintf(stderr,
+						"Unrecognizable hex digit text in file %s: %c (%d)\n"
+						, file_name
+						, (int)byte_buf
+						, (int)byte_buf
+					);
+					return -1;
+				}
+
+				/* Successfully assembled hex number */
+				*byte_ptr = (char)val;
+				return ret;
+			}
+
+			/* Hex not used, return byte exactly as it was read */
+			*byte_ptr = (char)byte_buf;
+			return ret;
+		}
+
+		/* Should never get here */
+		break;
+	}
+
+	/* Should never get here */
+	fprintf(stderr, "Internal error in file reading: %d\n", ret);
+	return -1;
+}
+
+
+void *
+stdin_loop(void *args)
+{
+	struct in_args_t	*in_args;
+	char			**args_ptr;
+	snd_midi_event_t	*parser;
+	snd_seq_t		*handle;
+	snd_seq_event_t		ev;
+	long			ct_bytes = 0;
+	int			cli_id;
+	int			port_id;
+	int			target_cli;
+	int			target_port;
+	int			spacing_us;
+	int			wait_sec;
+	int			is_read;
+	int			is_hex;
+	int			r;
+	int			i;
+	int			is_active = 1;
+	int			ct_events = 0;
+	int			ct_congested = 0;
+	char			bytein;
+
+	/* Recover arguments */
+	in_args = (struct in_args_t *)args;
+	args_ptr    = in_args->args_ptr;
+	is_hex      = in_args->is_hex;
+	handle      = in_args->handle;
+	cli_id      = in_args->cli_id;
+	port_id     = in_args->port_id;
+	target_cli  = in_args->target_cli;
+	target_port = in_args->target_port;
+	spacing_us  = in_args->spacing_us;
+	wait_sec    = in_args->wait_sec;
+	is_read     = in_args->is_read;
+
+	snd_midi_event_new(DEFAULT_BUFSIZE, &parser);
+	snd_midi_event_init(parser);
+	snd_midi_event_reset_decode(parser);
+
+	snd_midi_event_no_status(parser, 1);
+
+	/* Reset event */
+	snd_seq_ev_clear(&ev);
+
+	/* FUTURE: Might need to maintain our own buffer,
+	 * to overcome ALSA SysEx size limitation */
+	while (is_active) {
+		/* Read from input, either stdin or files */
+		r = input_byte(args_ptr, is_hex, &bytein);
+		switch (r) {
+		case 0:		/* Clean EOF */
+			is_active = 0;
+			break;
+
+		case 1:		/* Byte received */
+			/* No action necessary */
+			ct_bytes++;
+			break;
+
+		default:
+			/* Error messages already printed by input_byte() */
+			is_active = 0;
+			break;
+		}
+
+		/* Feed byte into parser, as int */
+		i = bytein;
+		r = snd_midi_event_encode_byte(parser, i, &ev);
+		switch (r) {
+		case 0:		/* More bytes needed for event */
+			/* No action necessary */
+			break;
+
+		case 1:		/* Message complete */
+			/* Send completed event into ALSA */
+			r = write_event(handle, &ev, port_id,
+					target_cli, target_port, spacing_us);
+			if (r < 0) {
+				fprintf(stderr,
+					"Event write error: %s\n",
+					strerror(-r));
+				is_active = 0;
+				break;
+			}
+
+			/* The return value was the number of retries */
+			if (r > 0)
+				ct_congested++;
+
+			/* Reset event after write */
+			snd_seq_ev_clear(&ev);
+			ct_events++;
+
+			/* Wait for spacing between events, if desired */
+			microsleep(spacing_us);
+			break;
+
+		default:	/* Error */
+			fprintf(stderr,
+				"Internal error, from ALSA event encode byte: %s\n"
+				, strerror(-r)
+			);
+			is_active = 0;
+			break;
+		}
+	}
+
+	/* Input finished */
+	/* FUTURE: Perhaps an option to suppress this */
+	fprintf(stderr,
+		"Input total: %d MIDI messages, %ld bytes",
+		ct_events, ct_bytes);
+	if (ct_congested > 0) {
+		fprintf(stderr,
+			", %d events congested", ct_congested);
+	}
+	fprintf(stderr, "\n");
+
+	/* Give some time for output-only if the user desires */
+	microsleep(1000000 * wait_sec);
+
+	/* Set global variable, under mutex, so other thread sees it */
+	pthread_mutex_lock(&g_mutex);
+	g_is_endinput = 1;
+	pthread_mutex_unlock(&g_mutex);
+
+	/* Tell the reader thread that we are done */
+	if (is_read) {
+		/* Send a dummy message to ourself,
+		 * so ALSA gets unblocked in other thread */
+		snd_seq_ev_clear(&ev);
+		r = write_event(handle, &ev, port_id, cli_id,
+				port_id, spacing_us);
+
+		/* FUTURE: Indicate error result to reader thread */
+		if (r < 0) {
+			fprintf(stderr,
+				"Final event write error: %s\n"
+				, strerror(-r)
+			);
+		}
+	}
+
+	snd_midi_event_free(parser);
+
+	/* FUTURE: Perhaps bubble up an error result */
+	return NULL;
+}
+
+
+void *
+stdout_loop(void *args)
+{
+	struct out_args_t	*out_args;
+	unsigned char		*buffer;
+	snd_seq_t		*handle;
+	snd_midi_event_t	*parser;
+	snd_seq_event_t		*evptr;
+	long			size_ev;
+	long			ct_bytes = 0;
+	int			is_hex;
+	int			r;
+	int			is_active = 1;
+	int			ct_overruns = 0;
+	int			ct_events = 0;
+	int			ct_nonevents = 0;
+	int			is_endinput;
+
+	/* Recover arguments */
+	out_args = (struct out_args_t *)args;
+	handle = out_args->handle;
+	is_hex = out_args->is_hex;
+
+	snd_midi_event_new(DEFAULT_BUFSIZE, &parser);
+	snd_midi_event_init(parser);
+	snd_midi_event_reset_decode(parser);
+
+	snd_midi_event_no_status(parser, 1);
+
+	/* Allocate buffer */
+	buffer = malloc(DEFAULT_BUFSIZE);
+
+	while (is_active) {
+		/* ALSA will set this pointer to somewhere within itself,
+		 * if successful */
+		/* FUTURE: This is not threadsafe, but since this is
+		 * the only thread that does ALSA event input,
+		 * hopefully it's OK for now */
+		evptr = NULL;
+
+		/* BLOCK until event comes in from ALSA */
+		r = snd_seq_event_input(handle, &evptr);
+		if (r < 0) {
+			/* ENOSPC indicates that ALSA's internal buffer
+			 * overran and we lost some events */
+			if (-ENOSPC == r) {
+				/* FUTURE: Only show this
+				 * if verbose is turned on */
+				fprintf(stderr,
+					"Reported overrun while reading from ALSA to output\n"
+				);
+				ct_overruns++;
+				continue;
+			} else {
+				fprintf(stderr,
+					"Error reading event from ALSA to output: %s\n"
+					, strerror(-r)
+				);
+				is_active = 0;
+				break;
+			}
+		}
+		if (NULL == evptr) {
+			/* FUTURE: Shouldn't happen, perhaps remove this */
+			fprintf(stderr,
+				"Internal error reading event from ALSA\n");
+			is_active = 0;
+			break;
+		}
+
+		/* Check global flag, under lock,
+		 * see if other thread is telling us to exit */
+		pthread_mutex_lock(&g_mutex);
+		is_endinput = g_is_endinput;
+		pthread_mutex_unlock(&g_mutex);
+		if (is_endinput) {
+			/* Clean exit */
+			is_active = 0;
+			break;
+		}
+
+		/* Unpack event into bytes */
+		size_ev = snd_midi_event_decode(parser, buffer,
+						DEFAULT_BUFSIZE, evptr);
+		if (size_ev < 0) {
+			/* ENOENT indicates an event that is
+			 * not a MIDI message, silently skip it */
+			if (-ENOENT == size_ev) {
+				ct_nonevents++;
+				/* FUTURE: Suppress this with quiet option */
+				fprintf(stderr,
+					"Received non-MIDI message\n");
+				continue;
+			} else {
+				fprintf(stderr,
+					"Error decoding event from ALSA to output: %s\n"
+					, strerror(-size_ev)
+				);
+				is_active = 0;
+				break;
+			}
+		}
+
+		/* FUTURE: Might need some code here to cat multiple
+		 * SysEx events together (0xF0 ... 0xF7), to overcome ALSA
+		 * internal size limit splitting them */
+
+		/* Output to stdout */
+		if (size_ev > 0) {
+			r = write_stdout(buffer, size_ev, is_hex);
+			if (r < 0) {
+				fprintf(stderr,
+					"Error writing output: %s\n",
+					strerror(errno));
+				is_active = 0;
+				break;
+			}
+
+			ct_bytes += size_ev;
+		}
+
+		ct_events++;
+	}
+
+	/* This block will only be reached
+	 * once other thread tells us to exit */
+	/* If blocked in ALSA above,
+	 * a dummy event will need to be faked up,
+	 * to get ALSA to return */
+	/* FUTURE: Perhaps suppress this text with quiet option */
+	fprintf(stderr,
+		"Output total: %d MIDI messages, %ld bytes",
+		ct_events, ct_bytes);
+	if (ct_nonevents > 0)
+		fprintf(stderr, ", %d non-MIDI events", ct_nonevents);
+	if (ct_overruns > 0)
+		fprintf(stderr, ", %d ALSA read overruns", ct_overruns);
+	fprintf(stderr, "\n");
+
+	free(buffer);
+	snd_midi_event_free(parser);
+
+	/* FUTURE: Perhaps bubble up an error result */
+	return NULL;
+}
+
+
+/* Trivial function */
+void
+show_version(void)
+{
+	printf("amidicat version %s by Joshua Lehan <alsa@krellan.com>\n",
+	       SND_UTIL_VERSION_STR);
+}
+
+
+void
+help_screen(char *exename)
+{
+	/* Put help screen on stdout, not stderr,
+	 * because help screen replaces normal output */
+	/* Apologies for the awkward line breaks,
+	 * the mandate of the checkpatch script left no alternative */
+	printf("Usage: %s\n", exename);
+	printf("       [--help] [--version] [--list]\n");
+	printf("       [--name STRING]\n");
+	printf("       [--port CLIENT:PORT] [--addr CLIENT:PORT]\n");
+	printf("       [--hex] [--verbose] [--nowrite] [--noread]\n");
+	printf("       [--delay MILLISECONDS] [--wait SECONDS]\n");
+	printf("       input files....\n");
+	printf(
+		"amidicat(1) hooks up standard input and standard output to the ALSA sequencer.\n"
+	);
+	printf(
+		"Like cat(1), this program will concatenate multiple input files together.\n"
+	);
+	printf("--help    = Show this help screen and exit.\n");
+	printf("--version = Show version line and exit.  This version: %s\n",
+	       SND_UTIL_VERSION_STR);
+	printf(
+		"--list    = Show list of all ALSA sequencer devices and exit:\n"
+	);
+	printf(
+		" For each usable ALSA client and port, number and name are shown,\n"
+	);
+	printf(
+		" and flags: r,w = port can be read from or written to directly,\n"
+	);
+	printf(
+		"            R,W = also can use ALSA \"subscription\" to read or write.\n"
+	);
+	printf(
+		"--name    = Sets name of this program's ALSA connection, as shown\n"
+	);
+	printf("            in --list, default is \"%s\".\n",
+	       DEFAULT_CLI_NAME);
+	printf(
+		"--port    = Makes direct connection to ALSA port, for reading and\n"
+	);
+	printf(
+		"            writing, instead of the default which is just to set up\n"
+	);
+	printf(
+		"            a passive connection for use with ALSA \"subscription\".\n"
+	);
+	printf(
+		" Syntax is either numeric CLIENT:PORT (example: 128:0), or name of\n"
+	);
+	printf(
+		" another program's ALSA connection (use --list to see available).\n"
+	);
+	printf(
+		"--addr    = For compatibility, an exact synonym of the --port option.\n"
+	);
+	printf(
+		"--hex     = Change input and output to be human-readable hex\n"
+	);
+	printf(
+		"            digits (example: 90 3C 7F) instead of binary MIDI bytes.\n"
+	);
+	printf(
+		"--verbose = Provide additional, useful, output to standard error.\n"
+	);
+	printf(
+		"--nowrite = Disable writing data to ALSA from standard input.\n"
+	);
+	printf(
+		"            Intended for allowing connection to read-only devices.\n"
+	);
+	printf(
+		"            Input files may not be given when this option is used.\n"
+	);
+	printf(
+		"--noread  = Disable reading data from ALSA for standard output.\n"
+	);
+	printf(
+		"            Intended for allowing connection to write-only devices.\n"
+	);
+	printf(
+		"--delay   = Inserts a delay, in milliseconds, between each MIDI\n"
+	);
+	printf("            event submitted to ALSA from standard input.\n");
+	printf(
+		"            Intended for avoiding event loss due to queue congestion.\n"
+	);
+	printf(
+		"--wait    = After all input is finished, continue running program for\n"
+	);
+	printf("            this amount of time, in seconds.\n");
+	printf(
+		"            Intended for allowing output to continue after input.\n"
+	);
+}
+
+
+int
+main(int argc, char **argv)
+{
+	/* For getopt */
+	struct option long_options[] = {
+		{ "help",	0, NULL, 'h' },
+		{ "list",	0, NULL, 'l' },
+		{ "name",	1, NULL, 'n' },
+		{ "port",	1, NULL, 'p' },
+		{ "addr",	1, NULL, 'a' },
+		{ "hex",	0, NULL, 'x' },
+		{ "delay",	1, NULL, 'd' },
+		{ "wait",	1, NULL, 'w' },
+		{ "verbose",	0, NULL, 'v' },
+		{ "version",	0, NULL, 'V' },
+		{ "nowrite",	0, NULL, 'W' },
+		{ "noread",	0, NULL, 'R' },
+		{ NULL,		0, NULL, 0 }
+	};
+
+	/* For threading */
+	struct in_args_t	in_args;
+	struct out_args_t	out_args;
+	pthread_t		in_thread;
+	pthread_t		out_thread;
+	int			is_in_started = 0;
+	int			is_out_started = 0;
+
+	snd_seq_t	*handle = NULL;
+	char		*cli_name;
+	int		cli_id;
+	int		port_id;
+	int		target_cli = -1;
+	int		target_port = -1;
+	int		ret;
+	int		c;
+	int		r;
+	int		is_write = 1;
+	int		is_read = 1;
+	int		is_done = 0;
+	int		is_list = 0;
+	int		is_hex = 0;
+	int		is_verbose = 0;
+	int		is_help = 0;
+	int		is_version = 0;
+	int		spacing_us = 0;
+	int		wait_sec = 0;
+
+	/* Initialize defaults */
+	cli_name = strdup(DEFAULT_CLI_NAME);
+
+	/* Parse options */
+	while (!is_done) {
+		c = getopt_long(argc, argv, "hln:p:a:xd:w:vVWR",
+				long_options, NULL);
+		switch (c) {
+		case 'h': /* --help */
+			is_help = 1;
+			break;
+
+		case 'l': /* --list */
+			is_list = 1;
+			break;
+
+		case 'n': /* --name */
+			free(cli_name);
+			cli_name = strdup(optarg);
+			break;
+
+		case 'p': /* --port */
+		case 'a': /* --addr, exactly the same */
+			/* Open ALSA sequencer early,
+			 * we need it for port lookup */
+			if (NULL == handle)
+				handle = seq_open(is_read, is_write);
+			if (NULL == handle) {
+				/* Abort program if unable to open */
+				ret = ERR_OPEN;
+				goto cleanup;
+			}
+			r = str_to_cli_port(handle, optarg,
+					    &target_cli, &target_port);
+			if (r < 0) {
+				/* Abort program if unable
+				 * to locate target port */
+				fprintf(stderr,
+					"Unable to find ALSA sequencer port: %s\n"
+					, optarg
+				);
+				ret = ERR_FINDPORT;
+				goto cleanup;
+			}
+			break;
+
+		case 'x': /* --hex */
+			is_hex = 1;
+			break;
+
+		case 'd': /* --delay */
+			spacing_us = better_atoi(optarg);
+			if (spacing_us < 0) {
+				fprintf(stderr,
+					"Parameter for --delay must be a positive integer: %s\n"
+					, optarg
+				);
+				ret = ERR_PARAM;
+				goto cleanup;
+			}
+			if (spacing_us > MAX_DELAY) {
+				fprintf(stderr,
+					"Parameter for --delay must be %d or less: %s\n"
+					, MAX_DELAY
+					, optarg
+				);
+				ret = ERR_PARAM;
+				goto cleanup;
+			}
+			/* Argument is in milliseconds,
+			 * but stored value is in microseconds */
+			spacing_us *= 1000;
+			break;
+
+		case 'w': /* --wait */
+			wait_sec = better_atoi(optarg);
+			if (wait_sec < 0) {
+				fprintf(stderr,
+					"Parameter for --wait must be a positive integer: %s\n"
+					, optarg
+				);
+				ret = ERR_PARAM;
+				goto cleanup;
+			}
+			if (wait_sec > MAX_WAIT) {
+				fprintf(stderr,
+					"Parameter for --wait must be %d or less: %s\n"
+					, MAX_WAIT
+					, optarg
+				);
+				ret = ERR_PARAM;
+				goto cleanup;
+			}
+			break;
+
+		case 'v': /* --verbose */
+			/* FUTURE: Perhaps multiple verbosity levels */
+			is_verbose = 1;
+			break;
+
+		case 'V': /* --version */
+			is_version = 1;
+			break;
+
+		case 'W': /* --nowrite */
+			is_write = 0;
+			break;
+
+		case 'R': /* --noread */
+			is_read = 0;
+			break;
+
+		case -1: /* Clean end of getopt processing */
+			is_done = 1;
+			break;
+
+		default: /* Error */
+			/* Error message already has been printed by getopt */
+			ret = ERR_PARAM;
+			goto cleanup;
+			break;
+		}
+	}
+
+	/* If both read and write are disabled, there's no point in running */
+	if (!is_read && !is_write) {
+		fprintf(stderr,
+			"Parameters --noread and --nowrite cannot coexist\n");
+		ret = ERR_PARAM;
+		goto cleanup;
+	}
+
+	/* If write to ALSA is disabled, nothing to do with input files */
+	if (!is_write) {
+		/* Command line must be empty after the options */
+		if (NULL != argv[optind]) {
+			fprintf(stderr,
+				"Parameter --nowrite must not be used with any input files\n"
+			);
+			ret = ERR_PARAM;
+			goto cleanup;
+		}
+	}
+
+	/* If write to ALSA is disabled, no input,
+	 * so no waiting after input */
+	if (!is_write) {
+		if (wait_sec > 0) {
+			fprintf(stderr,
+				"Parameters --nowrite and --wait cannot coexist\n"
+			);
+			ret = ERR_PARAM;
+			goto cleanup;
+		}
+	}
+
+	/* For --help, show help screen and exit successfully */
+	if (is_help) {
+		help_screen(argv[0]);
+		ret = 0;
+		goto cleanup;
+	}
+
+	/* For --version, show version line and exit successfully */
+	if (is_version) {
+		show_version();
+		ret = 0;
+		goto cleanup;
+	}
+
+	/* Open ALSA sequencer, if it is not already open */
+	if (NULL == handle)
+		handle = seq_open(is_read, is_write);
+	if (NULL == handle) {
+		/* Abort program if unable to open */
+		ret = ERR_OPEN;
+		goto cleanup;
+	}
+
+	/* Set up connection */
+	r = seq_setup(handle, cli_name, target_cli, target_port,
+		      is_read, is_write, &cli_id, &port_id);
+	if (r < 0) {
+		ret = ERR_CONNPORT;
+		goto cleanup;
+	}
+
+	/* For --list, show list of ports and exit successfully */
+	if (is_list) {
+		discover_ports(handle, NULL, 1, NULL, NULL);
+		ret = 0;
+		goto cleanup;
+	}
+
+	if (is_verbose) {
+		fprintf(stderr,
+			"Connected to ALSA sequencer on port %d:%d\n",
+			cli_id, port_id);
+	}
+
+	/* Initialize globals used for thread synchronization */
+	pthread_mutex_init(&g_mutex, NULL);
+	pthread_cond_init(&g_cond, NULL);
+	g_is_endinput = 0;
+
+	/* The output thread reads from ALSA and provides output */
+	if (is_read) {
+		/* Start output thread first, to avoid input backlog */
+		out_args.handle = handle;
+		out_args.is_hex = is_hex;
+
+		r = pthread_create(&out_thread, NULL, stdout_loop, &out_args);
+		if (r != 0) {
+			fprintf(stderr,
+				"Unable to start output thread: %s\n",
+				strerror(errno));
+			ret = ERR_THREAD;
+			goto threadwait;
+		}
+
+		is_out_started = 1;
+	}
+
+	/* The input thread takes input and writes to ALSA */
+	if (is_write) {
+		/* Start input thread */
+		/* Save pointer to rest of command line, after options */
+		in_args.args_ptr    = &(argv[optind]);
+		in_args.handle      = handle;
+		in_args.is_hex      = is_hex;
+		in_args.cli_id      = cli_id;
+		in_args.port_id     = port_id;
+		in_args.target_cli  = target_cli;
+		in_args.target_port = target_port;
+		in_args.spacing_us  = spacing_us;
+		in_args.wait_sec    = wait_sec;
+		in_args.is_read     = is_read;
+
+		r = pthread_create(&in_thread, NULL, stdin_loop, &in_args);
+		if (r != 0) {
+			fprintf(stderr,
+				"Unable to start input thread: %s\n",
+				strerror(errno));
+			ret = ERR_THREAD;
+			goto threadwait;
+		}
+
+		is_in_started = 1;
+	}
+
+	/* Initialization successful, now just wait for threads to exit */
+	ret = 0;
+
+threadwait:
+	/* Reap threads */
+	if (is_in_started)
+		pthread_join(in_thread, NULL);
+	if (is_out_started)
+		pthread_join(out_thread, NULL);
+
+cleanup:
+	/* Cleanup */
+	if (handle != NULL)
+		seq_close(handle);
+	free(cli_name);
+
+	return ret;
+}