fix(windows): address review findings on native player refactor

Bugs:
- Guard against clock-skew underflow when computing cached-sample heldMs
- Stop audio thread before the presentation clock in StopPlayback
- InitWASAPI no longer takes ownership of pSourceAudioFormat; caller transfers
- Triple-buffer Skia bitmaps to remove producer/Compose read race
- dispose() avoids runBlocking on AWT EDT to prevent potential deadlock

Performance:
- SeekMedia wakes the audio thread via event before taking csAudioFeed
- Underflow-safe UINT32 arithmetic on framesFree

Cleanup:
- HLSPlayer stored via ComPtr<HLSPlayer> instead of raw pointer
- HLSPlayer frame buffer uses std::vector<BYTE> instead of new[]/delete[]
- Replace 0xC00D36C4 magic with MF_E_UNSUPPORTED_BYTESTREAM_TYPE
- Extract NVP_MIN/MAX_PLAYBACK_SPEED constants in NativeVideoPlayer.h
- Extract startVideoPipeline() helper to dedupe producer/consumer launches
- Replace goto cleanup in InitWASAPI with a RAII scope guard
- GetMediaDuration returns S_FALSE when duration is absent (vs hard error)
- Shutdown logs a warning when live instances remain
- Skip redundant 2ch/48kHz audio fallback when already canonical
- Drop the default nullptr argument on InitWASAPI
- Remove dead clearAllResourcesSync path

Author: kdroidFilter

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

