From fc2099dede649eeeda019802b43bb453a5155663 Mon Sep 17 00:00:00 2001 From: Philippe Simons Date: Sat, 7 Oct 2023 12:33:21 +0200 Subject: [PATCH 1/4] MIDI: Ignore SysEx messages ignore SysEx messages instead of choking --- .../androidx/media3/decoder/midi/TrackEvent.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java b/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java index 0dca5d33bb4..a9e68904c87 100644 --- a/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java +++ b/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java @@ -37,6 +37,8 @@ public static final int DATA_FIELD_UNSET = Integer.MIN_VALUE; private static final int TICKS_UNSET = -1; + private static final int SYSEX_BEGIN_STATUS = 0xF0; + private static final int SYSEX_END_STATUS = 0xF7; private static final int META_EVENT_STATUS = 0xFF; private static final int META_END_OF_TRACK = 0x2F; private static final int META_TEMPO_CHANGE = 0x51; @@ -139,10 +141,17 @@ public boolean populateFrom(ParsableByteArray parsableTrackEventBytes, int previ default: // Ignore all other Meta events. parsableTrackEventBytes.skipBytes(eventLength); } - } else { + } else if (firstByte == SYSEX_BEGIN_STATUS) { // TODO(b/228838584): Handle this gracefully. + statusByte = firstByte; + + int currentByte; + do { // eat SysEx Message + currentByte = parsableTrackEventBytes.readUnsignedByte(); + } while (currentByte != SYSEX_END_STATUS); + } else { throw ParserException.createForUnsupportedContainerFeature( - "SysEx track events are not yet supported."); + "Unknown track events."); } } @@ -155,7 +164,7 @@ public boolean populateFrom(ParsableByteArray parsableTrackEventBytes, int previ public boolean isMidiEvent() { // TODO(b/228838584): Update with SysEx event check when implemented. - return statusByte != META_EVENT_STATUS; + return statusByte != META_EVENT_STATUS && statusByte != SYSEX_BEGIN_STATUS; } public boolean isNoteChannelEvent() { From b27b65199ada2db2a77665e64aaef89623e52c51 Mon Sep 17 00:00:00 2001 From: Philippe Simons Date: Thu, 19 Oct 2023 21:53:15 +0200 Subject: [PATCH 2/4] Update TrackEvent.java --- .../media3/decoder/midi/TrackEvent.java | 72 +++++++++---------- 1 file changed, 35 insertions(+), 37 deletions(-) diff --git a/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java b/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java index a9e68904c87..a83ab980c88 100644 --- a/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java +++ b/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java @@ -115,44 +115,42 @@ public boolean populateFrom(ParsableByteArray parsableTrackEventBytes, int previ } statusByte = firstByte; - } else { - if (firstByte == META_EVENT_STATUS) { // This is a Meta event. - int metaEventMessageType = parsableTrackEventBytes.readUnsignedByte(); - int eventLength = readVariableLengthInt(parsableTrackEventBytes); - - statusByte = firstByte; - - switch (metaEventMessageType) { - case META_TEMPO_CHANGE: - usPerQuarterNote = parsableTrackEventBytes.readUnsignedInt24(); - - if (usPerQuarterNote <= 0) { - throw ParserException.createForUnsupportedContainerFeature( - "Tempo event data value must be a non-zero positive value. Parsed value: " - + usPerQuarterNote); - } - - parsableTrackEventBytes.skipBytes(eventLength - /* tempoDataLength */ 3); - break; - case META_END_OF_TRACK: - parsableTrackEventBytes.setPosition(startingPosition); - reset(); - return false; - default: // Ignore all other Meta events. - parsableTrackEventBytes.skipBytes(eventLength); - } - } else if (firstByte == SYSEX_BEGIN_STATUS) { - // TODO(b/228838584): Handle this gracefully. - statusByte = firstByte; - - int currentByte; - do { // eat SysEx Message - currentByte = parsableTrackEventBytes.readUnsignedByte(); - } while (currentByte != SYSEX_END_STATUS); - } else { - throw ParserException.createForUnsupportedContainerFeature( - "Unknown track events."); + } else if (firstByte == META_EVENT_STATUS) { // This is a Meta event. + int metaEventMessageType = parsableTrackEventBytes.readUnsignedByte(); + int eventLength = readVariableLengthInt(parsableTrackEventBytes); + + statusByte = firstByte; + + switch (metaEventMessageType) { + case META_TEMPO_CHANGE: + usPerQuarterNote = parsableTrackEventBytes.readUnsignedInt24(); + + if (usPerQuarterNote <= 0) { + throw ParserException.createForUnsupportedContainerFeature( + "Tempo event data value must be a non-zero positive value. Parsed value: " + + usPerQuarterNote); + } + + parsableTrackEventBytes.skipBytes(eventLength - /* tempoDataLength */ 3); + break; + case META_END_OF_TRACK: + parsableTrackEventBytes.setPosition(startingPosition); + reset(); + return false; + default: // Ignore all other Meta events. + parsableTrackEventBytes.skipBytes(eventLength); } + } else if (firstByte == SYSEX_BEGIN_STATUS) { + // TODO(b/228838584): Handle this gracefully. + statusByte = firstByte; + + int currentByte; + do { // eat SysEx Message + currentByte = parsableTrackEventBytes.readUnsignedByte(); + } while (currentByte != SYSEX_END_STATUS); + } else { + throw ParserException.createForUnsupportedContainerFeature( + "Unknown track event: " + firstByte); } eventFileSizeBytes = parsableTrackEventBytes.getPosition() - startingPosition; From a06430a513903d90c76131236fdd088cdf9a649e Mon Sep 17 00:00:00 2001 From: Philippe Simons Date: Thu, 19 Oct 2023 21:54:15 +0200 Subject: [PATCH 3/4] Update TrackEvent.java --- .../src/main/java/androidx/media3/decoder/midi/TrackEvent.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java b/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java index a83ab980c88..4ccbfb6c468 100644 --- a/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java +++ b/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java @@ -145,7 +145,7 @@ public boolean populateFrom(ParsableByteArray parsableTrackEventBytes, int previ statusByte = firstByte; int currentByte; - do { // eat SysEx Message + do { // Consume SysEx message. currentByte = parsableTrackEventBytes.readUnsignedByte(); } while (currentByte != SYSEX_END_STATUS); } else { From 71602767ca7ae60e2c46509a4b809e496a59ee3f Mon Sep 17 00:00:00 2001 From: christosts Date: Fri, 20 Oct 2023 11:13:11 +0000 Subject: [PATCH 4/4] Restructure TrackEvent.populateFrom() --- RELEASENOTES.md | 2 + .../media3/decoder/midi/TrackEvent.java | 72 +++++++++---------- 2 files changed, 35 insertions(+), 39 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 884884f770e..adb59ff9df4 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -38,6 +38,8 @@ * Smooth Streaming Extension: * RTSP Extension: * Decoder Extensions (FFmpeg, VP9, AV1, MIDI, etc.): + * MIDI decoder: Ignore SysEx event messages + ([#710](https://github.com/androidx/media/pull/710)). * Leanback extension: * Cast Extension: * Test Utilities: diff --git a/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java b/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java index 4ccbfb6c468..5e7c830b4a7 100644 --- a/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java +++ b/libraries/decoder_midi/src/main/java/androidx/media3/decoder/midi/TrackEvent.java @@ -84,9 +84,40 @@ public boolean populateFrom(ParsableByteArray parsableTrackEventBytes, int previ int firstByte = parsableTrackEventBytes.readUnsignedByte(); eventDecoderSizeBytes = 1; - if ((firstByte & 0xF0) != 0xF0) { - // Most significant nibble is not 0xF, this is a MIDI channel event. + if (firstByte == SYSEX_BEGIN_STATUS) { + // TODO(b/228838584): Handle this gracefully. + statusByte = firstByte; + + int currentByte; + do { // Consume SysEx message. + currentByte = parsableTrackEventBytes.readUnsignedByte(); + } while (currentByte != SYSEX_END_STATUS); + } else if (firstByte == META_EVENT_STATUS) { // This is a Meta event. + int metaEventMessageType = parsableTrackEventBytes.readUnsignedByte(); + int eventLength = readVariableLengthInt(parsableTrackEventBytes); + + statusByte = firstByte; + + switch (metaEventMessageType) { + case META_TEMPO_CHANGE: + usPerQuarterNote = parsableTrackEventBytes.readUnsignedInt24(); + + if (usPerQuarterNote <= 0) { + throw ParserException.createForUnsupportedContainerFeature( + "Tempo event data value must be a non-zero positive value. Parsed value: " + + usPerQuarterNote); + } + parsableTrackEventBytes.skipBytes(eventLength - /* tempoDataLength */ 3); + break; + case META_END_OF_TRACK: + parsableTrackEventBytes.setPosition(startingPosition); + reset(); + return false; + default: // Ignore all other Meta events. + parsableTrackEventBytes.skipBytes(eventLength); + } + } else { // This is a MIDI channel event. // Check for running status, an occurrence where the statusByte has been omitted from the // bytes of this event. The standard expects us to assume that this command has the same // statusByte as the last command. @@ -115,42 +146,6 @@ public boolean populateFrom(ParsableByteArray parsableTrackEventBytes, int previ } statusByte = firstByte; - } else if (firstByte == META_EVENT_STATUS) { // This is a Meta event. - int metaEventMessageType = parsableTrackEventBytes.readUnsignedByte(); - int eventLength = readVariableLengthInt(parsableTrackEventBytes); - - statusByte = firstByte; - - switch (metaEventMessageType) { - case META_TEMPO_CHANGE: - usPerQuarterNote = parsableTrackEventBytes.readUnsignedInt24(); - - if (usPerQuarterNote <= 0) { - throw ParserException.createForUnsupportedContainerFeature( - "Tempo event data value must be a non-zero positive value. Parsed value: " - + usPerQuarterNote); - } - - parsableTrackEventBytes.skipBytes(eventLength - /* tempoDataLength */ 3); - break; - case META_END_OF_TRACK: - parsableTrackEventBytes.setPosition(startingPosition); - reset(); - return false; - default: // Ignore all other Meta events. - parsableTrackEventBytes.skipBytes(eventLength); - } - } else if (firstByte == SYSEX_BEGIN_STATUS) { - // TODO(b/228838584): Handle this gracefully. - statusByte = firstByte; - - int currentByte; - do { // Consume SysEx message. - currentByte = parsableTrackEventBytes.readUnsignedByte(); - } while (currentByte != SYSEX_END_STATUS); - } else { - throw ParserException.createForUnsupportedContainerFeature( - "Unknown track event: " + firstByte); } eventFileSizeBytes = parsableTrackEventBytes.getPosition() - startingPosition; @@ -161,7 +156,6 @@ public boolean populateFrom(ParsableByteArray parsableTrackEventBytes, int previ } public boolean isMidiEvent() { - // TODO(b/228838584): Update with SysEx event check when implemented. return statusByte != META_EVENT_STATUS && statusByte != SYSEX_BEGIN_STATUS; }