Voice Message Playback Scrolling Support #5404

onurays · 2022-03-02T14:55:42Z

vm_scrollbar.mp4

Voice Message related UI and state classes became to be messy. We should plan to refactor them by rethinking all states and flow between them.

github-actions · 2022-03-02T15:21:26Z

Unit Test Results

106 files ±0 106 suites ±0 1m 9s ⏱️ +6s
188 tests ±0 188 ✔️ ±0 0 💤 ±0 0 ❌ ±0
622 runs ±0 622 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 7ead3f9. ± Comparison against base commit 20b2af4.

♻️ This comment has been updated with latest results.

…ackground.

… LTR.

bmarty

Some minor remark.
Static review OK!
It should deserve some manual test though.

bmarty · 2022-03-04T15:26:06Z

...or/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageViews.kt

@@ -69,6 +83,8 @@ class VoiceMessageViews(
        observeMicButton(actions)
    }

+    private fun getTouchedPositionPercentage(motionEvent: MotionEvent, view: View) = motionEvent.x / view.width


Not tested, but show probably use coerceIn(0,1) to avoid overflow.

More secure, done.

bmarty · 2022-03-04T15:27:53Z

...src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt

@@ -695,8 +709,7 @@ class MessageItemFactory @Inject constructor(
        return this
                ?.filterNotNull()
                ?.map {
-                    // Value comes from AudioRecordView.maxReportableAmp, and 1024 is the max value in the Matrix spec


This comment is still valuable, no? Maybe just update the part about AudioRecordView.maxReportableAmp to AudioWaveformView.MAX_FFT

Right, done.

bmarty · 2022-03-04T15:31:12Z

vector/src/main/res/values/audio_waveform_attr.xml

@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="utf-8"?>


This file should be moved to https://github.com/vector-im/element-android/tree/main/library/ui-styles/src/main/res/values and its name should start with stylable_

ouchadam · 2022-03-16T11:24:56Z

when scrubbing the play head, should the audio continue to play? at the moment we're pausing each time we change the play head position 🤔

ouchadam · 2022-03-16T11:32:05Z

vector/src/main/java/im/vector/app/features/home/room/detail/composer/VoiceMessageHelper.kt

        mediaPlayer?.stop()
        stopPlaybackTicker()
    }

+    fun movePlaybackTo(id: String, percentage: Float, totalDuration: Int) {


if we use the mediaPlayer?.duration then we'll be able to avoid inferring the duration from the UI countdown ticker
would that be possible? (it would assume the media player is in the prepared state)

We have a single mediaplayer which is initialized when users start the playback. But users are able to scroll idle recordings and the mediaplayer is null at this stage.

ouchadam · 2022-03-16T11:41:09Z

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt

-                    is VoiceMessagePlaybackTracker.Listener.State.Idle    -> renderIdleState(holder)
-                    is VoiceMessagePlaybackTracker.Listener.State.Playing -> renderPlayingState(holder, state)
-                    is VoiceMessagePlaybackTracker.Listener.State.Paused  -> renderPausedState(holder, state)
+        // Don't track and don't try to update UI before view is present


is this because we're relying on holder.voicePlaybackWaveform.post? if so we should move the tracker logic to the same post body to better couple the logic together ~~(it seems that we're relying on the audio view to be fully drawn because it only updates its internal state as a side effect of the drawing)~~

actually... do we need any of the posting? 🤔

Yes we need post because the width of the view is 0 (not calculated yet) when it is bind. Do you mean holder.voicePlaybackWaveform.setOnTouchListener should be inside post block?

it might be safer to rely on viewWithLateWidth.doOnPreDraw { } or doOnLayout { } if we need the width/height at some point~

for the moving I meant moving the tracker calls to be inside the same holder.voicePlaybackWaveform.post as the comment suggests the view doesn't exist yet but they should be inflated at the same time as the parent 🤔 so I'm assuming the lack of data inside the voicePlaybackWaveform is causing trouble, which would be fixed by moving the tracker into the same post block 🤞 If I've understood correctly!

Thanks for clarification! Done, if I understood you correctly.

onurays · 2022-03-17T12:13:03Z

when scrubbing the play head, should the audio continue to play? at the moment we're pausing each time we change the play head position 🤔

Ideally yes, I couldn't find an easy way to implement it and I am not sure this is a blocker.

ouchadam

thanks for the update! 💯

DoM1niC · 2022-03-22T23:21:52Z

@onurays the Player visual events freeze for me after skip play and stop a Voice Message and play a another one above the timeline.

* develop: (429 commits) fixing the onboarding sanity test failing - adds tapping the new take me home button within the sanity test Fix lint issues on weblate sync fixing view model tests not collecting flow results - the switch from runBlockingTest to runTest means we need to provide a separate scope from the test in order to asynchronously collect the flow results Do not suggest collapse if there is only one section Translated using Weblate (Spanish) Translated using Weblate (Spanish) runBlocking -> runTest https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-test/MIGRATION.md runBlockingTest -> runTest https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-test/MIGRATION.md Small cleanup The `.exhaustive` trick is not needed anymore in Kotlin 1.6.0 https://kotlinlang.org/docs/whatsnew16.html#stable-exhaustive-when-statements-for-enum-sealed-and-boolean-subjects Also upgrade the coroutine lib Fix compilation warning (exhaustive when) Fix compilation warning (exhaustive when) Format file (no other change) Fix compilation warning (exhaustive when) Bump moshi from 1.12.0 to 1.13.0 Bump kotlin-gradle-plugin from 1.5.31 to 1.6.0 fixing presence icon anchoring to the middle of the room icon - creates a secondary verification shield and aligns to the start of the room title when presence is present PR remarks Increase the thread summaries limit ... # Conflicts: # vector/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageRecorderView.kt # vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt

onurays · 2022-03-23T11:32:44Z

@onurays the Player visual events freeze for me after skip play and stop a Voice Message and play a another one above the timeline.

Good catch, thank you. I will fix this in another PR.

onurays added 2 commits March 2, 2022 17:45

Create a custom audio waveform view.

ab2001c

Remove 3rd party waveform library.

243a714

onurays requested a review from bmarty March 2, 2022 14:55

onurays added 8 commits March 3, 2022 17:59

Support scrolling playback on timeline.

4254f46

Fix voice recorder start/pause states.

3bd4a4c

Support scrolling playback on recorded audio before sending.

5168d71

Always stop all voice actions and media player if app enters to the b…

aae75ce

…ackground.

Support ltr and rtl flow of the recording waveform.

601f10a

Changelog added.

e09b123

Do not allow to flow RTL after summarized, playback time always flows…

4cb432e

… LTR.

Merge branch 'develop' into feature/ons/voice_message_scrubbing

2c61179

onurays marked this pull request as ready for review March 4, 2022 15:05

bmarty reviewed Mar 4, 2022

View reviewed changes

onurays added 2 commits March 7, 2022 15:52

Code review fixes.

3156410

Lint fixes.

6fef2f6

onurays requested a review from bmarty March 8, 2022 08:54

ouchadam reviewed Mar 16, 2022

View reviewed changes

Code review fixes.

24bdad3

ouchadam approved these changes Mar 22, 2022

View reviewed changes

Onuray Sahin added 2 commits March 23, 2022 13:32

Remove exhaustive.

7ead3f9

onurays merged commit 6d0b823 into develop Mar 23, 2022

onurays deleted the feature/ons/voice_message_scrubbing branch March 23, 2022 11:33

bmarty mentioned this pull request Apr 6, 2022

[Release] Element Android v1.4.10 - v1.4.11 #5699

Closed

59 tasks

jmartinesp mentioned this pull request Jul 14, 2022

Fixes wrong voice message being displayed and played on the timeline. #6556

Merged

15 tasks

RayBB mentioned this pull request Nov 29, 2022

Improve the experience of scrolling / scrubbing backwards in voice messages #7661

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Voice Message Playback Scrolling Support #5404

Voice Message Playback Scrolling Support #5404

onurays commented Mar 2, 2022 •

edited

Loading

github-actions bot commented Mar 2, 2022 •

edited

Loading

bmarty left a comment •

edited

Loading

bmarty Mar 4, 2022

onurays Mar 7, 2022

bmarty Mar 4, 2022

onurays Mar 7, 2022

bmarty Mar 4, 2022

onurays Mar 7, 2022

ouchadam commented Mar 16, 2022

ouchadam Mar 16, 2022

onurays Mar 17, 2022

ouchadam Mar 16, 2022 •

edited

Loading

onurays Mar 17, 2022 •

edited

Loading

ouchadam Mar 18, 2022 •

edited

Loading

onurays Mar 22, 2022

onurays commented Mar 17, 2022

ouchadam left a comment

DoM1niC commented Mar 22, 2022 •

edited

Loading

onurays commented Mar 23, 2022

Voice Message Playback Scrolling Support #5404

Voice Message Playback Scrolling Support #5404

Conversation

onurays commented Mar 2, 2022 • edited Loading

github-actions bot commented Mar 2, 2022 • edited Loading

Unit Test Results

bmarty left a comment • edited Loading

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

ouchadam commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam Mar 16, 2022 • edited Loading

Choose a reason for hiding this comment

onurays Mar 17, 2022 • edited Loading

Choose a reason for hiding this comment

ouchadam Mar 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onurays commented Mar 17, 2022

ouchadam left a comment

Choose a reason for hiding this comment

DoM1niC commented Mar 22, 2022 • edited Loading

onurays commented Mar 23, 2022

onurays commented Mar 2, 2022 •

edited

Loading

github-actions bot commented Mar 2, 2022 •

edited

Loading

bmarty left a comment •

edited

Loading

ouchadam Mar 16, 2022 •

edited

Loading

onurays Mar 17, 2022 •

edited

Loading

ouchadam Mar 18, 2022 •

edited

Loading

DoM1niC commented Mar 22, 2022 •

edited

Loading