search

googlearchive / android-audio-high-performance

We now recommend you use the Oboe libraries:
https://github.com/google/oboe
Apache License 2.0
720 stars 275 forks source link

audio-echo bugfix #83

Closed ggfan closed 7 years ago

ggfan commented 7 years ago

when tearing down echo engine, close play stream first, then close record stream

dturner commented 7 years ago

I think another problem might be that stopStream() calls stream->requestStop which is asynchronous. This means there's no guarantee that the streams have actually stopped before they are restarted. Maybe stopStream() should block until the stream state changes to STOPPED, although this may not be desirable behaviour if called from the UI thread.

Phil - what do you think?

ggfan commented 7 years ago

the behavior is exposed through stopStreams(), the stop then close logic is there already. The ones inside destructor may not be the cause of the problem, just add the similar change there to make is consistent.

philburk commented 7 years ago

Close() will block until the stream is stopped. Then it deletes the stream.

philburk commented 7 years ago

Good. Closing the output stream first will stop the data callback. If you close the input stream first then the output callback could try to read from a closed stream, which has been deleted. So this is the correct order.

Do you want to merge it? Or do you want me to merge it?

dturner commented 7 years ago

Phil, do you mean AAudioStream_requestStop() will block until the stream is stopped? The docs explicitly state that the call is async: https://developer.android.com/ndk/reference/group___audio.html#ga6727be00b9cb4330981c20242ce32299

dturner commented 7 years ago

Oh, I see:

AAudioStream_requestStop() - asynchronous AAudioStream_close() - synchronous

I will comment the code to this effect.