Skip to content

Commit a572047

Browse files
authored
Merge pull request #188 from kdroidFilter/fix/windows-audio-timing-master
fix(windows): rewrite audio timing model and adaptive frame polling
2 parents bfdc842 + 0483240 commit a572047

12 files changed

Lines changed: 549 additions & 351 deletions

File tree

.gitignore

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,14 @@ Pods/
1717
*yarn.lock
1818
# Native compiled binaries (built locally or in CI)
1919
/mediaplayer/src/jvmMain/resources/composemediaplayer/native/
20+
/mediaplayer/src/jvmMain/resources/win32-x86-64/
21+
/mediaplayer/src/jvmMain/resources/win32-arm64/
2022

2123
# Native build artifacts
2224
/mediaplayer/src/jvmMain/native/windows/build-x64/
2325
/mediaplayer/src/jvmMain/native/windows/build-arm64/
26+
/mediaplayer/src/jvmMain/native/windows/build-test/
2427
*.log
2528
/sample/composeApp/debug/
29+
NUL
30+
.claude/

mediaplayer/ComposeMediaPlayer.podspec

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,5 +41,5 @@ Pod::Spec.new do |spec|
4141
SCRIPT
4242
}
4343
]
44-
spec.resources = ['build/compose/cocoapods/compose-resources']
44+
spec.resources = ['build\compose\cocoapods\compose-resources']
4545
end

mediaplayer/src/jvmMain/kotlin/io/github/kdroidfilter/composemediaplayer/linux/LinuxVideoPlayerState.kt

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -718,26 +718,32 @@ class LinuxVideoPlayerState : VideoPlayerState {
718718
uiUpdateJob?.cancel()
719719
playerScope.cancel()
720720

721-
// Dispose the native player synchronously to guarantee cleanup before
722-
// ioScope is cancelled — otherwise GStreamer keeps running (audio leak).
721+
// Clear the pointer atomically so no background task can use it
723722
val ptrToDispose = playerPtrAtomic.getAndSet(0L)
724723

725-
skiaBitmapA?.close()
726-
skiaBitmapB?.close()
727-
skiaBitmapA = null
728-
skiaBitmapB = null
729-
skiaBitmapWidth = 0
730-
skiaBitmapHeight = 0
731-
nextSkiaBitmapA = true
732-
733-
if (ptrToDispose != 0L) {
724+
// Native cleanup on a background thread to avoid blocking the UI.
725+
Thread {
734726
try {
735-
LinuxNativeBridge.nDisposePlayer(ptrToDispose)
727+
skiaBitmapA?.close()
728+
skiaBitmapB?.close()
729+
skiaBitmapA = null
730+
skiaBitmapB = null
731+
skiaBitmapWidth = 0
732+
skiaBitmapHeight = 0
733+
nextSkiaBitmapA = true
736734
} catch (e: Exception) {
737-
if (e is CancellationException) throw e
738-
linuxLogger.e { "Error disposing player: ${e.message}" }
735+
linuxLogger.e { "Error releasing bitmaps: ${e.message}" }
739736
}
740-
}
737+
738+
if (ptrToDispose != 0L) {
739+
try {
740+
LinuxNativeBridge.nDisposePlayer(ptrToDispose)
741+
} catch (e: Exception) {
742+
if (e is CancellationException) throw e
743+
linuxLogger.e { "Error disposing player: ${e.message}" }
744+
}
745+
}
746+
}.start()
741747

742748
ioScope.cancel()
743749
}

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

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -861,32 +861,37 @@ class MacVideoPlayerState : VideoPlayerState {
861861
uiUpdateJob?.cancel()
862862
playerScope.cancel()
863863

864-
// Dispose synchronously to guarantee cleanup before ioScope is cancelled —
865-
// otherwise AVPlayer keeps running (audio leak).
866-
// Use frameDispatcher to safely close bitmaps (rendering accesses them there).
867-
val ptrToDispose = runBlocking(frameDispatcher) {
868-
val ptr = playerPtrAtomic.getAndSet(0L)
869-
870-
skiaBitmapA?.close()
871-
skiaBitmapB?.close()
872-
skiaBitmapA = null
873-
skiaBitmapB = null
874-
skiaBitmapWidth = 0
875-
skiaBitmapHeight = 0
876-
nextSkiaBitmapA = true
877-
878-
ptr
879-
}
864+
// Clear the pointer atomically so no background task can use it
865+
val ptrToDispose = playerPtrAtomic.getAndSet(0L)
880866

881-
if (ptrToDispose != 0L) {
882-
macLogger.d { "dispose() - Disposing native player" }
867+
// Release bitmaps on the frame dispatcher (rendering accesses them there)
868+
// then dispose the native player — all on a background thread to avoid
869+
// blocking the main/UI thread.
870+
Thread {
883871
try {
884-
MacNativeBridge.nDisposePlayer(ptrToDispose)
872+
// Close bitmaps (not thread-safe with rendering, but frame updates
873+
// are already cancelled above and playerPtr is zeroed)
874+
skiaBitmapA?.close()
875+
skiaBitmapB?.close()
876+
skiaBitmapA = null
877+
skiaBitmapB = null
878+
skiaBitmapWidth = 0
879+
skiaBitmapHeight = 0
880+
nextSkiaBitmapA = true
885881
} catch (e: Exception) {
886-
if (e is CancellationException) throw e
887-
macLogger.e { "Error disposing player: ${e.message}" }
882+
macLogger.e { "Error releasing bitmaps: ${e.message}" }
888883
}
889-
}
884+
885+
if (ptrToDispose != 0L) {
886+
macLogger.d { "dispose() - Disposing native player" }
887+
try {
888+
MacNativeBridge.nDisposePlayer(ptrToDispose)
889+
} catch (e: Exception) {
890+
if (e is CancellationException) throw e
891+
macLogger.e { "Error disposing player: ${e.message}" }
892+
}
893+
}
894+
}.start()
890895

891896
ioScope.cancel()
892897
}

