fix(eq): re-apply equalizer when the native audio session rotates
ExoPlayer assigns a new audio session id after transient audio-focus interruptions (navigation prompts, radar warnings), leaving the AndroidEqualizer attached to the dead session so playback resumed without equalization until the next station switch. The session-id listener now detects genuine rotations through a dedicated guard and re-activates the equalizer with the current preset, gated on EQ availability to stay clear of player teardown/rebuild.
This commit is contained in:
@@ -0,0 +1,171 @@
|
||||
# Design: EQ Re-Apply After Audio-Focus Ducking
|
||||
|
||||
## Technical Approach
|
||||
|
||||
Extend the existing `_androidAudioSessionIdSub` listener in `PluriWaveAudioHandler`
|
||||
(`lib/servicios/servicio_audio.dart:258-265`) so that, after it broadcasts a new
|
||||
native session id, it re-attaches the equalizer and re-pushes the CURRENT gains
|
||||
when the id genuinely rotated mid-playback. The fix is a single conditional call
|
||||
to the already-idempotent `_activarEcualizador()` choke-point, guarded by a
|
||||
dedicated change-guard field and the `_eqDisponible` lifecycle flag. Mirrors the
|
||||
proven in-repo pattern `VisualizadorAudio._onSessionId`
|
||||
(`lib/widgets/visualizador_audio.dart:82-88`). Entirely service-layer; no
|
||||
`ServicioAudioSession`, `EstadoEcualizador`, or Kotlin changes. Realizes spec
|
||||
`eq-audiofocus-reapply`.
|
||||
|
||||
## Architecture Decisions
|
||||
|
||||
### Decision: Where the re-apply logic lives
|
||||
|
||||
| Option | Tradeoff | Decision |
|
||||
|--------|----------|----------|
|
||||
| New listener on `androidAudioSessionIdStream` | Duplicate subscription lifecycle, extra teardown in `_recrearPlayer`/`onTaskRemoved` | Rejected |
|
||||
| Hook into `ServicioAudioSession.interruptionEventStream` | Couples EQ to app-focus events, not the actual native session signal; misses non-focus rotations | Rejected |
|
||||
| Add conditional inside the EXISTING `_androidAudioSessionIdSub` listener | Zero new subscriptions; reuses teardown already in place; precise native signal | **Chosen** |
|
||||
|
||||
**Rationale**: The listener already fires on every native session id and already
|
||||
owns its cancel path in `_recrearPlayer()` (L492) and `onTaskRemoved()` (L671).
|
||||
Adding one branch there is the smallest, lowest-risk surface and uses the most
|
||||
precise trigger (the session id itself, per the just_audio internals traced in
|
||||
exploration).
|
||||
|
||||
### Decision: Change-guard field separate from the broadcast field
|
||||
|
||||
| Option | Tradeoff | Decision |
|
||||
|--------|----------|----------|
|
||||
| Reuse `_androidAudioSessionId` as the guard | Guard would need to read the field AFTER it is overwritten for broadcast; entangles EQ semantics with external-consumer broadcast state | Rejected |
|
||||
| New private `int? _ultimaSessionIdEq` dedicated to the EQ guard | One extra field; keeps broadcast semantics untouched for `ServicioAudio`/`VisualizadorAudio` consumers | **Chosen** |
|
||||
|
||||
**Rationale**: Per proposal note, `_androidAudioSessionId` must keep pure
|
||||
broadcast semantics so external consumers are unaffected. A dedicated
|
||||
`_ultimaSessionIdEq` lets the EQ compare-and-swap independently. It is reset to
|
||||
`null` inside `_recrearPlayer()` alongside `_androidAudioSessionId` so a rebuilt
|
||||
player's first id is treated as fresh, not a rotation.
|
||||
|
||||
### Decision: Re-apply call target and current-gains source
|
||||
|
||||
| Option | Tradeoff | Decision |
|
||||
|--------|----------|----------|
|
||||
| `aplicarPreset(_presetActual)` only | Skips `_eq.parameters` re-probe / `setEnabled`; assumes params already valid on the new session | Rejected as sole call |
|
||||
| `_activarEcualizador()` | Re-probes `_eq.parameters`, re-sets enabled state, then calls `aplicarPreset(_presetActual)` | **Chosen** |
|
||||
|
||||
**Rationale**: `_activarEcualizador()` (L526) is the exact activate/attach
|
||||
choke-point used by the working station-switch path; on a rotated session the
|
||||
native effect must be re-probed and re-enabled before gains land, which
|
||||
`aplicarPreset` alone does not do. Both paths read gains from the single source
|
||||
of truth `_presetActual` (field L194), which is also mutated by `aplicarPreset`
|
||||
(L541), `setBanda` (L568), and `setEcualizadorActivo` (L600) — so re-apply always
|
||||
uses LIVE state, never a stale snapshot. No gain value is captured or copied at
|
||||
subscribe time.
|
||||
|
||||
## Data Flow
|
||||
|
||||
Another app ducks/interrupts
|
||||
│
|
||||
ExoPlayer rotates native session id (issue #5302)
|
||||
│
|
||||
_player.androidAudioSessionIdStream emits new id
|
||||
│
|
||||
_androidAudioSessionIdSub listener:
|
||||
1. _androidAudioSessionId = id (broadcast state, unchanged)
|
||||
2. controller.add(id) (external consumers: unchanged)
|
||||
3. re-apply gate ↓
|
||||
│
|
||||
id == null ? ──yes──► return (no-op)
|
||||
│ no
|
||||
id == _ultimaSessionIdEq ? ──yes──► return (redundant, guard)
|
||||
│ no
|
||||
_eqDisponible == false ? ──yes──► return (mid-teardown / not attached)
|
||||
│ no
|
||||
_ultimaSessionIdEq = id
|
||||
unawaited(_activarEcualizador()) ──► re-probe + setEnabled + aplicarPreset(_presetActual)
|
||||
│
|
||||
Native EQ re-attached to new session → music stays equalized
|
||||
|
||||
## Race Analysis (`_recrearPlayer`, L488-508)
|
||||
|
||||
Teardown order makes the guard safe. `_recrearPlayer()`:
|
||||
1. `await _androidAudioSessionIdSub?.cancel()` (L492) — old listener silenced.
|
||||
2. `_eq = AndroidEqualizer()` (L502) — fresh effect, empty params.
|
||||
3. `_eqDisponible = false` (L503) — **gate closes BEFORE any new id can arrive**.
|
||||
4. `_androidAudioSessionId = null` (L504) + reset `_ultimaSessionIdEq = null`.
|
||||
5. `_player = _crearPlayer()` (L505) then `_conectarStreamsPlayer()` (L507)
|
||||
re-subscribes.
|
||||
|
||||
Because `_eqDisponible` is set `false` (step 3) before the new subscription
|
||||
exists (step 5), any id the rebuilt player emits is dropped by the gate until the
|
||||
station-switch path's own `_activarEcualizador()` (L449) flips `_eqDisponible`
|
||||
back to `true`. Once true, that same call sets `_ultimaSessionIdEq`, so the very
|
||||
first post-rebuild rotation does not double-fire.
|
||||
|
||||
**Rapid consecutive rotations**: the compare-and-swap on `_ultimaSessionIdEq`
|
||||
collapses duplicate emissions of the same id. Distinct ids each trigger one
|
||||
`_activarEcualizador()`; the call is `unawaited` and internally guarded/idempotent
|
||||
(`aplicarPreset` returns early if `!_eqDisponible`), so overlapping in-flight
|
||||
re-applies converge on `_presetActual` without corruption. No lock needed.
|
||||
|
||||
## File Changes
|
||||
|
||||
| File | Action | Description |
|
||||
|------|--------|-------------|
|
||||
| `lib/servicios/servicio_audio.dart` | Modify | Add `int? _ultimaSessionIdEq`; extend `_androidAudioSessionIdSub` listener (after L263) with the guarded re-apply; reset guard in `_recrearPlayer` (near L504); extract the pure decision predicate for tests |
|
||||
| `test/servicios/servicio_audio_eq_reapply_test.dart` | Create | Unit-test the extracted predicate: rotation, first-activation no-op, null id, teardown gate, rapid duplicates |
|
||||
|
||||
Explicitly NOT touched: `servicio_audio_session.dart`, `estado_ecualizador.dart`,
|
||||
Kotlin. Fix is orthogonal to `_eqMultiDeviceEnabled` (an `EstadoEcualizador`
|
||||
concern absent from this file) — it operates on the single live `_presetActual`,
|
||||
so it applies regardless of that toggle.
|
||||
|
||||
## Interfaces / Contracts
|
||||
|
||||
Testable seam (minimal refactor — the handler cannot be constructed in unit
|
||||
tests because its constructor builds a real `AudioPlayer` needing MethodChannels,
|
||||
confirmed by `servicio_audio_source_switch_test.dart:13-18`). Extract the pure
|
||||
decision so tests exercise it directly, mirroring how that test isolates
|
||||
`ControladorReconexion`:
|
||||
|
||||
```dart
|
||||
/// Pure re-apply decision for a native session-id emission. No side effects.
|
||||
/// Returns true only when the EQ must re-attach + re-push current gains.
|
||||
@visibleForTesting
|
||||
static bool debeReaplicarEcualizador({
|
||||
required int? sessionId,
|
||||
required int? ultimaSessionIdEq,
|
||||
required bool eqDisponible,
|
||||
}) =>
|
||||
sessionId != null && sessionId != ultimaSessionIdEq && eqDisponible;
|
||||
```
|
||||
|
||||
The listener calls this predicate; on `true` it sets
|
||||
`_ultimaSessionIdEq = sessionId` and `unawaited(_activarEcualizador())`. Keeping
|
||||
the mutation + async call in the listener (not the predicate) preserves the "no
|
||||
side effects in the tested function" boundary and needs no fake player.
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
| Layer | What to Test | Approach |
|
||||
|-------|-------------|----------|
|
||||
| Unit | `debeReaplicarEcualizador` truth table | Direct static calls; no handler instance |
|
||||
| Unit | Rotation → true (new non-null id, eq available) | `debeReaplicarEcualizador(sessionId:2, ultimaSessionIdEq:1, eqDisponible:true) == true` |
|
||||
| Unit | First activation no-op (same id re-emitted) | `sessionId:1, ultimaSessionIdEq:1 → false` |
|
||||
| Unit | Null id → false | `sessionId:null → false` |
|
||||
| Unit | Teardown gate (`_eqDisponible==false`) → false | `eqDisponible:false, sessionId:9 → false` |
|
||||
| Unit | Rapid duplicate emissions | Two calls same id: second sees updated guard → false |
|
||||
| Regression | Station-switch + manual slider paths intact | Full `flutter test` (Strict TDD) — call sites unchanged |
|
||||
|
||||
Strict TDD: write each predicate case RED first, then implement the predicate and
|
||||
wire the listener GREEN. Integration-level "gains actually re-pushed to native"
|
||||
is not unit-testable without platform channels; it is covered by the predicate
|
||||
contract plus the unchanged, already-tested `_activarEcualizador`/`aplicarPreset`
|
||||
idempotency.
|
||||
|
||||
## Migration / Rollout
|
||||
|
||||
No migration required. No persistence, schema, or public API change. Revert the
|
||||
single commit to restore prior behavior.
|
||||
|
||||
## Open Questions
|
||||
|
||||
- [ ] None blocking. Predicate seam chosen because full-handler construction is
|
||||
not unit-testable in this repo; if a future integration harness fakes the
|
||||
player, an end-to-end re-apply assertion could be added then.
|
||||
Reference in New Issue
Block a user