Fix for WDM KS WaveRT low-latency microphone capture quality

[feoff3]

In my tests with latency < 40 ms on Win10 with embeded Realtek sound card, WDM KS capture resulted in sound with lots of cracks.
I digged into the code and found out 1) there is a wrong assumption that host buffer position should be either 0 or 1/2 of buffer size when event is handled, 2) ensured that host ring buffer is read both from the end and the beginning if necessary (the exisiting code might resulted in reading non-initialized data beyond the end of the host buffer)

[RossBencina]

Thank you for your contribution, please fix the trailing whitespace issue on line 6620 flagged by CI:

error: src/hostapi/wdmks/pa_win_wdmks.c(6620) trailing whitespace:
b' '

[RossBencina]

We notice that you've cut-and-pasted the same buffer copying code to two different places: PaPinRenderSubmitHandler_WaveCyclic, PaPinCaptureEventHandler_WaveRTPolled -- can you please confirm that both of these code paths have actually been tested on your machine? Copy-pasta is always a risk and I'd like to be sure that both codes are correct and tested.

[feoff3]

Yeap, I was considering having just one function instead of two because they are almost identical right now. Let me double check if the code works fine on both path, and if it is possible to reduce the code duplication there. вт, 22 нояб. 2022 г. в 05:15, Ross Bencina ***@***.***>:

…

We notice that you've cut-and-pasted the same buffer copying code to two different places: PaPinRenderSubmitHandler_WaveCyclic, PaPinCaptureEventHandler_WaveRTPolled -- can you please confirm that both of these code paths have actually been tested on your machine? Copy-pasta is always a risk and I'd like to be sure that both codes are correct and tested. — Reply to this email directly, view it on GitHub <#752 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV5W5SX2FR2BMSQHMCKEF3WJQCRPANCNFSM6AAAAAASCJ4LVU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[feoff3]

@RossBencina I have tested the SubType_kPolled type of capture code, and it produces distorted sound with ~10ms latency both with my updated code and original code.
On the other hand, my code improves SubType_kNotification type of capture significantly. So for now, I return the original code for SubType_kPolled, and push my code for SubType_kNotification.

I leave the SubType_kPolled type as is until the further investigation.

[RossBencina]

@feoff3 thank you. please fix the whitespace issue as previously requested.

[RossBencina]

@feoff3 do you think this should be merged as-is or were you planning to do more work on it? If merge, we'll fix the whitespace.

[feoff3]

Oh sorry, I just forgot about the whitespace - xmas and NY has swamped me into other stuff. Please feel free to fix the whitespace and merge ;)

…

On Tue, 10 Jan 2023, 05:23 Ross Bencina, ***@***.***> wrote: @feoff3 <https://github.com/feoff3> do you think this should be merged as-is or were you planning to do more work on it? If merge, we'll fix the whitespace. — Reply to this email directly, view it on GitHub <#752 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV5W5QQNT7XTC7TWOL7JMLWRSQGVANCNFSM6AAAAAASCJ4LVU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[dechamps]

Currently this PR only touches the PaPinCaptureEventHandler_WaveRTEvent() function. @feoff3 Is this because these issues don't apply to the render side, or because you didn't look at the render side at all? I'm wondering if similar issues on the render side could be causing #763.

[feoff3]

Hey there. Yes I didn't touch render side as I experienced the issue on capture side only. I can take a quick look on render side a little bit later... I guess the original developer used a similar approach to the render as was used for capture (which is prone to data loss and thus choppy sound)

…

On Sat, 21 Jan 2023, 21:28 Etienne Dechamps, ***@***.***> wrote: Currently this PR only touches the PaPinCaptureEventHandler_WaveRTEvent() function. @feoff3 <https://github.com/feoff3> Is this because these issues don't apply to the render side, or because you didn't look at the render side at all? I'm wondering if similar issues on the render side could be causing #763 <#763>. — Reply to this email directly, view it on GitHub <#752 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV5W5T4WSGUILX4WJULH2TWTQBRDANCNFSM6AAAAAASCJ4LVU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[RossBencina]

@feoff3 could you review our comments and commits please? It would be great if you could test the changes on your system too.

[RossBencina]

Awaiting response to our requests for change, and review of our commits.

[feoff3]

@RossBencina @philburk @dechamps Guys, thanks for the feedback. Give me several days to check your comments and run a couple of tests on my side.

[feoff3]

sorry guys, still stuck with other tasks. gonna do it next week

[feoff3]

yeap, makes sense

[feoff3]

changes looks good, thank you guys

[feoff3]

@RossBencina
For some reason "change requested" is still there and I am not able to approve your changes via Github UI (it allows me to comment only, no approval).
I have incorporated the suggested changes into a new commit.
I have tested the commit against a small test app on my side, see the app: https://gist.github.com/feoff3/c989e054d6a88369995ba7860c9eaabd