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 Message not being send with correct mimeType (WPB-3534) #2029

Merged
merged 12 commits into from
Aug 1, 2023

Conversation

alexandreferris
Copy link
Contributor


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

Audio messages sent from Android would play on another Android device or Web, but not on iOS devices.

Causes (Optional)

We have 3 ways to send audio messages:

  • Audio recording feature (issue)
  • Attach File from additional options menu (no issues)
  • Share from outside the app (no issues)

When sending from the recording feature, due to using MediaRecorder the end file mimeType resolves into video/mp4 and not audio/mp4, this doesn't occur when sending an audio message from other flows.

Solutions

Unify sending/handling asset (normal asset and audio asset) with its respective Bundle classes and if it is an Audio asset, then we force its mimeType to audio/mp4 with extension as .m4a so its playable in all platforms.

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

  • Open App
  • Open Conversation
  • Send an Audio Message (record the audio and send from the feature, not sharing/picking the file from device).
  • Audio is sent successfully and fully playable from any iOS device.

@CLAassistant
Copy link

CLAassistant commented Jul 31, 2023

CLA assistant check
All committers have signed the CLA.

@alexandreferris alexandreferris changed the base branch from develop to release/candidate July 31, 2023 13:33
Comment on lines +120 to +122
val mimeType = if (audioPath != null) AUDIO_MIME_TYPE else attachmentUri
.getMimeType(context)
.orDefault(DEFAULT_FILE_MIME_TYPE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part specifically, if we go by getMimeType(..).. it would render as a VIDEO file due to its current mimeType being video/mp4 despite having only audio and no video data due to the way MediaRecorder creates the file.

@alexandreferris alexandreferris changed the title fix: Audio Message not being send with correct mimeType (WPB-3534) fix: Audio Message not being send with correct mimeType [WPB-3534] Jul 31, 2023
@AndroidBob
Copy link
Collaborator

Build 777 failed.

@alexandreferris alexandreferris changed the title fix: Audio Message not being send with correct mimeType [WPB-3534] fix: Audio Message not being send with correct mimeType (WPB-3534) Jul 31, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 31, 2023

Test Results

512 tests   511 ✔️  28s ⏱️
  74 suites      1 💤
  74 files        0

Results for commit 5de073b.

♻️ This comment has been updated with latest results.

@AndroidBob
Copy link
Collaborator

Build 778 failed.

@github-actions
Copy link
Contributor

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 779 succeeded.

The build produced the following APK's:

@alexandreferris alexandreferris requested review from a team, gongracr, yamilmedina, Garzas, trOnk12 and ohassine and removed request for a team July 31, 2023 17:56
Copy link
Contributor

@Garzas Garzas left a comment

Choose a reason for hiding this comment

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

Nicely done 👍 Just one comment about adding one more test

@AndroidBob
Copy link
Collaborator

Build 788 failed.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 789 failed.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@alexandreferris alexandreferris enabled auto-merge (squash) August 1, 2023 12:57
@AndroidBob
Copy link
Collaborator

Build 794 succeeded.

The build produced the following APK's:

@AndroidBob
Copy link
Collaborator

Build 799 succeeded.

The build produced the following APK's:

@alexandreferris alexandreferris merged commit 12fd721 into release/candidate Aug 1, 2023
12 checks passed
@alexandreferris alexandreferris deleted the fix/sending_audio_message_to_ios branch August 1, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants