ankidroid / Anki-Android

AnkiDroid: Anki flashcards on Android. Your secret trick to achieve superhuman information retention.
GNU General Public License v3.0
8.41k stars 2.19k forks source link

[Bug] Recording audio crashes background apps #8637

Open MarkusJLechner opened 3 years ago

MarkusJLechner commented 3 years ago
Reproduction Steps
  1. Constantly press record Audio shortcut (shift + v) and play (v)
  2. After some intervals its possible to have both states active
  3. It is not possible to active another state anymore
Expected Result

Switching between recording and play audio should trigger the desired states without side effects

Actual Result

Backgound services crash. (I also had to restart the phone). Both states can be triggered the same time. After happened it is not possible to change states anymore.

Screenshot_20210417-105758 Screenshot_20210417-105843

I am using a one plus 8 pro with android 11. Normally i never experienced background crashes, but could trigger this behavior 2 times when reproducing this bug

I'm using the 2.15 alpha 43

Debug info

Refer to the support page if you are unsure where to get the "debug info".

Research

Enter an [x] character to confirm the points below:

AnkiDroid Version = 2.15alpha43

Android Version = 11

Manufacturer = OnePlus

Model = IN2023

Hardware = qcom

Webview User Agent = Mozilla/5.0 (Linux; Android 11; IN2023 Build/RP1A.201005.001; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/90.0.4430.66 Mobile Safari/537.36

ACRA UUID = ed0ca8ca-f925-4399-a89a-409ede3efc4c

Scheduler = std2

Crash Reports Enabled = true

DatabaseV2 Enabled = true

SilentCruzer commented 3 years ago

I would like to work on this issue if it is accepted.

SilentCruzer commented 3 years ago

I am new to this codebase, so can I know where to start on this issue.

david-allison commented 3 years ago

https://github.com/ankidroid/Anki-Android/blob/95b310de511f67cd0af0504a64696ce59590fe73/AnkiDroid/src/main/java/com/ichi2/anki/multimediacard/AudioView.java

And Reviewer.java for the usage of the class

SilentCruzer commented 3 years ago

I tried to reproduce the mentioned error and noticed that the uploaded screenshot is when we are trying to view the card and not in the edit view of the card. When we are viewing a card with audio, only the play button should be present and there should not be a record or a stop button, like this:

Before Shortcut

The record option should only be present when we are creating or editing a card, but the screenshot uploaded in the issue is when the card is viewed. I tried to reproduce the error when we are recording the audio and the shortcuts shift+v and v didn't make any changes. After we record the audio and create the card, if I use the shortcut keys when viewing the card, the record button pops up:

After shortcut

There will be two instances of MediaPlayer after pressing the shortcuts. The recording button function as it normally does. After recording if we click the new play button, the recently recorded audio will play. But in this new popped up media player, the error mentioned above happens. When we use the shortcuts, it is possible to make both states active.

The app didn't crash for me when both states were active but it crashed when the record button showed up when I used the shortcut key on the card view screen. The error only occurs when we use keyboard shortcuts.

Is the record button showing up in the card view a bug? just want to make sure.

MarkusJLechner commented 3 years ago

Should be a feature not a bug, as far as i know :). While reviewing you can open the menu (3 dots) and show the recording buttons, just like if pressing a recording shortcut (v or shift + v). It is meant to improve pronunciation and ghosting.

I just wanted to be clear and did a stresstest without the keyboard to active this locked state (constantly pressing record and play). I could not archive both states to happen the same time, however it was possible that the record button freezes and after a period of time and no input recognition the app crashed

Screenshot_20210423-062934

However this should not happen on a daily use

SilentCruzer commented 3 years ago

I have tried to fix this issue, but I am having trouble with something. The following functions in AudioView is the reason behind the issue: https://github.com/ankidroid/Anki-Android/blob/95b310de511f67cd0af0504a64696ce59590fe73/AnkiDroid/src/main/java/com/ichi2/anki/multimediacard/AudioView.java#L230-L244

After pressing the shortcuts multiple times, the mStatus variable gets a value other than Status.RECORDING even when it's recording. This makes the condition fail and executes line 235 and makes both buttons active and the app crashes. I tried adding many else conditions and the main issue is that the mStatus gets the wrong value. Does anyone know why it gets the wrong value only after spamming the shortcuts many times and why does not it happen initially?

mikehardy commented 3 years ago

It must be race conditions with the audio view listeners getting events from another thread:

https://github.com/ankidroid/Anki-Android/blob/95b310de511f67cd0af0504a64696ce59590fe73/AnkiDroid/src/main/java/com/ichi2/anki/multimediacard/AudioView.java#L107-L110

My guess is that access to mStatus should be synchronized somehow ? Maybe adding a "double click prevention" click delay on the buttons would be useful too - that exists for the reviewer buttons and the pattern could be copied

david-allison commented 3 years ago

My guess is that access to mStatus should be synchronized somehow ? Maybe adding a "double click prevention" click delay on the buttons would be useful too - that exists for the reviewer buttons and the pattern could be copied

An alternate would be to queue up the actions and only perform them once the player has completed the previous action.

I don't think we've recorded the stack trace or logs here to find out why this is occurring. That'd be the first step for me. Is this due to trying to record/play twice, or an interaction between the player and the recorder?

SilentCruzer commented 3 years ago

The error is due to the interaction between player and recorder. The issue happens when we press V when it is recording. At first, it gives the expected result - the recording stops and the audio starts playing, but after using it multiple times, it starts playing without stopping the recording, after this, the recording button is stuck in the recording state, even if the mStatus variable has other values. Used the logs to find out that the mstatus variable gets the value "STOPPED" even when it's recording and it happens after using the shortcut keys multiple times. As you suggested I added a delay for the buttons, but the issue still remains.

david-allison commented 3 years ago

Could you post logs of the "before", it's much easier to understand what's going on from that than from a text-based description

SilentCruzer commented 3 years ago

This is the log when it behaves normally: The following screenshot is when you press shift +v and then v. (I have added a log before the if statement in the stopRecording function to see if the program goes inside the if statement)

Before Struck

After pressing it multiple times:

After struck

In the second screenshot, we can see that it's not entering the if block, and just starts playing even when the recording is still active. This is where the app crashes. mStatus gets the value "RECORDING" in the second line and I don't know how mStatus gets the value "STOPPED" after exiting toggleRecord function.

SilentCruzer commented 3 years ago

So does anyone have any other alternatives to fix this issue?. So far I have tried adding a delay to the buttons, and it didn't work.

mikehardy commented 3 years ago

So far no - but I'm going to review #8781 shortly and there may be some wisdom from that one if you want to have a look - it's adding play/pause to reviewer and it may have similar issues that it had to work around (or! it may not be handling the same kind of crash here and not be aware of it...)

github-actions[bot] commented 3 years ago

Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

dae commented 1 year ago

A few years have passed - is this still an issue?

mikehardy commented 1 year ago

Unknown, needs a re-triage to see if it reproduces. It appears from the comments above reproduction steps are detailed, so anyone can try to reproduce again, we may close if it no longer reproduces