@@ -269,11 +269,12 @@ class WindowsVideoPlayerState : VideoPlayerState {
     private val videoReaderMutex = Mutex()
     private val isSeeking = AtomicBoolean(false)
 
-    // Memory optimization for frame processing
-    private val frameQueueSize = 1
+    // Frame channel: one slot, drop-oldest. With triple-buffering on the
+    // producer side, overflow simply means the consumer was slow — safe to
+    // drop. Capacity >1 would just let the pipeline pile up.
     private val frameChannel =
         Channel<FrameData>(
-            capacity = frameQueueSize,
+            capacity = 1,
             onBufferOverflow = BufferOverflow.DROP_OLDEST,
         )
 
@@ -283,10 +284,13 @@ class WindowsVideoPlayerState : VideoPlayerState {
         val timestamp: Double,
     )
 
-    // Double-buffering for zero-copy frame rendering
-    private var skiaBitmapA: Bitmap? = null
-    private var skiaBitmapB: Bitmap? = null
-    private var nextSkiaBitmapA: Boolean = true
+    // Triple-buffering for zero-copy frame rendering: the consumer may still
+    // be driving a frame onto Compose (via currentFrameState) when the
+    // producer writes the next frame. Two bitmaps is racy — with three, the
+    // buffer the producer writes is guaranteed to be distinct from both the
+    // one currently bound to ImageBitmap and the one Compose just finished.
+    private val skiaBitmaps = arrayOfNulls<Bitmap>(3)
+    private var nextBitmapIndex: Int = 0
     @Volatile
     private var lastFrameHash: Int = Int.MIN_VALUE
     private var skiaBitmapWidth: Int = 0
@@ -359,13 +363,26 @@ class WindowsVideoPlayerState : VideoPlayerState {
         // StopAudioThread + MF teardown) but guarantees a clean shutdown.
         if (instance != 0L) {
             if (jobToJoin != null) {
-                try {
-                    kotlinx.coroutines.runBlocking {
-                        kotlinx.coroutines.withTimeoutOrNull(500) {
-                            jobToJoin.join()
-                        }
+                // Avoid runBlocking on the AWT Event Dispatch Thread: if any
+                // child coroutine of `scope` ever chains on Dispatchers.Main
+                // (even indirectly, e.g. Compose effects), joining here would
+                // deadlock. Fall back to a plain Thread.join on EDT — the
+                // 500 ms cap keeps the UI from hanging if the native side
+                // stalls.
+                val deadlineNs = System.nanoTime() + 500_000_000L
+                if (java.awt.EventQueue.isDispatchThread()) {
+                    while (jobToJoin.isActive && System.nanoTime() < deadlineNs) {
+                        try { Thread.sleep(10) } catch (_: InterruptedException) { break }
                     }
-                } catch (_: Exception) { /* ignore */ }
+                } else {
+                    try {
+                        kotlinx.coroutines.runBlocking {
+                            kotlinx.coroutines.withTimeoutOrNull(500) {
+                                jobToJoin.join()
+                            }
+                        }
+                    } catch (_: Exception) { /* ignore */ }
+                }
             }
 
             try {
@@ -389,81 +406,38 @@ class WindowsVideoPlayerState : VideoPlayerState {
         scope.cancel()
     }
 
-    private fun clearAllResourcesSync() {
-        // Clear the frame channel synchronously
-        while (frameChannel.tryReceive().isSuccess) {
-            // Drain the channel
-        }
-
-        // Free bitmaps and frame buffers
-        bitmapLock.write {
-            _currentFrame = null
-            currentFrameState.value = null
-
-            // Don't close bitmaps — see comment in releaseAllResources().
-            skiaBitmapA = null
-            skiaBitmapB = null
-            skiaBitmapWidth = 0
-            skiaBitmapHeight = 0
-            nextSkiaBitmapA = true
-            lastFrameHash = Int.MIN_VALUE
-            // Deferred-close queue: drop refs, let Skia cleaner finalize them
-            // (AWT may still hold the newest one).
-            pendingCloseBitmaps.clear()
-        }
-
-        // Reset all state
-        _currentTime = 0.0
-        _duration = 0.0
-        _progress = 0f
-        _metadata = VideoMetadata()
-        userPaused = false
-        isLoading = false
-        errorMessage = null
-        _error = null
-
-        // Reset initialFrameRead flag to ensure we read an initial frame when reinitialized
-        initialFrameRead.set(false)
-    }
-
     private fun releaseAllResources() {
         // Cancel any remaining jobs related to video processing
         videoJob?.cancel()
         resizeJob?.cancel()
 
-        // Drain the frame channel (tryReceive is non-suspending)
         clearFrameChannel()
 
-        // Free bitmaps and frame buffers
+        // Free bitmaps and frame buffers.
+        // Do NOT close the triple-buffer bitmaps here: the ImageBitmap exposed
+        // via currentFrameState shares the same native pixel memory
+        // (asComposeImageBitmap is zero-copy). Compose may still be rendering
+        // the last frame on the AWT-EventQueue thread. Closing now would free
+        // the native memory while Skia reads it, causing an access violation.
+        // Nullifying the references lets the Skia Managed cleaner release them
+        // once Compose (and any other holder) drops its reference.
         bitmapLock.write {
             _currentFrame = null
             currentFrameState.value = null
 
-            // Do NOT close the double-buffer bitmaps here: the ImageBitmap
-            // exposed via currentFrameState shares the same native pixel memory
-            // (asComposeImageBitmap is zero-copy). Compose may still be rendering
-            // the last frame on the AWT-EventQueue thread. Closing now would free
-            // the native memory while Skia reads it, causing an access violation.
-            // Nullifying the references lets the Skia Managed cleaner release them
-            // once Compose (and any other holder) drops its reference.
-            skiaBitmapA = null
-            skiaBitmapB = null
+            for (i in skiaBitmaps.indices) skiaBitmaps[i] = null
             skiaBitmapWidth = 0
             skiaBitmapHeight = 0
-            nextSkiaBitmapA = true
+            nextBitmapIndex = 0
             lastFrameHash = Int.MIN_VALUE
             pendingCloseBitmaps.clear()
         }
 
-        // Reset initialFrameRead flag to ensure we read an initial frame when reinitialized
         initialFrameRead.set(false)
     }
 
     private fun clearFrameChannel() {
-        // Drain the frame channel to ensure all items are removed
-        while (frameChannel.tryReceive().isSuccess) {
-            // Intentionally empty - just draining the channel
-        }
+        while (frameChannel.tryReceive().isSuccess) { /* drain */ }
     }
 
     /**
@@ -665,11 +639,7 @@ class WindowsVideoPlayerState : VideoPlayerState {
                         _isPlaying = startPlayback
 
                         // Start video processing
-                        videoJob =
-                            scope.launch {
-                                launch { produceFrames() }
-                                launch { consumeFrames() }
-                            }
+                        videoJob = startVideoPipeline()
                     }
                 } catch (e: Exception) {
                     setError("Error while opening media: ${e.message}")
@@ -681,6 +651,15 @@ class WindowsVideoPlayerState : VideoPlayerState {
         }
     }
 
+    /**
+     * Launches the producer/consumer coroutine pair that reads frames from
+     * the native side and pushes them to Compose.
+     */
+    private fun startVideoPipeline(): Job = scope.launch {
+        launch { produceFrames() }
+        launch { consumeFrames() }
+    }
+
     /**
      * Zero-copy optimized frame producer using double-buffering and direct memory access.
      *
@@ -819,26 +798,31 @@ class WindowsVideoPlayerState : VideoPlayerState {
         }
         lastFrameHash = newHash
 
-        if (skiaBitmapA == null || skiaBitmapWidth != width || skiaBitmapHeight != height) {
+        if (skiaBitmaps[0] == null || skiaBitmapWidth != width || skiaBitmapHeight != height) {
             bitmapLock.write {
                 // Queue previous bitmaps for deferred close instead of leaking them
                 // to the Skia managed cleaner: closing now would race with Compose
                 // still drawing the last frame on the AWT thread.
-                skiaBitmapA?.let { pendingCloseBitmaps.addLast(PendingCloseBitmap(it, pendingCloseGraceFrames)) }
-                skiaBitmapB?.let { pendingCloseBitmaps.addLast(PendingCloseBitmap(it, pendingCloseGraceFrames)) }
+                for (i in skiaBitmaps.indices) {
+                    skiaBitmaps[i]?.let {
+                        pendingCloseBitmaps.addLast(PendingCloseBitmap(it, pendingCloseGraceFrames))
+                    }
+                    skiaBitmaps[i] = null
+                }
                 val imageInfo = createVideoImageInfo()
-                skiaBitmapA = Bitmap().apply { allocPixels(imageInfo) }
-                skiaBitmapB = Bitmap().apply { allocPixels(imageInfo) }
+                for (i in skiaBitmaps.indices) {
+                    skiaBitmaps[i] = Bitmap().apply { allocPixels(imageInfo) }
+                }
                 skiaBitmapWidth = width
                 skiaBitmapHeight = height
-                nextSkiaBitmapA = true
+                nextBitmapIndex = 0
             }
         }
 
         drainPendingCloseBitmaps()
 
-        val targetBitmap = if (nextSkiaBitmapA) skiaBitmapA!! else skiaBitmapB!!
-        nextSkiaBitmapA = !nextSkiaBitmapA
+        val targetBitmap = skiaBitmaps[nextBitmapIndex]!!
+        nextBitmapIndex = (nextBitmapIndex + 1) % skiaBitmaps.size
 
         val pixmap = targetBitmap.peekPixels()
         if (pixmap == null) {
@@ -990,11 +974,7 @@ class WindowsVideoPlayerState : VideoPlayerState {
         }
 
         if (_hasMedia && (videoJob == null || videoJob?.isActive == false)) {
-            videoJob =
-                scope.launch {
-                    launch { produceFrames() }
-                    launch { consumeFrames() }
-                }
+            videoJob = startVideoPipeline()
         }
     }
 
@@ -1146,10 +1126,7 @@ class WindowsVideoPlayerState : VideoPlayerState {
                 // If the producer was never started (e.g. stop() was called
                 // before the first play), start it now so the new frame shows.
                 if (!isDisposing.get() && (videoJob == null || videoJob?.isActive == false)) {
-                    videoJob = scope.launch {
-                        launch { produceFrames() }
-                        launch { consumeFrames() }
-                    }
+                    videoJob = startVideoPipeline()
                 }
             }
         } finally {

mediaplayer/src/jvmMain/native/windows/AudioManager.cpp

@@ -95,7 +95,8 @@ int FeedOneSample(VideoPlayerInstance* inst, IMFSourceReader* audioReader,
 {
     UINT32 framesPadding = 0;
     if (FAILED(inst->pAudioClient->GetCurrentPadding(&framesPadding))) return -1;
-    UINT32 framesFree = engineBufferFrames - framesPadding;
+    UINT32 framesFree = (framesPadding < engineBufferFrames)
+        ? engineBufferFrames - framesPadding : 0;
     if (framesFree == 0) return 0;
 
     const UINT32 sampleRate = inst->pSourceAudioFormat
@@ -149,9 +150,13 @@ int FeedOneSample(VideoPlayerInstance* inst, IMFSourceReader* audioReader,
 
         UINT32 wantFrames = (std::min)(totalOutputFrames - outputDone, framesFree);
         if (wantFrames == 0) {
+            // Render buffer full. Wait on hAudioSamplesReadyEvent for the
+            // driver to free room — short timeout keeps us responsive to
+            // seek/shutdown signals.
             WaitForSingleObject(inst->hAudioSamplesReadyEvent.Get(), 5);
             if (FAILED(inst->pAudioClient->GetCurrentPadding(&framesPadding))) break;
-            framesFree = engineBufferFrames - framesPadding;
+            framesFree = (framesPadding < engineBufferFrames)
+                ? engineBufferFrames - framesPadding : 0;
             continue;
         }
 
@@ -206,7 +211,8 @@ int FeedOneSample(VideoPlayerInstance* inst, IMFSourceReader* audioReader,
         outputDone += wantFrames;
 
         if (FAILED(inst->pAudioClient->GetCurrentPadding(&framesPadding))) break;
-        framesFree = engineBufferFrames - framesPadding;
+        framesFree = (framesPadding < engineBufferFrames)
+            ? engineBufferFrames - framesPadding : 0;
     }
 
     if (needsResample) {
@@ -276,7 +282,7 @@ DWORD WINAPI AudioThreadProc(LPVOID lpParam) {
 } // namespace
 
 HRESULT InitWASAPI(VideoPlayerInstance* inst, const WAVEFORMATEX* srcFmt) {
-    if (!inst) return E_INVALIDARG;
+    if (!inst || !srcFmt) return E_INVALIDARG;
     if (inst->pAudioClient && inst->pRenderClient) {
         inst->bAudioInitialized = true;
         return S_OK;
@@ -285,74 +291,54 @@ HRESULT InitWASAPI(VideoPlayerInstance* inst, const WAVEFORMATEX* srcFmt) {
     IMMDeviceEnumerator* enumerator = MediaFoundation::GetDeviceEnumerator();
     if (!enumerator) return E_FAIL;
 
+    // RAII cleanup: released by name on the single success return.
+    struct CleanupGuard {
+        VideoPlayerInstance* inst;
+        bool armed = true;
+        ~CleanupGuard() {
+            if (!armed) return;
+            inst->pRenderClient.Reset();
+            inst->pAudioClient.Reset();
+            inst->pAudioEndpointVolume.Reset();
+            inst->pDevice.Reset();
+            inst->hAudioSamplesReadyEvent.Reset();
+            inst->bAudioInitialized = false;
+        }
+    } guard{inst};
+
     HRESULT hr = enumerator->GetDefaultAudioEndpoint(
         eRender, eConsole, inst->pDevice.ReleaseAndGetAddressOf());
     if (FAILED(hr)) return hr;
 
     hr = inst->pDevice->Activate(__uuidof(IAudioClient), CLSCTX_ALL, nullptr,
         reinterpret_cast<void**>(inst->pAudioClient.ReleaseAndGetAddressOf()));
-    if (FAILED(hr)) goto cleanup;
+    if (FAILED(hr)) return hr;
 
     hr = inst->pDevice->Activate(__uuidof(IAudioEndpointVolume), CLSCTX_ALL, nullptr,
         reinterpret_cast<void**>(inst->pAudioEndpointVolume.ReleaseAndGetAddressOf()));
-    if (FAILED(hr)) goto cleanup;
-
-    {
-        WAVEFORMATEX* mixFmt = nullptr;
-        const WAVEFORMATEX* useFmt = srcFmt;
-        if (!useFmt) {
-            hr = inst->pAudioClient->GetMixFormat(&mixFmt);
-            if (FAILED(hr)) goto cleanup;
-            useFmt = mixFmt;
-        }
-
-        size_t totalSize = sizeof(WAVEFORMATEX) + useFmt->cbSize;
-        inst->pSourceAudioFormat = static_cast<WAVEFORMATEX*>(CoTaskMemAlloc(totalSize));
-        if (!inst->pSourceAudioFormat) {
-            if (mixFmt) CoTaskMemFree(mixFmt);
-            hr = E_OUTOFMEMORY;
-            goto cleanup;
-        }
-        memcpy(inst->pSourceAudioFormat, useFmt, totalSize);
-        if (mixFmt) CoTaskMemFree(mixFmt);
-    }
+    if (FAILED(hr)) return hr;
 
     if (!inst->hAudioSamplesReadyEvent) {
         inst->hAudioSamplesReadyEvent.Reset(CreateEventW(nullptr, FALSE, FALSE, nullptr));
-        if (!inst->hAudioSamplesReadyEvent) {
-            hr = HRESULT_FROM_WIN32(GetLastError());
-            goto cleanup;
-        }
+        if (!inst->hAudioSamplesReadyEvent) return HRESULT_FROM_WIN32(GetLastError());
     }
 
     hr = inst->pAudioClient->Initialize(AUDCLNT_SHAREMODE_SHARED,
                                         AUDCLNT_STREAMFLAGS_EVENTCALLBACK,
                                         kTargetBufferDuration100ns, 0,
-                                        inst->pSourceAudioFormat, nullptr);
-    if (FAILED(hr)) goto cleanup;
+                                        srcFmt, nullptr);
+    if (FAILED(hr)) return hr;
 
     hr = inst->pAudioClient->SetEventHandle(inst->hAudioSamplesReadyEvent.Get());
-    if (FAILED(hr)) goto cleanup;
+    if (FAILED(hr)) return hr;
 
     hr = inst->pAudioClient->GetService(__uuidof(IAudioRenderClient),
         reinterpret_cast<void**>(inst->pRenderClient.ReleaseAndGetAddressOf()));
-    if (FAILED(hr)) goto cleanup;
+    if (FAILED(hr)) return hr;
 
     inst->bAudioInitialized = true;
+    guard.armed = false;
     return S_OK;
-
-cleanup:
-    inst->pRenderClient.Reset();
-    inst->pAudioClient.Reset();
-    inst->pAudioEndpointVolume.Reset();
-    inst->pDevice.Reset();
-    if (inst->pSourceAudioFormat) {
-        CoTaskMemFree(inst->pSourceAudioFormat);
-        inst->pSourceAudioFormat = nullptr;
-    }
-    inst->hAudioSamplesReadyEvent.Reset();
-    inst->bAudioInitialized = false;
-    return hr;
 }
 
 HRESULT PreFillAudioBuffer(VideoPlayerInstance* inst) {