diff --git a/docs/linux-desktop.md b/docs/linux-desktop.md index a898bf1c..6c44442c 100644 --- a/docs/linux-desktop.md +++ b/docs/linux-desktop.md @@ -286,7 +286,7 @@ not rendered at all. | The seam | `lib/core/services/audio_output_device_service.dart` | | Linux implementation | `lib/core/services/linux_audio_output_device_service.dart` | | Platform split | `lib/core/services/platform_audio_output_device_service.dart` | -| Policy (restore, fallback, what is remembered) | `lib/features/settings/playback/audio_output_controller.dart` | +| Policy (restore, fallback, hotplug, what is remembered) | `lib/features/settings/playback/audio_output_controller.dart` | Four decisions worth knowing: @@ -351,6 +351,66 @@ desktop: | Pick a USB output, quit, unplug it, relaunch | Playback uses the system default and the card says the saved output is unavailable. | | Pick a USB output, quit, plug it back in, relaunch | Playback goes back to that output on its own. | +### Device hotplug + +Devices come and go while music is playing: headphones are unplugged, a +Bluetooth speaker drops out of range and comes back, an HDMI sink appears when a +monitor wakes, the desktop moves its default sink +([issue #403](https://github.com/thezupzup/linthra/issues/403)). The same seam +handles all of it — `AudioOutputDeviceService.deviceChanges` is libmpv's +`audio-device-list` *observed* rather than read once, and the rules for reacting +live in `AudioOutputController` next to the ones that already decide which +output is chosen and whether it is remembered. + +| The host does this | Linthra does this | +| --- | --- | +| A device appears | Adds it to the list. Playback is not moved. | +| An unrelated device disappears | Updates the list. Playback is not moved. | +| The system default moves, and nothing was chosen | Nothing. "System default" means the host decides, including when it changes its mind. | +| The chosen device is still listed | Nothing. libmpv carried playback through with no gap, and re-routing to a sink audio is already on would be an interruption caused purely by the recovery code. | +| The chosen device disappears | Falls back to the system default so audio stays audible, and the card says why it moved. The preference is **kept**. | +| The chosen device comes back | Hands playback back to it and clears the notice. | +| The fallback is refused too | Surfaces a recoverable "playback may be silent" state with a **Try again**, rather than leaving playback pointed at a sink that is not there. | + +Three properties are what keep this from causing the bugs it is meant to fix: + +* **Nothing is ever re-loaded.** Recovery is a *routing* decision: the queue, + the track and the position are untouched, and no second player is created, so + a device event cannot produce duplicate playback. +* **Exactly one subscription.** The controller opens one device-change + subscription in `build` and closes it with the notifier, and the Linux service + keeps at most one device-list listener per live player, keyed by the player + id. One unplug is handled once. +* **Nothing polls.** The service re-attaches when the engine rebuilds its + player, driven by the vendored plugin's `livePlayersChanged` signal + (`third_party/just_audio_media_kit/PATCHES.md`) rather than by a timer. + +**Memory remembers what disk forgets.** A saved output that was *never seen* on +this machine is dropped at launch — that is the "saved on another machine" rule +and it has not changed. But a device that has been playing this session and then +vanished is a hotplug, not a stale preference, so the preference survives it and +a reconnect restores the choice. Without that split, a Bluetooth dropout plus a +refresh would quietly lose what the listener picked. + +**When nothing is playing there are no events.** Watching needs a live player, +and Linthra will not create one just to watch — a diagnostics or settings screen +that spun up a second libmpv handle would be the next bug. There is nothing to +recover in that state either, because no audio is being interrupted; the next +play, or the card's Refresh, picks up whatever changed. + +`test/features/settings/playback/audio_output_hotplug_test.dart` drives every +row of the table above against a fake backend, including a flapping device over +repeated connect/disconnect cycles. On a real desktop: + +| Check | Expected | +| --- | --- | +| Play something on the built-in output, plug in wired headphones | Audio keeps playing; the new device appears in the list without playback moving. | +| Choose the headphones, then unplug them mid-track | Audio continues on the system default, the track does not restart, and the card explains the move. | +| Plug them back in | Playback returns to them on its own. | +| Choose a Bluetooth speaker, walk out of range and back | Same: fall back, then hand back, with no duplicated audio and no restart. | +| Choose an HDMI output, put the monitor to sleep, wake it | Same. | +| Change the desktop's default sink while playing on "System default" | Audio follows the desktop; Linthra does not fight it. | + libmpv provides broad codec/container support and PulseAudio/PipeWire output. It is a native runtime dependency, not a binary downloaded when Linthra starts. The Flatpak manifest therefore builds libmpv as a declared module and bundles @@ -489,6 +549,7 @@ loaded: | Local tag reading | Supported | `FilesystemLocalMetadataReader` reads title, artist, album artist, album, track number and duration from ID3, Vorbis comments, MP4 atoms, APEv2 and RIFF INFO through `audio_metadata_reader` ([issue #407](https://github.com/TheZupZup/Linthra/issues/407)). An unreadable or untagged file still appears, from its filename. Android is deliberately unchanged: its tags come from the native SAF walk. | | Local embedded artwork | Unsupported | Tags are read without pulling cover images out of every file during a scan. Extracting and caching embedded art on desktop is [issue #408](https://github.com/TheZupZup/Linthra/issues/408); tracks keep the placeholder until then. | | **Audio output device** | Supported | Settings → Music & playback → Audio output lists libmpv's `audio-device-list` and routes playback with `audio-device` ([issue #402](https://github.com/TheZupZup/Linthra/issues/402)). A saved device is re-applied at launch, and one that is no longer present falls back to the system default. See [Audio output device](#audio-output-device). | +| **Device hotplug** | Supported | A headset, Bluetooth speaker or HDMI sink appearing or disappearing mid-playback is recovered without restarting the track or creating a second player, and a chosen device that comes back takes playback back ([issue #403](https://github.com/thezupzup/linthra/issues/403)). If even the system default is refused, the card says so and offers a retry. See [Device hotplug](#device-hotplug). | | **Playback diagnostics** | Supported | Settings → Diagnostics & support → Linux playback builds a copyable report of the backend, libmpv, the selected output subsystem and recent failure kinds ([issue #406](https://github.com/thezupzup/linthra/issues/406)). Safe by construction: no field can hold a URL, token, header, path, device name or raw error. See [Playback diagnostics](#playback-diagnostics). | | Chromecast | Android/iOS only | Already gated in `cast_providers.dart`; Linux keeps the honest "cast unavailable" service. | | Share sheet, launcher-icon switching | Android-only, by design | No desktop equivalent; the UI simply omits them. | diff --git a/lib/core/services/audio_output_device_service.dart b/lib/core/services/audio_output_device_service.dart index bd022557..f503d67e 100644 --- a/lib/core/services/audio_output_device_service.dart +++ b/lib/core/services/audio_output_device_service.dart @@ -36,4 +36,24 @@ abstract interface class AudioOutputDeviceService { /// as done: it is the difference between remembering an output that is /// playing and remembering one that never started. Future select(AudioOutputDevice device); + + /// Emits the host's output list whenever the backend reports that it changed + /// — a headset plugged in or pulled out, a Bluetooth speaker connecting or + /// dropping, an HDMI sink appearing when a monitor wakes, the system default + /// moving. + /// + /// Each event is the full list in the same shape [devices] returns, so a + /// listener compares lists rather than reconstructing a diff from events it + /// might have missed. + /// + /// This is *observation only*: it starts, stops and re-routes nothing. What + /// to do about a device that vanished is policy, and it lives in + /// `AudioOutputController` — the same place that already owns which output + /// is chosen and whether it is remembered. + /// + /// A broadcast stream: several listeners are fine and none of them changes + /// what the others see. Implementations that cannot observe (every platform + /// but Linux) return an empty stream rather than throwing, so a caller never + /// has to ask whether watching is supported before listening. + Stream> get deviceChanges; } diff --git a/lib/core/services/linux_audio_output_device_service.dart b/lib/core/services/linux_audio_output_device_service.dart index aa27d349..6a462f20 100644 --- a/lib/core/services/linux_audio_output_device_service.dart +++ b/lib/core/services/linux_audio_output_device_service.dart @@ -13,6 +13,14 @@ typedef LinuxAudioDeviceProbe = Future> /// Writes a device name back to the backend. typedef LinuxAudioDeviceApply = Future Function(String deviceId); +/// Watches the backend's raw output list, re-attaching as players come and go. +/// +/// A seam so the *policy* around hotplug (see `AudioOutputController`) can be +/// tested without libmpv, and so the attachment strategy underneath can change +/// without the policy noticing. +typedef LinuxAudioDeviceWatch = Stream> + Function(); + /// Linux output-device routing, through media_kit/libmpv. /// /// libmpv already models exactly what this feature needs, so nothing here goes @@ -35,15 +43,42 @@ typedef LinuxAudioDeviceApply = Future Function(String deviceId); /// nothing is playing — libmpv reports `audio-device-list` on a fresh handle /// without ever opening an output, so listing outputs from Settings never makes /// a sound or grabs a device. +/// +/// ## Watching for hotplug +/// +/// [deviceChanges] is the same `audio-device-list` property, observed rather +/// than read once: libmpv republishes it when a headset is plugged in, a +/// Bluetooth sink connects or drops, an HDMI output appears, or the system +/// default moves. Three rules keep that from becoming a source of bugs of its +/// own: +/// +/// * **One subscription per live player, ever.** The service keys them by the +/// just_audio player id, so re-attaching is idempotent and a device event +/// can never be delivered twice — which is what would turn one hotplug into +/// two recovery attempts, and one recovery into duplicate playback. +/// * **No player is created to watch.** Enumeration may build a throwaway +/// handle; watching never does. When nothing is playing there is no player, +/// so there are no events — and nothing to recover either, because no audio +/// is being interrupted. +/// * **It re-attaches on a signal, not on a timer.** The engine tears a player +/// down and builds a new one on a stop, on suspend/resume and on some source +/// switches, so a listener attached to one player has to follow. The +/// vendored plugin publishes `livePlayersChanged` for exactly this +/// (`third_party/just_audio_media_kit/PATCHES.md`), so nothing polls. class LinuxAudioOutputDeviceService implements AudioOutputDeviceService { LinuxAudioOutputDeviceService({ LinuxAudioDeviceProbe? probe, LinuxAudioDeviceApply? apply, + LinuxAudioDeviceWatch? watch, }) : _probe = probe ?? _probeThroughMediaKit, - _apply = apply ?? _applyThroughMediaKit; + _apply = apply ?? _applyThroughMediaKit, + _watch = watch ?? _watchThroughMediaKit; final LinuxAudioDeviceProbe _probe; final LinuxAudioDeviceApply _apply; + final LinuxAudioDeviceWatch _watch; + + Stream>? _deviceChanges; /// How long libmpv gets to report its device list before Linthra gives up. /// @@ -65,6 +100,24 @@ class LinuxAudioOutputDeviceService implements AudioOutputDeviceService { } } + /// The host's output list, re-emitted whenever the backend reports a change. + /// + /// Built once and shared: the underlying watch attaches to the backend on the + /// first listen and detaches on the last, so a second listener costs nothing + /// and a page nobody opened holds no subscription at all. + /// + /// A list that could not be read is dropped rather than emitted as empty — + /// "the backend did not answer" is not the same as "this machine has no + /// outputs", and a policy that acted on the difference would route playback + /// away from a device the listener is still using. + @override + Stream> get deviceChanges { + return _deviceChanges ??= _watch() + .map(audioOutputDevicesFromBackend) + .handleError((Object _) {}) + .asBroadcastStream(); + } + @override Future select(AudioOutputDevice device) async { try { @@ -79,6 +132,61 @@ class LinuxAudioOutputDeviceService implements AudioOutputDeviceService { } } + /// Follows libmpv's `audio-device-list` across the players the engine + /// creates and destroys. + /// + /// The bookkeeping is deliberately boring: a map of player id → subscription, + /// re-synced on every `livePlayersChanged` event and on the first listen. + /// Attaching is idempotent (a player already in the map is skipped) and + /// detaching is total (the last listener leaves nothing behind), which + /// together are what keep one hotplug from being seen twice. + static Stream> + _watchThroughMediaKit() { + final Map>> attached = + >>{}; + StreamSubscription? registry; + late StreamController> controller; + + void sync() { + final Map live = JustAudioMediaKit.livePlayers; + for (final MapEntry entry in live.entries) { + if (attached.containsKey(entry.key)) continue; + attached[entry.key] = entry.value.stream.audioDevices.listen( + (List devices) { + if (_isUnpopulated(devices)) return; + controller.add(<({String id, String description})>[ + for (final AudioDevice device in devices) + (id: device.name, description: device.description), + ]); + }, + onError: (Object _) {}, + ); + } + for (final String id in attached.keys.toList()) { + if (live.containsKey(id)) continue; + unawaited(attached.remove(id)?.cancel()); + } + } + + controller = StreamController>( + onListen: () { + registry = JustAudioMediaKit.livePlayersChanged.stream + .listen((void _) => sync()); + sync(); + }, + onCancel: () async { + await registry?.cancel(); + registry = null; + for (final StreamSubscription> subscription + in attached.values.toList()) { + await subscription.cancel(); + } + attached.clear(); + }, + ); + return controller.stream; + } + static Future> _probeThroughMediaKit() async { final Iterable live = JustAudioMediaKit.livePlayers.values; diff --git a/lib/core/services/noop_audio_output_device_service.dart b/lib/core/services/noop_audio_output_device_service.dart index 215d75a1..49136ba1 100644 --- a/lib/core/services/noop_audio_output_device_service.dart +++ b/lib/core/services/noop_audio_output_device_service.dart @@ -20,4 +20,10 @@ class NoopAudioOutputDeviceService implements AudioOutputDeviceService { @override Future select(AudioOutputDevice device) async => false; + + /// Nothing to observe: an empty stream that closes immediately, so a listener + /// is never left waiting on events that cannot come. + @override + Stream> get deviceChanges => + const Stream>.empty(); } diff --git a/lib/core/services/platform_audio_output_device_service.dart b/lib/core/services/platform_audio_output_device_service.dart index 8ab98b5e..23d3f0c6 100644 --- a/lib/core/services/platform_audio_output_device_service.dart +++ b/lib/core/services/platform_audio_output_device_service.dart @@ -46,4 +46,7 @@ class PlatformAudioOutputDeviceService implements AudioOutputDeviceService { @override Future select(AudioOutputDevice device) => _delegate.select(device); + + @override + Stream> get deviceChanges => _delegate.deviceChanges; } diff --git a/lib/features/settings/playback/audio_output_controller.dart b/lib/features/settings/playback/audio_output_controller.dart index 59133aca..bccce9b0 100644 --- a/lib/features/settings/playback/audio_output_controller.dart +++ b/lib/features/settings/playback/audio_output_controller.dart @@ -1,5 +1,6 @@ import 'dart:async'; +import 'package:flutter/foundation.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../../../core/models/audio_output_device.dart'; @@ -15,6 +16,7 @@ class AudioOutputSettingsState { this.hasEnumerated = false, this.savedDeviceUnavailable = false, this.selectionFailed = false, + this.outputRecoveryFailed = false, }); /// The outputs the host offers, system default first. Empty either because @@ -37,6 +39,20 @@ class AudioOutputSettingsState { /// playback is still on [selected]. Cleared by the next successful choice. final bool selectionFailed; + /// Whether an output disappeared *and* the fall back to the system default + /// was refused too, so audio may not be coming out anywhere. + /// + /// This is the "recovery is impossible" state the hotplug work has to surface + /// rather than leave silently muted (#403). It is recoverable, not terminal: + /// the card offers a retry, and the next device change or refresh clears it + /// on its own if the backend comes back. + final bool outputRecoveryFailed; + + /// Whether playback is currently somewhere other than the output the + /// listener asked for. What the card shows a "your output is not available" + /// note for. + bool get isUsingFallback => savedDeviceUnavailable; + /// Whether [selected] will still be in effect after a restart. False for the /// system default (there is nothing to remember) and for devices named by an /// unstable handle, which Linthra deliberately does not store. @@ -48,6 +64,7 @@ class AudioOutputSettingsState { bool? hasEnumerated, bool? savedDeviceUnavailable, bool? selectionFailed, + bool? outputRecoveryFailed, }) { return AudioOutputSettingsState( devices: devices ?? this.devices, @@ -56,6 +73,7 @@ class AudioOutputSettingsState { savedDeviceUnavailable: savedDeviceUnavailable ?? this.savedDeviceUnavailable, selectionFailed: selectionFailed ?? this.selectionFailed, + outputRecoveryFailed: outputRecoveryFailed ?? this.outputRecoveryFailed, ); } } @@ -79,6 +97,37 @@ class AudioOutputSettingsState { /// [build] is deliberately cheap when nothing is stored: it does not enumerate, /// so launching the app never probes the audio backend just to confirm the /// default. The Settings card calls [refresh] when it is opened. +/// +/// ## Hotplug (#403) +/// +/// It also owns what happens when the *host* changes an output underneath +/// playback — a headset unplugged, a Bluetooth speaker dropping and coming +/// back, an HDMI sink appearing when a monitor wakes, the system default +/// moving. [AudioOutputDeviceService.deviceChanges] reports those; the rules +/// for reacting live here, next to the ones above, because they are the same +/// decision seen from the other side. +/// +/// The rules, in the order [onDevicesChanged] applies them: +/// +/// * **The chosen output is still listed → do nothing.** libmpv keeps playing +/// through a sink that is still there, including across a default-sink +/// change, and re-routing to a device audio is already on would be an +/// audible interruption caused entirely by the recovery code. +/// * **It is listed again after being lost → take it back.** This is the +/// Bluetooth reconnect case, and it is why a hotplug loss deliberately does +/// *not* forget the stored preference. +/// * **It is gone → fall back to the system default and say so.** Playback +/// keeps going somewhere audible, and the card explains why it moved. The +/// preference is kept, so the device coming back restores it. +/// * **The fall back was refused too → surface it.** +/// [AudioOutputSettingsState.outputRecoveryFailed] with a retry, rather than +/// leaving playback silently pointed at a sink that is not there. +/// +/// Nothing here starts, stops or reloads playback: recovery is a *routing* +/// decision, so a device event can never produce a second stream. And the +/// device-change subscription is a single one, opened in [build] and closed +/// with the notifier, so a rebuild cannot leave two of them reacting to one +/// unplug. class AudioOutputController extends AsyncNotifier { /// The output currently pushed at the backend. It starts as the system /// default because that is what a fresh process is already on, and it is what @@ -87,6 +136,24 @@ class AudioOutputController extends AsyncNotifier { /// retried rather than assumed done. String _appliedId = AudioOutputDevice.systemDefaultId; + /// The output the listener actually wants, remembered for the session even + /// while it is unplugged. + /// + /// Deliberately separate from the *stored* preference. Disk forgets a device + /// that was not there at startup — that is the "saved on another machine" + /// rule from #402 and it stays — but a device that has been seen working this + /// session and then vanished is a hotplug, not a stale preference, so memory + /// keeps it and [onDevicesChanged] can hand playback back when it returns. + String? _desiredId; + + /// Whether [_desiredId] has been observed present since launch. What tells a + /// hotplug loss apart from a preference that never applied here. + bool _desiredSeen = false; + + /// The live device-change subscription, or null when nothing is being + /// watched. Exactly one is ever held. + StreamSubscription>? _deviceChanges; + /// Serializes [select] and [refresh]. /// /// Both route audio, write the preference and publish state. Two of them in @@ -101,6 +168,8 @@ class AudioOutputController extends AsyncNotifier { ref.read(audioOutputDeviceServiceProvider); if (!service.isSupported) return const AudioOutputSettingsState(); + _watchDevices(service); + final String? storedId = await ref.read(playbackPreferencesProvider).audioOutputDeviceId(); if (storedId == null) return const AudioOutputSettingsState(); @@ -109,10 +178,114 @@ class AudioOutputController extends AsyncNotifier { return _resolve(storedId, AudioOutputDevice.systemDefault); } + /// Opens the one device-change subscription, closing it with the notifier. + /// + /// Idempotent by construction — it is called once, from [build], and a + /// rebuild disposes the previous notifier first — but the null check makes + /// that explicit: two subscriptions would mean one unplug handled twice. + void _watchDevices(AudioOutputDeviceService service) { + if (_deviceChanges != null) return; + _deviceChanges = service.deviceChanges.listen( + onDevicesChanged, + // A backend that errors out of its own watch is not a reason to take + // playback anywhere; the list simply stops updating until a refresh. + onError: (Object _) {}, + ); + ref.onDispose(() { + final StreamSubscription>? subscription = + _deviceChanges; + _deviceChanges = null; + subscription?.cancel(); + }); + } + + /// Applies the hotplug rules to a fresh device list from the backend. + /// + /// See the class doc for the rules and why each one is what it is. Queued + /// behind any in-flight select/refresh for the same reason those queue behind + /// each other: two routing decisions in flight can complete in the wrong + /// order and leave playback on the one that was decided first. + /// + /// Visible for the tests that drive the lifecycle directly; the running app + /// reaches it only through [_watchDevices]. + @visibleForTesting + Future onDevicesChanged(List devices) { + return _enqueue(() async { + if (devices.isEmpty) return; + final AudioOutputSettingsState current = + state.valueOrNull ?? const AudioOutputSettingsState(); + final String desired = _desiredId ?? AudioOutputDevice.systemDefaultId; + + AudioOutputDevice? present; + for (final AudioOutputDevice device in devices) { + if (device.id == desired) present = device; + } + + if (desired == AudioOutputDevice.systemDefaultId) { + // Nothing was chosen, so the system default is the choice and it + // follows the host by itself — including when the host moves it. + // Refresh the list and leave playback exactly where it is. + state = AsyncData( + current.copyWith(devices: devices, hasEnumerated: true), + ); + return; + } + + if (present != null) { + _desiredSeen = true; + if (_appliedId == present.id) { + // Still on it: the backend carried playback through the change with + // no gap, which is the outcome worth protecting. Nothing to re-route. + state = AsyncData( + current.copyWith( + devices: devices, + hasEnumerated: true, + savedDeviceUnavailable: false, + outputRecoveryFailed: false, + ), + ); + return; + } + // It is back after being lost — a Bluetooth speaker reconnecting, a + // monitor waking its HDMI sink. Hand playback back to it. + final bool routed = await _apply(present); + state = AsyncData( + current.copyWith( + devices: devices, + hasEnumerated: true, + selected: routed ? present : current.selected, + savedDeviceUnavailable: !routed, + outputRecoveryFailed: false, + selectionFailed: false, + ), + ); + if (routed) await _rememberIfStable(present); + return; + } + + // The chosen output is gone. Keep audio audible on the system default and + // explain the move; the preference is deliberately kept so the device + // coming back restores it. + final bool recovered = await _apply(AudioOutputDevice.systemDefault); + state = AsyncData( + current.copyWith( + devices: devices, + hasEnumerated: true, + selected: + recovered ? AudioOutputDevice.systemDefault : current.selected, + savedDeviceUnavailable: true, + outputRecoveryFailed: !recovered, + ), + ); + }); + } + /// Re-reads the host's output list and re-applies the current choice. /// /// Called when the Settings card is opened and by its refresh action, so a - /// device plugged in while the app was running shows up. + /// device plugged in while the app was running shows up. Also the retry + /// behind [AudioOutputSettingsState.outputRecoveryFailed]: it re-enumerates + /// and re-applies, which is exactly what a failed recovery needs. Future refresh() async { final AudioOutputDeviceService service = ref.read(audioOutputDeviceServiceProvider); @@ -174,19 +347,30 @@ class AudioOutputController extends AsyncNotifier { return; } - await ref.read(playbackPreferencesProvider).setAudioOutputDeviceId( - AudioOutputDevice.isPersistableId(device.id) ? device.id : null, - ); + // The listener has said what they want. Remembered for the session even + // if the device later disappears, so a reconnect can hand playback back. + _desiredId = device.id; + _desiredSeen = true; + await _rememberIfStable(device); state = AsyncData( current.copyWith( selected: device, savedDeviceUnavailable: false, selectionFailed: false, + outputRecoveryFailed: false, ), ); }); } + /// Persists [device] when its id will still mean the same sink after a + /// reboot, and clears the stored value when it will not. + Future _rememberIfStable(AudioOutputDevice device) { + return ref.read(playbackPreferencesProvider).setAudioOutputDeviceId( + AudioOutputDevice.isPersistableId(device.id) ? device.id : null, + ); + } + /// Runs [operation] after every operation already queued. Future _enqueue(Future Function() operation) { final Future queued = _operations.then((_) => operation()); @@ -215,6 +399,9 @@ class AudioOutputController extends AsyncNotifier { String desiredId, AudioOutputDevice currentSelection, ) async { + if (desiredId != AudioOutputDevice.systemDefaultId) { + _desiredId = desiredId; + } final AudioOutputDeviceService service = ref.read(audioOutputDeviceServiceProvider); final List devices = await service.devices(); @@ -233,6 +420,7 @@ class AudioOutputController extends AsyncNotifier { for (final AudioOutputDevice device in devices) { if (device.id != desiredId) continue; + _desiredSeen = true; final bool routed = await _apply(device); return AudioOutputSettingsState( devices: devices, @@ -243,11 +431,18 @@ class AudioOutputController extends AsyncNotifier { } // The chosen output is not on this machine any more. Fall back to the - // system default and forget it, so the next launch does not keep trying a - // name that no longer means anything. + // system default, and forget it *unless* it has been working this session: + // an id that was never seen here is a preference saved on another machine + // and the next launch should not keep trying it, but one that played five + // minutes ago is an unplugged headset, and forgetting it would mean a + // refresh during a Bluetooth dropout quietly loses the listener's choice. final bool wasStored = desiredId != AudioOutputDevice.systemDefaultId; if (wasStored) { - await ref.read(playbackPreferencesProvider).setAudioOutputDeviceId(null); + if (!_desiredSeen) { + await ref + .read(playbackPreferencesProvider) + .setAudioOutputDeviceId(null); + } await _apply(AudioOutputDevice.systemDefault); } return AudioOutputSettingsState( diff --git a/lib/features/settings/playback/audio_output_settings_section.dart b/lib/features/settings/playback/audio_output_settings_section.dart index c5ccd482..cb56505b 100644 --- a/lib/features/settings/playback/audio_output_settings_section.dart +++ b/lib/features/settings/playback/audio_output_settings_section.dart @@ -95,12 +95,33 @@ class _AudioOutputSettingsSectionState ), const SizedBox(height: AppSpacing.md), _OutputPicker(state: state, isBusy: async.isLoading), - if (state.savedDeviceUnavailable) ...[ + if (state.outputRecoveryFailed) ...[ + const SizedBox(height: AppSpacing.sm), + // The one case where playback may not be coming out anywhere: + // the chosen output went away and the system default was refused + // too. Recoverable, and said so — never left silently muted. + _Note( + icon: Icons.volume_off_outlined, + text: 'The audio output went away and Linthra could not fall ' + 'back to the system default, so playback may be silent. ' + 'Try again once your device is back.', + color: theme.colorScheme.error, + action: ( + label: 'Try again', + onPressed: async.isLoading + ? null + : () => ref + .read(audioOutputControllerProvider.notifier) + .refresh(), + ), + ), + ] else if (state.savedDeviceUnavailable) ...[ const SizedBox(height: AppSpacing.sm), _Note( icon: Icons.info_outline, text: 'Your saved output is not available right now, so ' - 'playback is using the system default.', + 'playback is using the system default. It will be used ' + 'again as soon as it is back.', color: theme.colorScheme.tertiary, ), ], @@ -199,24 +220,47 @@ class _OutputPicker extends ConsumerWidget { } class _Note extends StatelessWidget { - const _Note({required this.icon, required this.text, required this.color}); + const _Note({ + required this.icon, + required this.text, + required this.color, + this.action, + }); final IconData icon; final String text; final Color color; + /// An optional way out of the state the note describes — the retry a failed + /// recovery needs, so "playback may be silent" is never a dead end. + final ({String label, VoidCallback? onPressed})? action; + @override Widget build(BuildContext context) { final ThemeData theme = Theme.of(context); + final ({String label, VoidCallback? onPressed})? action = this.action; return Row( crossAxisAlignment: CrossAxisAlignment.start, children: [ Icon(icon, size: 16, color: color), const SizedBox(width: AppSpacing.sm), Expanded( - child: Text( - text, - style: theme.textTheme.bodySmall?.copyWith(color: color), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + text, + style: theme.textTheme.bodySmall?.copyWith(color: color), + ), + if (action != null) + Align( + alignment: Alignment.centerLeft, + child: TextButton( + onPressed: action.onPressed, + child: Text(action.label), + ), + ), + ], ), ), ], diff --git a/test/core/services/platform_audio_output_device_service_test.dart b/test/core/services/platform_audio_output_device_service_test.dart index ad0d1440..4cb2a798 100644 --- a/test/core/services/platform_audio_output_device_service_test.dart +++ b/test/core/services/platform_audio_output_device_service_test.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:flutter_test/flutter_test.dart'; import 'package:linthra/core/models/audio_output_device.dart'; import 'package:linthra/core/platform/host_platform.dart'; @@ -23,6 +25,12 @@ class _RecordingService implements AudioOutputDeviceService { selected = device; return true; } + + final StreamController> changes = + StreamController>.broadcast(); + + @override + Stream> get deviceChanges => changes.stream; } void main() { @@ -45,6 +53,13 @@ void main() { await service.select(headset); expect(linux.selected, headset); expect(fallback.selected, isNull); + // Hotplug watching follows the same split: a Linux build observes the + // real backend, and the fallback is never subscribed to. + final Future> watched = + service.deviceChanges.first; + linux.changes.add(const [headset]); + expect(await watched, const [headset]); + expect(fallback.changes.hasListener, isFalse); }); test('on Android, output routing stays with the system', () async { @@ -57,6 +72,8 @@ void main() { expect(service.isSupported, isFalse); expect(await service.devices(), isEmpty); + expect(await service.deviceChanges.toList(), isEmpty, + reason: 'Android has nothing to watch: the system owns routing'); await service.select(headset); expect(linux.selected, isNull); }); diff --git a/test/features/settings/diagnostics/linux_playback_diagnostics_collector_test.dart b/test/features/settings/diagnostics/linux_playback_diagnostics_collector_test.dart index 10ed0195..02dd38d8 100644 --- a/test/features/settings/diagnostics/linux_playback_diagnostics_collector_test.dart +++ b/test/features/settings/diagnostics/linux_playback_diagnostics_collector_test.dart @@ -49,6 +49,13 @@ class _FakeOutputService implements AudioOutputDeviceService { @override Future select(AudioOutputDevice device) async => true; + + /// Hotplug watching (#403). The diagnostics collector never listens, but the + /// seam requires it, and an empty stream is the honest answer for a fake that + /// nothing plugs into. + @override + Stream> get deviceChanges => + const Stream>.empty(); } const AudioOutputDevice _usbDac = AudioOutputDevice( diff --git a/test/features/settings/playback/audio_output_controller_test.dart b/test/features/settings/playback/audio_output_controller_test.dart index eb76fb07..bdc05fea 100644 --- a/test/features/settings/playback/audio_output_controller_test.dart +++ b/test/features/settings/playback/audio_output_controller_test.dart @@ -50,6 +50,26 @@ class _FakeService implements AudioOutputDeviceService { routed.add(device); return true; } + + /// The hotplug channel (#403): the test pushes a device list the way libmpv + /// republishes `audio-device-list` when hardware comes or goes. + final StreamController> changes = + StreamController>.broadcast(); + + int deviceChangeListeners = 0; + + @override + Stream> get deviceChanges { + deviceChangeListeners++; + return changes.stream; + } + + /// Publishes [devices] as the host's new output list and also makes them what + /// a later enumeration reports, the way a real unplug does both. + void hotplug(List devices) { + available = devices; + changes.add(devices); + } } void main() { diff --git a/test/features/settings/playback/audio_output_hotplug_test.dart b/test/features/settings/playback/audio_output_hotplug_test.dart new file mode 100644 index 00000000..7e1e6ab5 --- /dev/null +++ b/test/features/settings/playback/audio_output_hotplug_test.dart @@ -0,0 +1,382 @@ +import 'dart:async'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:linthra/core/models/audio_output_device.dart'; +import 'package:linthra/core/services/audio_output_device_service.dart'; +import 'package:linthra/data/repositories/audio_output_device_service_provider.dart'; +import 'package:linthra/data/repositories/in_memory_playback_preferences.dart'; +import 'package:linthra/data/repositories/playback_preferences_provider.dart'; +import 'package:linthra/features/settings/playback/audio_output_controller.dart'; + +/// Linux audio-device hotplug recovery (#403). +/// +/// Everything here drives the *policy*: the backend is a fake that reports +/// whatever device list the scenario says the host has, so a headphone unplug, +/// a Bluetooth dropout and reconnect, an HDMI sink appearing and a system +/// default moving are all deterministic and run on any machine. + +/// A stand-in for libmpv that can also republish its device list. +class _FakeService implements AudioOutputDeviceService { + _FakeService({this.isSupported = true, List? devices}) + : available = devices ?? []; + + @override + final bool isSupported; + + List available; + final List routed = []; + + /// Device ids the backend refuses to route to, standing in for a sink that + /// is listed but cannot be opened. + final Set refuses = {}; + + int enumerations = 0; + int listenCount = 0; + + final StreamController> _changes = + StreamController>.broadcast(); + + @override + Future> devices() async { + enumerations++; + return available; + } + + @override + Future select(AudioOutputDevice device) async { + if (refuses.contains(device.id)) return false; + routed.add(device); + return true; + } + + @override + Stream> get deviceChanges { + listenCount++; + return _changes.stream; + } + + /// The host's outputs changed: the new list is what enumeration reports from + /// now on *and* what the backend republishes, the way a real unplug does + /// both. + void hotplug(List devices) { + available = devices; + _changes.add(devices); + } + + /// Whether anything is currently watching, so a test can assert the + /// subscription is actually released. + bool get isWatched => _changes.hasListener; + + Future dispose() => _changes.close(); +} + +const AudioOutputDevice _builtIn = AudioOutputDevice( + id: 'pipewire/alsa_output.pci-0000_00_1f.3.analog-stereo', + label: 'Built-in Audio', +); +const AudioOutputDevice _headphones = AudioOutputDevice( + id: 'pipewire/alsa_output.pci-0000_00_1f.3.analog-stereo.headphones', + label: 'Headphones', +); +const AudioOutputDevice _bluetooth = AudioOutputDevice( + id: 'pipewire/bluez_output.AC_12_2F_3D_4E_5F.1', + label: 'Living room speaker', +); +const AudioOutputDevice _hdmi = AudioOutputDevice( + id: 'pipewire/alsa_output.pci-0000_01_00.1.hdmi-stereo', + label: 'HDMI Audio', +); + +List _hostWith(List devices) => + [AudioOutputDevice.systemDefault, ...devices]; + +void main() { + late _FakeService service; + late InMemoryPlaybackPreferences preferences; + late ProviderContainer container; + + /// Builds a controller already routed to [chosen], the way it is after the + /// listener picks an output in Settings. + Future chooseOutput(AudioOutputDevice chosen) async { + await container.read(audioOutputControllerProvider.future); + await container.read(audioOutputControllerProvider.notifier).select(chosen); + } + + AudioOutputSettingsState currentState() => + container.read(audioOutputControllerProvider).valueOrNull ?? + const AudioOutputSettingsState(); + + Future hotplug(List devices) async { + service.hotplug(devices); + // Let the broadcast reach the controller and its queued work settle. + await pumpEventQueue(); + } + + void setUpWith({ + List? devices, + String? savedDeviceId, + }) { + service = + _FakeService(devices: devices ?? _hostWith([])); + preferences = + InMemoryPlaybackPreferences(audioOutputDeviceId: savedDeviceId); + container = ProviderContainer( + overrides: [ + audioOutputDeviceServiceProvider.overrideWithValue(service), + playbackPreferencesProvider.overrideWithValue(preferences), + ], + ); + addTearDown(container.dispose); + addTearDown(service.dispose); + } + + group('the watch itself', () { + test( + 'exactly one subscription is opened, and it is closed with the ' + 'notifier', () async { + setUpWith(); + await container.read(audioOutputControllerProvider.future); + expect(service.listenCount, 1); + + // A second read must not open another: two subscriptions would mean one + // unplug handled twice, and one recovery becoming two. + await container.read(audioOutputControllerProvider.future); + expect(service.listenCount, 1); + + expect(service.isWatched, isTrue); + container.dispose(); + await pumpEventQueue(); + expect(service.isWatched, isFalse, + reason: 'a disposed notifier must not keep reacting to hotplug'); + }); + + test('an unsupported backend is never watched', () async { + service = _FakeService(isSupported: false); + preferences = InMemoryPlaybackPreferences(); + container = ProviderContainer( + overrides: [ + audioOutputDeviceServiceProvider.overrideWithValue(service), + playbackPreferencesProvider.overrideWithValue(preferences), + ], + ); + addTearDown(container.dispose); + addTearDown(service.dispose); + + await container.read(audioOutputControllerProvider.future); + expect(service.listenCount, 0); + }); + }); + + group('playback continues transparently where it can', () { + test('a device appearing does not move playback', () async { + setUpWith(devices: _hostWith([_builtIn])); + await chooseOutput(_builtIn); + final int routedBefore = service.routed.length; + + // A monitor wakes and its HDMI sink shows up. + await hotplug(_hostWith([_builtIn, _hdmi])); + + expect(service.routed.length, routedBefore, + reason: 'nothing was re-routed: audio never had to move'); + expect(currentState().selected, _builtIn); + expect(currentState().devices, contains(_hdmi)); + expect(currentState().savedDeviceUnavailable, isFalse); + }); + + test('an unrelated device disappearing does not move playback', () async { + setUpWith(devices: _hostWith([_builtIn, _hdmi])); + await chooseOutput(_builtIn); + final int routedBefore = service.routed.length; + + await hotplug(_hostWith([_builtIn])); + + expect(service.routed.length, routedBefore); + expect(currentState().selected, _builtIn); + expect(currentState().savedDeviceUnavailable, isFalse); + }); + + test('the system default moving is the system\'s business, not ours', + () async { + // Nothing was chosen, so playback follows the host by itself. Re-routing + // here would be an audible interruption caused only by the recovery code. + setUpWith(devices: _hostWith([_builtIn])); + await container.read(audioOutputControllerProvider.future); + + await hotplug(_hostWith([_builtIn, _headphones])); + + expect(service.routed, isEmpty); + expect(currentState().selected, AudioOutputDevice.systemDefault); + expect(currentState().devices, contains(_headphones)); + }); + }); + + group('the selected output going away', () { + test('wired headphones unplugged: playback falls back and says so', + () async { + setUpWith(devices: _hostWith([_builtIn, _headphones])); + await chooseOutput(_headphones); + + await hotplug(_hostWith([_builtIn])); + + expect(service.routed.last, AudioOutputDevice.systemDefault); + expect(currentState().selected, AudioOutputDevice.systemDefault); + expect(currentState().savedDeviceUnavailable, isTrue); + expect(currentState().outputRecoveryFailed, isFalse); + }); + + test('the preference is kept, so the device coming back can restore it', + () async { + setUpWith(devices: _hostWith([_builtIn, _bluetooth])); + await chooseOutput(_bluetooth); + expect(await preferences.audioOutputDeviceId(), _bluetooth.id); + + await hotplug(_hostWith([_builtIn])); + + expect(await preferences.audioOutputDeviceId(), _bluetooth.id, + reason: 'a dropout is not a reason to forget the listener\'s choice'); + }); + + test('a refused fallback is surfaced, not left silently muted', () async { + setUpWith(devices: _hostWith([_builtIn, _headphones])); + await chooseOutput(_headphones); + service.refuses.add(AudioOutputDevice.systemDefaultId); + + await hotplug(_hostWith([_builtIn])); + + expect(currentState().outputRecoveryFailed, isTrue); + expect(currentState().savedDeviceUnavailable, isTrue); + // Playback is still shown on the output it was on, because that is where + // the backend left it. + expect(currentState().selected, _headphones); + }); + + test('retrying after the backend recovers clears the failure', () async { + setUpWith(devices: _hostWith([_builtIn, _headphones])); + await chooseOutput(_headphones); + service.refuses.add(AudioOutputDevice.systemDefaultId); + await hotplug(_hostWith([_builtIn])); + expect(currentState().outputRecoveryFailed, isTrue); + + service.refuses.clear(); + await hotplug(_hostWith([_builtIn])); + + expect(currentState().outputRecoveryFailed, isFalse); + expect(currentState().selected, AudioOutputDevice.systemDefault); + }); + }); + + group('reconnecting', () { + test('a Bluetooth speaker coming back takes playback back', () async { + setUpWith(devices: _hostWith([_builtIn, _bluetooth])); + await chooseOutput(_bluetooth); + + await hotplug(_hostWith([_builtIn])); + expect(currentState().selected, AudioOutputDevice.systemDefault); + + await hotplug(_hostWith([_builtIn, _bluetooth])); + + expect(currentState().selected, _bluetooth); + expect(currentState().savedDeviceUnavailable, isFalse); + expect(currentState().outputRecoveryFailed, isFalse); + }); + + test('a reconnect routes once, never twice', () async { + setUpWith(devices: _hostWith([_builtIn, _bluetooth])); + await chooseOutput(_bluetooth); + await hotplug(_hostWith([_builtIn])); + await hotplug(_hostWith([_builtIn, _bluetooth])); + + final int routesBack = service.routed + .where((AudioOutputDevice d) => d.id == _bluetooth.id) + .length; + // Once when the listener chose it, once when it came back. Not more. + expect(routesBack, 2); + + // A repeat of the same list changes nothing at all. + final int before = service.routed.length; + await hotplug(_hostWith([_builtIn, _bluetooth])); + expect(service.routed.length, before); + }); + + test('a flapping device survives repeated cycles', () async { + setUpWith(devices: _hostWith([_builtIn, _bluetooth])); + await chooseOutput(_bluetooth); + + for (int i = 0; i < 5; i++) { + await hotplug(_hostWith([_builtIn])); + expect(currentState().selected, AudioOutputDevice.systemDefault); + expect(currentState().savedDeviceUnavailable, isTrue); + + await hotplug(_hostWith([_builtIn, _bluetooth])); + expect(currentState().selected, _bluetooth); + expect(currentState().savedDeviceUnavailable, isFalse); + } + + expect(service.listenCount, 1, reason: 'still one subscription'); + expect(await preferences.audioOutputDeviceId(), _bluetooth.id); + }); + }); + + group('the list the UI shows follows the host', () { + test('every change republishes the real device list', () async { + setUpWith(devices: _hostWith([_builtIn])); + await chooseOutput(_builtIn); + + await hotplug( + _hostWith([_builtIn, _hdmi, _bluetooth])); + expect( + currentState().devices, + _hostWith([ + _builtIn, + _hdmi, + _bluetooth, + ])); + expect(currentState().hasEnumerated, isTrue); + + await hotplug(_hostWith([_builtIn])); + expect(currentState().devices, _hostWith([_builtIn])); + }); + + test('an empty report is not treated as "every output is gone"', () async { + // A backend that could not be asked is not evidence the sink vanished; + // routing away on it would move audio the listener is still using. + setUpWith(devices: _hostWith([_builtIn, _headphones])); + await chooseOutput(_headphones); + final int routedBefore = service.routed.length; + + await hotplug(const []); + + expect(service.routed.length, routedBefore); + expect(currentState().selected, _headphones); + expect(currentState().savedDeviceUnavailable, isFalse); + }); + }); + + group('startup rules from #402 still hold', () { + test('a saved device never seen here is still forgotten', () async { + // The "saved on another machine" rule: nothing has played on it this + // session, so the preference is dropped rather than retried every launch. + setUpWith( + devices: _hostWith([_builtIn]), + savedDeviceId: _headphones.id, + ); + + await container.read(audioOutputControllerProvider.future); + + expect(await preferences.audioOutputDeviceId(), isNull); + expect(currentState().savedDeviceUnavailable, isTrue); + }); + + test('a refresh during a dropout keeps a device that worked this session', + () async { + setUpWith(devices: _hostWith([_builtIn, _bluetooth])); + await chooseOutput(_bluetooth); + await hotplug(_hostWith([_builtIn])); + + await container.read(audioOutputControllerProvider.notifier).refresh(); + + expect(await preferences.audioOutputDeviceId(), _bluetooth.id); + }); + }); +} diff --git a/test/features/settings/playback/audio_output_settings_section_test.dart b/test/features/settings/playback/audio_output_settings_section_test.dart index e738f03e..0cd406ed 100644 --- a/test/features/settings/playback/audio_output_settings_section_test.dart +++ b/test/features/settings/playback/audio_output_settings_section_test.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -28,6 +30,12 @@ class _FakeService implements AudioOutputDeviceService { routed.add(device); return true; } + + final StreamController> changes = + StreamController>.broadcast(); + + @override + Stream> get deviceChanges => changes.stream; } void main() { @@ -156,4 +164,69 @@ void main() { expect(find.textContaining('could not be used'), findsOneWidget); expect(await preferences.audioOutputDeviceId(), isNull); }); + + group('an output that went away (#403)', () { + testWidgets('a fallback the backend refused is surfaced with a way out', + (tester) async { + final _FakeService service = _FakeService( + devices: const [ + AudioOutputDevice.systemDefault, + headset, + ], + ); + addTearDown(() => service.changes.close()); + await pump(tester, service); + + // The listener is on the headset, then it is unplugged and the backend + // refuses the system default too. + await tester.tap(find.text('System default')); + await tester.pumpAndSettle(); + await tester.tap(find.text(headset.label).last); + await tester.pumpAndSettle(); + + service.routingSucceeds = false; + service.changes.add(const [ + AudioOutputDevice.systemDefault, + ]); + await tester.pumpAndSettle(); + + expect(find.textContaining('playback may be silent'), findsOneWidget); + expect(find.text('Try again'), findsOneWidget); + + // The retry re-enumerates and re-applies, which is what a recovered + // backend needs. + service.routingSucceeds = true; + await tester.tap(find.text('Try again')); + await tester.pumpAndSettle(); + expect(find.textContaining('playback may be silent'), findsNothing); + }); + + testWidgets('a plain dropout says the output will be used again', + (tester) async { + final _FakeService service = _FakeService( + devices: const [ + AudioOutputDevice.systemDefault, + headset, + ], + ); + addTearDown(() => service.changes.close()); + await pump(tester, service); + + await tester.tap(find.text('System default')); + await tester.pumpAndSettle(); + await tester.tap(find.text(headset.label).last); + await tester.pumpAndSettle(); + + service.changes.add(const [ + AudioOutputDevice.systemDefault, + ]); + await tester.pumpAndSettle(); + + expect( + find.textContaining('as soon as it is back'), + findsOneWidget, + ); + expect(find.text('Try again'), findsNothing); + }); + }); } diff --git a/third_party/just_audio_media_kit/PATCHES.md b/third_party/just_audio_media_kit/PATCHES.md index ef7bba6f..7489d52c 100644 --- a/third_party/just_audio_media_kit/PATCHES.md +++ b/third_party/just_audio_media_kit/PATCHES.md @@ -32,7 +32,7 @@ this package resolves deterministically. ## The patch -Two additions, in two files, no deletions. +Three additions, in two files, no deletions. **1. `mpvProperties` — extra libmpv options at player creation.** @@ -50,11 +50,20 @@ Two additions, in two files, no deletions. construction and removes it in `release()`, so the map only ever holds players that are alive. +**3. `livePlayersChanged` — a signal that the registry changed.** + +- `lib/just_audio_media_kit.dart` adds + `JustAudioMediaKit.livePlayersChanged`, a broadcast `StreamController` + (and the `dart:async` import it needs). +- `lib/mediakit_player.dart` adds one event beside each of the two writes + above — one after the insert, one after the removal. + Nothing else changes: no new dependency, no new I/O, no new process or library loading, and no call into libmpv that upstream does not already make. With the -map left empty and the registry unread, the generated libmpv calls are -byte-for-byte what upstream makes; Linthra does not leave them unused (see -below), so the difference from upstream is exactly what it does with them. +map left empty and neither the registry nor the signal read, the generated +libmpv calls are byte-for-byte what upstream makes; Linthra does not leave them +unused (see below), so the difference from upstream is exactly what it does +with them. ### Why @@ -76,6 +85,16 @@ player, reads the device list from it, and calls media_kit's own `setAudioDevice` on it, so audio that is *already playing* moves to the chosen output ([#402](https://github.com/thezupzup/linthra/issues/402)). +**`livePlayersChanged`.** Hotplug recovery +([#403](https://github.com/thezupzup/linthra/issues/403)) needs to *watch* +libmpv's `audio-device-list`, which means holding a listener on a live player's +device-list stream. The engine tears a player down and builds a new one on a +stop, on suspend/resume and on some source switches, so that listener has to be +re-attached — and without a signal the only way to notice would be to poll +`livePlayers` on a timer for the whole life of the app, which is exactly the +kind of idle wake-up the battery work removed. One event on each map write +costs nothing and replaces the timer. + ### Who sets `mpvProperties` **Three callers, and two of them are production.** This hook is *not* CI-only — @@ -93,14 +112,23 @@ cache configuration production uses, with only the output device swapped. The output-device service merges too, for the same reason: an output change must not drop `cache-on-disk=no`. -### Who reads `livePlayers` +### Who reads `livePlayers` and `livePlayersChanged` + +One caller for both: `lib/core/services/linux_audio_output_device_service.dart`. + +It reads a live player to enumerate `audio-device-list` and calls +`setAudioDevice` on every entry when the listener picks an output. When the map +is empty (nothing is playing) it builds its own short-lived `Player` for +enumeration instead and disposes it, so the registry is never a requirement for +listing outputs — only for moving audio that is already playing. -One caller: `lib/core/services/linux_audio_output_device_service.dart`. It reads -a live player to enumerate `audio-device-list` and calls `setAudioDevice` on -every entry when the listener picks an output. When the map is empty (nothing is -playing) it builds its own short-lived `Player` for enumeration instead and -disposes it, so the registry is never a requirement for listing outputs — only -for moving audio that is already playing. +It listens to `livePlayersChanged` to keep exactly one device-list subscription +per live player: on each event it attaches to players it is not watching and +drops the ones that are gone. That is the whole reason the signal exists — +without it the service would either miss hotplug events after the engine +rebuilt its player, or poll the map forever. The subscription is only held +while something is listening to `deviceChanges`, so an app nobody has asked to +watch outputs holds none. ## Auditing this directory @@ -123,9 +151,10 @@ set). CI runs it on every PR. 3. Re-apply the hunks (`git apply upstream.patch`, or by hand if they moved) and regenerate `upstream.patch` from the pristine-vs-vendored diff. **Do not drop them because upstream gained something similar** without checking every - caller in *Who sets `mpvProperties`* and *Who reads `livePlayers`* first: - shipped playback and output-device selection both depend on these, so losing - one is a silent behaviour change, not a test-only regression. + caller in *Who sets `mpvProperties`* and *Who reads `livePlayers` and + `livePlayersChanged`* first: shipped playback, output-device selection and + hotplug recovery all depend on these, so losing one is a silent behaviour + change, not a test-only regression. 4. Update the tables above, refresh `pubspec.lock`, and run `./scripts/check_vendored_packages.sh`. 5. If upstream ever adds its own public hooks — player-creation properties, or diff --git a/third_party/just_audio_media_kit/lib/just_audio_media_kit.dart b/third_party/just_audio_media_kit/lib/just_audio_media_kit.dart index a757bdd1..580fc5a9 100644 --- a/third_party/just_audio_media_kit/lib/just_audio_media_kit.dart +++ b/third_party/just_audio_media_kit/lib/just_audio_media_kit.dart @@ -1,6 +1,7 @@ /// `package:media_kit` bindings for `just_audio` to support Linux and Windows. library just_audio_media_kit; +import 'dart:async'; import 'dart:collection'; import 'package:flutter/services.dart'; @@ -65,6 +66,17 @@ class JustAudioMediaKit extends JustAudioPlatform { /// never handed out. static final Map livePlayers = {}; + /// Broadcasts whenever [livePlayers] gains or loses an entry. + /// + /// The engine tears a player down and builds a new one on a stop, on + /// suspend/resume and on some source switches, so a reader that attached a + /// listener to one player's device-list stream has to know when to re-attach. + /// Without a signal the only way to notice would be to poll the map on a + /// timer, for the whole life of the app. Nothing is carried on the stream: it + /// means "look at [livePlayers] again" and nothing more. + static final StreamController livePlayersChanged = + StreamController.broadcast(); + static final _logger = Logger('JustAudioMediaKit'); final _players = HashMap(); diff --git a/third_party/just_audio_media_kit/lib/mediakit_player.dart b/third_party/just_audio_media_kit/lib/mediakit_player.dart index 7e445549..9ef1bd56 100644 --- a/third_party/just_audio_media_kit/lib/mediakit_player.dart +++ b/third_party/just_audio_media_kit/lib/mediakit_player.dart @@ -66,6 +66,7 @@ class MediaKitPlayer extends AudioPlayerPlatform { } JustAudioMediaKit.livePlayers[id] = _player; + JustAudioMediaKit.livePlayersChanged.add(null); if (JustAudioMediaKit.prefetchPlaylist) { setProperty(_player, 'prefetch-playlist', 'yes'); @@ -397,6 +398,7 @@ class MediaKitPlayer extends AudioPlayerPlatform { Future release() async { _logger.info('releasing player resources'); JustAudioMediaKit.livePlayers.remove(id); + JustAudioMediaKit.livePlayersChanged.add(null); _mediaOpened = false; await _player.dispose(); // cancel all stream subscriptions diff --git a/third_party/just_audio_media_kit/upstream.patch b/third_party/just_audio_media_kit/upstream.patch index 889b438d..973305bf 100644 --- a/third_party/just_audio_media_kit/upstream.patch +++ b/third_party/just_audio_media_kit/upstream.patch @@ -1,6 +1,14 @@ --- a/lib/just_audio_media_kit.dart +++ b/lib/just_audio_media_kit.dart -@@ -48,6 +48,23 @@ +@@ -1,6 +1,7 @@ + /// `package:media_kit` bindings for `just_audio` to support Linux and Windows. + library just_audio_media_kit; + ++import 'dart:async'; + import 'dart:collection'; + + import 'package:flutter/services.dart'; +@@ -48,6 +49,34 @@ /// [the related issue](https://github.com/Pato05/just_audio_media_kit/issues/11) for more information static bool prefetchPlaylist = false; @@ -20,13 +28,24 @@ + /// is created and is removed when it is released, so a stale [Player] is + /// never handed out. + static final Map livePlayers = {}; ++ ++ /// Broadcasts whenever [livePlayers] gains or loses an entry. ++ /// ++ /// The engine tears a player down and builds a new one on a stop, on ++ /// suspend/resume and on some source switches, so a reader that attached a ++ /// listener to one player's device-list stream has to know when to re-attach. ++ /// Without a signal the only way to notice would be to poll the map on a ++ /// timer, for the whole life of the app. Nothing is carried on the stream: it ++ /// means "look at [livePlayers] again" and nothing more. ++ static final StreamController livePlayersChanged = ++ StreamController.broadcast(); + static final _logger = Logger('JustAudioMediaKit'); final _players = HashMap(); --- a/lib/mediakit_player.dart +++ b/lib/mediakit_player.dart -@@ -61,6 +61,12 @@ +@@ -61,6 +61,13 @@ ready: () => _readyCompleter.complete(), )); @@ -35,15 +54,17 @@ + } + + JustAudioMediaKit.livePlayers[id] = _player; ++ JustAudioMediaKit.livePlayersChanged.add(null); + if (JustAudioMediaKit.prefetchPlaylist) { setProperty(_player, 'prefetch-playlist', 'yes'); } -@@ -390,6 +396,7 @@ +@@ -390,6 +397,8 @@ /// Release the resources used by this player. Future release() async { _logger.info('releasing player resources'); + JustAudioMediaKit.livePlayers.remove(id); ++ JustAudioMediaKit.livePlayersChanged.add(null); _mediaOpened = false; await _player.dispose(); // cancel all stream subscriptions