On Wed, 15 Nov 2023 16:26:56 GMT, Alec Su <[email protected]> wrote:

>> JVM attempts to reuse the buffer for sending MIDI out data when the buffer 
>> size is enough. It use `dwBytesRecorded` in `MIDIHDR` structure to indicate 
>> the actual size of the data. However, `midiOutLongMsg()` ignores 
>> `dwBytesRecorded`, although it did not mentioned in the documentation. I've 
>> tested on Windows 7, 10 and 11. All of them have the same behavior.
>> 
>> The bug cannot be easily reproduced because some MIDI drivers filter out any 
>> malformed MIDI data. The example code below create a special case to make 
>> sure all MIDI data are legally when the bug is triggered.
>> 
>> 
>> import javax.sound.midi.*;
>> 
>> public class MidiTest {
>>     public static class RawMidiMessage extends MidiMessage {
>>         public RawMidiMessage(byte[] data) {
>>             super(data);
>>         }
>> 
>>         @Override
>>         public Object clone() {
>>             return new RawMidiMessage(this.getMessage());
>>         }
>>     }
>> 
>>     public static void main(String[] args) {
>>         var deviceInfos = MidiSystem.getMidiDeviceInfo();
>>         for (var info : deviceInfos) {
>>             try (MidiDevice device = MidiSystem.getMidiDevice(info)) {
>>                 if (device.getMaxReceivers() != 0) {
>>                     System.out.println("Open MIDI port: " + info.getName());
>>                     device.open();
>>                     Receiver receiver = device.getReceiver();
>>                     // Send two sysex messages at once
>>                     receiver.send(new RawMidiMessage(new byte[]{
>>                             (byte) 0xF0, 0x7D, 0x01, (byte) 0xF7,
>>                             (byte) 0xF0, 0x7D, 0x02, (byte) 0xF7
>>                         }), -1);
>>                     // Send another sysex message
>>                     receiver.send(new RawMidiMessage(new byte[]{(byte) 0xF0, 
>> 0x7D, 0x03, (byte) 0xF7}), -1);
>>                 }
>>             } catch (MidiUnavailableException e) {
>>                 e.printStackTrace();
>>             }
>>         }
>>     }
>> }
>> 
>> 
>> The expected messages received should be the following three messages
>> 
>> F0 7D 01 F7
>> F0 7D 02 F7
>> F0 7D 03 F7
>> 
>> 
>> But acually four messages was received with the second message repeated 
>> twice.
>> 
>> F0 7D 01 F7
>> F0 7D 02 F7
>> F0 7D 03 F7
>> F0 7D 02 F7
>> 
>> 
>> To resolve the issue, I add a new variable to backup the actual buffer size 
>> and set `dwBufferLength` of `MIDIHDR` structure to the size of MIDI data. 
>> After calling `midiOutLongMsg()`, I restore the original buffer size if the 
>> buf...
>
> Alec Su has updated the pull request with a new target base due to a merge or 
> a rebase. The incremental webrev excludes the unrelated changes brought in by 
> the merge/rebase. The pull request contains six additional commits since the 
> last revision:
> 
>  - Remove the testcase and reuse the testcase added in #16477
>  - Merge branch 'master' into sysex-win
>  - Remove OS check in the test
>  - Code cleanup
>  - Add a testcase for the buffer issue on Windows
>  - Resolve the message size issue when sending SysexMessage on Windows

I ran this change through our CI testing system and it all looks good.

-------------

Marked as reviewed by prr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16399#pullrequestreview-1735250581

Reply via email to