mediaplayer/src/jvmMain/kotlin/io/github/kdroidfilter/composemediaplayer/windows/WindowsVideoPlayerState.kt

Lines changed: 49 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,11 @@ class WindowsVideoPlayerState : VideoPlayerState {
263263
private var skiaBitmapWidth: Int = 0
264264
private var skiaBitmapHeight: Int = 0
265265

266+
// Adaptive frame interval (ms) based on the video's native frame rate.
267+
// Mirrors macOS approach: poll at the video frame rate, not faster.
268+
// This prevents starving the audio thread on the shared SourceReader.
269+
private var frameIntervalMs: Long = 16L // Default ~60fps, updated after open
270+
266271
// Variable to store the last opened URI
267272
private var lastUri: String? = null
268273

@@ -292,65 +297,43 @@ class WindowsVideoPlayerState : VideoPlayerState {
292297
return // Already disposing
293298
}
294299

295-
// Cancel the scope immediately to stop all coroutines
296-
scope.cancel()
297-
298-
// Use runBlocking to ensure resources are cleaned up synchronously
299-
runBlocking {
300-
try {
301-
// Cancel all jobs with immediate effect
302-
videoJob?.cancel()
303-
resizeJob?.cancel()
304-
305-
// Wait a bit for coroutines to cancel
306-
delay(50)
307-
308-
mediaOperationMutex.withLock {
309-
// Stop playing if active
310-
_isPlaying = false
311-
val instance = videoPlayerInstance
312-
if (instance != 0L) {
313-
try {
314-
// Stop playback before releasing resources
315-
val hr = player.SetPlaybackState(instance, false, true)
316-
if (hr < 0) {
317-
windowsLogger.e { "Error stopping playback (hr=0x${hr.toString(16)})" }
318-
}
319-
} catch (e: Exception) {
320-
windowsLogger.e { "Exception stopping playback: ${e.message}" }
321-
}
322-
323-
// Close the media
324-
try {
325-
player.CloseMedia(instance)
326-
} catch (e: Exception) {
327-
windowsLogger.e { "Exception closing media: ${e.message}" }
328-
}
329-
330-
// Remove volume setting for this instance
331-
instanceVolumes.remove(instance)
300+
// Stop coroutines first — non-blocking
301+
videoJob?.cancel()
302+
resizeJob?.cancel()
303+
_isPlaying = false
304+
_hasMedia = false
332305

333-
// Destroy the player instance
334-
try {
335-
WindowsNativeBridge.destroyInstance(instance)
336-
} catch (e: Exception) {
337-
windowsLogger.e { "Exception destroying instance: ${e.message}" }
338-
}
306+
// Release Kotlin-side resources immediately (bitmaps, channel)
307+
releaseAllResources()
339308

340-
videoPlayerInstance = 0L
341-
}
309+
// Native cleanup on a background thread so dispose() never blocks the UI.
310+
// scope is about to be cancelled, so use a detached thread.
311+
val instance = videoPlayerInstance
312+
videoPlayerInstance = 0L
313+
lastUri = null
342314

343-
// Clear all resources
344-
clearAllResourcesSync()
315+
if (instance != 0L) {
316+
Thread {
317+
try {
318+
player.SetPlaybackState(instance, false, true)
319+
} catch (e: Exception) {
320+
windowsLogger.e { "Exception stopping playback: ${e.message}" }
345321
}
346-
} catch (e: Exception) {
347-
windowsLogger.e { "Error during dispose: ${e.message}" }
348-
} finally {
349-
// Mark player as uninitialized
350-
_hasMedia = false
351-
lastUri = null
352-
}
322+
try {
323+
player.CloseMedia(instance)
324+
} catch (e: Exception) {
325+
windowsLogger.e { "Exception closing media: ${e.message}" }
326+
}
327+
instanceVolumes.remove(instance)
328+
try {
329+
WindowsNativeBridge.destroyInstance(instance)
330+
} catch (e: Exception) {
331+
windowsLogger.e { "Exception destroying instance: ${e.message}" }
332+
}
333+
}.start()
353334
}
335+
336+
scope.cancel()
354337
}
355338

356339
private fun clearAllResourcesSync() {
@@ -592,6 +575,16 @@ class WindowsVideoPlayerState : VideoPlayerState {
592575
)
593576
}
594577

578+
// Query the native frame rate to compute an adaptive polling interval
579+
// like macOS does with captureFrameRate.
580+
val rateArr = IntArray(2)
581+
if (player.nGetVideoFrameRate(instance, rateArr) >= 0 && rateArr[0] > 0) {
582+
val fps = rateArr[0].toDouble() / rateArr[1].coerceAtLeast(1).toDouble()
583+
frameIntervalMs = (1000.0 / fps).toLong().coerceIn(8L, 50L)
584+
} else {
585+
frameIntervalMs = 16L // fallback ~60fps
586+
}
587+
595588
// Set _hasMedia to true only if everything succeeded
596589
_hasMedia = true
597590

@@ -807,6 +800,8 @@ class WindowsVideoPlayerState : VideoPlayerState {
807800
// Send frame to channel
808801
frameChannel.trySend(FrameData(targetBitmap, frameTime))
809802

803+
// Native AcquireNextSample already paces video to the audio
804+
// clock via PreciseSleepHighRes — no additional delay needed.
810805
delay(1)
811806
} catch (e: CancellationException) {
812807
break

0 commit comments

Comments
 (0)