Merge pull request #8012 from vector-im/bugfix/fre/fix_vb_scrubbing

[Voice Broadcast] Use internal playback timer to compute the playback position

changelog.d/8012.bugfix

@@ -0,0 +1 @@
+[Voice Broadcast] Use internal playback timer to compute the current playback position

library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/VideoViewHolder.kt

@@ -102,7 +102,7 @@ class VideoViewHolder constructor(itemView: View) :
 
         views.videoView.setOnPreparedListener {
             stopTimer()
-            countUpTimer = CountUpTimer(100).also {
+            countUpTimer = CountUpTimer(intervalInMs = 100).also {
                 it.tickListener = CountUpTimer.TickListener {
                     val duration = views.videoView.duration
                     val progress = views.videoView.currentPosition

library/core-utils/src/main/java/im/vector/lib/core/utils/timer/Clock.kt

@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.lib.core.utils.timer
+
+interface Clock {
+    fun epochMillis(): Long
+}
+
+class DefaultClock : Clock {
+
+    /**
+     * Provides a UTC epoch in milliseconds
+     *
+     * This value is not guaranteed to be correct with reality
+     * as a User can override the system time and date to any values.
+     */
+    override fun epochMillis(): Long {
+        return System.currentTimeMillis()
+    }
+}

library/core-utils/src/main/java/im/vector/lib/core/utils/timer/CountUpTimer.kt

@@ -28,41 +28,50 @@ import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicLong
 
 @OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class)
-class CountUpTimer(private val intervalInMs: Long = 1_000) {
+class CountUpTimer(initialTime: Long = 0L, private val intervalInMs: Long = 1_000) {
 
     private val coroutineScope = CoroutineScope(Dispatchers.Main)
-    private val elapsedTime: AtomicLong = AtomicLong()
     private val resumed: AtomicBoolean = AtomicBoolean(false)
 
+    private val clock: Clock = DefaultClock()
+    private val lastTime: AtomicLong = AtomicLong()
+    private val elapsedTime: AtomicLong = AtomicLong(initialTime)
+
     init {
         startCounter()
     }
 
     private fun startCounter() {
-        tickerFlow(coroutineScope, intervalInMs / 10)
+        tickerFlow(coroutineScope, intervalInMs)
                 .filter { resumed.get() }
-                .map { elapsedTime.addAndGet(intervalInMs / 10) }
-                .filter { it % intervalInMs == 0L }
-                .onEach {
-                    tickListener?.onTick(it)
-                }.launchIn(coroutineScope)
+                .map { elapsedTime() }
+                .onEach { tickListener?.onTick(it) }
+                .launchIn(coroutineScope)
     }
 
     var tickListener: TickListener? = null
 
     fun elapsedTime(): Long {
-        return elapsedTime.get()
+        return if (resumed.get()) {
+            val now = clock.epochMillis()
+            elapsedTime.addAndGet(now - lastTime.getAndSet(now))
+        } else {
+            elapsedTime.get()
+        }
     }
 
     fun pause() {
+        tickListener?.onTick(elapsedTime())
         resumed.set(false)
     }
 
     fun resume() {
+        lastTime.set(clock.epochMillis())
         resumed.set(true)
     }
 
     fun stop() {
+        tickListener?.onTick(elapsedTime())
         coroutineScope.cancel()
     }
 

tools/check/forbidden_strings_in_code.txt

@@ -176,7 +176,7 @@ PreferenceManager\.getDefaultSharedPreferences==2
 R\.string\.template_
 
 ### Use the Clock interface, or use `measureTimeMillis`
-System\.currentTimeMillis\(\)===2
+System\.currentTimeMillis\(\)===3
 
 ### Remove extra space between the name and the description
 \* @\w+ \w+  +

vector/src/main/java/im/vector/app/features/call/webrtc/WebRtcCall.kt

@@ -166,7 +166,7 @@ class WebRtcCall(
     private var videoSender: RtpSender? = null
     private var screenSender: RtpSender? = null
 
-    private val timer = CountUpTimer(1000L).apply {
+    private val timer = CountUpTimer(intervalInMs = 1000L).apply {
         tickListener = CountUpTimer.TickListener { milliseconds ->
             val formattedDuration = formatDuration(Duration.ofMillis(milliseconds))
             listeners.forEach {

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

@@ -198,7 +198,7 @@ class AudioMessageHelper @Inject constructor(
 
     private fun startRecordingAmplitudes() {
         amplitudeTicker?.stop()
-        amplitudeTicker = CountUpTimer(50).apply {
+        amplitudeTicker = CountUpTimer(intervalInMs = 50).apply {
             tickListener = CountUpTimer.TickListener { onAmplitudeTick() }
             resume()
         }

vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationUserItem.kt

@@ -105,7 +105,7 @@ abstract class LiveLocationUserItem : VectorEpoxyModel<LiveLocationUserItem.Hold
     }
 
     class Holder : VectorEpoxyHolder() {
-        val timer: CountUpTimer = CountUpTimer(1000)
+        val timer: CountUpTimer = CountUpTimer(intervalInMs = 1000)
         val itemUserAvatarImageView by bind<ImageView>(R.id.itemUserAvatarImageView)
         val itemUserDisplayNameTextView by bind<TextView>(R.id.itemUserDisplayNameTextView)
         val itemRemainingTimeTextView by bind<TextView>(R.id.itemRemainingTimeTextView)

vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt

@@ -206,7 +206,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
                 }
             }
             State.Buffering -> {
-                val savedPosition = currentVoiceBroadcast?.voiceBroadcastId?.let { playbackTracker.getPlaybackTime(it) }
+                val savedPosition = currentVoiceBroadcast?.let { playbackTracker.getPlaybackTime(it.voiceBroadcastId) }
                 when {
                     // resume playback from the next sequence item
                     playlist.currentSequence != null -> playlist.getNextItem()?.let { startPlayback(it.startTime) }
@@ -223,27 +223,45 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
         }
     }
 
-    private fun startPlayback(position: Int) {
+    private fun startPlayback(playbackPosition: Int) {
         stopPlayer()
+        playingState = State.Buffering
+
+        val playlistItem = playlist.findByPosition(playbackPosition) ?: run {
+            Timber.w("## Voice Broadcast | No content to play at position $playbackPosition"); stop(); return
+        }
+        val sequence = playlistItem.sequence ?: run {
+            Timber.w("## Voice Broadcast | Playlist item has no sequence"); stop(); return
+        }
+
+        currentVoiceBroadcast?.let {
+            val percentage = tryOrNull { playbackPosition.toFloat() / playlist.duration } ?: 0f
+            playbackTracker.updatePausedAtPlaybackTime(it.voiceBroadcastId, playbackPosition, percentage)
+        }
 
-        val playlistItem = playlist.findByPosition(position)
-        val content = playlistItem?.audioEvent?.content ?: run { Timber.w("## Voice Broadcast | No content to play at position $position"); return }
-        val sequence = playlistItem.sequence ?: run { Timber.w("## Voice Broadcast | Playlist item has no sequence"); return }
-        val sequencePosition = position - playlistItem.startTime
         prepareCurrentPlayerJob = sessionScope.launch {
             try {
-                val mp = prepareMediaPlayer(content)
+                val mp = prepareMediaPlayer(playlistItem.audioEvent.content)
+
+                // Take the difference between the duration given from the media player and the duration given from the chunk event
+                // If the offset is smaller than 500ms, we consider there is no offset to keep the normal behaviour
+                val offset = (mp.duration - playlistItem.duration).takeUnless { it < 500 }?.coerceAtLeast(0) ?: 0
+                val sequencePosition = offset + (playbackPosition - playlistItem.startTime)
+
                 playlist.currentSequence = sequence - 1 // will be incremented in onNextMediaPlayerStarted
                 mp.start()
                 if (sequencePosition > 0) {
                     mp.seekTo(sequencePosition)
                 }
+
                 onNextMediaPlayerStarted(mp)
             } catch (failure: VoiceBroadcastFailure.ListeningError) {
+                if (failure.cause !is CancellationException) {
                     playingState = State.Error(failure)
                 }
             }
         }
+    }
 
     private fun pausePlayback() {
         playingState = State.Paused // This will trigger a playing state update and save the current position
@@ -259,7 +277,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
             playingState = State.Playing
             currentMediaPlayer?.start()
         } else {
-            val savedPosition = currentVoiceBroadcast?.voiceBroadcastId?.let { playbackTracker.getPlaybackTime(it) } ?: 0
+            val savedPosition = currentVoiceBroadcast?.let { playbackTracker.getPlaybackTime(it.voiceBroadcastId) } ?: 0
             startPlayback(savedPosition)
         }
     }
@@ -301,7 +319,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
                 } catch (failure: VoiceBroadcastFailure.ListeningError) {
                     // Do not change the playingState if the current player is still valid,
                     // the error will be thrown again when switching to the next player
-                    if (playingState == State.Buffering || tryOrNull { currentMediaPlayer?.isPlaying } != true) {
+                    if (failure.cause !is CancellationException && (playingState == State.Buffering || tryOrNull { currentMediaPlayer?.isPlaying } != true)) {
                         playingState = State.Error(failure)
                     }
                 }
@@ -355,6 +373,8 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
 
     private fun stopPlayer() {
         tryOrNull { currentMediaPlayer?.stop() }
+        playbackTicker.stopPlaybackTicker()
+
         currentMediaPlayer?.release()
         currentMediaPlayer = null
 
@@ -376,7 +396,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
                 State.Paused,
                 State.Buffering,
                 is State.Error,
-                State.Idle -> playbackTicker.stopPlaybackTicker(voiceBroadcastId)
+                State.Idle -> playbackTicker.stopPlaybackTicker()
             }
 
             // Notify playback tracker about error
@@ -416,22 +436,6 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
         prepareNextMediaPlayer()
     }
 
-    private fun getCurrentPlaybackPosition(): Int? {
-        val voiceBroadcastId = currentVoiceBroadcast?.voiceBroadcastId ?: return null
-        val computedPosition = tryOrNull { currentMediaPlayer?.currentPosition }?.let { playlist.currentItem?.startTime?.plus(it) }
-        val savedPosition = playbackTracker.getPlaybackTime(voiceBroadcastId)
-        return computedPosition ?: savedPosition
-    }
-
-    private fun getCurrentPlaybackPercentage(): Float? {
-        val playlistPosition = playlist.currentItem?.startTime
-        val computedPosition = tryOrNull { currentMediaPlayer?.currentPosition }?.let { playlistPosition?.plus(it) } ?: playlistPosition
-        val duration = playlist.duration
-        val computedPercentage = if (computedPosition != null && duration > 0) computedPosition.toFloat() / duration else null
-        val savedPercentage = currentVoiceBroadcast?.voiceBroadcastId?.let { playbackTracker.getPercentage(it) }
-        return computedPercentage ?: savedPercentage
-    }
-
     private inner class MediaPlayerListener :
             MediaPlayer.OnInfoListener,
             MediaPlayer.OnCompletionListener,
@@ -488,40 +492,41 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
 
         fun startPlaybackTicker(id: String) {
             playbackTicker?.stop()
-            playbackTicker = CountUpTimer(50L).apply {
-                tickListener = CountUpTimer.TickListener { onPlaybackTick(id) }
+            playbackTicker = CountUpTimer(
+                    initialTime = playbackTracker.getPlaybackTime(id)?.toLong() ?: 0L,
+                    intervalInMs = 50L
+            ).apply {
+                tickListener = CountUpTimer.TickListener { onPlaybackTick(id, it.toInt()) }
                 resume()
             }
-            onPlaybackTick(id)
         }
 
-        fun stopPlaybackTicker(id: String) {
+        fun stopPlaybackTicker() {
             playbackTicker?.stop()
+            playbackTicker?.tickListener = null
             playbackTicker = null
-            onPlaybackTick(id)
         }
 
-        private fun onPlaybackTick(id: String) {
-            val playbackTime = getCurrentPlaybackPosition()
-            val percentage = getCurrentPlaybackPercentage()
+        private fun onPlaybackTick(id: String, position: Int) {
+            val percentage = tryOrNull { position.toFloat() / playlist.duration }
             when (playingState) {
                 State.Playing -> {
-                    if (playbackTime != null && percentage != null) {
-                        playbackTracker.updatePlayingAtPlaybackTime(id, playbackTime, percentage)
+                    if (percentage != null) {
+                        playbackTracker.updatePlayingAtPlaybackTime(id, position, percentage)
                     }
                 }
                 State.Paused,
                 State.Buffering -> {
-                    if (playbackTime != null && percentage != null) {
-                        playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage)
+                    if (percentage != null) {
+                        playbackTracker.updatePausedAtPlaybackTime(id, position, percentage)
                     }
                 }
                 State.Idle -> {
-                    // restart the playback time if player completed with less than 250 ms remaining time
-                    if (playbackTime == null || percentage == null || (playlist.duration - playbackTime) < 250) {
+                    // restart the playback time if player completed with less than 1s remaining time
+                    if (percentage == null || (playlist.duration - position) < 1000) {
                         playbackTracker.stopPlayback(id)
                     } else {
-                        playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage)
+                        playbackTracker.updatePausedAtPlaybackTime(id, position, percentage)
                     }
                 }
                 is State.Error -> Unit

vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlaylist.kt

@@ -65,6 +65,6 @@ class VoiceBroadcastPlaylist(
 }
 
 data class PlaylistItem(val audioEvent: MessageAudioEvent, val startTime: Int) {
-    val sequence: Int?
-        get() = audioEvent.sequence
+    val sequence: Int? = audioEvent.sequence
+    val duration: Int = audioEvent.duration
 }

vector/src/main/java/im/vector/app/features/voicebroadcast/model/MessageVoiceBroadcastInfoContent.kt

@@ -23,7 +23,6 @@ import org.matrix.android.sdk.api.session.events.model.Content
 import org.matrix.android.sdk.api.session.room.model.message.MessageContent
 import org.matrix.android.sdk.api.session.room.model.message.MessageType.MSGTYPE_VOICE_BROADCAST_INFO
 import org.matrix.android.sdk.api.session.room.model.relation.RelationDefaultContent
-import timber.log.Timber
 
 /**
  * Content of the state event of type [VoiceBroadcastConstants.STATE_ROOM_VOICE_BROADCAST_INFO].
@@ -50,8 +49,4 @@ data class MessageVoiceBroadcastInfoContent(
 
     val voiceBroadcastState: VoiceBroadcastState? = VoiceBroadcastState.values()
             .find { it.value == voiceBroadcastStateStr }
-            ?: run {
-                Timber.w("Invalid value for state: `$voiceBroadcastStateStr`")
-                null
-            }
 }

vector/src/main/java/im/vector/app/features/voicebroadcast/recording/VoiceBroadcastRecorderQ.kt

@@ -248,31 +248,21 @@ class VoiceBroadcastRecorderQ(
             recordingTicker = CountUpTimer().apply {
                 tickListener = CountUpTimer.TickListener { onTick(elapsedTime()) }
                 resume()
-                onTick(elapsedTime())
             }
         }
 
         fun pause() {
-            recordingTicker?.apply {
-                pause()
-                onTick(elapsedTime())
-            }
+            recordingTicker?.pause()
         }
 
         fun resume() {
-            recordingTicker?.apply {
-                resume()
-                onTick(elapsedTime())
-            }
+            recordingTicker?.resume()
         }
 
         fun stop() {
-            recordingTicker?.apply {
-                stop()
-                onTick(elapsedTime())
+            recordingTicker?.stop()
             recordingTicker = null
         }
-        }
 
         private fun onTick(elapsedTimeMillis: Long) {
             onElapsedTimeUpdated(elapsedTimeMillis)

vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml

@@ -147,9 +147,11 @@
         android:id="@+id/seekBar"
         android:layout_width="0dp"
         android:layout_height="wrap_content"
-        android:layout_marginTop="24dp"
+        android:layout_marginTop="20dp"
         android:paddingStart="0dp"
+        android:paddingTop="4dp"
         android:paddingEnd="0dp"
+        android:paddingBottom="4dp"
         android:progressDrawable="@drawable/bg_seek_bar"
         android:thumbOffset="3dp"
         android:thumbTint="?vctr_content_secondary"
@@ -164,7 +166,7 @@
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_marginStart="4dp"
-        android:layout_marginTop="-3dp"
+        android:layout_marginTop="-7dp"
         android:textColor="?vctr_content_tertiary"
         app:layout_constraintStart_toStartOf="parent"
         app:layout_constraintTop_toBottomOf="@id/seekBar"
@@ -176,7 +178,7 @@
         style="@style/Widget.Vector.TextView.Caption"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
-        android:layout_marginTop="-3dp"
+        android:layout_marginTop="-7dp"
         android:layout_marginEnd="4dp"
         android:textColor="?vctr_content_tertiary"
         app:layout_constraintEnd_toEndOf="parent"