2020-06-20 04:26:01

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [PATCH V2 1/2] midi: Fix SysEx parser in MIDI plugin

From: Tedd Ho-Jeong An <[email protected]>

The SysEx end of message parser didn't consider the fact that
timestampsLow are in the stream and it might have the EOX (F7)
byte as well.

Fix that by always assuming the first system message byte is
the timestampLow.

Future work would involve support other type of system message
bytes, such as real-time.
---
profiles/midi/libmidi.c | 38 +++++++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/profiles/midi/libmidi.c b/profiles/midi/libmidi.c
index 4b4df799f..7d57e7335 100644
--- a/profiles/midi/libmidi.c
+++ b/profiles/midi/libmidi.c
@@ -331,6 +331,30 @@ static size_t handle_end_of_sysex(struct midi_read_parser *parser,
return sysex_length + 1; /* +1 because of timestampLow */
}

+/* Searches the end of a SysEx message that contains a timestampLow
+ * before the SysEx end byte. Returns the number of bytes of valid
+ * SysEx payload in the buffer.
+ */
+static size_t sysex_eox_len(const uint8_t *data, size_t size)
+{
+ size_t i = 0;
+
+ MIDI_ASSERT(size > 0);
+
+ if (data[i] == 0xF0)
+ i++;
+
+ /* Search for timestamp low */
+ while (i < size) {
+ if ((data[i] & 0x80)) {
+ i++;
+ break;
+ }
+ i++;
+ }
+
+ return (i < size && data[i] == 0xF7) ? i : 0;
+}


size_t midi_read_raw(struct midi_read_parser *parser, const uint8_t *data,
@@ -368,14 +392,14 @@ size_t midi_read_raw(struct midi_read_parser *parser, const uint8_t *data,

/* System Common Messages */
case 0xF0: /* SysEx Start */ {
- uint8_t *pos;
+ size_t sysex_length;

/* cleanup Running Status Message */
parser->rstatus = 0;

- /* Avoid parsing if SysEx is contained in one BLE packet */
- if ((pos = memchr(data + i, 0xF7, size - i))) {
- const size_t sysex_length = pos - (data + i);
+ sysex_length = sysex_eox_len(data + i, size - i);
+ /* Search for End of SysEx message in one BLE message */
+ if (sysex_length > 0) {
midi_size = handle_end_of_sysex(parser, ev, data + i,
sysex_length);
} else {
@@ -424,10 +448,10 @@ size_t midi_read_raw(struct midi_read_parser *parser, const uint8_t *data,

/* Check for SysEx messages */
if (parser->sysex_stream.len > 0) {
- uint8_t *pos;
+ size_t sysex_length;

- if ((pos = memchr(data + i, 0xF7, size - i))) {
- const size_t sysex_length = pos - (data + i);
+ sysex_length = sysex_eox_len(data + i, size - i);
+ if (sysex_length > 0) {
midi_size = handle_end_of_sysex(parser, ev, data + i,
sysex_length);
} else {
--
2.25.4


2020-06-22 19:29:36

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] midi: Fix SysEx parser in MIDI plugin

Hi Tedd,

On Fri, Jun 19, 2020 at 9:27 PM <[email protected]> wrote:
>
> From: Tedd Ho-Jeong An <[email protected]>
>
> The SysEx end of message parser didn't consider the fact that
> timestampsLow are in the stream and it might have the EOX (F7)
> byte as well.
>
> Fix that by always assuming the first system message byte is
> the timestampLow.
>
> Future work would involve support other type of system message
> bytes, such as real-time.
> ---
> profiles/midi/libmidi.c | 38 +++++++++++++++++++++++++++++++-------
> 1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/profiles/midi/libmidi.c b/profiles/midi/libmidi.c
> index 4b4df799f..7d57e7335 100644
> --- a/profiles/midi/libmidi.c
> +++ b/profiles/midi/libmidi.c
> @@ -331,6 +331,30 @@ static size_t handle_end_of_sysex(struct midi_read_parser *parser,
> return sysex_length + 1; /* +1 because of timestampLow */
> }
>
> +/* Searches the end of a SysEx message that contains a timestampLow
> + * before the SysEx end byte. Returns the number of bytes of valid
> + * SysEx payload in the buffer.
> + */
> +static size_t sysex_eox_len(const uint8_t *data, size_t size)
> +{
> + size_t i = 0;
> +
> + MIDI_ASSERT(size > 0);
> +
> + if (data[i] == 0xF0)
> + i++;
> +
> + /* Search for timestamp low */
> + while (i < size) {
> + if ((data[i] & 0x80)) {
> + i++;
> + break;
> + }
> + i++;
> + }
> +
> + return (i < size && data[i] == 0xF7) ? i : 0;
> +}
>
>
> size_t midi_read_raw(struct midi_read_parser *parser, const uint8_t *data,
> @@ -368,14 +392,14 @@ size_t midi_read_raw(struct midi_read_parser *parser, const uint8_t *data,
>
> /* System Common Messages */
> case 0xF0: /* SysEx Start */ {
> - uint8_t *pos;
> + size_t sysex_length;
>
> /* cleanup Running Status Message */
> parser->rstatus = 0;
>
> - /* Avoid parsing if SysEx is contained in one BLE packet */
> - if ((pos = memchr(data + i, 0xF7, size - i))) {
> - const size_t sysex_length = pos - (data + i);
> + sysex_length = sysex_eox_len(data + i, size - i);
> + /* Search for End of SysEx message in one BLE message */
> + if (sysex_length > 0) {
> midi_size = handle_end_of_sysex(parser, ev, data + i,
> sysex_length);
> } else {
> @@ -424,10 +448,10 @@ size_t midi_read_raw(struct midi_read_parser *parser, const uint8_t *data,
>
> /* Check for SysEx messages */
> if (parser->sysex_stream.len > 0) {
> - uint8_t *pos;
> + size_t sysex_length;
>
> - if ((pos = memchr(data + i, 0xF7, size - i))) {
> - const size_t sysex_length = pos - (data + i);
> + sysex_length = sysex_eox_len(data + i, size - i);
> + if (sysex_length > 0) {
> midi_size = handle_end_of_sysex(parser, ev, data + i,
> sysex_length);
> } else {
> --
> 2.25.4
>

Applied, thanks.

--
Luiz Augusto von Dentz