I'll need to look at this again another time as the tests don't quite look correct.

According to this:
https://docs.microsoft.com/en-us/previous-versions/windows/xna/ff433726(v=xnagamestudio.40)

The BufferNeeded event is raised in the following situations:

• During DynamicSoundEffectInstance playback when the pending buffer count is less then or equal to 2.
• Every time the DynamicSoundEffectInstance instance completes playing a buffer, and updates the pending buffer count.
• When a DynamicSoundEffectInstance instance transitions from the stopped to playing state, and the pending buffer count is less than or equal to 2.

However some of the tests seem to be checking that only a single BufferNeeded event is raised per buffer used or play call. In other words only points 2 & 3 of the above, not point 1. This doesn't seem to match XNA which keeps raising the BufferNeeded event during playback until 3 buffers are queued.

Edit: It seems the tests are based upon not submitting data when the BufferNeeded event is raised. This is where my PR doesn't match XNA. However the current MonoGame implementation doesn't match XNA when data is actually submitted with the event (which can be seen with the sample project).

I'll need to look at this again and update the PR to match XNA, but also add tests for submitting data with BufferNeeded events as this is more important for correct playback operation.

Point 1 makes me think that the check/event happens on every tick.
However for point 3, mg implementation sets a flag during PlatformPlay and the events is invoked during the gameloop tick.
If we were to add a queue check in update it make the check on PlatformPlay irrelevant. The documentation explicitly states 3 as separate case which makes me think that XNA would invoke the event directly from PlatformPlay(). Probably the same is true for point 2.

Yeah, during normal playback some of the points seem to be asking for the same thing.

However I think the points begin to be different when SubmitBuffer is called outside of BufferNeeded and when no SubmitBuffer occurs during a BufferNeeded event.

My next step is to create a table of XNA results with the combinations of SubmitBuffer before playback, SubmitBuffer outside of BufferNeeded and no SubmitBuffer during BufferNeeded.

I'm hoping to do that in the coming weeks and revise the PR. For now the PR will fix the choppy audio glitch but shouldn't be committed yet as it will cause tests to fail.

Something that seems to work is to make _bufferNeeded boolean and call the event once per frame.

        internal void UpdateQueue()
        {
            // Update the buffers
            PlatformUpdateQueue();

            // Raise the event
            if (_bufferNeeded || (PendingBufferCount < TargetPendingBufferCount))
            {
               var bufferNeededHandler = BufferNeeded;
               if (bufferNeededHandler != null)
                   bufferNeededHandler(this, EventArgs.Empty);
               _bufferNeeded = false;
            }            
        }

edit
Obviously a boolean will not work if frames takes longer that a single buffer.

I did quickly try the above, however I'm still getting failed tests with that.

