diff --git a/lib/servicios/servicio_audio.dart b/lib/servicios/servicio_audio.dart index e99229c..298c5c6 100644 --- a/lib/servicios/servicio_audio.dart +++ b/lib/servicios/servicio_audio.dart @@ -687,44 +687,51 @@ DecisionToggleEq decidirToggleEq({ requiereLlamadaNativa: eqDisponible, ); -/// Translates a gain on the app's fixed ±12 dB slider scale to the range the -/// device's native equalizer actually reports -/// (`AndroidEqualizerParameters.min/maxDecibels`, itself derived from -/// `Equalizer.getBandLevelRange()`). +/// Delivers a gain from the app's ±12 dB slider to the device's native +/// equalizer, clamped by what the device reports it can do +/// (`AndroidEqualizerParameters.min/maxDecibels`, itself +/// `Equalizer.getBandLevelRange()` in millibels divided by 1000). /// /// Top-level and pure so the mapping is testable without a device. /// -/// THE DEFECT THIS REPLACES, and the likely source of the reported «suena muy -/// alto»: the previous implementation normalised across the whole range and -/// interpolated linearly, +/// THE CONTRACT: the decibels the user reads are the decibels the device is +/// asked for. The native range BOUNDS the request; it is not a scale to +/// normalise into. Both sides are already the same unit — `just_audio` +/// documents `setGain` as taking decibels and its Android bridge does +/// `setBandLevel(band, round(gain * 1000.0))`, plain dB to millibels with no +/// normalisation — so multiplying by the device's headroom was a unit error. /// -/// minDecibels + ((db + 12) / 24) * (maxDecibels - minDecibels) +/// WHY IT MATTERS, in the app's own terms. The slider is hard-coded +/// `min: -12.0, max: 12.0`, the label under each band prints +/// `'${banda.toStringAsFixed(1)}dB'`, and TalkBack reads `equalizerBandValue` +/// = "{value} decibels": one promise, made three ways. Presets are persisted +/// and exported as those same raw slider values (`PresetEcualizador.toJson`), +/// so scaling at this boundary made an exported backup mean a different SOUND +/// on a different phone while displaying identical numbers — and on the +/// common asymmetric shape [-12, +19] it multiplied boosts by 1.58 and cuts +/// by 1.0, deforming a preset's shape rather than just its depth. /// -/// which puts 0 dB at the MIDPOINT of the native range. That is only 0 when -/// the range is symmetric, and Android guarantees no such thing — the -/// Equalizer contract only promises a min/max pair. On a device reporting, -/// say, [-12, +19] dB, every band of a FLAT preset was pushed to +3.5 dB of -/// real boost: audibly louder, with the on/off button still reading "off" -/// and nothing in the UI to explain it. -/// -/// The contract here instead: 0 dB is always exactly 0, and each side of the -/// scale is stretched independently against its own end of the native range, -/// so a cut can never become a boost. A range with no headroom on one side -/// (or none at all) collapses that side to 0 rather than inverting it. +/// WHAT IS DELIBERATELY KEPT from the mapping this replaces — every invariant +/// the «suena muy alto» fix earned. Note the clamp window is widened to +/// always contain 0: a naive `db.clamp(minDecibels, maxDecibels)` would, on a +/// device reporting a wholly positive range such as [+3, +19], turn a FLAT +/// preset's 0 dB into +3 dB of real boost on every band — exactly the bug +/// that was fixed. So 0 dB is always exactly 0, the sign of the user's intent +/// is never inverted, the result never escapes the native range, a device +/// with no headroom above unity can never boost, and a zero-width range +/// collapses to 0. double mapearGananciaNativa( double db, { required double minDecibels, required double maxDecibels, }) { final limitado = db.clamp(-12.0, 12.0); - if (limitado == 0) return 0; - if (limitado > 0) { - // Only genuine headroom above unity counts as boost. - final techo = maxDecibels > 0 ? maxDecibels : 0.0; - return (limitado / 12.0) * techo; - } + // The clamp window is the device's range widened to include 0, so that a + // device reporting no headroom on one side collapses that side to "no + // change" instead of forcing a gain the user never asked for. final suelo = minDecibels < 0 ? minDecibels : 0.0; - return (limitado.abs() / 12.0) * suelo; + final techo = maxDecibels > 0 ? maxDecibels : 0.0; + return limitado.clamp(suelo, techo); } /// Advances to the NEXT factory preset after [actual] in [presets] order @@ -2720,10 +2727,12 @@ class PluriWaveAudioHandler extends BaseAudioHandler try { final params = await _eq.parameters; _eqDisponible = params.bands.isNotEmpty; - // eq-estado-unico item E: the ONE number that decides whether - // [mapearGananciaNativa] can be silently boosting a FLAT preset on - // this device. `Equalizer.getBandLevelRange()` is not required to be - // symmetric, and nothing else in the app can observe what it returned. + // eq-estado-unico item E: the ONE number that decides how much of the + // ±12 dB slider [mapearGananciaNativa] can actually honour on this + // device — anything past this range is clamped, so a report of "the + // slider stops doing anything past N" is answered from this line. + // `Equalizer.getBandLevelRange()` is not required to be symmetric, and + // nothing else in the app can observe what it returned. // `debugPrint` (never `dart:developer`'s `log`) so it reaches logcat in // the release build, which is the only one that ever runs in a car: // diff --git a/test/servicios/servicio_audio_eq_ganancia_test.dart b/test/servicios/servicio_audio_eq_ganancia_test.dart index aa20a96..d472dac 100644 --- a/test/servicios/servicio_audio_eq_ganancia_test.dart +++ b/test/servicios/servicio_audio_eq_ganancia_test.dart @@ -1,28 +1,43 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:pluriwave/servicios/servicio_audio.dart'; -/// eq-estado-unico item E — `mapearGananciaNativa`, the translation from the -/// app's fixed ±12 dB slider scale to whatever range the device's native -/// `Equalizer.getBandLevelRange()` reports. +/// `mapearGananciaNativa` — the hand-off from the app's ±12 dB slider to the +/// device's native `Equalizer`, whose capability is reported as +/// `AndroidEqualizerParameters.min/maxDecibels` +/// (`Equalizer.getBandLevelRange()` in millibels, divided by 1000). /// -/// This is the only source-plausible explanation for the reported «suena muy -/// alto» half of the bug. The original implementation normalised the input -/// across the WHOLE range and mapped it linearly: +/// WHY THIS CONTRACT CHANGED — the previous one stretched each side of the +/// slider against its own end of the native range, so `+6` on a device +/// reporting `[-12, +20]` was delivered as `+10`. Both sides of the mapping +/// are already the SAME unit, so that multiplication was a unit error: /// -/// normalizado = (db.clamp(-12, 12) + 12) / 24 -/// return minDecibels + normalizado * (maxDecibels - minDecibels) +/// * `just_audio` documents `setGain` as "Sets the gain for this band in +/// decibels", and its Android bridge does `setBandLevel(band, +/// round(gain * 1000.0))` — plain dB to millibels, no normalisation. +/// `min/maxDecibels` are the device's absolute CAPABILITY in dB, i.e. a +/// bound on the control, not a scale to normalise into. +/// * The app makes the user a decibel promise in three places at once: the +/// slider is hard-coded `min: -12.0, max: 12.0`, the label under each +/// band prints `'${banda.toStringAsFixed(1)}dB'`, and TalkBack reads out +/// `equalizerBandValue` = "{value} decibels". Stretching made that label +/// a lie on every device whose range is not exactly ±12. +/// * Presets are persisted and EXPORTED as those same raw slider dB +/// (`PresetEcualizador.toJson`), so under the old mapping a backup +/// restored on a wider-range phone showed identical numbers and played +/// louder — and on the common asymmetric shape `[-12, +19]` boosts were +/// multiplied by 1.58 while cuts were not, deforming the preset's SHAPE +/// rather than merely its depth. /// -/// which sends 0 dB to the MIDPOINT of the native range. That is only 0 when -/// the range happens to be symmetric. Android does not guarantee that: the -/// AudioEffect Equalizer contract only requires a min/max pair, and real -/// devices ship asymmetric ranges. On such a device a FLAT preset — every -/// band 0 dB — was silently pushing a positive boost into every band, which -/// is audibly louder while the on/off button still reads "off". +/// So: the number the user reads is the number the device is asked for. The +/// native range only CLAMPS it. /// -/// The contract asserted here: 0 dB always maps to exactly 0, and the two -/// sides of the scale are stretched INDEPENDENTLY against their own end of -/// the native range, so the sign of the user's intent is never inverted and -/// the extremes still reach the device's real limits. +/// What this deliberately KEEPS from the previous contract — every invariant +/// the «suena muy alto» fix actually earned. 0 dB is always exactly 0 (a +/// naive `db.clamp(minDecibels, maxDecibels)` would regress that on a wholly +/// positive reported range, turning a FLAT preset into a boost again), the +/// sign of the user's intent is never inverted, the result never escapes the +/// native range, a device with no headroom above unity can never boost, and a +/// zero-width range collapses to 0. void main() { group('mapearGananciaNativa — 0 dB is always exactly 0', () { test('symmetric range (the common case) is unchanged', () { @@ -43,6 +58,10 @@ void main() { }); test('a wholly positive range still cannot boost a FLAT preset', () { + // This is precisely why the mapping cannot be a plain + // `db.clamp(minDecibels, maxDecibels)`: that would answer +3 here and + // bring the «suena muy alto» bug straight back. The clamp window has + // to be widened so that it always contains 0. expect(mapearGananciaNativa(0, minDecibels: 3, maxDecibels: 19), 0); }); @@ -51,36 +70,61 @@ void main() { }); }); - group('mapearGananciaNativa — the extremes reach the native limits', () { - test('+12 dB maps to the native maximum', () { - expect(mapearGananciaNativa(12, minDecibels: -12, maxDecibels: 19), 19); + group('mapearGananciaNativa — the slider dB reach the device literally', () { + test('+12 dB is delivered as +12 dB, not stretched to the native max', () { + // CONTRACT CHANGE: this used to assert 19, i.e. the whole of the + // device's headroom. The slider says "12.0dB" and the accessibility + // label says "12.0 decibels", so 12 dB is what the device must be + // asked for. The 7 dB of extra hardware headroom is unreachable by + // design until the slider itself is widened and says so. + expect(mapearGananciaNativa(12, minDecibels: -12, maxDecibels: 19), 12); }); - test('-12 dB maps to the native minimum', () { + test('-12 dB is delivered as -12 dB', () { expect(mapearGananciaNativa(-12, minDecibels: -12, maxDecibels: 19), -12); }); - test('values beyond the slider scale are clamped, not extrapolated', () { - expect(mapearGananciaNativa(40, minDecibels: -15, maxDecibels: 15), 15); - expect(mapearGananciaNativa(-40, minDecibels: -15, maxDecibels: 15), -15); + test('values beyond the slider scale clamp to the slider limit', () { + // CONTRACT CHANGE: these used to answer the NATIVE extremes (±15). + // The slider scale is the first bound; the device range is the second. + expect(mapearGananciaNativa(40, minDecibels: -15, maxDecibels: 15), 12); + expect(mapearGananciaNativa(-40, minDecibels: -15, maxDecibels: 15), -12); }); }); - group('mapearGananciaNativa — each side scales against its own end', () { - test('half boost is half of the positive headroom', () { + group('mapearGananciaNativa — the label is the value the device gets', () { + test('+6 dB on a wide-range device is +6 dB, never 10', () { + // CONTRACT CHANGE: this used to assert closeTo(10) — "half boost is + // half of the positive headroom". A slider reading "6.0dB" that + // produced +10 dB of real boost is exactly what made a restored backup + // sound different on a different phone. expect( mapearGananciaNativa(6, minDecibels: -12, maxDecibels: 20), - closeTo(10, 1e-9), + closeTo(6, 1e-9), ); }); - test('half cut is half of the negative headroom', () { + test('-6 dB on that same device is -6 dB', () { expect( mapearGananciaNativa(-6, minDecibels: -12, maxDecibels: 20), closeTo(-6, 1e-9), ); }); + test('the six factory presets keep their shape on an asymmetric device', () { + // Jazz, authored in true dB before any scaling existed. Under the old + // mapping [-12, +19] delivered it as [4.75, -1, -1.5, 3.17, 6.33]: a + // different tonal curve, not merely a louder one. + const jazz = [3.0, -1.0, -1.5, 2.0, 4.0]; + final entregado = jazz + .map( + (db) => + mapearGananciaNativa(db, minDecibels: -12, maxDecibels: 19), + ) + .toList(); + expect(entregado, jazz); + }); + test('the sign of the user intent is never inverted', () { for (final db in [-12.0, -6.0, -1.0, 1.0, 6.0, 12.0]) { final nativo = mapearGananciaNativa( @@ -97,12 +141,44 @@ void main() { }); }); + group('mapearGananciaNativa — a device narrower than the slider', () { + test('a request that fits is still delivered literally', () { + // CONTRACT CHANGE: the old mapping shrank this to (3/12)*6 = 1.5 dB, + // so a modest device silently under-delivered every request too. + expect(mapearGananciaNativa(3, minDecibels: -6, maxDecibels: 6), 3); + expect(mapearGananciaNativa(-3, minDecibels: -6, maxDecibels: 6), -3); + }); + + test('a request beyond the device range clamps to the device limit', () { + expect(mapearGananciaNativa(12, minDecibels: -6, maxDecibels: 6), 6); + expect(mapearGananciaNativa(-12, minDecibels: -6, maxDecibels: 6), -6); + }); + + test('a very narrow device still gets a sane, in-range value', () { + for (final db in [-12.0, -5.0, 0.0, 5.0, 12.0]) { + final nativo = mapearGananciaNativa( + db, + minDecibels: -1.5, + maxDecibels: 1.5, + ); + expect(nativo, greaterThanOrEqualTo(-1.5)); + expect(nativo, lessThanOrEqualTo(1.5)); + expect(nativo.sign, db.sign); + } + }); + }); + group('mapearGananciaNativa — degenerate ranges reported by the device', () { - test('a range with no headroom on one side clamps that side to 0', () { - // A device that reports max == 0 can only cut. Asking for a boost must - // resolve to "no change", never to a negative value. + test('a device with no headroom above unity can never boost', () { expect(mapearGananciaNativa(12, minDecibels: -15, maxDecibels: 0), 0); - expect(mapearGananciaNativa(-12, minDecibels: -15, maxDecibels: 0), -15); + expect(mapearGananciaNativa(6, minDecibels: -15, maxDecibels: 0), 0); + }); + + test('a cut the device could honour exactly is not over-delivered', () { + // CONTRACT CHANGE: this used to answer -15, spending the device's whole + // range on a request for -12 dB. The user asked for -12; -12 is + // representable here, so -12 is what is sent. + expect(mapearGananciaNativa(-12, minDecibels: -15, maxDecibels: 0), -12); }); test('a zero-width range collapses everything to 0', () {