Closed BinhT1 closed 1 year ago
BinhT1
commented
1 year ago 
chipweinberger
commented
1 year ago this looks mostly good!
the Thread.join(1) change does not seem right. That will cause the join to timeout and do nothing.
you are right that there are bugs tho.
I think we need to add another check for playbackRunning, because there are 2 locations we could be waiting.
private void startPlaybackThread() {
playbackThread = new Thread(() -> {
android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_AUDIO);
while (playbackRunning) {
// suspend / resume
synchronized (playbackLock) {
while (playbackSuspended) {
try {
playbackLock.wait(); // we could be waiting here!
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
}
if (playbackRunning) { // i think we need to add this check otherwise we will sleep again
try {
if (mIsPlaying) {
while (mSamplesIsEmpty() == false) {
ByteBuffer data = mSamplesPop().duplicate();
if (data != null && mAudioTrack != null) {
mAudioTrack.write(data, data.remaining(), AudioTrack.WRITE_BLOCKING);
}
// If nearing buffer underflow, feed
if (mSamplesRemainingFrames() <= mFeedThreshold && mDidInvokeFeedCallback == false) {
mDidInvokeFeedCallback = true;
mainThreadHandler.post(() -> invokeFeedCallback());
}
}
}
// avoid excessive CPU usage
Thread.sleep(5); // we could be waiting here!
} catch (InterruptedException e) {
}
}
}
});
playbackThread.setPriority(Thread.MAX_PRIORITY);
playbackThread.start();
}Agree with you about Thread.join(1) is not the correct answer. But i see the problem is in stopPlaybackThread function. if we don't set: playbackSuspended = false, then we use Thread.interrupt(); The app will be crashed when we use release() function. Because playbackSuspended = true will make the code below work
synchronized (playbackLock) {
while (playbackSuspended) {
try {
playbackLock.wait(); // we could be waiting here!
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
}playbackLock will call wait() then the thread will still be working when i use release() and back or navigate to other screen. This make the app crash.
The code below you can see example
private void stopPlaybackThread() {
if (playbackThread != null) {
playbackRunning = false;
playbackThread.interrupt();
try {
playbackThread.join();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
playbackThread = null;
}
}
// CRASH when use release() private void stopPlaybackThread() {
if (playbackThread != null) {
playbackSuspended = false;
playbackRunning = false;
playbackThread.interrupt();
try {
playbackThread.join();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
playbackThread = null;
}
}
// NOT CRASH when use release()i think we don't need if(playbackRunning){} inside the loop while(playbackRunning){}
chipweinberger
commented
1 year ago what is the crash?
BinhT1
commented
1 year ago The problem is when navigator. My test app has 2 screens: Home screen and PCM screen. My workflow is: HomeScreen -> PcmScreen -> play -> stop -> release -> back to HomeScreen -> PcmScreen -> play -> stop -> release -> again.
I test my app with a check log:
synchronized (playbackLock) {
while (playbackSuspended) {
try {
Log.d(TAG, "CHECK PlaybackSuspend");
playbackLock.wait(); // we could be waiting here!
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
}and result is when i am in HomeScreen, the checklog is still printed.
And if i continue my workflow, the app will be crashed with thread error:
F/example.exampl( 5594): runtime.cc:655] Runtime aborting...
F/example.exampl( 5594): runtime.cc:655] Dumping all threads without mutator lock held
F/example.exampl( 5594): runtime.cc:655] All threads:
F/example.exampl( 5594): runtime.cc:655] DALVIK THREADS (24):The error is about hundreds of lines, so i show you some first lines.
chipweinberger
commented
1 year ago thanks. what is stacktrace of the crashing thread?
i think we don't need if(playbackRunning){} inside the loop while(playbackRunning){}
why not? the state of playbackRunning can change at anytime. This could happen:
playbackLock.wait();stopPlaybackThread()playbackLock.wait()that would be bad ^
We want to avoid step 3 & 4 from happening after we call stopPlaybackThread()
BinhT1
commented
1 year ago here are those backtrace:
#00 pc 00000b99 [vdso] (__kernel_vsyscall+9)
#01 pc 0005ad68 /apex/com.android.runtime/lib/bionic/libc.so (syscall+40) (BuildId: 6e3a0180fa6637b68c0d181c343e6806)
#02 pc 00076511 /apex/com.android.runtime/lib/bionic/libc.so (abort+209) (BuildId: 6e3a0180fa6637b68c0d181c343e6806)
#03 pc 00639a4d /apex/com.android.art/lib/libart.so (art::Runtime::Abort(char const*)+2477) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#04 pc 00025a23 /apex/com.android.art/lib/libartbase.so (std::__1::__function::__func<void (*)(char const*), std::__1::allocator<void (*)(char const*)>, void (char const*)>::operator()(char const*&&)+35) (BuildId: 41e9e0cbb5db4bb6875333d66af6569f)
#05 pc 0001588f /system/lib/libbase.so (android::base::SetAborter(std::__1::function<void (char const*)>&&)::$_3::__invoke(char const*)+79) (BuildId: 3abc3ce4c3b633a64b14c50cb931a64b)
#06 pc 00006dbd /system/lib/liblog.so (__android_log_assert+285) (BuildId: bbac430fc6349b937996bb914e70c060)
#07 pc 0007095f /system/lib/libaudioclient.so (android::ClientProxy::releaseBuffer(android::Proxy::Buffer*)+223) (BuildId: 3c6e8d5a4558d5ae7029f6d00fe52408)
#08 pc 000680d4 /system/lib/libaudioclient.so (android::AudioTrack::releaseBuffer(android::AudioTrack::Buffer const*)+180) (BuildId: 3c6e8d5a4558d5ae7029f6d00fe52408)
#09 pc 00068369 /system/lib/libaudioclient.so (android::AudioTrack::write(void const*, unsigned int, bool)+457) (BuildId: 3c6e8d5a4558d5ae7029f6d00fe52408)
#10 pc 0013ab1a /system/lib/libandroid_runtime.so (int writeToTrack<signed char>(android::sp<android::AudioTrack> const&, int, signed char const*, int, int, bool)+170) (BuildId: 588f2cd5873ff4273bb25b25edb82606)
#11 pc 00136d7b /system/lib/libandroid_runtime.so (int android_media_AudioTrack_writeArray<_jbyteArray*>(_JNIEnv*, _jobject*, _jbyteArray*, int, int, int, unsigned char)+203) (BuildId: 588f2cd5873ff4273bb25b25edb82606)
#12 pc 0022f85c /system/framework/x86/boot-framework.oat (art_jni_trampoline+172) (BuildId: 9a9778e61b43d349325d0bb85244bd9bc95ff387)
#13 pc 020081a4 /memfd:jit-cache (deleted) (offset 0x2000000) (android.media.AudioTrack.write+372)
#14 pc 02009838 /memfd:jit-cache (deleted) (offset 0x2000000) (com.lib.flutter_pcm_sound.FlutterPcmSoundPlugin.lambda$startPlaybackThread$1$com-lib-flutter_pcm_sound-FlutterPcmSoundPlugin+440)
#15 pc 0013b922 /apex/com.android.art/lib/libart.so (art_quick_invoke_stub+338) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#16 pc 001d0381 /apex/com.android.art/lib/libart.so (art::ArtMethod::Invoke(art::Thread*, unsigned int*, unsigned int, art::JValue*, char const*)+241) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#17 pc 00386701 /apex/com.android.art/lib/libart.so (art::interpreter::ArtInterpreterToCompiledCodeBridge(art::Thread*, art::ArtMethod*, art::ShadowFrame*, unsigned short, art::JValue*)+385) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#18 pc 0037aa3e /apex/com.android.art/lib/libart.so (bool art::interpreter::DoCall<false, false>(art::ArtMethod*, art::Thread*, art::ShadowFrame&, art::Instruction const*, unsigned short, art::JValue*)+1070) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#19 pc 007a11b7 /apex/com.android.art/lib/libart.so (MterpInvokeVirtual+967) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#20 pc 001357a1 /apex/com.android.art/lib/libart.so (mterp_op_invoke_virtual+33) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#21 pc 0011e968 [anon:dalvik-classes.dex extracted in memory from /data/app/~~B3wm7qt1wf5NfpWorRvm9w==/com.example.example-xvBYEFs-XCDXbLVVQlZW3Q==/base.apk] (com.lib.flutter_pcm_sound.FlutterPcmSoundPlugin$$ExternalSyntheticLambda1.run+4)
#22 pc 007a355e /apex/com.android.art/lib/libart.so (MterpInvokeInterface+2126) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#23 pc 001359a1 /apex/com.android.art/lib/libart.so (mterp_op_invoke_interface+33) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#24 pc 000eb7d0 /apex/com.android.art/javalib/core-oj.jar (java.lang.Thread.run+8)
#25 pc 0036fb02 /apex/com.android.art/lib/libart.so (art::interpreter::Execute(art::Thread*, art::CodeItemDataAccessor const&, art::ShadowFrame&, art::JValue, bool, bool) (.llvm.16375758241455872412)+370) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#26 pc 00379b00 /apex/com.android.art/lib/libart.so (art::interpreter::EnterInterpreterFromEntryPoint(art::Thread*, art::CodeItemDataAccessor const&, art::ShadowFrame*)+176) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#27 pc 0078b325 /apex/com.android.art/lib/libart.so (artQuickToInterpreterBridge+1061) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#28 pc 0014220d /apex/com.android.art/lib/libart.so (art_quick_to_interpreter_bridge+77) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#29 pc 0013b922 /apex/com.android.art/lib/libart.so (art_quick_invoke_stub+338) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#30 pc 001d0381 /apex/com.android.art/lib/libart.so (art::ArtMethod::Invoke(art::Thread*, unsigned int*, unsigned int, art::JValue*, char const*)+241) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#31 pc 0062f37c /apex/com.android.art/lib/libart.so (art::JValue art::InvokeVirtualOrInterfaceWithJValues<art::ArtMethod*>(art::ScopedObjectAccessAlreadyRunnable const&, _jobject*, art::ArtMethod*, jvalue const*)+620) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#32 pc 0062f595 /apex/com.android.art/lib/libart.so (art::JValue art::InvokeVirtualOrInterfaceWithJValues<_jmethodID*>(art::ScopedObjectAccessAlreadyRunnable const&, _jobject*, _jmethodID*, jvalue const*)+85) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#33 pc 00697701 /apex/com.android.art/lib/libart.so (art::Thread::CreateCallback(void*)+1537) (BuildId: 8191579dfafff37a5cbca70f9a73020f)
#34 pc 000e6974 /apex/com.android.runtime/lib/bionic/libc.so (__pthread_start(void*)+100) (BuildId: 6e3a0180fa6637b68c0d181c343e6806)
#35 pc 00078567 /apex/com.android.runtime/lib/bionic/libc.so (__start_thread+71) (BuildId: 6e3a0180fa6637b68c0d181c343e6806)We want to avoid step 3 & 4 from happening after we call
stopPlaybackThread()
playback thread leaves playbackLock.wait(). this is not concerning in the playbackRunning state.
As above comment, when i play() pcm, stop(), then release(), then back to HomseScreen, the checklog still work although playbackRunning is false when i release(), playbackLock.wait() is called many times when i'am in HomeScreen. I think when i am in HomeScreen, playbackThread in PcmScreen is NOT going to work. So we must set playbackSuspended = false in stopPlaybackThread() to make palybackThread is not going to work in other screens.
chipweinberger
commented
1 year ago So we must set playbackSuspended = false
correct. We must do that, and we must add another check for playbackRunning. we must do both.
BinhT1
commented
1 year ago I have updated my branch after our discussion. Please merge the branch to fix that issue, close issue and PR. I am so happy with the first time i have made a Pull Request to contribute. 😊😊😊
chipweinberger
commented
1 year ago the new code solves the crash?
BinhT1
commented
1 year ago correct. We must do that, and we must add another check for
playbackRunning. we must do both.
Yes, i do both and when i do my workflow to test, my app is not crashed. You can check my code, or create an example with HomeScreen and PcmScreen, do my workflow again to test. The workflow should be tested above 10 times.
chipweinberger
commented
1 year ago merged!
Btw, usually you should give your commits a better message, and have a single commit! but for a first PR it's no big deal! I don't care too much =)
congrats on your first PR!
I will push an update to this library soon.
chipweinberger
commented
1 year ago fixed in 1.1.0