Noise when seeking with wasapi:exclusive

[ponyo]

If you pause seek unpause it sounds like there is some sound from the before seek time for a moment. Its not a problem in shared mode.

[kevmitch]

This is a known issue for which the cause much less the solution is unknown. It has been observed it to go away with a smaller buffer size, however, then there tend to be much worse crackling artifacts on seek/pause which sometimes continue for the remainder of playback.

[ghost]

@kevmitch: do you know whether this issue is with windows/the driver, or the AO, or with the mpv common code?

[kevmitch]

I have a suspicion that it is a windows/driver issue, but I feel it's lazy to proclaim that definitively.
I have been meaning to test if these problems persist if I re-replace the COM messaging since that was the commit that seemed to break things for me.

I should see I can reproduce similar behaviour with chrome.

[ponyo]

Which buffer are you refering to. Does clearing the buffer before resuming playback not eliminate the problem.

[ponyo]

I tried --audio-buffer=0 but it didnt seem to change anything.

Cycling the audio track on and off completely gets rid of the noise though. I expect this reinitializes the buffers?

[kevmitch]

I'm pretty sure that controls the size of mpv internal ring buffer. I'm talking about the wasapi buffer to which that's copied. It's size is specified by bufferPeriod at ao_wasapi_utils.c:522. There is no command line option.

Cycling the audio track re-initializes the entire audio driver. Though I seem to remember hearing this problem even at the very beginning of playback right after the audio driver has been initialized.

[ghost]

Note that seeking is entirely different from stream switching and full audio reinit. When seeking, the player will suspend and resume the audio output, which is obviously more complicated.

One thing that catches my eye: isn't the way how IAudioClient_GetCurrentPadding is called a bit strange? Is it really only needed for shared mode?

[kevmitch]

Supposedly, in exclusive mode, there are two buffers so that both the audio device and us each have control over a buffer at any given time. the two buffers are exchanged in their entirety some time after IAudioRenderClient_ReleaseBuffer is called. Therefore the offset should always be zero.

[kevmitch]

Yes here it is:

For an exclusive-mode rendering or capture stream that was initialized with the AUDCLNT_STREAMFLAGS_EVENTCALLBACK flag, the client typically has no use for the padding value reported by GetCurrentPadding. Instead, the client accesses an entire buffer during each processing pass. Each time a buffer becomes available for processing, the audio engine notifies the client by signaling the client's event handle. For more information about this flag, see IAudioClient::Initialize.

It is kind of vague on what the meaning of this value is in that case. It is however giving information on something.

--msg-level=ao=debug tells me that my device period is

[ao/wasapi] Device period: 10 ms

and it correctly uses a 2400 frame buffer (for 48000Hz)

[ao/wasapi] Buffer frame count: 2400 (50 ms)

I tried modifying the code so that it still calls GetCurrentPadding even in exclusive mode. In this case --msg-level=ao=trace tells me about the reported padding values:

the first read right after intialization:

audio_resume
[ao/wasapi] Frame to fill: 2400. Padding: 0
[ao/wasapi] Device delay: 0 samples (0 ms)
(this fills one of the buffers before the stream starts)

the next (and subsequent) chunks:

[ao/wasapi] Frame to fill: 2400. Padding: 2400
[ao/wasapi] Device delay: 2400 samples (50 ms)

on pause:

[ao/wasapi] audio_reset
[ao/wasapi] Frame to fill: 2400. Padding: 0
[ao/wasapi] Device delay: 0 samples (0 ms)
[ao/wasapi] Frame to fill: 2400. Padding: 2400
[ao/wasapi] Device delay: 2400 samples (50 ms)

(it seems to request two more reads after IAudioClient_Stop/)IAudioClient_Reset are called)

on resume:

[ao/wasapi] audio_resume
[ao/wasapi] Frame to fill: 2400. Padding: 4800
[ao/wasapi] Device delay: 4800 samples (100 ms)
(again this is filling the buffer before calling IAudioClient_Start) note that now the padding is reported as twice the buffer length

The next read after rIAudioClient_Start reports the same padding

[ao/wasapi] Frame to fill: 2400. Padding: 4800
[ao/wasapi] Device delay: 7200 samples (150 ms)

The read after that returns to normal

[ao/wasapi] Frame to fill: 2400. Padding: 2400
[ao/wasapi] Device delay: 7200 samples (150 ms)

[kevmitch]

mmm. it looks as though the two extra reads happen after

IAudioClient_Stop(state->pAudioClientProxy);

in other words Stop returns before it has actually stopped the stream.

This means that the subsequent call to

IAudioClient_Reset(state->pAudioClientProxy);
atomic_store(&state->sample_count, 0);

is actually happening before the two final reads.

If I sleep (at least) two buffer lengths after the Stop with

mp_sleep_us(100); 

then the reset happens after the two reads have finished.

Then on the next resume things go more smoothly

