| Message ID | 20240626161017.9479-1-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: seq: Improve UMP SysEx packing | expand |
On Wed, 26 Jun 2024 18:10:15 +0200, Takashi Iwai wrote: > > The initial implementation of 7bit SysEx message conversion to UMP did > a trick to deal with the first SysEx start byte (0xf7). The converter > trims it and sends the first packet with the content of 5 bytes. > Although this is valid and should work, it's not ideal and a bit > confusing as if something goes wrong. > > This patch improves the handling to try filling always 6 bytes in the > package. > > Fixes: e9e02819a98a ("ALSA: seq: Automatic conversion of UMP events") > Signed-off-by: Takashi Iwai <tiwai@suse.de> Scratch this one. It won't work properly when the sysex message is split. Will rework on it. Takashi > --- > sound/core/seq/seq_ump_convert.c | 48 +++++++++++++++++++------------- > 1 file changed, 28 insertions(+), 20 deletions(-) > > diff --git a/sound/core/seq/seq_ump_convert.c b/sound/core/seq/seq_ump_convert.c > index e90b27a135e6..952e64c499dd 100644 > --- a/sound/core/seq/seq_ump_convert.c > +++ b/sound/core/seq/seq_ump_convert.c > @@ -1192,7 +1192,7 @@ static int cvt_sysex_to_ump(struct snd_seq_client *dest, > { > struct snd_seq_ump_event ev_cvt; > unsigned char status; > - u8 buf[6], *xbuf; > + u8 buf[6]; > int offset = 0; > int len, err; > > @@ -1200,36 +1200,44 @@ static int cvt_sysex_to_ump(struct snd_seq_client *dest, > return 0; > > setup_ump_event(&ev_cvt, event); > + len = 0; > + status = UMP_SYSEX_STATUS_START; > for (;;) { > - len = snd_seq_expand_var_event_at(event, sizeof(buf), buf, offset); > - if (len <= 0) > + err = snd_seq_expand_var_event_at(event, sizeof(buf) - len, > + buf + len, offset); > + if (err < 0) > + break; > + len += err; > + if (!len) > break; > if (WARN_ON(len > 6)) > break; > - offset += len; > - xbuf = buf; > - if (*xbuf == UMP_MIDI1_MSG_SYSEX_START) { > - status = UMP_SYSEX_STATUS_START; > - xbuf++; > + offset += err; > + if (status == UMP_SYSEX_STATUS_START && > + buf[0] == UMP_MIDI1_MSG_SYSEX_START) { > len--; > - if (len > 0 && xbuf[len - 1] == UMP_MIDI1_MSG_SYSEX_END) { > - status = UMP_SYSEX_STATUS_SINGLE; > - len--; > - } > - } else { > - if (xbuf[len - 1] == UMP_MIDI1_MSG_SYSEX_END) { > - status = UMP_SYSEX_STATUS_END; > - len--; > - } else { > - status = UMP_SYSEX_STATUS_CONTINUE; > - } > + memmove(buf, buf + 1, len); > + continue; /* fill one more byte */ > } > - fill_sysex7_ump(dest_port, ev_cvt.ump, status, xbuf, len); > + if (buf[len - 1] == UMP_MIDI1_MSG_SYSEX_END) { > + len--; > + if (status == UMP_SYSEX_STATUS_START) > + status = UMP_SYSEX_STATUS_SINGLE; > + else > + status = UMP_SYSEX_STATUS_END; > + } > + fill_sysex7_ump(dest_port, ev_cvt.ump, status, buf, len); > err = __snd_seq_deliver_single_event(dest, dest_port, > (struct snd_seq_event *)&ev_cvt, > atomic, hop); > if (err < 0) > return err; > + > + if (status == UMP_SYSEX_STATUS_START) > + status = UMP_SYSEX_STATUS_CONTINUE; > + else if (status != UMP_SYSEX_STATUS_CONTINUE) > + break; > + len = 0; /* reset filling */ > } > return 0; > } > -- > 2.43.0 >
diff --git a/sound/core/seq/seq_ump_convert.c b/sound/core/seq/seq_ump_convert.c index e90b27a135e6..952e64c499dd 100644 --- a/sound/core/seq/seq_ump_convert.c +++ b/sound/core/seq/seq_ump_convert.c @@ -1192,7 +1192,7 @@ static int cvt_sysex_to_ump(struct snd_seq_client *dest, { struct snd_seq_ump_event ev_cvt; unsigned char status; - u8 buf[6], *xbuf; + u8 buf[6]; int offset = 0; int len, err; @@ -1200,36 +1200,44 @@ static int cvt_sysex_to_ump(struct snd_seq_client *dest, return 0; setup_ump_event(&ev_cvt, event); + len = 0; + status = UMP_SYSEX_STATUS_START; for (;;) { - len = snd_seq_expand_var_event_at(event, sizeof(buf), buf, offset); - if (len <= 0) + err = snd_seq_expand_var_event_at(event, sizeof(buf) - len, + buf + len, offset); + if (err < 0) + break; + len += err; + if (!len) break; if (WARN_ON(len > 6)) break; - offset += len; - xbuf = buf; - if (*xbuf == UMP_MIDI1_MSG_SYSEX_START) { - status = UMP_SYSEX_STATUS_START; - xbuf++; + offset += err; + if (status == UMP_SYSEX_STATUS_START && + buf[0] == UMP_MIDI1_MSG_SYSEX_START) { len--; - if (len > 0 && xbuf[len - 1] == UMP_MIDI1_MSG_SYSEX_END) { - status = UMP_SYSEX_STATUS_SINGLE; - len--; - } - } else { - if (xbuf[len - 1] == UMP_MIDI1_MSG_SYSEX_END) { - status = UMP_SYSEX_STATUS_END; - len--; - } else { - status = UMP_SYSEX_STATUS_CONTINUE; - } + memmove(buf, buf + 1, len); + continue; /* fill one more byte */ } - fill_sysex7_ump(dest_port, ev_cvt.ump, status, xbuf, len); + if (buf[len - 1] == UMP_MIDI1_MSG_SYSEX_END) { + len--; + if (status == UMP_SYSEX_STATUS_START) + status = UMP_SYSEX_STATUS_SINGLE; + else + status = UMP_SYSEX_STATUS_END; + } + fill_sysex7_ump(dest_port, ev_cvt.ump, status, buf, len); err = __snd_seq_deliver_single_event(dest, dest_port, (struct snd_seq_event *)&ev_cvt, atomic, hop); if (err < 0) return err; + + if (status == UMP_SYSEX_STATUS_START) + status = UMP_SYSEX_STATUS_CONTINUE; + else if (status != UMP_SYSEX_STATUS_CONTINUE) + break; + len = 0; /* reset filling */ } return 0; }
The initial implementation of 7bit SysEx message conversion to UMP did a trick to deal with the first SysEx start byte (0xf7). The converter trims it and sends the first packet with the content of 5 bytes. Although this is valid and should work, it's not ideal and a bit confusing as if something goes wrong. This patch improves the handling to try filling always 6 bytes in the package. Fixes: e9e02819a98a ("ALSA: seq: Automatic conversion of UMP events") Signed-off-by: Takashi Iwai <tiwai@suse.de> --- sound/core/seq/seq_ump_convert.c | 48 +++++++++++++++++++------------- 1 file changed, 28 insertions(+), 20 deletions(-)