Skip to content

fix(playback): resolve race condition to prevent OSD takeover#429

Merged
RadicalMuffinMan merged 2 commits into
Moonfin-Client:mainfrom
mattsigal:osd-fix
Jun 8, 2026
Merged

fix(playback): resolve race condition to prevent OSD takeover#429
RadicalMuffinMan merged 2 commits into
Moonfin-Client:mainfrom
mattsigal:osd-fix

Conversation

@mattsigal

Copy link
Copy Markdown
Contributor

resolve race condition to prevent OSD takeover

Summary

Resolves a race condition on desktop platforms using media_kit where the play/pause button state gets stuck on the Play icon and the OSD controls fail to auto-hide when video playback begins.

Related Issues

None - bug found during playback testing

  • Closes #
  • Fixes #
  • Related to #

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Performance improvement
  • UI/UX update
  • Documentation update
  • Build/CI change
  • Other (describe):

Changes Made

  • Implemented the _mergeWithStale<T> helper method in MediaKitPlayerBackend which merges the underlying streams with _player.stream.playlist events.
  • Updated playingStream and bufferingStream getters to utilize this helper to ensure they are re-evaluated and re-emitted once _isStale becomes false when media loads.

Platform

  • Android
  • iOS
  • macOS
  • Windows
  • Linux
  • All / Shared code (but only impact Desktop clients)

Testing

Describe how this change was tested.

  • Tested on emulator / simulator
  • Tested on physical device
  • Manual testing completed
  • Not tested (explain why):

Test Steps

  1. Build and run the Windows application.
  2. Play any video.
  3. Verify that the play button switches to the Pause icon once playback begins.
  4. Verify that the OSD correctly auto-hides after the inactivity delay.

Checklist

  • Code builds successfully
  • Code follows project style and conventions
  • No unnecessary commented-out code
  • No new warnings introduced

@RadicalMuffinMan RadicalMuffinMan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A getter like get playingStream => _mergeWithStale(...) re-runs its right-hand side every single time something reads playingStream. So each read builds a fresh StreamController and wires up its own two subscriptions to the player. Today the code reads it only once, so you never notice. But the moment two places read playingStream, you end up with two separate controllers both hooked into the same player, and the spare one just sits there listening forever because nothing ever tears it down.

late final is Dart's way of saying "build this once, then keep handing back the same thing." The first time the field gets read, Dart runs the initializer (the _mergeWithStale(...) call), saves the result, and from then on every read returns that saved value. So no matter how many times anyone touches playingStream, the stream is built a single time.

The getter now just returns that saved field (get playingStream => _playingStream), so nothing changes for the people using it. Callers still write backend.playingStream.listen(...) exactly like before.

Comment thread lib/playback/media_kit_player_backend.dart
Comment thread lib/playback/media_kit_player_backend.dart Outdated
Comment thread lib/playback/media_kit_player_backend.dart Outdated
Comment thread lib/playback/media_kit_player_backend.dart
mattsigal added 2 commits June 8, 2026 09:28
…ing streams

- Introduce _mergeWithStale helper to combine the native player stream with playlist updates.
- Ensure playingStream and bufferingStream correctly re-evaluate and emit their values once the media loads and _isStale becomes false.
- Resolves the play/pause button state getting stuck on the Play icon and prevents the OSD controls from getting stuck on screen.
@mattsigal

Copy link
Copy Markdown
Contributor Author

Fixes incorporated, thanks!

@RadicalMuffinMan RadicalMuffinMan merged commit 70664ac into Moonfin-Client:main Jun 8, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants