Skip to content

Commit eb2a32a

Browse files
committed
fix(lt): multiple issues
- fixes host/guest desync during quick actions - fixes minor ui glitches
1 parent d57e481 commit eb2a32a

File tree

3 files changed

+107
-17
lines changed

3 files changed

+107
-17
lines changed

app/src/main/kotlin/com/metrolist/music/listentogether/ListenTogetherManager.kt

Lines changed: 96 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ class ListenTogetherManager @Inject constructor(
4747
) {
4848
companion object {
4949
private const val TAG = "ListenTogetherManager"
50+
// Debounce threshold for playback syncs - prevents excessive seeking/pausing
51+
private const val SYNC_DEBOUNCE_THRESHOLD_MS = 200L
52+
// Position tolerance - only seek if difference exceeds this (prevents micro-adjustments)
53+
private const val POSITION_TOLERANCE_MS = 500L
5054
}
5155

5256
private val scope = CoroutineScope(Dispatchers.Main + SupervisorJob())
@@ -76,11 +80,18 @@ class ListenTogetherManager @Inject constructor(
7680
private var lastSyncedIsPlaying: Boolean? = null
7781
private var lastSyncedTrackId: String? = null
7882

83+
// Track last sync action time for debouncing (prevents excessive seeking/pausing)
84+
private var lastSyncActionTime: Long = 0L
85+
7986
// Track ID being buffered
8087
private var bufferingTrackId: String? = null
8188

8289
// Track active sync job to cancel it if a better update arrives
8390
private var activeSyncJob: Job? = null
91+
92+
// Generation ID for track changes - incremented on each new track change
93+
// Used to prevent old coroutines from overwriting newer track loads
94+
private var currentTrackGeneration: Int = 0
8495

8596
// Pending sync to apply after buffering completes for guest
8697
private var pendingSyncState: SyncStatePayload? = null
@@ -674,6 +685,8 @@ class ListenTogetherManager @Inject constructor(
674685
isSyncing = false
675686
bufferCompleteReceivedForTrack = null
676687
lastRole = RoomRole.NONE
688+
lastSyncActionTime = 0L // Reset sync debouncing
689+
++currentTrackGeneration // Increment to invalidate any pending track-change coroutines
677690
}
678691

679692
private fun updateGuestMuteState() {
@@ -699,17 +712,22 @@ class ListenTogetherManager @Inject constructor(
699712
/**
700713
* Restore the mute state that was saved when joining the room.
701714
* This is called when leaving the room to ensure the user's
702-
* mute preference is restored (unmuted by default if they muted during session).
715+
* mute preference is restored to what it was before joining Listen Together.
703716
*/
704717
private fun restoreGuestMuteState() {
705718
val connection = playerConnection ?: return
706719
val savedState = previousMuteState
707720

708-
// If player is currently muted, unmute it when leaving Listen Together
709-
// This addresses the issue where mute state persists after leaving
710-
if (connection.isMuted.value) {
711-
Timber.tag(TAG).d("Unmuting player on leave (was muted during Listen Together session)")
712-
connection.setMuted(false)
721+
if (savedState != null) {
722+
Timber.tag(TAG).d("Restoring mute state on leave: was muted=$savedState, currently muted=${connection.isMuted.value}")
723+
connection.setMuted(savedState)
724+
} else {
725+
// No saved state means we never properly saved (e.g., player wasn't ready on join)
726+
// In this case, if currently muted, unmute as a fallback
727+
if (connection.isMuted.value) {
728+
Timber.tag(TAG).d("No saved mute state on leave, unmuting player as fallback")
729+
connection.setMuted(false)
730+
}
713731
}
714732

715733
previousMuteState = null
@@ -795,50 +813,73 @@ class ListenTogetherManager @Inject constructor(
795813
return
796814
}
797815

798-
// Seek first for precision, then play
799-
if (kotlin.math.abs(player.currentPosition - adjustedPos) > 100) {
816+
// Seek first for precision, then play (use larger position tolerance to reduce stuttering)
817+
if (kotlin.math.abs(player.currentPosition - adjustedPos) > POSITION_TOLERANCE_MS) {
800818
connection.seekTo(adjustedPos)
819+
Timber.tag(TAG).d("Guest: PLAY seeking from ${player.currentPosition} to $adjustedPos (diff > ${POSITION_TOLERANCE_MS}ms)")
801820
}
802821
// Start playback immediately for tighter sync
803822
connection.play()
823+
lastSyncActionTime = now
804824
}
805825

806826
PlaybackActions.PAUSE -> {
807827
val pos = action.position ?: 0L
828+
val now = System.currentTimeMillis()
829+
808830
Timber.tag(TAG).d("Guest: PAUSE at position $pos")
809831

810832
if (bufferingTrackId != null) {
811833
pendingSyncState = (pendingSyncState ?: SyncStatePayload(
812834
currentTrack = roomState.value?.currentTrack,
813835
isPlaying = false,
814836
position = pos,
815-
lastUpdate = System.currentTimeMillis()
837+
lastUpdate = now
816838
)).copy(
817839
isPlaying = false,
818840
position = pos,
819-
lastUpdate = System.currentTimeMillis()
841+
lastUpdate = now
820842
)
821843
applyPendingSyncIfReady()
822844
return
823845
}
824846

825-
// Pause first, then seek for accuracy
847+
// Pause first, then seek for accuracy (use larger position tolerance)
826848
connection.pause()
827-
if (kotlin.math.abs(player.currentPosition - pos) > 100) {
849+
if (kotlin.math.abs(player.currentPosition - pos) > POSITION_TOLERANCE_MS) {
828850
connection.seekTo(pos)
851+
Timber.tag(TAG).d("Guest: PAUSE seeking from ${player.currentPosition} to $pos (diff > ${POSITION_TOLERANCE_MS}ms)")
829852
}
853+
lastSyncActionTime = now
830854
}
831855

832856
PlaybackActions.SEEK -> {
833857
val pos = action.position ?: 0L
834-
Timber.tag(TAG).d("Guest: SEEK to $pos")
835-
connection.seekTo(pos)
858+
val now = System.currentTimeMillis()
859+
860+
// Debounce SEEK actions - don't seek if one just happened
861+
if (now - lastSyncActionTime < SYNC_DEBOUNCE_THRESHOLD_MS) {
862+
Timber.tag(TAG).d("Guest: SEEK debounced (only ${now - lastSyncActionTime}ms since last sync)")
863+
return
864+
}
865+
866+
// Use larger position tolerance
867+
if (kotlin.math.abs(player.currentPosition - pos) > POSITION_TOLERANCE_MS) {
868+
Timber.tag(TAG).d("Guest: SEEK to $pos from ${player.currentPosition} (diff > ${POSITION_TOLERANCE_MS}ms)")
869+
connection.seekTo(pos)
870+
lastSyncActionTime = now
871+
} else {
872+
Timber.tag(TAG).d("Guest: SEEK ignored (position diff < ${POSITION_TOLERANCE_MS}ms)")
873+
}
836874
}
837875

838876
PlaybackActions.CHANGE_TRACK -> {
839877
action.trackInfo?.let { track ->
840878
Timber.tag(TAG).d("Guest: CHANGE_TRACK to ${track.title}, queue size=${action.queue?.size}")
841879

880+
// Reset sync debounce timer on track change - this is a fresh sync cycle
881+
lastSyncActionTime = 0L
882+
842883
// If we have a queue, use it! This is the "smart" sync path.
843884
if (action.queue != null && action.queue.isNotEmpty()) {
844885
val queueTitle = action.queueTitle
@@ -1030,7 +1071,14 @@ class ListenTogetherManager @Inject constructor(
10301071
// If no track, just pause and clear/set queue
10311072
if (currentTrack == null) {
10321073
Timber.tag(TAG).d("No track in state, pausing")
1074+
val generation = ++currentTrackGeneration
10331075
scope.launch(Dispatchers.Main) {
1076+
// Verify we're still on the same track generation (no newer track change arrived)
1077+
if (currentTrackGeneration != generation) {
1078+
Timber.tag(TAG).d("Skipping stale track generation: $generation vs current $currentTrackGeneration")
1079+
return@launch
1080+
}
1081+
10341082
if (playerConnection !== connection) return@launch
10351083
isSyncing = true
10361084
connection.allowInternalSync = true
@@ -1053,13 +1101,26 @@ class ListenTogetherManager @Inject constructor(
10531101
}
10541102

10551103
bufferingTrackId = currentTrack.id
1104+
val generation = ++currentTrackGeneration
10561105

10571106
scope.launch(Dispatchers.Main) {
1107+
// Verify we're still on the same track generation (no newer track change arrived)
1108+
if (currentTrackGeneration != generation) {
1109+
Timber.tag(TAG).d("Skipping stale track generation: $generation vs current $currentTrackGeneration (track ${currentTrack.id})")
1110+
return@launch
1111+
}
1112+
10581113
if (playerConnection !== connection) return@launch
10591114
isSyncing = true
10601115
connection.allowInternalSync = true
10611116

10621117
try {
1118+
// Re-verify generation before applying media items (critical section)
1119+
if (currentTrackGeneration != generation) {
1120+
Timber.tag(TAG).d("Stale generation detected before setMediaItems: $generation vs $currentTrackGeneration")
1121+
return@launch
1122+
}
1123+
10631124
// Apply queue/media (same)
10641125
if (queue != null && queue.isNotEmpty()) {
10651126
val mediaItems = queue.map { it.toMediaMetadata().toMediaItem() }
@@ -1150,14 +1211,29 @@ class ListenTogetherManager @Inject constructor(
11501211

11511212
// Track which buffer-complete we expect for this load
11521213
bufferingTrackId = track.id
1214+
val generation = currentTrackGeneration
11531215

11541216
activeSyncJob?.cancel()
11551217
activeSyncJob = scope.launch(Dispatchers.IO) {
11561218
try {
1219+
// Check if a newer track change arrived - skip this load if stale
1220+
if (currentTrackGeneration != generation) {
1221+
Timber.tag(TAG).d("Skipping stale syncToTrack for ${track.id} (generation $generation vs $currentTrackGeneration)")
1222+
isSyncing = false
1223+
return@launch
1224+
}
1225+
11571226
// Use YouTube API to play the track by ID
11581227
YouTube.queue(listOf(track.id)).onSuccess { queue ->
11591228
Timber.tag(TAG).d("Got queue for track ${track.id}")
11601229
launch(Dispatchers.Main) {
1230+
// Final generation check before applying changes
1231+
if (currentTrackGeneration != generation) {
1232+
Timber.tag(TAG).d("Skipping stale track application for ${track.id} (generation $generation vs $currentTrackGeneration)")
1233+
isSyncing = false
1234+
return@launch
1235+
}
1236+
11611237
val connection = playerConnection ?: run {
11621238
isSyncing = false
11631239
return@launch
@@ -1185,6 +1261,12 @@ class ListenTogetherManager @Inject constructor(
11851261
// Wait for player to be ready - monitor actual player state
11861262
var waitCount = 0
11871263
while (waitCount < 40) { // Max 2 seconds (40 * 50ms)
1264+
// Check generation again while waiting
1265+
if (currentTrackGeneration != generation) {
1266+
Timber.tag(TAG).d("Generation changed while waiting for player ready - aborting sync for ${track.id}")
1267+
isSyncing = false
1268+
return@launch
1269+
}
11881270
try {
11891271
val player = connection.player
11901272
if (player.playbackState == Player.STATE_READY) {

app/src/main/kotlin/com/metrolist/music/ui/component/Dialog.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import androidx.compose.foundation.layout.Spacer
1818
import androidx.compose.foundation.layout.fillMaxSize
1919
import androidx.compose.foundation.layout.fillMaxWidth
2020
import androidx.compose.foundation.layout.height
21+
import androidx.compose.foundation.layout.imePadding
2122
import androidx.compose.foundation.layout.padding
2223
import androidx.compose.foundation.lazy.LazyColumn
2324
import androidx.compose.foundation.lazy.LazyListScope
@@ -248,7 +249,9 @@ fun ListDialog(
248249
) {
249250
Column(
250251
horizontalAlignment = Alignment.CenterHorizontally,
251-
modifier = modifier.padding(vertical = 24.dp),
252+
modifier = modifier
253+
.padding(vertical = 24.dp)
254+
.imePadding(),
252255
) {
253256
LazyColumn(content = content)
254257
}

app/src/main/kotlin/com/metrolist/music/ui/screens/ListenTogetherScreen.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import androidx.compose.foundation.layout.asPaddingValues
2929
import androidx.compose.foundation.layout.fillMaxSize
3030
import androidx.compose.foundation.layout.fillMaxWidth
3131
import androidx.compose.foundation.layout.height
32+
import androidx.compose.foundation.layout.imePadding
3233
import androidx.compose.foundation.layout.offset
3334
import androidx.compose.foundation.layout.padding
3435
import androidx.compose.foundation.layout.size
@@ -232,7 +233,8 @@ fun ListenTogetherScreen(
232233
state = lazyListState,
233234
modifier = Modifier
234235
.fillMaxSize()
235-
.background(MaterialTheme.colorScheme.background),
236+
.background(MaterialTheme.colorScheme.background)
237+
.imePadding(),
236238
contentPadding = PaddingValues(
237239
start = 16.dp,
238240
end = 16.dp,
@@ -353,6 +355,7 @@ fun ListenTogetherScreen(
353355
isJoiningRoom = isJoiningRoom,
354356
joinErrorMessage = joinErrorMessage,
355357
waitingForApprovalText = waitingForApprovalText,
358+
bringIntoViewRequester = bringIntoViewRequester,
356359
onCreateRoom = {
357360
val username = usernameInput.takeIf { it.isNotBlank() } ?: savedUsername
358361
val finalUsername = username.trim()
@@ -389,7 +392,7 @@ fun ListenTogetherScreen(
389392
},
390393
onFieldFocused = {
391394
coroutineScope.launch {
392-
lazyListState.animateScrollToItem(3)
395+
bringIntoViewRequester.bringIntoView()
393396
}
394397
}
395398
)
@@ -1019,6 +1022,7 @@ private fun JoinCreateRoomSection(
10191022
isJoiningRoom: Boolean,
10201023
joinErrorMessage: String?,
10211024
waitingForApprovalText: String,
1025+
bringIntoViewRequester: BringIntoViewRequester,
10221026
onCreateRoom: () -> Unit,
10231027
onJoinRoom: () -> Unit,
10241028
onFieldFocused: () -> Unit = {}
@@ -1100,6 +1104,7 @@ private fun JoinCreateRoomSection(
11001104
),
11011105
modifier = Modifier
11021106
.fillMaxWidth()
1107+
.bringIntoViewRequester(bringIntoViewRequester)
11031108
.onFocusChanged { if (it.isFocused) onFieldFocused() }
11041109
)
11051110

0 commit comments

Comments
 (0)