-
Notifications
You must be signed in to change notification settings - Fork 858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(Will PR) MediaPlayer on Android is *not* thread safe, but it is used in multiple threads, causing subtle bugs #1524
Comments
It depends from where it's called. Usually it helps if wrapping the call in the main loop handler. But we should think about running the asynchronous stuff also on the main thread. |
UPDATE The cause of multi-threading is not what I originally thought (I wrongly thought Flutter calls the kotlin code on non-ui threads). Instead, it is this: class AudioplayersPlugin : FlutterPlugin, IUpdateCallback {
...
methods = MethodChannel(binding.binaryMessenger, "xyz.luan/audioplayers")
methods.setMethodCallHandler { call, response -> safeCall(call, response, ::methodHandler) }
...
private fun safeCall(
call: MethodCall,
response: MethodChannel.Result,
handler: FlutterHandler,
) {
mainScope.launch(Dispatchers.IO) {
try {
handler(call, response)
} catch (e: Exception) {
response.error("Unexpected AndroidAudioError", e.message, e)
}
}
} The Dispatchers.IO says:
Thus, it is this |
ANOTHER UPDATE ExoPlayer (related: #1526) says that, it should (usually) be called directly on the ui thread. Therefore, if we refactor into ExoPlayer, we can safely remove the safeCall without the worry that running things on ui thread may block anything. |
Closing in favor of #1525 |
I can make a PR for this.
https://developer.android.com/reference/android/media/MediaPlayer says:
However, I have made some logging and realize the call to MediaPlayer in this library is from multiple threads.
For example, I have seen:
mediaPlayer.reset (thread=6854, this=e3ed388, )
mediaPlayer.stop (thread=2, this=e3ed388, )
If you are interested, my logging code looks like:
The text was updated successfully, but these errors were encountered: