Skip to content

fix: audio recording and playing crashes [WPB-16687] [WPB-16835] #3955

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

Merged

Conversation

saleniuk
Copy link
Contributor

@saleniuk saleniuk commented Apr 2, 2025

BugWPB-16687 [Android] App crashes when second audio is recorded after playing the first before sending


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

This PR contains fixes for two issues related to recording and playing audio messages:
https://wearezeta.atlassian.net/browse/WPB-16687
https://wearezeta.atlassian.net/browse/WPB-16835

Issues

The app crashes when attempting to send an audio recording for the second time because the user played the 1st recording before sending it.
The app crashes when attempting to play an audio message after sending it.

Causes (Optional)

  • MediaPlayer being released after closing or sending a message so it cannot be reused again
  • audio focus not being abandoned correctly and receiving audiofocus loss when trying to record again
  • trying to pause audio when there is no audio being played and MediaPlayer is not initialised
  • asset file path is being changed from temporary to proper one after uploading and player keeps the temporary path

Solutions

  • do not release MediaPlayer after sending or closing, it can be reused then (the same instance is persisted as long as the view model exists)
  • abandon focus when closing the recording
  • fix abandonExclusive function to abandon exclusiveFocusRequest and not regular focusRequest
  • check if there is any current audio set before trying to pause or resume
  • implement missing "uploading" audio message state so that view model is not injected while the path can change
  • execute getting asset again when the resulting file does not exist anymore

Testing

How to Test

Steps to Reproduce:

1
-record an audio message
-play the recording before sending it
-send the audio message
-attempt to make another audio recording

2
-record an audio message
-send it
-attempt to play it from the audio message cell

Attachments (Optional)

record-play-send-and-record-again.mp4
play_after_uploading.mp4
image

PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

Copy link

sonarqubecloud bot commented Apr 2, 2025

Copy link

codecov bot commented Apr 2, 2025

Codecov Report

Attention: Patch coverage is 32.25806% with 21 lines in your changes missing coverage. Please review.

Project coverage is 45.91%. Comparing base (add996f) to head (972100d).

Files with missing lines Patch % Lines
...oid/media/audiomessage/RecordAudioMessagePlayer.kt 0.00% 11 Missing ⚠️
...tions/model/messagetypes/audio/AudioMessageType.kt 0.00% 8 Missing ⚠️
...ire/android/media/audiomessage/AudioFocusHelper.kt 0.00% 1 Missing ⚠️
...dia/audiomessage/ConversationAudioMessagePlayer.kt 90.90% 0 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (32.25%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@                  Coverage Diff                  @@
##           release/candidate    #3955      +/-   ##
=====================================================
- Coverage              45.93%   45.91%   -0.03%     
=====================================================
  Files                    492      492              
  Lines                  17163    17175      +12     
  Branches                2859     2862       +3     
=====================================================
+ Hits                    7884     7886       +2     
- Misses                  8482     8492      +10     
  Partials                 797      797              
Files with missing lines Coverage Δ
...essagecomposer/recordaudio/RecordAudioViewModel.kt 65.28% <ø> (+0.15%) ⬆️
...ire/android/media/audiomessage/AudioFocusHelper.kt 0.00% <0.00%> (ø)
...dia/audiomessage/ConversationAudioMessagePlayer.kt 66.04% <90.90%> (+0.38%) ⬆️
...tions/model/messagetypes/audio/AudioMessageType.kt 0.00% <0.00%> (ø)
...oid/media/audiomessage/RecordAudioMessagePlayer.kt 1.13% <0.00%> (-0.05%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update add996f...972100d. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

github-actions bot commented Apr 2, 2025

Built wire-android-staging-compat-pr-3955.apk is available for download

@saleniuk saleniuk added this pull request to the merge queue Apr 2, 2025
@MohamadJaara MohamadJaara removed this pull request from the merge queue due to a manual request Apr 2, 2025
@MohamadJaara MohamadJaara merged commit 58ae553 into release/candidate Apr 2, 2025
22 of 23 checks passed
@MohamadJaara MohamadJaara deleted the fix/audio-recording-and-playing-crashes branch April 2, 2025 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants