Skip to content

Commit 734b9e5

Browse files
committed
Address review findings: instant replay after stop, end-event loss, dispose and resize races
- play() after stop() no longer stalls 10 s waiting for _hasMedia: reload lastUri immediately when no media is loaded and no open is in flight. - produceFrames(): re-read the duration before treating a consumed didPlayToEnd event as a live-stream artifact, so a finite file whose duration resolved late still fires onPlaybackEnded/loop. - dispose(): log a warning when the pipeline-join or videoReaderMutex timeouts expire instead of silently proceeding to nDisposePlayer. - onResized(): a cancelled resize job no longer clears isResizing set by the superseding call; the flag can also no longer stay stuck when the job is cancelled during the debounce delay.
1 parent 196cdbf commit 734b9e5

1 file changed

Lines changed: 33 additions & 5 deletions

File tree

mediaplayer/src/jvmMain/kotlin/io/github/kdroidfilter/composemediaplayer/mac/MacVideoPlayerState.kt

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,17 @@ class MacVideoPlayerState : VideoPlayerState {
710710
if (_isPlaying && !_userDragging && !isSeeking.get() && !seekInFlight.get() &&
711711
MacNativeBridge.nConsumeDidPlayToEnd(ptr)
712712
) {
713+
if (_duration <= 0.0) {
714+
// The end event is already consumed, but the duration may simply not have
715+
// resolved yet (finite file still reporting NaN at open time). Try one last
716+
// read before writing the event off as a live-stream artifact, otherwise a
717+
// real end-of-playback would be silently swallowed.
718+
val d = readDuration(ptr)
719+
if (d > 0.0) {
720+
_duration = d
721+
metadata.duration = (d * 1000).toLong().takeIf { it > 0 }
722+
}
723+
}
713724
if (_duration <= 0.0) {
714725
// Live stream — wait and continue.
715726
delay(1000)
@@ -995,6 +1006,14 @@ class MacVideoPlayerState : VideoPlayerState {
9951006
scope.launch {
9961007
try {
9971008
withTimeout(10_000) { initReady.await() }
1009+
// No media and no open in flight (e.g. play() after stop()): _hasMedia will never
1010+
// flip on its own, so reload immediately instead of stalling on the full timeout.
1011+
if (!_hasMedia && !isOpening.get()) {
1012+
lastUri?.takeIf { it.isNotEmpty() }?.let { uri ->
1013+
openUriInternal(uri, InitialPlayerState.PLAY)
1014+
}
1015+
return@launch
1016+
}
9981017
withTimeout(10_000) {
9991018
snapshotFlow { _hasMedia }.filter { it }.first()
10001019
}
@@ -1266,15 +1285,17 @@ class MacVideoPlayerState : VideoPlayerState {
12661285
surfaceWidth = width
12671286
surfaceHeight = height
12681287

1269-
isResizing.set(true)
12701288
resizeJob?.cancel()
1289+
isResizing.set(true)
12711290
resizeJob =
12721291
scope.launch {
1273-
delay(120)
12741292
try {
1293+
delay(120)
12751294
applyOutputScaling()
12761295
} finally {
1277-
isResizing.set(false)
1296+
// Only the job that still owns the resize clears the flag: a superseding
1297+
// onResized() has already cancelled this job and re-set the flag for its own.
1298+
if (resizeJob === coroutineContext[Job]) isResizing.set(false)
12781299
}
12791300
}
12801301
}
@@ -1420,8 +1441,15 @@ class MacVideoPlayerState : VideoPlayerState {
14201441
Thread {
14211442
try {
14221443
runBlocking {
1423-
withTimeoutOrNull(500) { jobToJoin?.join() }
1424-
withTimeoutOrNull(1000) { videoReaderMutex.withLock { } }
1444+
if (jobToJoin != null && withTimeoutOrNull(500) { jobToJoin.join() } == null) {
1445+
macLogger.w { "dispose(): video pipeline did not stop within 500 ms" }
1446+
}
1447+
if (withTimeoutOrNull(1000) { videoReaderMutex.withLock { } } == null) {
1448+
macLogger.w {
1449+
"dispose(): videoReaderMutex not acquired within 1 s — " +
1450+
"a native frame read may still be in flight"
1451+
}
1452+
}
14251453
}
14261454
} catch (_: Throwable) { /* best effort */ }
14271455
try {

0 commit comments

Comments
 (0)