[ao/wasapi] audio_resume
[ao/wasapi] Frame to fill: 2400. Padding: 0
[ao/wasapi] Device delay: 0 samples (0 ms)
[ao/wasapi] IAudioClient_Start
[ao/wasapi] Frame to fill: 2400. Padding: 2400
[ao/wasapi] Device delay: 2400 samples (50 ms)

and the echo sound happens less frequently.

However, it does still happen sometimes for me, which I think may be an inevitable driver/windows bug because, like I said previously, I even hear it sometimes on initialization (starting mpv, or cycling audio tracks).

[kevmitch]

So I see three options for improving the situation:

1. sleep between Stop and Reset
2. switch on a state->stopped flag in audio_reset after calling Stop/Reset and re-reset after every buffer read until it is swiched off in audio_resume. Can I assume that audio_resume will be called after audio_reset?
3. re-reset in audio_resume.

[kevmitch]

oops, that's only sleeping 100us, not ms. it's weird that even makes a difference.

[ghost]

If it's a race condition, it should be fixed properly. The common ao code (pull.c in this case) actually doesn't want much, and it will return silence for any data read after the reset callback.

[kevmitch]

What do you mean by "properly"? Option 2) above seems the closest approximation I can see.

Based on my observations, having a non-reset buffer on resume is bad. It's possible the additional reads are even happening before audio_reset returns, which would explain why the buffer has non-silent garbage in it on resume.

Therefore IAudioClient_Reset/atomic_store(&state->sample_count, 0) needs to happen after the stream has really stopped. But wasapi doesn't provide any way to know when this is. As far as I can tell there isn't even a way to query whether the stream is started or stopped at a given moment. Basically I think some sort of hack is necessary to work around the broken wasapi api.

[ponyo]

I dont think that IAudioClient is asynchronous or anything.

Is it possible to swap buffers on a stopped stream.

[kevmitch]

It is definitely behaving asynchronously if the stream hasn't stopped when IAudioClient_Stop returns.

The "swapping" appears to be an implementation detail that we have no direct control over. Indeed the padding value seems to indicate that the endpoint device can "own" more than one buffer if they have not yet been played. In this case it will just create additional buffers for us to fill rather than overwriting ones that haven't been played.

This shouldn't really be a problem however if we could just do an actual stream reset and clear all the buffers.

[ponyo]

It is definitely behaving asynchronously if the stream hasn't stopped when IAudioClient_Stop returns.

Maybe it just appears not to have stopped because the buffers are still being swapped.

I read some more of the documentation.

It says to use GetBuffer() and ReleaseBuffer() before Start() in order to avoid glitches. That would mean that you can buffer a stopped stream.

So I am guessing that the race condition is between ReleaseBuffer() and Reset().

[kevmitch]

It says to use GetBuffer() and ReleaseBuffer() before Start() in order to avoid glitches. That would mean that you can buffer a stopped stream.

Yes, we do exactly in audio_resume with

SetEvent(state->hForceFeed);
WaitForSingleObject(state->hFeedDone, INFINITE);

The problem is that that is supposed to write audio heard immediately after IAudioClient_Start. However if the stream is not in a reset state when this is done, we'll hear the other garbage first.

So I am guessing that the race condition is between ReleaseBuffer() and Reset().

Yes, exactly. We need to make sure that there are no more GetBuffer/ReleaseBuffer after Reset. The problem is that wasapi keeps sending us state->hFeed events after we have called reset. I'm not sure if we're allowed to just ignore these, which is why I suggested forcing reset after ReleaseBuffer above.

[ghost]

So it sounds like Reset() and Start() should be called in the feed thread. audio_resume doesn't have to act immediately (there's simply little reason why it should), and its effect can be "delayed" until the feed thread gets to it. audio_resume only needs to make sure the feed thread gets back to work.

[kevmitch]

I tried moving Stop/Reset to a feed thread event. It still triggers a single feed event right after. That's an improvement because with the current COM messaging it triggers two, but still not a complete solution.

The other thing we could do is Reset() right before we fill the buffer in the hForceFeed event.

[ghost]

I'm not sure if I'm following, but can't we just not fill the buffer in the reset case?

[kevmitch]

@ponyo can you test the above pull request. Here is mpv.exe

[ponyo]

@kevmitch This doesnt fix it for me. I still have the same problem.

Your exe doesnt give the extra messages.

[kevmitch]

So there is no improvement at all? That definitely improved the stuttering sounds for me. Although I still hear it at the very start of playback.

Here is a different version which always unconditionally resets (clears the buffer) right before resuming.

If that doesn't help, then we could look at the device delay (which I still don't really understand).

[kevmitch]

I also put the device padding debugging messages back in the most recent version I posted at msg-level=ao=trace. I took them out of exclusive mode in the one i committed because the trace message was the only thing that used the value.

[kevmitch]

@ponyo ping
does the experimental mpv.exe improve anything?