billthefarmer / mididriver

Android midi driver using Sonivox EAS library
176 stars 52 forks source link

Oboe Callback blocked with mutex #47

Closed ygtorres closed 2 years ago

ygtorres commented 2 years ago

One of the requirements when using audio thread is that any object shared with the audio thread have to be locked free, so why you use a mute when calling EAS_Render. That is causing suboptimal timer accuracy.

billthefarmer commented 2 years ago

I agree that's what the docs say and expert advice says similar. However the Sonivox synthesizer isn't thread safe, so if an app calls EAS_WriteMIDIStream() while the synthesizer is processing EAS_Render() unexpected things happen. Using a mutex is the accepted way of handling this and that is what android does in MidiFile.cpp, which is the code that handles playing midi files. And that's where I originally got the ideas from to develop this driver.

ygtorres commented 2 years ago

Yes you are right, is not thread safe, and it's true in the code android has published to test the library, they use mutextes, only because it's converting midi to WAV. But that's no the right way. And if anyone needs to build a midi players, will notice the issue with time accuracy. I will close this issue, and when I have time I will share a solution for all of the people with that issue.

billthefarmer commented 2 years ago

I am somewhat confused as to what timer accuracy you are talking about. I think I have tested this before, but I just tested it again. This is on a MotoG, so it's rather dated now and not the fastest CPU compared with now.


// midi write
jboolean midi_write(EAS_U8 *bytes, jint length)
{
    EAS_RESULT result;

    if (pEASData == NULL || midiHandle == NULL)
        return JNI_FALSE;
    clock_t start = clock();
    // lock
    pthread_mutex_lock(&mutex);
    clock_t next = clock();
    result = EAS_WriteMIDIStream(pEASData, midiHandle, bytes, length);
    clock_t penult = clock();
    // unlock
    pthread_mutex_unlock(&mutex);
    clock_t end = clock();
    if (result != EAS_SUCCESS)
        return JNI_FALSE;

    double lock = ((double) (next - start)) / CLOCKS_PER_SEC;
    double write = ((double) (penult - next)) / CLOCKS_PER_SEC;
    double unlock = ((double) (end - penult)) / CLOCKS_PER_SEC;

    LOG_D(LOG_TAG, "Write %1.6f %1.6f %1.6f", lock, write, unlock);

    return JNI_TRUE;
}

Results:

01-12 13:51:37.354 21448 21448 D MidiDriver: Write 0.000015 0.000010 0.000007
01-12 13:51:39.526 21448 21448 D MidiDriver: Write 0.000044 0.000065 0.000011
01-12 13:51:39.527 21448 21448 D MidiDriver: Write 0.000016 0.000015 0.000010
01-12 13:51:39.527 21448 21448 D MidiDriver: Write 0.000012 0.000014 0.000010
01-12 13:51:40.638 21448 21448 D MidiDriver: Write 0.000032 0.000022 0.000014
01-12 13:51:40.638 21448 21448 D MidiDriver: Write 0.000016 0.000016 0.000013
01-12 13:51:40.638 21448 21448 D MidiDriver: Write 0.000016 0.000015 0.000014

This is just pressing and releasing one of the chord buttons on the midi test app. There is a fair bit of variation, but the delays are tens of microseconds. The latency in the phone audio system and the buffering system are going to be orders of magnitude greater than that.

ygtorres commented 2 years ago

Playing single notes it's ok, the issue arise when you need it for a more intensive task. For instance to create a live midi file player. That's where you really notice the consequences of using a mutex in Audio Thread

On Wed, Jan 12, 2022, 10:03 AM Bill Farmer @.***> wrote:

I am somewhat confused as to what timer accuracy you are talking about. I think I have tested this before, but I just tested it again. This is on a MotoG, so it's rather dated now and not the fastest CPU compared with now.

// midi write jboolean midi_write(EAS_U8 *bytes, jint length) { EAS_RESULT result;

if (pEASData == NULL || midiHandle == NULL)
    return JNI_FALSE;
clock_t start = clock();
// lock
pthread_mutex_lock(&mutex);
clock_t next = clock();
result = EAS_WriteMIDIStream(pEASData, midiHandle, bytes, length);
clock_t penult = clock();
// unlock
pthread_mutex_unlock(&mutex);
clock_t end = clock();
if (result != EAS_SUCCESS)
    return JNI_FALSE;

double lock = ((double) (next - start)) / CLOCKS_PER_SEC;
double write = ((double) (penult - next)) / CLOCKS_PER_SEC;
double unlock = ((double) (end - penult)) / CLOCKS_PER_SEC;

LOG_D(LOG_TAG, "Write %1.6f %1.6f %1.6f", lock, write, unlock);

return JNI_TRUE;

}

Results:

01-12 13:51:37.354 21448 21448 D MidiDriver: Write 0.000015 0.000010 0.000007 01-12 13:51:39.526 21448 21448 D MidiDriver: Write 0.000044 0.000065 0.000011 01-12 13:51:39.527 21448 21448 D MidiDriver: Write 0.000016 0.000015 0.000010 01-12 13:51:39.527 21448 21448 D MidiDriver: Write 0.000012 0.000014 0.000010 01-12 13:51:40.638 21448 21448 D MidiDriver: Write 0.000032 0.000022 0.000014 01-12 13:51:40.638 21448 21448 D MidiDriver: Write 0.000016 0.000016 0.000013 01-12 13:51:40.638 21448 21448 D MidiDriver: Write 0.000016 0.000015 0.000014

This is just pressing and releasing one of the chord buttons on the midi test app. There is a fair bit of variation, but the delays are tens of microseconds. The latency in the phone audio system and the buffering system are going to be orders of magnitude greater than that.

— Reply to this email directly, view it on GitHub https://github.com/billthefarmer/mididriver/issues/47#issuecomment-1011077085, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHIQAASARJKN5AMMXC6BUDDUVWC2NANCNFSM5LSXYYMQ . You are receiving this because you modified the open/close state.Message ID: @.***>

billthefarmer commented 2 years ago

If you are using this driver to play midi files you are using the wrong software. MidiFile.cpp uses the EAS_OpenFile()/EAS_Prepare()/EAS_CloseFile()/etc API to play files. This driver is intended for musical instrument emulators or games where the midi messages should be limited to 2 or 3 bytes as per the test app.

ygtorres commented 2 years ago

Hi, thank you very much for taking your time to reply, it's ok. Base on your driver I developed a new one it's not using any lock, I couldn't use any of you suggestion because my midi files are generated on the fly and while playing user can modify it's events so it's not possible to use a plain midi file. User may also change tempo. So I needed a high performance midi player, and now it's working perfectly fine. You can check it out here.

https://www.facebook.com/groups/402392864940112/?ref=share

On Thu, Jan 13, 2022, 7:29 AM Bill Farmer @.***> wrote:

If you are using this driver to play midi files you are using the wrong software. MidiFile.cpp uses the EAS_OpenFile()/EAS_Prepare()/ EAS_CloseFile()/etc API to play files. This driver is intended for musical instrument emulators or games where the midi messages should be limited to 2 or 3 bytes as per the test app.

— Reply to this email directly, view it on GitHub https://github.com/billthefarmer/mididriver/issues/47#issuecomment-1012049956, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHIQAAVMRCWHWIZ46NAR2DTUV2ZRLANCNFSM5LSXYYMQ . You are receiving this because you modified the open/close state.Message ID: @.***>