diff --git a/client/public/version.js b/client/public/version.js index da77a6e..e2711f5 100644 --- a/client/public/version.js +++ b/client/public/version.js @@ -1,5 +1,5 @@ // Maintainer-controlled web client version. // Format: YYYY.MM.DD Rn (example: 2026.02.20 R2) -window.CHGRID_WEB_VERSION = "2026.02.25 R235"; +window.CHGRID_WEB_VERSION = "2026.02.25 R236"; // Optional display timezone for timestamps. Falls back to America/Detroit if unset/invalid. window.CHGRID_TIME_ZONE = "America/Detroit"; diff --git a/client/src/audio/effects.ts b/client/src/audio/effects.ts index 00223df..9051d30 100644 --- a/client/src/audio/effects.ts +++ b/client/src/audio/effects.ts @@ -19,6 +19,8 @@ export type EffectRuntime = { flangerLfoGain: GainNode | null; }; +const reverbImpulseCache = new WeakMap>(); + export function clampEffectLevel(value: number): number { const clamped = Math.max(0, Math.min(100, value)); return Math.round(clamped * 10) / 10; @@ -96,7 +98,7 @@ export function connectEffectChain( if (effect === 'reverb') { const convolver = audioCtx.createConvolver(); - convolver.buffer = createImpulseResponse(audioCtx, 0.4 + effectMix * 4.2, 1 + effectMix * 3.6); + convolver.buffer = getCachedImpulseResponse(audioCtx, clampEffectLevel(effectValue)); const wetGain = audioCtx.createGain(); wetGain.gain.value = 0.06 + effectMix * 0.94; const dryGain = audioCtx.createGain(); @@ -145,6 +147,25 @@ export function connectEffectChain( return runtime; } +/** Returns a cached impulse response for a reverb amount bucket and sample rate. */ +function getCachedImpulseResponse(audioCtx: AudioContext, effectValue: number): AudioBuffer { + const roundedLevel = Math.round(Math.max(0, Math.min(100, effectValue))); + const cacheKey = `${audioCtx.sampleRate}:${roundedLevel}`; + let ctxCache = reverbImpulseCache.get(audioCtx); + if (!ctxCache) { + ctxCache = new Map(); + reverbImpulseCache.set(audioCtx, ctxCache); + } + const existing = ctxCache.get(cacheKey); + if (existing) { + return existing; + } + const mix = roundedLevel / 100; + const impulse = createImpulseResponse(audioCtx, 0.4 + mix * 4.2, 1 + mix * 3.6); + ctxCache.set(cacheKey, impulse); + return impulse; +} + /** Generates a synthetic impulse buffer used by the reverb convolver effect. */ function createImpulseResponse(audioCtx: AudioContext, duration: number, decay: number): AudioBuffer { const length = Math.floor(audioCtx.sampleRate * duration); diff --git a/client/src/main.ts b/client/src/main.ts index 5808da4..27009c4 100644 --- a/client/src/main.ts +++ b/client/src/main.ts @@ -718,6 +718,8 @@ function classifySystemMessageSound(message: string): keyof typeof SYSTEM_SOUND_ function resolveIncomingSoundUrl(url: string): string { const raw = String(url || '').trim(); if (!raw) return ''; + const lowered = raw.toLowerCase(); + if (lowered === 'none' || lowered === 'off') return ''; if (/^https?:/i.test(raw)) { return shouldProxyStreamUrl(raw) ? getProxyUrlForStream(raw) : raw; } diff --git a/docs/protocol-notes.md b/docs/protocol-notes.md index 520e795..cd13949 100644 --- a/docs/protocol-notes.md +++ b/docs/protocol-notes.md @@ -40,6 +40,7 @@ This is a behavior guide for packet semantics beyond raw schemas. - Piano runtime control no longer depends on parsing `item_action_result.message` text. - `item_piano_status` carries machine-readable piano events (`use_mode_entered`, record/playback transitions). - `item_use_sound` contains absolute item world coordinates (`x`, `y`) and sound path. + - For carried items, source coordinates resolve to the carrier's current position. - `item_piano_note` contains: - `itemId`, `senderId`, `keyId`, `midi`, `on` - resolved `instrument`, `voiceMode`, `octave`, `attack`, `decay`, `release`, `brightness`, `emitRange` @@ -69,6 +70,10 @@ This is a behavior guide for packet semantics beyond raw schemas. - Server is authoritative for all action validation and normalization. - Server is authoritative for movement acceptance (bounds + rate/delta checks). - Client validates incoming packet shapes and applies runtime behavior. +- Sound/media field normalization uses shared server policy helpers: + - `none/off` normalize to empty values + - bare filenames normalize to `sounds/` for sound-reference fields + - media URL-like fields are trimmed/validated consistently - Client-side item edit validation is convenience only; server remains source of truth. ## Heartbeat/Stale Recovery diff --git a/plans/audio-architecture-update-plan.md b/plans/audio-architecture-update-plan.md new file mode 100644 index 0000000..d59a39e --- /dev/null +++ b/plans/audio-architecture-update-plan.md @@ -0,0 +1,126 @@ +# Audio Architecture Update Proposal + +Date: 2026-02-25 + +## Goals + +1. Fix correctness issues first (sound origin for carried items). +2. Improve runtime stability without increasing server/upstream load. +3. Reduce duplicate work in audio runtime (shared streams, cached effects). +4. Keep server-first boundaries clear and avoid client/server drift. + +## Proposed Implementation Sequence + +### Phase 1: Correctness + low-risk fixes + +1. Carried-item `useSound` source position (server) +- Problem: `item_use_sound` currently uses `item.x/y`, which can be stale while carried. +- Change: resolve source position via carrier when `carrierId` is set, same pattern as piano. +- Files: + - `server/app/server.py` +- Acceptance: + - Using a carried item emits sound from carrier’s current square for all listeners. + +2. Stream retry policy hardening (client) +- Status: partially done (throttled retry + cap + cooldown). +- Follow-up: + - Add small inline debug counters in runtime (non-user-facing unless debug enabled). + - Ensure cooldown reset after successful play and cleanup path reset are covered by tests. +- Files: + - `client/src/audio/radioStationRuntime.ts` + - `client/src/audio/itemEmitRuntime.ts` +- Acceptance: + - No retry spam under repeated failures. + - Retry state recovers automatically when playback succeeds. + +### Phase 2: Performance and scaling improvements + +3. Emit source strategy +removed + +4. Reverb impulse cache (client) +- Problem: effect chain rebuilds can recreate impulse buffers frequently. +- Change: cache impulse responses by `(sampleRate, effectValueBucket)` in `effects.ts`. +- Files: + - `client/src/audio/effects.ts` +- Acceptance: + - Effect toggling no longer repeatedly regenerates same impulse buffers. + - No audible regressions in reverb behavior. + +### Phase 3: Consistency and maintainability + +5. Centralize sound URL normalization policy +- Problem: normalization logic exists in multiple places (server validator + client resolver + proxy behavior). +- Change: + - Define one policy doc and align implementation points: + - server validation/normalization + - client runtime resolution + - proxy Dropbox/http normalization behavior + - Move server normalization logic to shared item-sound helper(s), not tied to a specific item type module. +- Files: + - `server/app/items/...` shared validator/normalizer helper module + - per-item validators (`widget`, `radio_station`, and future sound-accepting items) call shared helper + - `client/src/main.ts` (`resolveIncomingSoundUrl`) + - `deploy/php/media_proxy.php` + - `docs/protocol-notes.md` or new dedicated audio policy section +- Acceptance: + - Same input URL/path yields predictable behavior across use/emit/radio. + - Fewer edge mismatches (`none/off`, `sounds/`, full URLs, Dropbox links). + +### Phase 4: Output routing + observability (defer) + +6. Output-device routing behavior +- Problem: `setSinkId` on muted element may not map to all WebAudio-rendered domains. +- Change options: + - A: Explicitly document browser limitation + current behavior. + - B: Investigate alternate routing architecture and apply if robust in target browsers. +- Recommendation: + - Ship A first (fast, clear UX), then evaluate B separately. +- Files: + - `docs/controls.md` and/or `docs/runtime-flow.md` + - optional runtime status text in settings UI +- Acceptance: + - Users get accurate expectation of output-device behavior. + +7. Audio runtime debug observability +- Change: + - Add optional debug object/report for: + - active radio shared sources + - active emit outputs/shared sources + - retry failures and cooldown state + - Keep disabled by default. +- Files: + - `client/src/audio/radioStationRuntime.ts` + - `client/src/audio/itemEmitRuntime.ts` + - optional small hook in `main.ts` for debug dump command +- Acceptance: + - Runtime state can be inspected quickly during field troubleshooting. + +## Risks and Mitigation + +1. Shared emit pooling could accidentally couple per-item controls. +- Mitigation: maintain per-item gain/effect nodes after shared source split. + +2. Output routing changes can be browser-fragile. +- Mitigation: document-first rollout, then narrow-scope prototype for alternate routing. + +3. Normalization centralization can break legacy links. +- Mitigation: add targeted tests for representative URL/path cases before refactor. + +## Suggested PR/Commit Breakdown + +1. Carried-item sound origin fix (server). +2. Emit shared source pooling. +3. Reverb impulse cache. +4. Sound normalization alignment (server/client/proxy + docs). +5. Output routing docs/UX clarification. +6. Optional debug observability layer. + +## Definition of Done + +1. Carried item sounds always originate from current carrier position. +2. No unbounded retry loops for stream failures. +3. Emit runtime reuses identical stream URLs. +4. Reverb buffer creation is cached and stable under effect churn. +5. Sound URL/path behavior is documented and consistent across server/client/proxy. +6. Audio runtime state is inspectable when debugging is enabled. diff --git a/server/app/items/sound_policy.py b/server/app/items/sound_policy.py new file mode 100644 index 0000000..b65af0d --- /dev/null +++ b/server/app/items/sound_policy.py @@ -0,0 +1,44 @@ +"""Shared normalization helpers for item sound/media URL parameters.""" + +from __future__ import annotations + + +def normalize_sound_reference(raw: object) -> str: + """Normalize sound value to empty/URL/or `sounds/`-relative path.""" + + token = str(raw or "").strip() + if not token: + return "" + lowered = token.lower() + if lowered in {"none", "off"}: + return "" + if lowered.startswith(("http://", "https://", "data:", "blob:")): + return token + if token.startswith("/sounds/"): + return token[1:] + if token.startswith("sounds/"): + return token + if "/" not in token: + return f"sounds/{token}" + return token + + +def normalize_media_reference(raw: object) -> str: + """Normalize media URL-like value while preserving path/query format.""" + + token = str(raw or "").strip() + if not token: + return "" + lowered = token.lower() + if lowered in {"none", "off"}: + return "" + return token + + +def enforce_max_length(value: str, *, max_length: int, field_name: str) -> str: + """Enforce max character length for normalized string fields.""" + + if len(value) > max_length: + raise ValueError(f"{field_name} must be {max_length} characters or less.") + return value + diff --git a/server/app/items/types/radio_station/validator.py b/server/app/items/types/radio_station/validator.py index 6aa3037..55b671c 100644 --- a/server/app/items/types/radio_station/validator.py +++ b/server/app/items/types/radio_station/validator.py @@ -3,6 +3,7 @@ from __future__ import annotations from ....models import WorldItem +from ...sound_policy import enforce_max_length, normalize_media_reference from ...helpers import keep_only_known_params from .definition import CHANNEL_OPTIONS, EFFECT_OPTIONS, PARAM_KEYS @@ -10,10 +11,11 @@ from .definition import CHANNEL_OPTIONS, EFFECT_OPTIONS, PARAM_KEYS def validate_update(item: WorldItem, next_params: dict) -> dict: """Validate and normalize radio params.""" - stream_url = str(next_params.get("streamUrl", "")).strip() - if len(stream_url) > 2048: - raise ValueError("streamUrl must be 2048 characters or less.") - next_params["streamUrl"] = stream_url + next_params["streamUrl"] = enforce_max_length( + normalize_media_reference(next_params.get("streamUrl", "")), + max_length=2048, + field_name="streamUrl", + ) enabled_value = next_params.get("enabled", True) if isinstance(enabled_value, bool): diff --git a/server/app/items/types/widget/validator.py b/server/app/items/types/widget/validator.py index e9754cb..8a1e8f6 100644 --- a/server/app/items/types/widget/validator.py +++ b/server/app/items/types/widget/validator.py @@ -3,30 +3,11 @@ from __future__ import annotations from ....models import WorldItem +from ...sound_policy import enforce_max_length, normalize_sound_reference from ...helpers import keep_only_known_params, parse_bool_like from .definition import EFFECT_OPTIONS, PARAM_KEYS -def _normalize_sound_value(raw: object) -> str: - """Normalize sound value to empty/URL/or sounds-relative path.""" - - token = str(raw or "").strip() - if not token: - return "" - lowered = token.lower() - if lowered in {"none", "off"}: - return "" - if lowered.startswith(("http://", "https://", "data:", "blob:")): - return token - if token.startswith("/sounds/"): - return token[1:] - if token.startswith("sounds/"): - return token - if "/" not in token: - return f"sounds/{token}" - return token - - def validate_update(item: WorldItem, next_params: dict) -> dict: """Validate and normalize widget params.""" @@ -88,10 +69,14 @@ def validate_update(item: WorldItem, next_params: dict) -> dict: raise ValueError("emitEffectValue must be between 0 and 100.") next_params["emitEffectValue"] = round(emit_effect_value, 1) - next_params["useSound"] = _normalize_sound_value(next_params.get("useSound", item.params.get("useSound", ""))) - next_params["emitSound"] = _normalize_sound_value(next_params.get("emitSound", item.params.get("emitSound", ""))) - if len(next_params["useSound"]) > 2048: - raise ValueError("useSound must be 2048 characters or less.") - if len(next_params["emitSound"]) > 2048: - raise ValueError("emitSound must be 2048 characters or less.") + next_params["useSound"] = enforce_max_length( + normalize_sound_reference(next_params.get("useSound", item.params.get("useSound", ""))), + max_length=2048, + field_name="useSound", + ) + next_params["emitSound"] = enforce_max_length( + normalize_sound_reference(next_params.get("emitSound", item.params.get("emitSound", ""))), + max_length=2048, + field_name="emitSound", + ) return keep_only_known_params(next_params, PARAM_KEYS) diff --git a/server/app/server.py b/server/app/server.py index 98bfa5e..03983d7 100644 --- a/server/app/server.py +++ b/server/app/server.py @@ -209,6 +209,15 @@ class SignalingServer: return item.useSound.strip() return None + def _get_item_sound_source_position(self, item: WorldItem) -> tuple[int, int]: + """Resolve source position for item-emitted one-shot sounds.""" + + if item.carrierId: + carrier = self._get_client_by_id(item.carrierId) + if carrier is not None: + return carrier.x, carrier.y + return item.x, item.y + def _get_client_by_id(self, client_id: str) -> ClientConnection | None: """Resolve one connected client by id.""" @@ -1141,13 +1150,14 @@ class SignalingServer: ) use_sound = self._resolve_item_use_sound(item) if use_sound: + sound_x, sound_y = self._get_item_sound_source_position(item) await self._broadcast( ItemUseSoundPacket( type="item_use_sound", itemId=item.id, sound=use_sound, - x=item.x, - y=item.y, + x=sound_x, + y=sound_y, ) ) if item.type == "piano": diff --git a/server/tests/test_item_use_cooldown.py b/server/tests/test_item_use_cooldown.py index eb48ca0..190ac5d 100644 --- a/server/tests/test_item_use_cooldown.py +++ b/server/tests/test_item_use_cooldown.py @@ -399,6 +399,44 @@ async def test_widget_update_and_use(monkeypatch: pytest.MonkeyPatch) -> None: assert "emitsoundtempo must be between 0 and 100" in send_payloads[-1].message.lower() +@pytest.mark.asyncio +async def test_carried_item_use_sound_uses_carrier_position(monkeypatch: pytest.MonkeyPatch) -> None: + server = SignalingServer("127.0.0.1", 8765, None, None) + ws = _fake_ws() + client = ClientConnection(websocket=ws, id="u1", nickname="tester", x=5, y=6) + server.clients[ws] = client + item = server.item_service.default_item(client, "widget") + item.params["useSound"] = "sounds/test.ogg" + item.carrierId = client.id + # Keep stale coordinates to verify carrier position is used for use-sound broadcasts. + item.x = 1 + item.y = 1 + server.item_service.add_item(item) + client.x = 9 + client.y = 10 + + send_payloads: list[object] = [] + broadcast_payloads: list[object] = [] + now_ms = 60_000 + + async def fake_send(websocket: ServerConnection, packet: object) -> None: + send_payloads.append(packet) + + async def fake_broadcast(packet: object, exclude: ServerConnection | None = None) -> None: + broadcast_payloads.append(packet) + + monkeypatch.setattr(server, "_send", fake_send) + monkeypatch.setattr(server, "_broadcast", fake_broadcast) + monkeypatch.setattr(server.item_service, "now_ms", lambda: now_ms) + + await server._handle_message(client, json.dumps({"type": "item_use", "itemId": item.id})) + assert send_payloads[-1].ok is True + sound_packets = [packet for packet in broadcast_payloads if getattr(packet, "type", "") == "item_use_sound"] + assert sound_packets + assert sound_packets[-1].x == 9 + assert sound_packets[-1].y == 10 + + @pytest.mark.asyncio async def test_piano_update_and_use(monkeypatch: pytest.MonkeyPatch) -> None: server = SignalingServer("127.0.0.1", 8765, None, None)