mozilla / DeepSpeech-examples

Examples of how to use or integrate DeepSpeech
816 stars 347 forks source link

DeepSpeechDemo app crashes on stop recording. #78

Open imart opened 4 years ago

imart commented 4 years ago

I downloaded android_mic_streaming example from here: STT-examples. Make the project and start on Android device (Android 10, Samsung A30), put files as mentioned in README. From the app UI click on the START RECORDING, observe recording and text recognition. When I click on STOP RECORDING the app crashes with 100% reproducibility. The exception occurs on the native lib layer:

13747-13898/org.deepspeechdemo A/libc: Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x1c in tid 13898 (AudioRecorder T), pid 13747 (.deepspeechdemo)

lissyx commented 4 years ago

@imart What branch? cc @erksch

imart commented 4 years ago

@lissyx I downloaded source code from r0.8 branch

zaptrem commented 4 years ago

Same issue here:

The app crashes whenever I click the "stop recording" button the demo crashes with this error in Logcat:

2020-08-17 22:29:17.514 24046-24046/org.deepspeechdemo A/libc: Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x7da47030 in tid 24046 (.deepspeechdemo), pid 24046 (.deepspeechdemo)

Any idea what could be causing that?

imart commented 4 years ago

Seems like the recording is stopped incorrectly. The error occurs not during recorder?.stop() call but after some time (asynchronous thread). It can't be caught manually by try-catch.

erksch commented 4 years ago

I'll take a look at it!

mschroederi commented 4 years ago

Moving the finishing of the transcription to the recorderThread solved the issue here. The recorderThread and the main thread seemed to interfere while stopping the recording.

erksch commented 4 years ago

@mschroederi Awesome! @lissyx Could you take a look at the PR? Looks good to me.

lissyx commented 4 years ago

Moving the finishing of the transcription to the recorderThread solved the issue here. The recorderThread and the main thread seemed to interfere while stopping the recording.

Make sense, can you issue the same PR on master branch?

imart commented 4 years ago

Seems PR changes fix the crash but lead to another issue: after recording stop and try to start again nothing works. The recordingThread is used the wrong way. Need a more accurate fix. @lissyx, @mschroederi fyi

lissyx commented 4 years ago

Seems PR changes fix the crash but lead to another issue: after recording stop and try to start again nothing works. The recordingThread is used the wrong way. Need a more accurate fix. @lissyx, @mschroederi fyi

I guess it lacks a .start() soemwhere. As soon as we have a PR I'll merge that, but please make PRs agaisnt both branches

reuben commented 4 years ago

I've made a few changes to the threading logic while working on the TFLite tutorial, and IIRC this problem was one of the things I fixed. I'll make a PR with my changes.

mschroederi commented 4 years ago

@reuben Alright, thanks! I'll then close my PR to the master

imart commented 4 years ago

This fixes the issue with record thread restart:

private fun transcribe() { val audioData = ShortArray(NUM_BUFFER_ELEMENTS)

    while (isRecording.get()) {
        recorder?.read(
            audioData,
            0,
            NUM_BUFFER_ELEMENTS
        )
        model?.feedAudioContent(streamContext, audioData, audioData.size)
        val decoded = model?.intermediateDecode(streamContext)
        runOnUiThread { transcription.text = decoded }
    }

    val decoded = model?.finishStream(streamContext)
    recorder?.stop()
    runOnUiThread {
        btnStartInference.text = "Start Recording"
        status.text = "Ready, waiting ..."
        transcription.text = decoded
        recordingThread?.interrupt()
        recordingThread = null
    }
}
reuben commented 4 years ago

@mschroederi @imart could you test the changes in #81?

imart commented 4 years ago

@reuben #81 doesn't fix all issues, transcriptionThread will not be restarted after stop ones. Please take a look at the previous comment. In your changes, the old thread never stops and it will lead to a memory leak.

reuben commented 4 years ago

@imart Have you actually tested it on a phone and reproduced the bug? I've tested this code and it works for multiple transcriptions, restarting the thread appropriately. When the stop recording button is tapped the loop finishes and the thread stops. A new one is always created when you click start recording.

mschroederi commented 4 years ago

@reuben It works fine for me. The transcription starts over when restarting the recording.

imart commented 4 years ago

@reuben I've tested changes and no issues observed. Just for an example everything works perfectly. As possible enhancements:

mschroederi commented 4 years ago

@reuben I took a look into the profiler and observed that many AudioRecord threads are created over time while the transcriptionThreads are stopped whenever the object is overwritten. Calling recorder.release() after recorder.stop() solved the issue with the sleeping AudioRecord threads for me. (After a few minutes a sleeping AudioRecord thread is shut down by the system. Nonetheless one should consider doing this manually)

reuben commented 4 years ago

@mschroederi thanks. weird that it doesn't release it when the recorder object goes out of scope. I've made that change.

reuben commented 4 years ago

Merged!

zaptrem commented 4 years ago

If I'm trying to build something with this demo which branch/commit/version should I use? @reuben when you said "Merged!" does that mean these issues are resolved in Master r0.8 now?

lissyx commented 4 years ago

If I'm trying to build something with this demo which branch/commit/version should I use? @reuben when you said "Merged!" does that mean these issues are resolved in Master now?

Currently supported branch is r0.8, I don't think @reuben sent a PR against master yet.

zaptrem commented 4 years ago

If I'm trying to build something with this demo which branch/commit/version should I use? @reuben when you said "Merged!" does that mean these issues are resolved in Master now?

Currently supported branch is r0.8, I don't think @reuben sent a PR against master yet.

Whoops, that's what I meant. Are these issues resolved in that branch?

lissyx commented 4 years ago

Whoops, that's what I meant. Are these issues resolved in that branch?

Try r0.8

zaptrem commented 4 years ago

Whoops, that's what I meant. Are these issues resolved in that branch?

Try r0.8

It appears to be resolved, but I don't know enough about Android development to verify the memory/thread leak fixes. I'm also getting an out-of-memory crash on a physical 1gb ram tablet when I tap "start recording" despite that app only using ~256mb of memory on the emulator, though that might not be related to the changes made here.