We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3633 - WASAPI_FlushCapture may handle empty-buffer return codes incorrectly
Summary: WASAPI_FlushCapture may handle empty-buffer return codes incorrectly
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: audio (show other bugs)
Version: HG 2.0
Hardware: All Windows 8
: P2 normal
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-18 14:37 UTC by Simon Hug
Modified: 2017-05-20 01:51 UTC (History)
0 users

See Also:

Attachments
WASAPI_FlushCapture (and bug 3632) workaround example. Don't commit this directly to the repository. (1.69 KB, patch)
2017-04-18 14:37 UTC, Simon Hug 		Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Hug 2017-04-18 14:37:55 UTC 
Created attachment 2724 [details]
WASAPI_FlushCapture (and bug 3632) workaround example. Don't commit this directly to the repository.

When WASAPI_FlushCapture [1] removes all data from the endpoint it uses the GetBuffer method in a loop until WasapiFailed [2] returns true. The way this is currently implemented seems to cause issues on my system (Windows 8.1 x86_64). For me, the GetBuffer method throws the AUDCLNT_S_BUFFER_EMPTY return code which WasapiFailed doesn't handle. This return code signals a success (according to the MSDN), but WasapiFailed immediately disables the device. No audio data will be captured.

This may be fixed if WasapiFailed handles AUDCLNT_S_BUFFER_EMPTY like S_OK and the loop in WASAPI_FlushCapture exits if the return code is AUDCLNT_S_BUFFER_EMPTY or WasapiFailed returns true.

I have to say that I don't know anything about WASAPI and I just looked a tiny bit at the SDL code for it. Since Microsoft has an interesting history with audio interfaces I thought it would be educational to poke around and stumbled over this issue.

Attached is an example workaround I used (including one for the issue mentioned in bug 3632). Don't commit this directly to the repository, because I really don't have enough experience to be certain that this is the proper way of doing it.

Also, SDL_wasapi.c mixes spaces and tabs for indentation. Sometimes on the same line. The Horror!

[1] https://hg.libsdl.org/SDL/file/10e16b1151b0/src/audio/wasapi/SDL_wasapi.c#l604
[2] https://hg.libsdl.org/SDL/file/10e16b1151b0/src/audio/wasapi/SDL_wasapi.c#l383
Comment 1 Ryan C. Gordon 2017-05-18 19:46:34 UTC 
> Attached is an example workaround I used (including one for the issue
> mentioned in bug 3632). Don't commit this directly to the repository,
> because I really don't have enough experience to be certain that this is the
> proper way of doing it.

My patch is similar: https://hg.libsdl.org/SDL/rev/cca512965b67

> Also, SDL_wasapi.c mixes spaces and tabs for indentation. Sometimes on the
> same line. The Horror!

The danger of popping into Visual Studio for finishing work that was started elsewhere.  :)

That is fixed now too: https://hg.libsdl.org/SDL/rev/96de7fe65e32

Thanks!

--ryan.
Comment 2 Simon Hug 2017-05-19 01:01:24 UTC 
I tested tip but still get a closed device.

I'm sorry I didn't mention this earlier, but WASAPI_CaptureFromDevice also suffers from this issue. The code checks for AUDCLNT_S_BUFFER_EMPTY after calling WasapiFailed [1].

[1] https://hg.libsdl.org/SDL/file/c2f18f1f7e97/src/audio/wasapi/SDL_wasapi.c#l565
Comment 3 Simon Hug 2017-05-19 01:07:58 UTC 
Correction for my last comment: I meant "disabled" not "closed". As in the call to SDL_OpenedAudioDeviceDisconnected sets the enabled member of SDL_AudioDevice to 0.

Oh, and a quick check shows that WasapiFailed is only called if the return code isn't AUDCLNT_S_BUFFER_EMPTY after that GetBuffer call in WASAPI_CaptureFromDevice will fix it.
Comment 4 Ryan C. Gordon 2017-05-19 02:11:49 UTC 
Yeah, whoops, just noticed this too. Fix incoming.
Comment 5 Ryan C. Gordon 2017-05-19 16:41:45 UTC 
Fixed (for real this time?) in https://hg.libsdl.org/SDL/rev/4210074a33c8

--ryan.
Comment 6 Simon Hug 2017-05-20 01:51:31 UTC 
Yes, it has been resolved.