Fix carried use-sound origin and centralize sound normalization
This commit is contained in:
@@ -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";
|
||||
|
||||
@@ -19,6 +19,8 @@ export type EffectRuntime = {
|
||||
flangerLfoGain: GainNode | null;
|
||||
};
|
||||
|
||||
const reverbImpulseCache = new WeakMap<AudioContext, Map<string, AudioBuffer>>();
|
||||
|
||||
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<string, AudioBuffer>();
|
||||
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);
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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/<name>` 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
|
||||
|
||||
126
plans/audio-architecture-update-plan.md
Normal file
126
plans/audio-architecture-update-plan.md
Normal file
@@ -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.
|
||||
44
server/app/items/sound_policy.py
Normal file
44
server/app/items/sound_policy.py
Normal file
@@ -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
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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":
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user