Skip to content
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

fix: audio converting time [WPB-9705] #3127

Merged
merged 5 commits into from
Jun 25, 2024
Merged

Conversation

Garzas
Copy link
Contributor

@Garzas Garzas commented Jun 25, 2024

BugWPB-9705 [Android] App freezes and delays when sending voice messages


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?

Issues

Converting audio file to m4a takes too long

Causes (Optional)

Bad user experience waiting for audio to be encoded

Solutions

  • change audio encoding to wav
  • introduce encoding state to be showed when user recorded long audio file

How to Test

  1. Record long audio file (about 3mins)
  2. After stop it should show Audio encoding message for few seconds
  3. Send audio message

Attachments (Optional)

Attachments like images, videos, etc. (drag and drop in the text box)

@Garzas Garzas self-assigned this Jun 25, 2024
Copy link
Contributor

@Garzas looks like you are rolling back kalium to a previous commitish.

This means that the PR's target branch (develop) is using a newer version of Kalium, and the changes in this PR will rollback Kalium to an older version.

develop This PR
f34993ab6b3ce61be7262a03bc512c6826a9df32 f619c5e87e1ecd94943c628dd04ad79731724e81

Is this intentional?

Copy link
Contributor

@Garzas looks like you are rolling back kalium to a previous commitish.

This means that the PR's target branch (develop) is using a newer version of Kalium, and the changes in this PR will rollback Kalium to an older version.

develop This PR
f34993ab6b3ce61be7262a03bc512c6826a9df32 f619c5e87e1ecd94943c628dd04ad79731724e81

Is this intentional?

Copy link

sonarcloud bot commented Jun 25, 2024

Copy link
Contributor

Built wire-android-staging-release-pr-3127.apk is available for download

Copy link
Contributor

Built wire-android-dev-debug-pr-3127.apk is available for download

Copy link
Contributor

@saleniuk saleniuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wav will be bigger in size than m4a, maybe it's not as big as to become a problem, but maybe we could think of compressing it not when recording or right after, but when sending - it won't be as problematic to the user as now. Also, maybe applying effect takes longer for wav than for m4a because of bigger file size, but I have no idea, just would be good to verify it. 😄
Anyway, well done!

Copy link
Contributor

@alexandreferris alexandreferris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work done here 👏🏻

Generation of the file with effects is now super fast as compared to before 🏃🏻‍♂️

Tested on Android + iOS + Mac wrapper and everything is as it should be, awesome!

@alexandreferris
Copy link
Contributor

Wav will be bigger in size than m4a, maybe it's not as big as to become a problem, but maybe we could think of compressing it not when recording or right after, but when sending - it won't be as problematic to the user as now. Also, maybe applying effect takes longer for wav than for m4a because of bigger file size, but I have no idea, just would be good to verify it. 😄 Anyway, well done!

The previous issue with m4a file was due to having to decode from m4a to wav and then from wav to m4a back again thus leaving us with a bit of delay when generating the file with effects, my personal opinion is that wav will still be faster than what/how we were doing before, but it would be good if we could run performance checks on both types 👌🏻

@Garzas Garzas added this pull request to the merge queue Jun 25, 2024
Merged via the queue into develop with commit 6b1abd6 Jun 25, 2024
10 checks passed
@Garzas Garzas deleted the fix/audio-converting-time branch June 25, 2024 12:23
@echoes-hq echoes-hq bot added the echoes: product-roadmap Work aligned with the customer-announced roadmap, targeting a specific release date. label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: product-roadmap Work aligned with the customer-announced roadmap, targeting a specific release date. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants