Skip to content

Commit

Permalink
8074211: javax.sound.midi: Error with send System Exclusive messages …
Browse files Browse the repository at this point in the history 
…of different length

8250667: MIDI sysex over USB scrambled when reply length matches previous message

Reviewed-by: prr
  • Loading branch information
@AlecJY @prrace
AlecJY authored and prrace committed Nov 21, 2023
1 parent 6d82436 commit e47cf61
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 26 deletions.
8 changes: 4 additions & 4 deletions src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_MidiIn.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -301,7 +301,7 @@ INT32 prepareBuffers(MidiDeviceHandle* handle) {
}
sysex = (SysExQueue*) handle->longBuffers;
for (i = 0; i<sysex->count; i++) {
MIDIHDR* hdr = &(sysex->header[i]);
MIDIHDR* hdr = &(sysex->headerInfo[i].header);
midiInPrepareHeader((HMIDIIN) handle->deviceHandle, hdr, sizeof(MIDIHDR));
err = midiInAddBuffer((HMIDIIN) handle->deviceHandle, hdr, sizeof(MIDIHDR));
}
Expand All @@ -320,7 +320,7 @@ INT32 unprepareBuffers(MidiDeviceHandle* handle) {
}
sysex = (SysExQueue*) handle->longBuffers;
for (i = 0; i<sysex->count; i++) {
err = midiInUnprepareHeader((HMIDIIN) handle->deviceHandle, &(sysex->header[i]), sizeof(MIDIHDR));
err = midiInUnprepareHeader((HMIDIIN) handle->deviceHandle, &(sysex->headerInfo[i].header), sizeof(MIDIHDR));
}
MIDIIN_CHECK_ERROR;
return (INT32) err;
Expand Down Expand Up @@ -502,7 +502,7 @@ void MIDI_IN_ReleaseMessage(MidiDeviceHandle* handle, MidiMessage* msg) {
}
sysex = (SysExQueue*) handle->longBuffers;
if (msg->type == LONG_MESSAGE && sysex) {
MIDIHDR* hdr = &(sysex->header[msg->data.l.index]);
MIDIHDR* hdr = &(sysex->headerInfo[msg->data.l.index].header);
//fprintf(stdout, "ReleaseMessage index %d\n", msg->data.l.index); fflush(stdout);
hdr->dwBytesRecorded = 0;
midiInAddBuffer((HMIDIIN) handle->deviceHandle, hdr, sizeof(MIDIHDR));
Expand Down
29 changes: 19 additions & 10 deletions src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_MidiOut.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -161,7 +161,7 @@ INT32 unprepareLongBuffers(MidiDeviceHandle* handle) {
}
sysex = (SysExQueue*) handle->longBuffers;
for (i = 0; i<sysex->count; i++) {
MIDIHDR* hdr = &(sysex->header[i]);
MIDIHDR* hdr = &(sysex->headerInfo[i].header);
if (hdr->dwFlags) {
err = midiOutUnprepareHeader((HMIDIOUT) handle->deviceHandle, hdr, sizeof(MIDIHDR));
}
Expand All @@ -170,8 +170,9 @@ INT32 unprepareLongBuffers(MidiDeviceHandle* handle) {
return (INT32) err;
}

INT32 freeLongBuffer(MIDIHDR* hdr, HMIDIOUT deviceHandle, INT32 minToLeaveData) {
INT32 freeLongBuffer(MidiHeaderInfo* info, HMIDIOUT deviceHandle, INT32 minToLeaveData) {
MMRESULT err = MMSYSERR_NOERROR;
MIDIHDR* hdr = &(info->header);

if (!hdr) {
ERROR0("MIDI_OUT_freeLongBuffer: hdr == NULL\n");
Expand All @@ -180,10 +181,11 @@ INT32 freeLongBuffer(MIDIHDR* hdr, HMIDIOUT deviceHandle, INT32 minToLeaveData)
if (hdr->dwFlags && deviceHandle) {
err = midiOutUnprepareHeader(deviceHandle, hdr, sizeof(MIDIHDR));
}
if (hdr->lpData && (((INT32) hdr->dwBufferLength) < minToLeaveData || minToLeaveData < 0)) {
if (hdr->lpData && (info->bufferLength < minToLeaveData || minToLeaveData < 0)) {
free(hdr->lpData);
hdr->lpData=NULL;
hdr->dwBufferLength=0;
info->bufferLength=0;
}
hdr->dwBytesRecorded=0;
hdr->dwFlags=0;
Expand All @@ -201,7 +203,7 @@ INT32 freeLongBuffers(MidiDeviceHandle* handle) {
}
sysex = (SysExQueue*) handle->longBuffers;
for (i = 0; i<sysex->count; i++) {
err = freeLongBuffer(&(sysex->header[i]), (HMIDIOUT) handle->deviceHandle, -1);
err = freeLongBuffer(&(sysex->headerInfo[i]), (HMIDIOUT) handle->deviceHandle, -1);
}
MIDIOUT_CHECK_ERROR;
return (INT32) err;
Expand Down Expand Up @@ -352,6 +354,7 @@ INT32 MIDI_OUT_SendShortMessage(MidiDeviceHandle* handle, UINT32 packedMsg, UINT
INT32 MIDI_OUT_SendLongMessage(MidiDeviceHandle* handle, UBYTE* data, UINT32 size, UINT32 timestamp) {
MMRESULT err;
SysExQueue* sysex;
MidiHeaderInfo* info = NULL;
MIDIHDR* hdr = NULL;
INT32 remainingSize;
int i;
Expand All @@ -378,10 +381,12 @@ INT32 MIDI_OUT_SendLongMessage(MidiDeviceHandle* handle, UBYTE* data, UINT32 siz
while (!hdr && handle->platformData) {
/* find a non-queued header */
for (i = 0; i < sysex->count; i++) {
hdr = &(sysex->header[i]);
info = &(sysex->headerInfo[i]);
hdr = &(info->header);
if ((hdr->dwFlags & MHDR_DONE) || (hdr->dwFlags == 0)) {
break;
}
info = NULL;
hdr = NULL;
}
/* wait for a buffer to free up */
Expand All @@ -404,22 +409,26 @@ INT32 MIDI_OUT_SendLongMessage(MidiDeviceHandle* handle, UBYTE* data, UINT32 siz
}

TRACE2("-> sending %d bytes with buffer index=%d\n", (int) size, (int) hdr->dwUser);
freeLongBuffer(hdr, handle->deviceHandle, (INT32) size);
freeLongBuffer(info, handle->deviceHandle, (INT32) size);
if (hdr->lpData == NULL) {
hdr->lpData = malloc(size);
hdr->dwBufferLength = size;
info->bufferLength = size;
}
// Because midiOutLongMsg() ignores dwBytesRecorded, set both
// dwBufferLength to the size of the data. The actual buffer
// size is recorded in info->bufferLength.
hdr->dwBufferLength = size;
hdr->dwBytesRecorded = size;
memcpy(hdr->lpData, data, size);
err = midiOutPrepareHeader((HMIDIOUT) handle->deviceHandle, hdr, sizeof(MIDIHDR));
if (err != MMSYSERR_NOERROR) {
freeLongBuffer(hdr, handle->deviceHandle, -1);
freeLongBuffer(info, handle->deviceHandle, -1);
MIDIOUT_CHECK_ERROR;
return (INT32) err;
}
err = midiOutLongMsg((HMIDIOUT) handle->deviceHandle, hdr, sizeof(MIDIHDR));
if (err != MMSYSERR_NOERROR) {
freeLongBuffer(hdr, handle->deviceHandle, -1);
freeLongBuffer(info, handle->deviceHandle, -1);
ERROR0("ERROR: MIDI_OUT_SendLongMessage: midiOutLongMsg returned error:\n");
MIDIOUT_CHECK_ERROR;
return (INT32) err;
Expand Down
11 changes: 6 additions & 5 deletions src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_Util.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2007, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -89,7 +89,7 @@ int MIDI_WinCreateLongBufferQueue(MidiDeviceHandle* handle, int count, int size,
SysExQueue* sysex;
int i;
UBYTE* dataPtr;
int structSize = sizeof(SysExQueue) + ((count - 1) * sizeof(MIDIHDR));
int structSize = sizeof(SysExQueue) + ((count - 1) * sizeof(MidiHeaderInfo));

sysex = (SysExQueue*) malloc(structSize);
if (!sysex) return FALSE;
Expand All @@ -112,10 +112,11 @@ int MIDI_WinCreateLongBufferQueue(MidiDeviceHandle* handle, int count, int size,
// set up headers
dataPtr = preAllocatedMem;
for (i=0; i<count; i++) {
sysex->header[i].lpData = dataPtr;
sysex->header[i].dwBufferLength = size;
sysex->headerInfo[i].header.lpData = dataPtr;
sysex->headerInfo[i].header.dwBufferLength = size;
sysex->headerInfo[i].bufferLength = size;
// user data is the index of the buffer
sysex->header[i].dwUser = (DWORD) i;
sysex->headerInfo[i].header.dwUser = (DWORD) i;
dataPtr += size;
}
return TRUE;
Expand Down
17 changes: 11 additions & 6 deletions src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_Util.h
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2007, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -46,12 +46,17 @@

#include "PlatformMidi.h"

typedef struct tag_MidiHeaderInfo {
MIDIHDR header; // Windows specific structure to hold meta info
INT32 bufferLength; // the actual length of the buffer in MIDIHDR
} MidiHeaderInfo;

typedef struct tag_SysExQueue {
int count; // number of sys ex headers
int size; // data size per sys ex header
int ownsLinearMem; // true when linearMem is to be disposed
UBYTE* linearMem; // where the actual sys ex data is, count*size bytes
MIDIHDR header[1]; // Windows specific structure to hold meta info
int count; // number of sys ex headers
int size; // data size per sys ex header
int ownsLinearMem; // true when linearMem is to be disposed
UBYTE* linearMem; // where the actual sys ex data is, count*size bytes
MidiHeaderInfo headerInfo[1]; // a structure to hold MIDIHDR and the actual buffer length
} SysExQueue;

/* set the startTime field in MidiDeviceHandle */
Expand Down
7 changes: 6 additions & 1 deletion test/jdk/javax/sound/midi/SysexMessage/SendRawSysexMessage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

/**
* @test
* @bug 8237495 8301310
* @bug 8074211 8237495 8301310
* @summary fail with memory errors when asked to send a sysex message starting
* with 0xF7
*/
Expand Down Expand Up @@ -114,6 +114,11 @@ private static void test(MidiDevice.Info info) throws Exception {
(byte) SPECIAL_SYSTEM_EXCLUSIVE}), -1);
System.err.println("note off");
r.send(new ShortMessage(ShortMessage.NOTE_OFF, 5, 5), -1);
// The three parts of the sysex below are added for
// JDK-8301310, but it can also used to test JDK-8074211.
// However, The testcase does not fail when JDK-8074211 occurs.
// It's recommended to setup a loopback MIDI device then check
// whether the sysex received is the same as the testcase.
System.err.println("sysex part 1 of 3");
r.send(new SysexMessage(new byte[]{
(byte) SYSTEM_EXCLUSIVE, 0x7D, 0x01, 0x02}, 4), -1);
Expand Down

3 comments on commit e47cf61

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

@JesperIRL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/tag jdk-22+25

@openjdk
Copy link

@openjdk openjdk bot commented on e47cf61 Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JesperIRL The tag jdk-22+25 was successfully created.

Please sign in to comment.