fix: address CodeRabbit review issues from PR #502
Android: - Fix stale wake lock detection by checking isHeld() before reacquiring - Wrap startForegroundService in try-catch to handle launch exceptions - Add START/STOP intent actions to prevent stop/start race condition HTML/Accessibility: - Convert preset and preamp spans to proper <label> elements - Add aria-controls to database collapse button - Sync aria-expanded state when toggling database panel JavaScript: - Guard auto-resume with source check to avoid resuming when not playing - Add _calculateQ() fallback in fast-path EQ filter updates - Persist graphic EQ gains and preamp to storage from setters - Include 'legacy' in EQ mode restoration check - Clear container before rebuilding GEQ bands for idempotency - Use AbortController for document-level listener cleanup - Apply gain calculation for non-parametric modes in touch snap - Validate stored GEQ values are finite numbers CSS: - Remove non-standard appearance: slider-vertical property
This commit is contained in:
tryptz
edideaur
parent
bd70b81a39
commit
da6c2e2dd9
7 changed files with 64 additions and 20 deletions
|
|
@ -33,6 +33,11 @@ public class AudioPlaybackService extends Service {
|
|||
|
||||
@Override
|
||||
public int onStartCommand(Intent intent, int flags, int startId) {
|
||||
if (intent != null && "STOP".equals(intent.getAction())) {
|
||||
stopSelf();
|
||||
return START_NOT_STICKY;
|
||||
}
|
||||
|
||||
Notification notification = buildNotification();
|
||||
|
||||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
|
||||
|
|
@ -95,6 +100,9 @@ public class AudioPlaybackService extends Service {
|
|||
}
|
||||
|
||||
private void acquireWakeLock() {
|
||||
if (wakeLock != null && !wakeLock.isHeld()) {
|
||||
wakeLock = null;
|
||||
}
|
||||
if (wakeLock == null) {
|
||||
PowerManager pm = (PowerManager) getSystemService(POWER_SERVICE);
|
||||
if (pm != null) {
|
||||
|
|
|
|||
|
|
@ -18,18 +18,24 @@ public class BackgroundAudioPlugin extends Plugin {
|
|||
|
||||
@PluginMethod
|
||||
public void start(PluginCall call) {
|
||||
Intent intent = new Intent(getContext(), AudioPlaybackService.class);
|
||||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
|
||||
getContext().startForegroundService(intent);
|
||||
} else {
|
||||
getContext().startService(intent);
|
||||
try {
|
||||
Intent intent = new Intent(getContext(), AudioPlaybackService.class);
|
||||
intent.setAction("START");
|
||||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
|
||||
getContext().startForegroundService(intent);
|
||||
} else {
|
||||
getContext().startService(intent);
|
||||
}
|
||||
call.resolve();
|
||||
} catch (Exception e) {
|
||||
call.reject("Failed to start audio service: " + e.getMessage(), e);
|
||||
}
|
||||
call.resolve();
|
||||
}
|
||||
|
||||
@PluginMethod
|
||||
public void stop(PluginCall call) {
|
||||
Intent intent = new Intent(getContext(), AudioPlaybackService.class);
|
||||
intent.setAction("STOP");
|
||||
getContext().stopService(intent);
|
||||
call.resolve();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4259,7 +4259,7 @@
|
|||
<!-- Legacy 16-Band Graphic EQ (visible in legacy mode) -->
|
||||
<div class="graphic-eq-section" id="graphic-eq-section" style="display: none">
|
||||
<div class="graphic-eq-preset-row">
|
||||
<span class="graphic-eq-preset-label">Preset</span>
|
||||
<label for="legacy-graphic-eq-preset-select" class="graphic-eq-preset-label">Preset</label>
|
||||
<select
|
||||
id="legacy-graphic-eq-preset-select"
|
||||
class="graphic-eq-preset-select"
|
||||
|
|
@ -4288,7 +4288,7 @@
|
|||
</div>
|
||||
<div class="graphic-eq-bottom-row">
|
||||
<div class="graphic-eq-preamp">
|
||||
<span class="graphic-eq-preamp-label">Preamp</span>
|
||||
<label for="legacy-graphic-eq-preamp-slider" class="graphic-eq-preamp-label">Preamp</label>
|
||||
<input
|
||||
type="range"
|
||||
id="legacy-graphic-eq-preamp-slider"
|
||||
|
|
@ -4368,6 +4368,7 @@
|
|||
id="autoeq-database-collapse"
|
||||
aria-label="Collapse database"
|
||||
aria-expanded="true"
|
||||
aria-controls="autoeq-database-body"
|
||||
>
|
||||
<use svg="!lucide/chevron-up.svg" size="18" />
|
||||
</button>
|
||||
|
|
|
|||
|
|
@ -353,7 +353,7 @@ class AudioContextManager {
|
|||
console.log(`[AudioContext] State changed to ${this.audioContext.state}, attempting resume`);
|
||||
// Use a short delay to let the system settle before resuming
|
||||
setTimeout(() => {
|
||||
if (this.audioContext && this.audioContext.state !== 'running') {
|
||||
if (this.audioContext && this.audioContext.state !== 'running' && this.source) {
|
||||
this.audioContext.resume().catch((e) => {
|
||||
console.warn('[AudioContext] Auto-resume failed:', e);
|
||||
});
|
||||
|
|
@ -847,7 +847,7 @@ class AudioContextManager {
|
|||
filter.type = newTypes[i] || 'peaking';
|
||||
filter.frequency.setTargetAtTime(newFrequencies[i], now, 0.005);
|
||||
filter.gain.setTargetAtTime(newGains[i], now, 0.005);
|
||||
filter.Q.setTargetAtTime(newQs[i], now, 0.005);
|
||||
filter.Q.setTargetAtTime(newQs[i] > 0 ? newQs[i] : this._calculateQ(i), now, 0.005);
|
||||
});
|
||||
} else {
|
||||
// Band count changed — must rebuild
|
||||
|
|
@ -1056,6 +1056,7 @@ class AudioContextManager {
|
|||
const now = this.audioContext.currentTime;
|
||||
this.geqFilters[bandIndex].gain.setTargetAtTime(this.geqGains[bandIndex], now, 0.01);
|
||||
}
|
||||
equalizerSettings.setGraphicEqGains([...this.geqGains]);
|
||||
}
|
||||
|
||||
setGraphicEqAllGains(gains) {
|
||||
|
|
@ -1068,6 +1069,7 @@ class AudioContextManager {
|
|||
this.geqFilters[i].gain.setTargetAtTime(this.geqGains[i], now, 0.01);
|
||||
}
|
||||
});
|
||||
equalizerSettings.setGraphicEqGains([...this.geqGains]);
|
||||
}
|
||||
|
||||
setGraphicEqPreamp(db) {
|
||||
|
|
@ -1077,6 +1079,7 @@ class AudioContextManager {
|
|||
const now = this.audioContext.currentTime;
|
||||
this.geqPreampNode.gain.setTargetAtTime(gainValue, now, 0.01);
|
||||
}
|
||||
equalizerSettings.setGraphicEqPreamp(this.geqPreamp);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -57,6 +57,8 @@ async function getButterchurnPresets(...args) {
|
|||
|
||||
// Module-level state for AutoEQ (persists across re-initializations)
|
||||
let _autoeqIndex = [];
|
||||
let _graphAbortController = null;
|
||||
let _graphResizeObserver = null;
|
||||
|
||||
export async function initializeSettings(scrobbler, player, api, ui) {
|
||||
// Restore last active settings tab
|
||||
|
|
@ -1287,6 +1289,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
|
|||
// Build 16 vertical slider bands into a container
|
||||
const buildGeqBands = (container, idPrefix) => {
|
||||
if (!container) return;
|
||||
container.innerHTML = '';
|
||||
GEQ_LABELS.forEach((_label, i) => {
|
||||
const band = document.createElement('div');
|
||||
band.className = 'graphic-eq-band';
|
||||
|
|
@ -2073,6 +2076,15 @@ export async function initializeSettings(scrobbler, player, api, ui) {
|
|||
return { x: e.clientX - rect.left, y: e.clientY - rect.top };
|
||||
};
|
||||
|
||||
// Clean up previous document-level listeners and observer on re-initialization
|
||||
if (_graphAbortController) _graphAbortController.abort();
|
||||
_graphAbortController = new AbortController();
|
||||
const graphSignal = _graphAbortController.signal;
|
||||
if (_graphResizeObserver) {
|
||||
_graphResizeObserver.disconnect();
|
||||
_graphResizeObserver = null;
|
||||
}
|
||||
|
||||
// Document-level mousemove so dragging continues outside the canvas
|
||||
document.addEventListener('mousemove', (e) => {
|
||||
if (draggedNode === null) return;
|
||||
|
|
@ -2116,7 +2128,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
|
|||
graphAnimFrame = null;
|
||||
});
|
||||
}
|
||||
});
|
||||
}, { signal: graphSignal });
|
||||
|
||||
// Canvas-only mousemove for hover cursor changes (when not dragging)
|
||||
autoeqCanvas.addEventListener('mousemove', (e) => {
|
||||
|
|
@ -2145,7 +2157,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
|
|||
draggedNode = null;
|
||||
autoeqCanvas.style.cursor = hoveredNode >= 0 ? 'grab' : 'crosshair';
|
||||
}
|
||||
});
|
||||
}, { signal: graphSignal });
|
||||
|
||||
autoeqCanvas.addEventListener('mouseleave', () => {
|
||||
// Only reset hover state, NOT drag state (drag continues outside canvas)
|
||||
|
|
@ -2291,6 +2303,11 @@ export async function initializeSettings(scrobbler, player, api, ui) {
|
|||
if (isParam) {
|
||||
const newGain = yToDb(coords.y - padTop, h, dbMin, dbMax);
|
||||
bands[touchNodeIdx].gain = Math.max(-30, Math.min(30, Math.round(newGain * 10) / 10));
|
||||
} else {
|
||||
const corrGain = interpolate(bands[touchNodeIdx].freq, autoeqCorrectedCurve || []);
|
||||
const newDb = yToDb(coords.y - padTop, h, dbMin, dbMax);
|
||||
const gainDelta = newDb - corrGain;
|
||||
bands[touchNodeIdx].gain = Math.max(-30, Math.min(30, bands[touchNodeIdx].gain + gainDelta * 0.3));
|
||||
}
|
||||
draggedNode = touchNodeIdx;
|
||||
computeCorrectedCurve();
|
||||
|
|
@ -2353,7 +2370,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
|
|||
}
|
||||
e.preventDefault();
|
||||
},
|
||||
{ passive: false }
|
||||
{ passive: false, signal: graphSignal }
|
||||
);
|
||||
|
||||
document.addEventListener('touchend', () => {
|
||||
|
|
@ -2361,14 +2378,14 @@ export async function initializeSettings(scrobbler, player, api, ui) {
|
|||
draggedNode = null;
|
||||
touchNodeIdx = -1;
|
||||
}
|
||||
});
|
||||
}, { signal: graphSignal });
|
||||
|
||||
// Resize observer for graph
|
||||
if (autoeqGraphWrapper) {
|
||||
const ro = new ResizeObserver(() => {
|
||||
_graphResizeObserver = new ResizeObserver(() => {
|
||||
scheduleDrawAutoEQGraph();
|
||||
});
|
||||
ro.observe(autoeqGraphWrapper);
|
||||
_graphResizeObserver.observe(autoeqGraphWrapper);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2529,6 +2546,10 @@ export async function initializeSettings(scrobbler, player, api, ui) {
|
|||
if (autoeqDatabaseCollapse) autoeqDatabaseCollapse.classList.toggle('collapsed');
|
||||
if (autoeqDatabaseBody)
|
||||
autoeqDatabaseBody.style.display = autoeqDatabaseBody.style.display === 'none' ? '' : 'none';
|
||||
if (autoeqDatabaseCollapse) {
|
||||
const isExpanded = !autoeqDatabaseCollapse.classList.contains('collapsed');
|
||||
autoeqDatabaseCollapse.setAttribute('aria-expanded', String(isExpanded));
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -4813,7 +4834,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
|
|||
|
||||
// Restore EQ mode on startup
|
||||
const savedEQMode = localStorage.getItem(EQ_MODE_KEY);
|
||||
if (savedEQMode && ['autoeq', 'parametric', 'speaker'].includes(savedEQMode)) {
|
||||
if (savedEQMode && ['autoeq', 'parametric', 'speaker', 'legacy'].includes(savedEQMode)) {
|
||||
setEQMode(savedEQMode);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1724,7 +1724,9 @@ export const equalizerSettings = {
|
|||
const stored = localStorage.getItem(this.GEQ_GAINS_KEY);
|
||||
if (stored) {
|
||||
const parsed = JSON.parse(stored);
|
||||
if (Array.isArray(parsed) && parsed.length === 16) return parsed;
|
||||
if (Array.isArray(parsed) && parsed.length === 16) {
|
||||
return parsed.map((v) => (Number.isFinite(v) ? v : 0));
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
/* ignore */
|
||||
|
|
@ -1743,7 +1745,11 @@ export const equalizerSettings = {
|
|||
getGraphicEqPreamp() {
|
||||
try {
|
||||
const val = localStorage.getItem(this.GEQ_PREAMP_KEY);
|
||||
return val !== null ? parseFloat(val) : 0;
|
||||
if (val !== null) {
|
||||
const num = parseFloat(val);
|
||||
return Number.isFinite(num) ? num : 0;
|
||||
}
|
||||
return 0;
|
||||
} catch {
|
||||
return 0;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7794,7 +7794,6 @@ body:has(#side-panel.active) #close-fullscreen-cover-btn {
|
|||
.graphic-eq-band-slider-wrap input[type='range'] {
|
||||
writing-mode: vertical-lr;
|
||||
direction: rtl;
|
||||
appearance: slider-vertical;
|
||||
width: 28px;
|
||||
height: 100%;
|
||||
accent-color: var(--foreground);
|
||||
|
|
|
|||
Loading…
Reference in a new issue