Record and send voice messages #1596

jonnyandrew · 2023-10-18T16:01:18Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Add ability to record and send voice messages

Note that where commented the UI and text is still placeholder content which will be replaced in follow up PRs.

Motivation and context

Part of [Story] Record and send a voice message element-meta#2084

Screenshots / GIFs

1000009088.mp4

Tests

Enable voice messages in developer settings
Go to a room
Tap and hold the voice message button

Tested devices

Physical
Emulator
OS version(s): Android 13

Checklist

Changes have been tested on an Android device or Android emulator with API 23
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#changelog
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR

github-actions · 2023-10-18T16:27:44Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/qx3eH9

codecov · 2023-10-19T13:01:06Z

Codecov Report

Attention: 143 lines in your changes are missing coverage. Please review.

Comparison is base (503efbf) 59.12% compared to head (4c1d431) 59.21%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1596      +/-   ##
===========================================
+ Coverage    59.12%   59.21%   +0.09%     
===========================================
  Files         1229     1246      +17     
  Lines        31881    32268     +387     
  Branches      6562     6618      +56     
===========================================
+ Hits         18849    19109     +260     
- Misses       10177    10289     +112     
- Partials      2855     2870      +15

Files	Coverage Δ
...ndroid/features/messages/impl/MessagesPresenter.kt	`83.06% <100.00%> (+0.18%)`	⬆️
...nt/android/features/messages/impl/MessagesState.kt	`100.00% <100.00%> (ø)`
...id/features/messages/impl/MessagesStateProvider.kt	`90.90% <100.00%> (+0.21%)`	⬆️
...s/impl/voicemessages/VoiceMessageComposerEvents.kt	`100.00% <100.00%> (ø)`
...es/impl/voicemessages/VoiceMessageComposerState.kt	`100.00% <100.00%> (ø)`
...voicemessages/VoiceMessageComposerStateProvider.kt	`87.50% <100.00%> (+1.78%)`	⬆️
...ssages/impl/voicemessages/VoiceMessageException.kt	`100.00% <100.00%> (ø)`
...droid/libraries/mediaupload/api/MediaUploadInfo.kt	`100.00% <100.00%> (+25.00%)`	⬆️
...ibraries/mediaupload/test/FakeMediaPreProcessor.kt	`100.00% <100.00%> (ø)`
...raries/permissions/api/PermissionsStateProvider.kt	`93.75% <100.00%> (+0.41%)`	⬆️
... and 24 more

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ElementBot · 2023-10-19T19:47:06Z

	Warnings
⚠️	gradle/libs.versions.toml#L24 - A newer version of androidx.compose:compose-bom than 2023.10.00 is available: 2023.10.01
⚠️	gradle/libs.versions.toml#L64 - A newer version of com.google.android.material:material than 1.9.0 is available: 1.10.0
⚠️	gradle/libs.versions.toml#L101 - A newer version of androidx.compose.material3:material3 than 1.2.0-alpha09 is available: 1.2.0-alpha10
⚠️	libraries/permissions/api/src/main/kotlin/io/element/android/libraries/permissions/api/PermissionsStateProvider.kt#L25 - Field requires API level 33 (current min is 23): `android.Manifest.permission#POST_NOTIFICATIONS`
⚠️	libraries/permissions/api/src/main/kotlin/io/element/android/libraries/permissions/api/PermissionsStateProvider.kt#L34 - Field requires API level 33 (current min is 23): `android.Manifest.permission#POST_NOTIFICATIONS`

Generated by 🚫 dangerJS against 4c1d431

julioromano · 2023-10-20T08:53:47Z

Do you think we can split this into logically independent smaller blocks?
It's gonna be tough to review it otherwise.
If we split it different reviewers can review different parts and a smaller PR usually gets also more attention from the reviewer itself.

jonnyandrew · 2023-10-20T08:59:07Z

Yeah it has grown pretty large! No problem, I can split this down.

jonnyandrew · 2023-10-20T11:29:49Z

...impl/src/main/kotlin/io/element/android/libraries/voicerecorder/impl/audio/DefaultEncoder.kt

+        encoder = encoderProvider.get().apply {
+            init(file.absolutePath, sampleRate)
+            setBitrate(bitRate)
+            // TODO check encoder application: 2048 (voice, default is typically 2049 as audio)


I've checked on this and it's not currently possible with libopusencoder-android and needs changes to the underlying libopusencoder library. After we merge this, I'll create an issue in this repo and link to element-hq/libopusencoder-android#4.

julioromano

Very well done! LGTM

Comments are mainly for simplifying some things but not really blocking.

julioromano · 2023-10-23T08:14:04Z

...main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerView.kt

@@ -71,10 +71,14 @@ internal fun MessageComposerView(
        }
    }

