FreeTLab
0ab63731d0
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.
172 lines
9.0 KiB
Markdown
172 lines
9.0 KiB
Markdown
# 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.
|