For some reason I thought I had tried the sample project with XNA, however I must have only compared it to the XNA docs. Point 1 is not being done by XNA and it will not automatically queue up 3 buffers (even though it's stated in the docs). Both XNA and MonoGame have the audio glitch when submitting buffers solely on the BufferNeeded event. Pre-submitting buffers before playback can fix this, but the documentation makes no mention of this as a requirement. Sorry for the confusion there.

Also I noticed with the MonoGame documentation (https://docs.monogame.net/api/Microsoft.Xna.Framework.Audio.DynamicSoundEffectInstance.html) that the BufferNeeded event should be called "when the number of queued audio buffers is less than or equal to 2". Again this matches the XNA documentation but hasn't been implemented in this way.

So going forward this raises the question should MonoGame match the documentation and the tests be revised or match XNA behaviour and the documentation be revised?

Another thing I noted just is in the tests the Thread.Sleep calls within the SleepWhileDispatching loop can take longer than expected and this can cause intermittent fails. Adding StopWatch to exit the loop early seemed to fix this issue.

Edit: Just noticed the remarks in the MonoGame documentation says "this event may occur when Play() is called or during playback when a buffer is completed", so that part is correct.

I guess the question then is should MonoGame be changed to always automatically queue up 3 buffers or should manually queuing up buffers before playback be a requirement in the documentation?

XNA implementation isn't that good. for example State isn't updated immediately after Play/Pause/Stop.
I bet what is described in the documentation is more correct behavior and what was their requirements/plan.
Today I also found this https://github.com/nkast/XNAGameStudio/blob/master/Documents/DynamicAudio_Article_4_0.docx
There's also this comment here "XNA posts DynamicSoundEffectInstance.BufferNeeded events always on the main thread."
I suppose what MG is doing then is correct to dispatch the events from update.

I 'll try a couple more things. I will bring back the eventNeeded counter, but I have removed the code entirely from Play().

Great. Nice find on the article. I need to read it some more, but there are a couple of points of interest.

// Start the sound, no currently pending buffers but sound will raise
// the BufferNeeded event giving us a chance to submit buffers for playback.

This comment implies buffers (multiple) should be sent on a BufferNeeded event, not just one.

void DynamicSoundInstance_BufferNeeded(object sender, EventArgs e)
{
    // Submitting the larger buffers as two smaller buffers ensures that
    // the event is raised before the buffers run out, avoiding glitches
    dynamicSoundInstance.SubmitBuffer(buffer, 0, buffer.Length/2);
    dynamicSoundInstance.SubmitBuffer(buffer, buffer.Length/2, buffer.Length/2);
}

This example is also submitting two buffers to "avoid glitches".

That implies it's probably working as expected and a single buffer per event will cause glitches, which is what is occurring in the sample project.

I guess the question now is should this be improved with MonoGame automatically calling BufferNeeded to properly maintain the TargetPendingBufferCount or should the documentation be improved to state that multiple buffers are required per BufferNeeded event.

by looking at the test i see that xna fired event only on buffer completion. I prefer the documented behavior better.
The implementation i currently have works fine. I might add a boolean and set it at Play(), then on update, and only if the user hasnt submited at least 2 buffers ,i will call the event two additional times. just to avoid initial glitses from small buffers.

By the way, thank you for the test project. It helped a lot.

No problem and thanks.

Just looking at the test failures again it seems these are due to what happens when no SubmitBuffer occurs during a BufferNeeded event. This should stop additional calls to build up to the TargetPendingBufferCount, but there should still be a call for each buffer consumed.

If I revert the PR (so that _buffersNeeded is counter of play/consumed events, not the number of buffers needed to reach the target) and change this

for (var i = 0; i < eventCount; i++)
{
    bufferNeededHandler(this, EventArgs.Empty);
}

To this

for (var i = 0; i < eventCount; i++)
{
    while (PendingBufferCount < TargetPendingBufferCount)
    {
        int lastPendingBufferCount = PendingBufferCount;
        bufferNeededHandler(this, EventArgs.Empty);
        if (PendingBufferCount == lastPendingBufferCount)
            break;
    }
}

Then this not only fixes the audio glitch but also passes all the existing tests as multiple BufferNeeded calls are ended early if the buffer count didn't change after a BufferNeeded event.

I need to test this some more but it seems to work so far.

the while loop will halt if the user write the following, unless you add a limit check

        private void DynamicSoundEffect_BufferNeeded(object sender, EventArgs e)
        {
            //TODO: tomorrow
        }

This is what i ended up with. Will probably break the tests.

        internal void Update()
        {
            lock (AudioService.SyncHandle)
            {
                // Update the buffers
                DynamicPlatformUpdate();

                // make sure initially we have at least 2 buffers
                if (_beginPlay)
                    _buffersNeeded = Math.Max(_buffersNeeded, TargetPendingBufferCount - 1 - PendingBufferCount);

                // Raise the event
                var bufferNeededHandler = BufferNeeded;
                if (bufferNeededHandler != null)
                {
                    // raise the event for each processed buffer
                    while(_buffersNeeded-- != 0)
                        bufferNeededHandler(this, EventArgs.Empty);

                    //raise the event once per frame if needed
                    if (State == SoundState.Playing && PendingBufferCount < TargetPendingBufferCount)
                        bufferNeededHandler(this, EventArgs.Empty);
                }

                _beginPlay = true;
                _buffersNeeded = 0;
            }
        }

the while loop will halt if the user write the following, unless you add a limit check

If you were referring to the while loop in my code that won't happen as it already breaks the loop if the PendingBufferCount doesn't increase.

With the code you posted I think it will fail the tests as it won't stop raising the event after SubmitBuffer isn't called. According to the document you linked:

Once the buffer count reaches 0, DynamicSoundEffectInstance assumes that the client is not interested in immediately submitting more buffers and stops raising this event. Of course, if the game queues more buffers, the event is raised again as the queued buffers are consumed.

The tests seem to be based around this, although this seems to go against point 1 of the XNA api (https://docs.microsoft.com/en-us/previous-versions/windows/xna/ff433726(v=xnagamestudio.40)) where events keep occurring in playback. The tests could be changed though if it was intended to keep raising events regardless of whether SubmitBuffer occurs. I'm not sure if this is desirable or not though.

Just with the code I posted, to make it clearer I've reordered it and commented although it does exactly the same as before

if (_buffersNeeded >= 1)
{
    // At least one event is needed after the initial play call and per processed buffer
    int minimumNoOfBufferNeededEvents = (_buffersNeeded < 3) ? _buffersNeeded : 3;

    // Multiple events may be raised to reach the target buffer count so long as a SubmitBuffer occurs for each event
    int noOfBuffersNeededEventsToReachTarget = TargetPendingBufferCount - PendingBufferCount;

    for (int i = 0; i < noOfBuffersNeededEventsToReachTarget; i++)
    {
        // Raise the BufferNeeded event and check if SubmitBuffer occurred
        int lastPendingBufferCount = PendingBufferCount;
        bufferNeededHandler(this, EventArgs.Empty);
        bool submitBufferOccurred = PendingBufferCount > lastPendingBufferCount;

        // Stop raising events if no SubmitBuffer occurred during BufferNeeded and the minimum number of events has already been raised
        if (!submitBufferOccurred && ((i + 1) >= minimumNoOfBufferNeededEvents))
            break;
    }
}