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.
8.4 KiB
Verify Report: EQ Re-Apply After Audio-Focus Ducking
Change: eq-audiofocus-reapply Verified: 2026-07-10 Verdict: PASS (0 CRITICAL, 0 WARNING, 2 SUGGESTION)
Scope of verification
Read spec (#2309), design (#2307), tasks (#2312), apply-progress (#2317) from engram. Re-read git diff -- lib/servicios/servicio_audio.dart, test/servicios/servicio_audio_eq_reapply_test.dart, and the surrounding implementation directly (not trusting apply-progress claims alone). Ran flutter analyze, the fallback explicit test set, estado_ecualizador_test.dart, and attempted the full flutter test suite.
Diff-to-design match (line-level)
| Design element | Expected | Found | Match |
|---|---|---|---|
| Predicate signature | @visibleForTesting static bool debeReaplicarEcualizador({required int? sessionId, required int? ultimaSessionIdEq, required bool eqDisponible}) | Line 561-566, exact body: sessionId != null && sessionId != ultimaSessionIdEq && eqDisponible | PASS |
| Change-guard field | Dedicated int? _ultimaSessionIdEq, separate from _androidAudioSessionId | Line 166, doc comment present, separate field confirmed | PASS |
| Wiring location | Inside EXISTING _androidAudioSessionIdSub listener, after existing broadcast add(sessionId) | Lines 265-280; conditional added strictly after _androidAudioSessionIdController.add(sessionId) (line 270) | PASS |
| Re-apply call | _activarEcualizador(), not aplicarPreset alone | unawaited(_activarEcualizador()) at line 278 | PASS |
| Guard reset location | Inside _recrearPlayer(), alongside _androidAudioSessionId = null, before _conectarStreamsPlayer() resubscribe | Line 520 (_ultimaSessionIdEq = null), after line 519 (_androidAudioSessionId = null), both after line 518 (_eqDisponible = false), before line 523 (_conectarStreamsPlayer()) | PASS |
Constraint compliance
- git status --short -- lib/ test/ shows only lib/servicios/servicio_audio.dart (M, +29 lines) and test/servicios/servicio_audio_eq_reapply_test.dart (new, untracked). No other lib/ or test/ file touched.
- servicio_audio_session.dart is absent from git status/diff entirely. Untouched.
- _cambiarFuente, setBanda, _eqMultiDeviceEnabled had zero matches when grepping the diff content. Non-goals respected; R3 (multi-device orthogonality) satisfied by absence -- this file has no dependency on EstadoEcualizador's toggle.
- Broadcast semantics (_androidAudioSessionIdController.add(sessionId), line 270) -- line itself unchanged; new logic added strictly after it, using the separate _ultimaSessionIdEq field. External consumers (ServicioAudio, VisualizadorAudio) unaffected.
- flutter build: never run, per instruction.
Test results
- flutter analyze --no-fatal-infos -> No issues found (0 issues).
- Fallback explicit set (servicio_audio_eq_reapply_test.dart + servicio_audio_source_switch_test.dart + servicio_audio_session_test.dart) -> 13/13 pass, including all 5 predicate truth-table cases.
- test/estado/estado_ecualizador_test.dart (regression check for R3 orthogonality) -> 38/38 pass.
- Full flutter test suite -> reproduced the pre-documented hang at ~241/245,
root cause test/estado/estado_alarmas_ejecuciones_test.dart (alarm
module, last touched in unrelated commit
079e19f, not present in this diff). Confirmed pre-existing and orthogonal to this change, matching apply-progress's own finding. Not a regression introduced here.
Spec requirement -> evidence mapping
Requirement: Session Id Rotation Triggers EQ Re-Apply
| Scenario | Verdict | Evidence |
|---|---|---|
| Session id rotates while playing | PASS | Predicate test 'rotation while playing returns true' (sessionId:2, ultimaSessionIdEq:1, eqDisponible:true) == true; listener wiring at lines 265-280 calls _activarEcualizador() on true and updates _ultimaSessionIdEq before the async call (compare-and-swap ordering matches design's Race Analysis for rapid duplicates) |
| Same id re-emitted produces no redundant re-apply | PASS | Predicate test 'same id re-emitted returns false' (sessionId:1, ultimaSessionIdEq:1) == false |
| First legitimate activation is not double-applied | PASS | Predicate test 'first activation / matching guard returns false' -- same shape as duplicate case, kept as a distinct named case per design's testing table to document intent (post-station-switch guard already set) |
Dedicated field requirement (change tracking separate from _androidAudioSessionId): PASS -- confirmed by diff, _ultimaSessionIdEq is a wholly separate field, never aliases the broadcast field.
Requirement: Re-Apply Is Gated On EQ Availability
| Scenario | Verdict | Evidence |
|---|---|---|
| Rotation during player teardown is safely skipped | PASS | Predicate test 'teardown gate returns false' (eqDisponible:false, sessionId:9) == false; _recrearPlayer() sets _eqDisponible = false (line 518) and resets _ultimaSessionIdEq = null (line 520) BEFORE _conectarStreamsPlayer() resubscribes (line 523), closing the gate before any new id can arrive -- matches design's Race Analysis step-by-step |
Plus predicate test 'null id returns false' (non-null precondition, not a named spec scenario but explicitly required by the Requirement text) -- PASS.
Requirement: Re-Apply Is Independent Of Multi-Device Toggle
| Scenario | Verdict | Evidence |
|---|---|---|
| Re-apply works with the toggle on or off | PASS (by design/absence, not a targeted test) | Zero references to _eqMultiDeviceEnabled or EstadoEcualizador in the diff or in servicio_audio.dart's relevant region -- this file operates on _presetActual directly and has no code path that reads the multi-device toggle. estado_ecualizador_test.dart (38/38 pass) confirms the toggle layer itself is unaffected as a regression check. Per spec's own Testability Matrix, on/off equivalence for the ducking scenario is manual QA (task 5.2) -- correctly deferred, not a gap in this apply. |
Non-Goals (regression verification)
- Station-switch path (_cambiarFuente -> _recrearPlayer -> _activarEcualizador() at line ~449/463): untouched, confirmed absent from diff. Fallback test set covers servicio_audio_source_switch_test.dart (3/3 pass).
- Manual slider path (setBanda): untouched, confirmed absent from diff.
- No new localized strings: confirmed, diff touches only Dart logic, no l10n files in the changeset.
Tasks / apply-progress consistency
- Tasks artifact (#2312) marks 1.1-4.2 as [x] DONE, 5.1-5.4 as [ ] NOT DONE with rationale (human-only gate, real MethodChannels/native EQ, no fake/injectable seam per spec's Testability Matrix). Confirmed consistent with actual code state -- no automatable coverage was skipped, and no checked task lacks corresponding evidence.
- Apply-progress (#2317) claims (+29 lines, RED->GREEN->REFACTOR->regression sequence, specific line numbers) all independently reproduced during this verification rather than taken on faith.
- Phase 5 (4 manual QA tasks) confirmed still unchecked in the artifact content read directly -- correctly left as the human on-device gate before archive.
Findings
No CRITICAL issues. No WARNING issues.
SUGGESTION 1 -- The full flutter test suite cannot be used as a clean gate for this or future changes until the pre-existing estado_alarmas_ejecuciones_test.dart hang is fixed (separate, unrelated alarm-module issue, already flagged in apply-progress and reproduced again here). Recommend filing that as its own fix outside this change's scope so flutter test becomes usable as a single command again.
SUGGESTION 2 -- Scenario 'First legitimate activation is not double-applied' and 'Same id re-emitted produces no redundant re-apply' are implemented as textually identical test bodies (both assert debeReaplicarEcualizador(sessionId:1, ultimaSessionIdEq:1, eqDisponible:true) == false). This is intentional per design's testing table (documents two distinct intents with the same input shape) and does not reduce coverage, but a future maintainer skimming the file could mistake it for accidental duplication. The in-test doc comments already explain the distinction clearly, so no action required -- noting only for awareness.
Recommendation
Automated portion of this change is complete and correct against spec, design, and tasks. Ready for sdd-archive once a human completes Phase 5 manual on-device QA (4 tasks: real ducking test, toggle on/off equivalence, rapid-switch-during-ducking race, and manual-slider/station-switch non-regression). Archive should NOT proceed before Phase 5 is completed by a human, per the spec's own Testability Matrix design.