-    fun onVoiceRecordButtonEvent(press: PressEvent) {
+    val onVoiceRecordButtonEvent = { press: PressEvent ->


Just wondering: why val instead of fun? What's the difference?

Actually here it shouldn't make a difference, but generally I'm starting to prefer val over fun so that changes in captured variables will trigger recomposition. With fun I've found that subtle bugs can sneak in easily.

https://gist.github.com/jonnyandrew/fd8d2762e2f6fcdb1656ea35c6da5eb9

julioromano · 2023-10-23T11:38:37Z

...kotlin/io/element/android/features/messages/impl/voicemessages/VoiceMessageComposerEvents.kt

@@ -16,10 +16,15 @@

 package io.element.android.features.messages.impl.voicemessages


I would suggest moving all the contents of this package to io.element.android.features.messages.impl.voicemessages.record so it'll match best with io.element.android.features.messages.impl.voicemessages.play from #1503

Can I make this refactor in a follow-up PR as I've built some features on top of this branch?

...rc/main/kotlin/io/element/android/libraries/textcomposer/components/VoiceMessageRecording.kt

julioromano · 2023-10-23T12:15:01Z

...der/api/src/main/kotlin/io/element/android/libraries/voicerecorder/api/VoiceRecorderState.kt

+     *
+     * @property level The current audio level of the recording as a fraction of 1.
+     */
+    data class Recording(val level: Double) : VoiceRecorderState()


Not that big of a deal but perhaps a Float is more than enough here since we don't care about absolute precision as much as we care about performance and responsiveness of the UI.

I couldn't find a source for this, but I think operations involving float and double have similar performance nowadays, since 64 bit processors became the norm? But happy to be corrected!

features/messages/impl/src/main/AndroidManifest.xml

.../impl/src/main/kotlin/io/element/android/libraries/voicerecorder/impl/audio/SampleRateExt.kt

...pl/src/main/kotlin/io/element/android/libraries/voicerecorder/impl/di/VoiceRecorderModule.kt

.../main/kotlin/io/element/android/libraries/voicerecorder/impl/file/DefaultVoiceFileManager.kt

… jonny/voice-message-sending

sonarcloud · 2023-10-23T16:49:24Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
2 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

julioromano · 2023-10-24T08:08:18Z

Super! Thanks for all the changes!

jonnyandrew force-pushed the jonny/voice-message-sending branch from c42b093 to c4328be Compare October 19, 2023 11:22

jonnyandrew force-pushed the jonny/voice-message-sending branch 2 times, most recently from 885e100 to be88988 Compare October 19, 2023 19:45

jonnyandrew force-pushed the jonny/voice-message-sending branch from be88988 to f28e9ee Compare October 20, 2023 08:45

jonnyandrew marked this pull request as ready for review October 20, 2023 08:45

jonnyandrew requested a review from a team as a code owner October 20, 2023 08:45

jonnyandrew requested review from julioromano and removed request for a team October 20, 2023 08:45

jonnyandrew marked this pull request as draft October 20, 2023 08:59

jonnyandrew and others added 5 commits October 20, 2023 11:24

Add changelog

926152a

Add voice recorder library

b6bf11b

Add voice message support to media sender

6211a1f

Add voice message sending UI

72ceeb3

Update screenshots

e3f0e64

jonnyandrew force-pushed the jonny/voice-message-sending branch from f28e9ee to e3f0e64 Compare October 20, 2023 10:25

jonnyandrew marked this pull request as ready for review October 20, 2023 10:26

jonnyandrew commented Oct 20, 2023

View reviewed changes

julioromano approved these changes Oct 23, 2023

View reviewed changes

jonnyandrew added 6 commits October 23, 2023 15:47

Enable placeholder recording level in all builds

a36ea65

Remove permission declarations from voice recorder library

d5e01ac

Remove unnecessary parameter to startRecord

b7ae49f

Change format of kbps value

8fc1c2d

Inject cacheDir directly

6282c2c

Reduce unused sample rate confurability

08c080f

jonnyandrew requested a review from julioromano October 23, 2023 16:08

Merge branch 'develop' of github.com:vector-im/element-x-android into…

4c1d431

… jonny/voice-message-sending

jonnyandrew force-pushed the jonny/voice-message-sending branch from ba7cf44 to 4c1d431 Compare October 23, 2023 16:34

jonnyandrew merged commit b476654 into develop Oct 23, 2023
14 of 15 checks passed

jonnyandrew deleted the jonny/voice-message-sending branch October 23, 2023 17:28

jonnyandrew mentioned this pull request Oct 24, 2023

Update package of voice message composer #1633

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Record and send voice messages #1596

Record and send voice messages #1596

jonnyandrew commented Oct 18, 2023 •

edited

github-actions bot commented Oct 18, 2023 •

edited

codecov bot commented Oct 19, 2023 •

edited

ElementBot commented Oct 19, 2023 •

edited

julioromano commented Oct 20, 2023

jonnyandrew commented Oct 20, 2023

jonnyandrew Oct 20, 2023

julioromano left a comment

julioromano Oct 23, 2023

jonnyandrew Oct 23, 2023

jonnyandrew Oct 24, 2023

julioromano Oct 23, 2023

jonnyandrew Oct 23, 2023

julioromano Oct 23, 2023

julioromano Oct 23, 2023

jonnyandrew Oct 23, 2023

sonarcloud bot commented Oct 23, 2023

julioromano commented Oct 24, 2023

		@@ -16,10 +16,15 @@

		package io.element.android.features.messages.impl.voicemessages

Record and send voice messages #1596

Record and send voice messages #1596

Conversation

jonnyandrew commented Oct 18, 2023 • edited

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented Oct 18, 2023 • edited

codecov bot commented Oct 19, 2023 • edited

Codecov Report

ElementBot commented Oct 19, 2023 • edited

julioromano commented Oct 20, 2023

jonnyandrew commented Oct 20, 2023

Choose a reason for hiding this comment

julioromano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 23, 2023

julioromano commented Oct 24, 2023

jonnyandrew commented Oct 18, 2023 •

edited

github-actions bot commented Oct 18, 2023 •

edited

codecov bot commented Oct 19, 2023 •

edited

ElementBot commented Oct 19, 2023 •

edited