Rework how drain stop the stream #529
Conversation
The logic for draining a stream was odd. After drain the AU stop was happening on the next callback. That could create race conditions especially when reinit is taking place at the same time. This patch triggers the stop of AU at the end of the same callback and set the shutdown flag in order to exit early any following callback.
|
From memory, we trigger the drain logic on the next callback to allow time for the buffer we just wrote to play out. With this change, I think we'll stop too early. It's probably quite hard to notice since we'd miss less than 11ms (default latency, could be lower) of audio in the typical case. It might be worth verifying with a higher latency stream/device (Bluetooth) and a short sound effect. |
|
I don't understand the problem the changes would solve here. If it's a solution for a different problem from #528, the changes can be tested without my devices. If the changes are for the same problem, I'll appreciate if you can state your opinions in the review comments. |
I did an experiment that the stream was drained on the first callback. This produced a very small (in terms of duration) sound effect. I guess that was the one filled buffer. I could try to hack the latency to something larger and try again. Can you recall if you had a specific problem that made you call the stop on the next callback or it was a "better safe than sorry" decision? |
The changes here include a fix for the problem in #528. I thought a patch would be a faster explanation of my idea. Sorry about that I will comment on #528. Nevertheless, I would appreciate if you can test it with your device and tell me what you see. Thank you in advance. |
I did an experiment similar to the above with a modified version of the backend that allowed big latency. Again, the stream drained at first callback. I verified that audio data of length equal to latency is written in the output. I started with a latency of 441 and ended with a latency of 4410 (for a rate of 44100). Further increase of the latency had no effect. |
|
Thanks for testing! Maybe there's no issue, and I don't remember any other details now. https://lists.apple.com/archives/coreaudio-api/2005/Dec/msg00049.html seems to suggest it works as you've observed, so I assume we waited an extra callback cycle just to be safe. |
|
Also https://lists.apple.com/archives/coreaudio-api/2005/Dec/msg00055.html, in particular:
|
|
Thanks for the links! That was a very interesting thread! |
This improves how we stop the stream when it is drained. Also, it addresses BMO 1572273. Unfortunately, I cannot verify that without the specific device mentioned there. @ChunMinChang can you please test it with your device? If the test passes we can land this one and skip #528 .