GNOME Bugzilla – Bug 793289
wasapi: Several improvements to latency and glitch-free playback
Last modified: 2018-02-08 16:14:22 UTC
With these patches, playback with WASAPI is always reliable and completely glitch-free on my machine with several different audio setups that I've tested. Shared mode engine period is always 10ms. On-board sound card: capture and render in shared and exclusive modes exclusive mode device period: 2.66ms USB webcam (C310): capture in shared and exclusive modes USB webcam (C920): capture in shared and exclusive modes exclusive mode device period: 3ms USB DAC: render in shared and exclusive modes exclusive mode device period: 3ms HDMI card: render in shared and exclusive modes exclusive mode device period: 3.33ms Bluetooth headphones: render in shared mode Bluetooth speakers: render in shared mode
Created attachment 368122 [details] [review] wasapi: Rename struct element for device name We will use ->device for storing a pointer to the IMMDevice structure which is needed for fetching the caps supported by devices in exclusive mode.
Created attachment 368123 [details] [review] wasapi: Allow opening devices in exclusive mode This provides much lower latency compared to opening in shared mode, but it also means that the device cannot be opened by any other application. The advantage is that the achievable latency is much lower. In shared mode, WASAPI's engine period is 10ms, and so that is the lowest latency achievable. In exclusive mode, the limit is the device period itself, which in my testing with USB DACs, on-board PCI sound-cards, and HDMI cards is between 2ms and 3.33ms. We set our audioringbuffer limits to match the device, so the achievable sink latency is 6-9ms. Further improvements can be made if needed.
Created attachment 368124 [details] [review] wasapi: Increase thread priority to reduce glitches This is particularly important when running in exclusive mode because any delays will immediately cause glitching. The MinGW version in Cerbero is too old, so we can only enable this when building with MSVC or when people build GStreamer for MSYS2 or other MinGW-based distributions. To force-enable this code when building with MinGW, build with CFLAGS="-DGST_FORCE_WIN_AVRT -lavrt".
Created attachment 368125 [details] [review] wasapi: Cover more HRESULT error messages This requires using allocated strings, but it's the best option. For instance, a call could fail because CoInitialize() wasn't called, or because some other thing in the stack failed.
Created attachment 368126 [details] [review] wasapi: Try to use latency-time and buffer-time So far, we have been completely discarding the values of latency-time and buffer-time and trying to always open the device in the lowest latency mode possible. However, sometimes this is a bad idea: 1. When we want to save power/CPU and don't want low latency 2. When the lowest latency setting causes glitches 3. Other audio-driver bugs Now we will try to follow the user-set values of latency-time and buffer-time in shared mode, and only latency-time in exclusive mode (we have no control over the hardware buffer size, and there is no use in setting GstAudioRingBuffer size to something larger). The elements will still try to open the devices in the lowest latency mode possible if you set the "low-latency" property to "true".
Created attachment 368127 [details] [review] wasapisink: pre-load the buffer with silence This reduces the chances of startup glitches, and also reduces the chances that we'll get garbled output due to driver bugs. Recommended by the WASAPI documentation.
Created attachment 368128 [details] [review] wasapisink: Re-align device period if necessary Sometimes the minimum period advertised by a card results in an unaligned buffer size error during initialization in exclusive mode. In that case, we can fetch the actual buffer size in frames and calculate the period from that. We can't do this pre-emptively because we can't call GetBufferSize till Initialize has been called at least once.
Review of attachment 368126 [details] [review]: ::: sys/wasapi/gstwasapiutil.c @@ +847,3 @@ + * we're writing to the other one. */ + use_buffer = use_period; + else { This is missing some {} :)
Comment on attachment 368127 [details] [review] wasapisink: pre-load the buffer with silence Only needed for the sink? And you don't actually fill the buffer with silence, but just get and release it. Intentional?
Comment on attachment 368128 [details] [review] wasapisink: Re-align device period if necessary This also makes sure to set the correct things in the ringbuffer spec? Then it should be fine, but ugly :)
Review of attachment 368123 [details] [review]: ::: sys/wasapi/gstwasapisink.c @@ +401,3 @@ + use_period = min_period; + /* For some reason, we need to call this another time for exclusive mode */ + CoInitialize (NULL); Isn't this basically refcounted, i.e. you need to uninitialize it the same number of times again? If prepare() fails, I'm not sure if unprepare() is always called
(In reply to Sebastian Dröge (slomo) from comment #9) > Comment on attachment 368127 [details] [review] [review] > wasapisink: pre-load the buffer with silence > > Only needed for the sink? And you don't actually fill the buffer with > silence, but just get and release it. Intentional? Yes, this is only needed for the sink because there's a possible race where WASAPI relinquishes the device to us and before the first streaming event hits us, the driver flips the buffer and writes out garbage or an old buffer. While releasing, we set the SILENT flag which tells wasapi to write out silence. (In reply to Sebastian Dröge (slomo) from comment #10) > Comment on attachment 368128 [details] [review] [review] > wasapisink: Re-align device period if necessary > > This also makes sure to set the correct things in the ringbuffer spec? Then > it should be fine, but ugly :) Yeah, we set this (potential) device period (in ms) on the device, and get the actual value back (in frames) from GetBufferSize. We then use that to set segsize and segtotal on the spec, and the acquire() function sets latency_time/buffer_time using that. (In reply to Sebastian Dröge (slomo) from comment #11) > Review of attachment 368123 [details] [review] [review]: > > ::: sys/wasapi/gstwasapisink.c > @@ +401,3 @@ > + use_period = min_period; > + /* For some reason, we need to call this another time for exclusive > mode */ > + CoInitialize (NULL); > > Isn't this basically refcounted, i.e. you need to uninitialize it the same > number of times again? If prepare() fails, I'm not sure if unprepare() is > always called You're right, it looks like it's not, so we need to release a bunch of other things that are released in unprepare() too, will add a new patch for that instead of trying to fiddle with rebase too much.
Created attachment 368130 [details] [review] wasapi: Unprepare when src/sink_prepare fails unprepare() is not called automatically on failure.
Attachment 368122 [details] pushed as 4b38881 - wasapi: Rename struct element for device name Attachment 368123 [details] pushed as 6ecbb75 - wasapi: Allow opening devices in exclusive mode Attachment 368124 [details] pushed as 62b6224 - wasapi: Increase thread priority to reduce glitches Attachment 368125 [details] pushed as 624de04 - wasapi: Cover more HRESULT error messages Attachment 368126 [details] pushed as 4dbca8d - wasapi: Try to use latency-time and buffer-time Attachment 368127 [details] pushed as 7f1d60d - wasapisink: pre-load the buffer with silence Attachment 368128 [details] pushed as cbe2fc4 - wasapisink: Re-align device period if necessary Attachment 368130 [details] pushed as 69b9022 - wasapi: Unprepare when src/sink_prepare fails
This is fantastic news! I look forward to trying it out in the near future. Thanks for your work.