Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7579.wip
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[Voice Broadcast] Improve the live indicator icon rendering in the timeline
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,26 @@ abstract class AbsMessageVoiceBroadcastItem<H : AbsMessageVoiceBroadcastItem.Hol
renderMetadata(holder)
}

private fun renderLiveIndicator(holder: H) {
abstract fun renderLiveIndicator(holder: H)

protected fun renderPlayingLiveIndicator(holder: H) {
with(holder) {
when (voiceBroadcastState) {
VoiceBroadcastState.STARTED,
VoiceBroadcastState.RESUMED -> {
liveIndicator.tintBackground(colorProvider.getColorFromAttribute(R.attr.colorError))
liveIndicator.isVisible = true
}
VoiceBroadcastState.PAUSED -> {
liveIndicator.tintBackground(colorProvider.getColorFromAttribute(R.attr.vctr_content_quaternary))
liveIndicator.isVisible = true
}
VoiceBroadcastState.STOPPED, null -> {
liveIndicator.isVisible = false
}
}
liveIndicator.tintBackground(colorProvider.getColorFromAttribute(R.attr.colorError))
liveIndicator.isVisible = true
}
}

protected fun renderPausedLiveIndicator(holder: H) {
with(holder) {
liveIndicator.tintBackground(colorProvider.getColorFromAttribute(R.attr.vctr_content_quaternary))
liveIndicator.isVisible = true
}
}

protected fun renderNoLiveIndicator(holder: H) {
holder.liveIndicator.isVisible = false
}

abstract fun renderMetadata(holder: H)

abstract class Holder(@IdRes stubId: Int) : AbsMessageItem.Holder(stubId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import im.vector.app.core.epoxy.onClick
import im.vector.app.features.home.room.detail.RoomDetailAction.VoiceBroadcastAction
import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker.Listener.State
import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer
import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState
import im.vector.app.features.voicebroadcast.views.VoiceBroadcastMetadataView

@EpoxyModelClass
Expand All @@ -43,7 +44,15 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem
}

private fun bindVoiceBroadcastItem(holder: Holder) {
playerListener = VoiceBroadcastPlayer.Listener { renderPlayingState(holder, it) }
playerListener = object : VoiceBroadcastPlayer.Listener {
override fun onPlayingStateChanged(state: VoiceBroadcastPlayer.State) {
renderPlayingState(holder, state)
}

override fun onLiveModeChanged(isLive: Boolean) {
renderLiveIndicator(holder)
}
}
player.addListener(voiceBroadcast, playerListener)
bindSeekBar(holder)
bindButtons(holder)
Expand Down Expand Up @@ -82,6 +91,14 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem
}
}

override fun renderLiveIndicator(holder: Holder) {
when {
voiceBroadcastState == null || voiceBroadcastState == VoiceBroadcastState.STOPPED -> renderNoLiveIndicator(holder)
voiceBroadcastState == VoiceBroadcastState.PAUSED || !player.isLiveListening -> renderPausedLiveIndicator(holder)
else -> renderPlayingLiveIndicator(holder)
}
}

private fun renderPlayingState(holder: Holder, state: VoiceBroadcastPlayer.State) {
with(holder) {
bufferingView.isVisible = state == VoiceBroadcastPlayer.State.BUFFERING
Expand All @@ -99,6 +116,8 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem
}
VoiceBroadcastPlayer.State.BUFFERING -> Unit
}

renderLiveIndicator(holder)
}
}

