Buffer underrun symptoms on Windows 10

[jps-aldridge]

Hello,

I'm using simpleaudio on Windows 10 to play a sound using code like

play = simpleaudio.play_buffer(
    audio_data=idata,
    num_channels=1, bytes_per_sample=2,
    sample_rate=Fs
)
play.wait_done()

and I often get broken-up sound. Looking at a recording of that sound, I see that it plays properly for some multiple of 0.05 seconds, then a short gap of silence is inserted, then playback resumes.

I experimented with the code, and found that changing

buffer_size = get_buffer_size(latency_us / NUM_BUFS, sample_rate, bytes_per_chan * num_channels);

to

buffer_size = get_buffer_size(2 * latency_us / NUM_BUFS, sample_rate, bytes_per_chan * num_channels);

in the function play_os() in simpleaudio_win.c (i.e. doubling the size of the buffers) was enough to make the problem stop happening, at least on my system.

Whilst investigating this problem, I also found (I think) two other potential ways in which sound corruption might occur, which I would like to offer for your consideration. It's possible I'm wrong about these, though, since I'm not very familiar with your code.

What's the best way of progressing this?

Cheers,
John

[hamiltron]

John,

The fix you described totally makes sense, though I'm surprised the issue came up. I had though the default buffer size was pretty generous for modern system (though there are always exceptions). I have wondered in the past whether this would come up and thought it might be smart to make the buffer size more dynamic. My first though was making that configurable by way of an environment variable since whatever setting works best will probably correlate with a particular system (OS, CPU, memory, background applications, etc) and would ideally be configurable without having to modify code (for users of the application/script in question who aren't developers). What are your thoughts on potential solutions?

I'd also welcome any information on potential bugs!

Thanks!

-Joe

[jps-aldridge]

My machine is an i7-3770 which although an old-ish machine, is still pretty competitive performance-wise. Allowing configuration via some environment variable sounds sensible, though I suspect that a modest increase in the buffer for everyone might actually be enough -- maybe 0.05 seconds is so short as to be affected by the task scheduler timeslice, or something like that.

FWIW, the code I'm using here at the moment actually looks like

/* Dividing by NUM_BUFS, as this code used to, results in buffers of only 0.05  */
/* seconds, which seems to be too short for smooth playback, at least on my     */
/* Windows 10 machine, and and that of a friend. Perhaps it's not even          */
/* desirable? This change results in each buffer being SA_LATENCY_US in length. */
buffer_size = get_buffer_size(latency_us, sample_rate, bytes_per_chan * num_channels);

(i.e. rather than multiplying by 2 as per the original comment, I've eliminated the division by NUM_BUFS, which comes to the same thing).

The other two potential issues are that

(a) I think that in fill_buffer() the replacement

    result = waveOutUnprepareHeader(audio_blob->handle, wave_header, sizeof(WAVEHDR));
    if (result != MMSYSERR_NOERROR) {return result;}
    memcpy(wave_header->lpData, &((char*)audio_blob->buffer_obj.buf)[audio_blob->used_bytes], have);

    /* Setting the buffer length should be done before the call to waveOutWrite */
    /* otherwise extra junk data might be sent in the last buffer if the sound  */
    /* was not a round number of buffers in length */
    wave_header->dwBufferLength = have;

    result = waveOutPrepareHeader(audio_blob->handle, wave_header, sizeof(WAVEHDR));
    if (result != MMSYSERR_NOERROR) {return result;}
    result = waveOutWrite(audio_blob->handle, wave_header, sizeof(WAVEHDR));
    if (result != MMSYSERR_NOERROR) {return result;}
    audio_blob->used_bytes += have;
/* ... no more audio left to buffer */
} else {

if probably a good idea, and

(b) At the end of play_os() I think this replacement might be safer...

for (i = 0; i < NUM_BUFS; i++) {
    temp_wave_hdr = PyMem_Malloc(sizeof(WAVEHDR));
    memset(temp_wave_hdr, 0, sizeof(WAVEHDR));
    temp_wave_hdr->lpData = PyMem_Malloc(buffer_size);
    temp_wave_hdr->dwBufferLength = buffer_size;

    /* I think there was a possible timing problem with the old code here,      */
    /* which used to call fill_buffer() directly. If Windows finishes with the  */
    /* first buffer at the same time as this code is creating and filling the   */
    /* second, then we could have two threads draining data from the audio blob */
    /* at the same time. It seems safer to use PostThreadMessage to pass this   */
    /* action off to the background thread.                                     */
    PostThreadMessage(thread_id, MM_WOM_DONE, (WPARAM)(audio_blob->handle), (LPARAM)temp_wave_hdr);
}

Hopefully the comments adequately explain the motivation for these changes, but if not I'm happy to discuss them further.

I'm also happy to submit a proper pull request, if you like, but didn't want to do that to start with in case you assumed they were properly considered changes, rather than suggestions for your consideration!

John

[Quefumas]

Hello,

I'd just like to bump this thread since I've been experiencing this issue for a long time now.
Also, my audio synthesis and processing library (Gensound) is relying on simpleAudio for playback, so it would be really great if this can be resolved.

Let me know if there's anything I can help with. I'm not well-versed in hardware-related C, but maybe I can run tests on my end (or something else you might have in mind). Here are some details in case that's relevant:

• I'm running win10 with i7 6500u,
• I always make sure to put the relevant buffer of audio samples as contiguous bytes in memory (through NumPy),
• I typically run it through an interactive shell in an IDE, but the underrun can also happen via the basic CLI as well,
• this issue seems to be independent of the current state of system resources and the actual audio.

Thanks!