Expand All @@ -121,6 +140,7 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem
}
playbackTracker.track(voiceBroadcast.voiceBroadcastId) { playbackState ->
renderBackwardForwardButtons(holder, playbackState)
renderLiveIndicator(holder)
if (!isUserSeeking) {
holder.seekBar.progress = playbackTracker.getPlaybackTime(voiceBroadcast.voiceBroadcastId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ abstract class MessageVoiceBroadcastRecordingItem : AbsMessageVoiceBroadcastItem
}
}

override fun renderLiveIndicator(holder: Holder) {
when (voiceBroadcastState) {
VoiceBroadcastState.STARTED,
VoiceBroadcastState.RESUMED -> renderPlayingLiveIndicator(holder)
VoiceBroadcastState.PAUSED -> renderPausedLiveIndicator(holder)
VoiceBroadcastState.STOPPED, null -> renderNoLiveIndicator(holder)
}
}

override fun renderMetadata(holder: Holder) {
with(holder) {
listenersCountMetadata.isVisible = false
Expand Down Expand Up @@ -104,6 +113,10 @@ abstract class MessageVoiceBroadcastRecordingItem : AbsMessageVoiceBroadcastItem
super.unbind(holder)
recorderListener?.let { recorder?.removeListener(it) }
recorderListener = null
with(holder) {
recordButton.onClick(null)
stopRecordButton.onClick(null)
}
}

override fun getViewStubId() = STUB_ID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ val MessageAudioEvent.sequence: Int? get() = getVoiceBroadcastChunk()?.sequence

val MessageAudioEvent.duration get() = content.audioInfo?.duration ?: content.audioWaveformInfo?.duration ?: 0

val VoiceBroadcastEvent.voiceBroadcastId
get() = reference?.eventId

val VoiceBroadcastEvent.isLive
get() = content?.isLive.orFalse()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ interface VoiceBroadcastPlayer {
*/
val playingState: State

/**
* Tells whether the player is listening a live voice broadcast in "live" position.
*/
val isLiveListening: Boolean

/**
* Start playback of the given voice broadcast.
*/
Expand Down Expand Up @@ -73,10 +78,15 @@ interface VoiceBroadcastPlayer {
/**
* Listener related to [VoiceBroadcastPlayer].
*/
fun interface Listener {
interface Listener {
/**
* Notify about [VoiceBroadcastPlayer.playingState] changes.
*/
fun onStateChanged(state: State)
fun onPlayingStateChanged(state: State) = Unit

/**
* Notify about [VoiceBroadcastPlayer.isLiveListening] changes.
*/
fun onLiveModeChanged(isLive: Boolean) = Unit
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer.Stat
import im.vector.app.features.voicebroadcast.listening.usecase.GetLiveVoiceBroadcastChunksUseCase
import im.vector.app.features.voicebroadcast.model.VoiceBroadcast
import im.vector.app.features.voicebroadcast.model.VoiceBroadcastEvent
import im.vector.app.features.voicebroadcast.sequence
import im.vector.app.features.voicebroadcast.usecase.GetVoiceBroadcastEventUseCase
import im.vector.lib.core.utils.timer.CountUpTimer
import kotlinx.coroutines.Job
Expand Down Expand Up @@ -70,18 +69,27 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
private var currentVoiceBroadcastEvent: VoiceBroadcastEvent? = null

override var currentVoiceBroadcast: VoiceBroadcast? = null
override var isLiveListening: Boolean = false
@MainThread
set(value) {
if (field != value) {
Timber.w("isLiveListening: $field -> $value")
field = value
onLiveListeningChanged(value)
}
}

override var playingState = State.IDLE
@MainThread
set(value) {
if (field != value) {
Timber.w("## VoiceBroadcastPlayer state: $field -> $value")
Timber.w("playingState: $field -> $value")
field = value
onPlayingStateChanged(value)
}
}

/** Map voiceBroadcastId to listeners.*/
/** Map voiceBroadcastId to listeners. */
private val listeners: MutableMap<String, CopyOnWriteArrayList<Listener>> = mutableMapOf()

override fun playOrResume(voiceBroadcast: VoiceBroadcast) {
Expand Down Expand Up @@ -121,7 +129,8 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
listeners[voiceBroadcast.voiceBroadcastId]?.add(listener) ?: run {
listeners[voiceBroadcast.voiceBroadcastId] = CopyOnWriteArrayList<Listener>().apply { add(listener) }
}
listener.onStateChanged(if (voiceBroadcast == currentVoiceBroadcast) playingState else State.IDLE)
listener.onPlayingStateChanged(if (voiceBroadcast == currentVoiceBroadcast) playingState else State.IDLE)
listener.onLiveModeChanged(if (voiceBroadcast == currentVoiceBroadcast) isLiveListening else false)
}

override fun removeListener(voiceBroadcast: VoiceBroadcast, listener: Listener) {
Expand All @@ -142,7 +151,10 @@ class VoiceBroadcastPlayerImpl @Inject constructor(

private fun observeVoiceBroadcastLiveState(voiceBroadcast: VoiceBroadcast) {
voiceBroadcastStateObserver = getVoiceBroadcastEventUseCase.execute(voiceBroadcast)
.onEach { currentVoiceBroadcastEvent = it.getOrNull() }
.onEach {
currentVoiceBroadcastEvent = it.getOrNull()
updateLiveListeningMode()
}
.launchIn(sessionScope)
}

Expand Down Expand Up @@ -190,7 +202,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
else -> playlist.firstOrNull()
}
val content = playlistItem?.audioEvent?.content ?: run { Timber.w("## VoiceBroadcastPlayer: No content to play"); return }
val sequence = playlistItem.audioEvent.sequence ?: run { Timber.w("## VoiceBroadcastPlayer: playlist item has no sequence"); return }
val sequence = playlistItem.sequence ?: run { Timber.w("## VoiceBroadcastPlayer: playlist item has no sequence"); return }
val sequencePosition = position?.let { it - playlistItem.startTime } ?: 0
sessionScope.launch {
try {
Expand Down Expand Up @@ -241,6 +253,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
playbackTracker.updatePausedAtPlaybackTime(voiceBroadcast.voiceBroadcastId, positionMillis, positionMillis.toFloat() / duration)
}
playingState == State.PLAYING || playingState == State.BUFFERING -> {
updateLiveListeningMode(positionMillis)
startPlayback(positionMillis)
}
playingState == State.IDLE || playingState == State.PAUSED -> {
Expand Down Expand Up @@ -302,15 +315,53 @@ class VoiceBroadcastPlayerImpl @Inject constructor(
}

private fun onPlayingStateChanged(playingState: State) {
// Notify state change to all the listeners attached to the current voice broadcast id
// Update live playback flag
updateLiveListeningMode()

currentVoiceBroadcast?.voiceBroadcastId?.let { voiceBroadcastId ->
// Start or stop playback ticker
when (playingState) {
State.PLAYING -> playbackTicker.startPlaybackTicker(voiceBroadcastId)
State.PAUSED,
State.BUFFERING,
State.IDLE -> playbackTicker.stopPlaybackTicker(voiceBroadcastId)
}
listeners[voiceBroadcastId]?.forEach { listener -> listener.onStateChanged(playingState) }
// Notify state change to all the listeners attached to the current voice broadcast id
listeners[voiceBroadcastId]?.forEach { listener -> listener.onPlayingStateChanged(playingState) }
}
}

/**
* Update the live listening state according to:
* - the voice broadcast state (started/paused/resumed/stopped),
* - the playing state (IDLE, PLAYING, PAUSED, BUFFERING),
* - the potential seek position (backward/forward).
*/
private fun updateLiveListeningMode(seekPosition: Int? = null) {
isLiveListening = when {
// the current voice broadcast is not live (ended)
!currentVoiceBroadcastEvent?.isLive.orFalse() -> false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Little nit pick here: I'd either use .not() at the end or .isLive == false, the current version makes the starting ! easy to miss.

// the player is stopped or paused
playingState == State.IDLE || playingState == State.PAUSED -> false
// the user has sought
seekPosition != null -> {
val seekDirection = seekPosition.compareTo(getCurrentPlaybackPosition() ?: 0)
when {
// backward
seekDirection < 0 -> false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, that's what caused one of the issues I mentioned. I guess it's a expected behaviour then.

// forward: check if new sequence is the last one

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: last -> latest?

else -> playlist.findByPosition(seekPosition)?.sequence == playlist.lastOrNull()?.sequence
}
}
// otherwise, stay in live or go in live if we reached the last sequence
else -> isLiveListening || playlist.currentSequence == playlist.lastOrNull()?.sequence
}
}

private fun onLiveListeningChanged(isLiveListening: Boolean) {
currentVoiceBroadcast?.voiceBroadcastId?.let { voiceBroadcastId ->
// Notify live mode change to all the listeners attached to the current voice broadcast id
listeners[voiceBroadcastId]?.forEach { listener -> listener.onLiveModeChanged(isLiveListening) }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,15 @@ class VoiceBroadcastPlaylist(
}

fun findBySequence(sequenceNumber: Int): PlaylistItem? {
return items.find { it.audioEvent.sequence == sequenceNumber }
return items.find { it.sequence == sequenceNumber }
}

fun getNextItem() = findBySequence(currentSequence?.plus(1) ?: 1)

fun firstOrNull() = findBySequence(1)
}

data class PlaylistItem(val audioEvent: MessageAudioEvent, val startTime: Int)
data class PlaylistItem(val audioEvent: MessageAudioEvent, val startTime: Int) {
val sequence: Int?
get() = audioEvent.sequence
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import im.vector.app.features.voicebroadcast.model.VoiceBroadcastState
import im.vector.app.features.voicebroadcast.model.asVoiceBroadcastEvent
import im.vector.app.features.voicebroadcast.sequence
import im.vector.app.features.voicebroadcast.usecase.GetVoiceBroadcastEventUseCase
import im.vector.app.features.voicebroadcast.voiceBroadcastId
import kotlinx.coroutines.channels.awaitClose
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.callbackFlow
Expand Down Expand Up @@ -73,14 +74,15 @@ class GetLiveVoiceBroadcastChunksUseCase @Inject constructor(

// Observe new timeline events
val listener = object : Timeline.Listener {
private var lastEventId: String? = null
private var latestEventId: String? = null
private var lastSequence: Int? = null

override fun onTimelineUpdated(snapshot: List<TimelineEvent>) {
val newEvents = lastEventId?.let { eventId -> snapshot.subList(0, snapshot.indexOfFirst { it.eventId == eventId }) } ?: snapshot
val latestEventIndex = latestEventId?.let { eventId -> snapshot.indexOfFirst { it.eventId == eventId } }
val newEvents = if (latestEventIndex != null) snapshot.subList(0, latestEventIndex) else snapshot

// Detect a potential stopped voice broadcast state event
val stopEvent = newEvents.findStopEvent()
val stopEvent = newEvents.findStopEvent(voiceBroadcast)
if (stopEvent != null) {
lastSequence = stopEvent.content?.lastChunkSequence
}
Expand All @@ -98,7 +100,7 @@ class GetLiveVoiceBroadcastChunksUseCase @Inject constructor(
timeline.dispose()
}

lastEventId = snapshot.firstOrNull()?.eventId
latestEventId = snapshot.firstOrNull()?.eventId
}
}

Expand All @@ -117,8 +119,8 @@ class GetLiveVoiceBroadcastChunksUseCase @Inject constructor(
/**
* Find a [VoiceBroadcastEvent] with a [VoiceBroadcastState.STOPPED] state.
*/
private fun List<TimelineEvent>.findStopEvent(): VoiceBroadcastEvent? =
this.mapNotNull { it.root.asVoiceBroadcastEvent() }
private fun List<TimelineEvent>.findStopEvent(voiceBroadcast: VoiceBroadcast): VoiceBroadcastEvent? =
this.mapNotNull { timelineEvent -> timelineEvent.root.asVoiceBroadcastEvent()?.takeIf { it.voiceBroadcastId == voiceBroadcast.voiceBroadcastId } }
.find { it.content?.voiceBroadcastState == VoiceBroadcastState.STOPPED }

/**
Expand Down
Loading