From 7c2c9843b78f3ef0b211a31b01a66dbf89f8a4d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 10:50:10 +0000 Subject: [PATCH] fix(client): make the screenshare volume slider actually change volume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects made the screenshare tile's volume slider ineffective: - The 0-200 slider mapped to element volume /200 clamped to [0,1], while the element attached at 1.0 — dragging the upper half did nothing. The screenshare slider is now 0-100 with 100 = 1.0 (HTMLAudioElement.volume cannot exceed 1.0; mic tiles keep the 0-200 boost range via LiveKit's GainNode-backed setVolume). - Setting a volume before the screenshare audio track attached was silently dropped. The per-user volume now persists independently of the element map and is applied on attach. - Changing the master output volume overwrote per-user screenshare volumes with just the master multiplier; they now scale together. The slider and mute button also initialize from persisted state when a tile is rebuilt, and unmuting via the button re-applies the restored volume. Fixes #121 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LwtnpHAoSFr1ZibQgQkNQK --- .../tauri-client/src/components/VideoGrid.ts | 26 +++++++---- Client/tauri-client/src/lib/audioElements.ts | 37 ++++++++++++---- Client/tauri-client/src/lib/livekitSession.ts | 5 +++ .../tests/unit/audio-elements.test.ts | 37 ++++++++++++++++ .../tests/unit/video-grid.test.ts | 43 +++++++++++++++++++ 5 files changed, 131 insertions(+), 17 deletions(-) diff --git a/Client/tauri-client/src/components/VideoGrid.ts b/Client/tauri-client/src/components/VideoGrid.ts index 23500363..19cf4391 100644 --- a/Client/tauri-client/src/components/VideoGrid.ts +++ b/Client/tauri-client/src/components/VideoGrid.ts @@ -6,6 +6,8 @@ import { createElement, appendChildren } from "@lib/dom"; import { createIcon } from "@lib/icons"; import { + getScreenshareAudioMuted, + getScreenshareAudioVolume, muteScreenshareAudio, setScreenshareAudioVolume, setUserVolume, @@ -304,8 +306,13 @@ export function createVideoGrid(): VideoGridComponent { // Add audio control overlay for remote tiles if (config !== undefined && !config.isSelf) { - let muted = false; - let currentVolume = 100; + // Screenshare audio state survives tile rebuilds — initialize from it. + // Screenshare sliders are 0-100 (HTMLAudioElement.volume caps at 1.0); + // mic sliders keep 0-200 (LiveKit setVolume supports boost up to 2.0). + let muted = config.isScreenshare ? getScreenshareAudioMuted(config.audioUserId) : false; + let currentVolume = config.isScreenshare + ? Math.round(getScreenshareAudioVolume(config.audioUserId) * 100) + : 100; const overlay = createElement("div", { class: "video-tile-overlay" }); @@ -313,8 +320,8 @@ export function createVideoGrid(): VideoGridComponent { const volumeSlider = createElement("input", { type: "range", min: "0", - max: "200", - value: "100", + max: config.isScreenshare ? "100" : "200", + value: String(currentVolume), class: "tile-volume-slider", "aria-label": "Volume", }); @@ -324,9 +331,10 @@ export function createVideoGrid(): VideoGridComponent { const wasMuted = muted; muted = currentVolume === 0; if (config.isScreenshare) { - // BUG-102: Set actual volume, not just mute toggle. + // BUG-102: Set actual volume, not just mute toggle. Slider 100 maps + // to element volume 1.0 (the attach-time default). muteScreenshareAudio(config.audioUserId, muted); - setScreenshareAudioVolume(config.audioUserId, currentVolume / 200); + setScreenshareAudioVolume(config.audioUserId, currentVolume / 100); } else { setUserVolume(config.audioUserId, currentVolume); } @@ -340,9 +348,10 @@ export function createVideoGrid(): VideoGridComponent { // Mute button const muteBtn = createElement("button", { class: "tile-mute-btn", - "aria-label": "Mute", + "aria-label": muted ? "Unmute" : "Mute", }); - muteBtn.appendChild(volumeIcon()); + muteBtn.appendChild(muted ? volumeXIcon() : volumeIcon()); + if (muted) overlay.classList.add("muted"); muteBtn.addEventListener("click", () => { muted = !muted; @@ -357,6 +366,7 @@ export function createVideoGrid(): VideoGridComponent { if (currentVolume === 0) currentVolume = 100; if (config.isScreenshare) { muteScreenshareAudio(config.audioUserId, false); + setScreenshareAudioVolume(config.audioUserId, currentVolume / 100); } else { setUserVolume(config.audioUserId, currentVolume); } diff --git a/Client/tauri-client/src/lib/audioElements.ts b/Client/tauri-client/src/lib/audioElements.ts index 7e2335bf..b78580b3 100644 --- a/Client/tauri-client/src/lib/audioElements.ts +++ b/Client/tauri-client/src/lib/audioElements.ts @@ -31,6 +31,9 @@ export class AudioElements { private screenshareAudioElements = new Map>(); /** Persisted mute state for screenshare audio so replacement tracks inherit UI state. */ private screenshareAudioMutedByUser = new Map(); + /** Per-user screenshare volume (0-1, default 1) — kept independent of the + * element map so a volume chosen before the track attaches still applies. */ + private screenshareVolumeByUser = new Map(); /** Master output volume multiplier (0-2.0). Per-user volumes are scaled by this. */ private outputVolumeMultiplier: number; @@ -54,8 +57,12 @@ export class AudioElements { return (userVol / 100) * this.outputVolumeMultiplier; } - private getScreenshareOutputVolume(): number { - return Math.max(0, Math.min(1, this.outputVolumeMultiplier)); + /** Effective element volume for a user's screenshare audio: per-user volume + * scaled by the master output multiplier, clamped to the 0-1 range that + * HTMLAudioElement.volume supports. */ + private getEffectiveScreenshareVolume(userId: number): number { + const userVol = this.screenshareVolumeByUser.get(userId) ?? 1; + return Math.max(0, Math.min(1, userVol * this.outputVolumeMultiplier)); } // --- Track subscription handlers --- @@ -80,7 +87,7 @@ export class AudioElements { const audioEl = track.attach(); audioEl.style.display = "none"; document.body.appendChild(audioEl); - audioEl.volume = this.getScreenshareOutputVolume(); + audioEl.volume = this.getEffectiveScreenshareVolume(userId); audioEl.muted = this.screenshareAudioMutedByUser.get(userId) ?? false; let audioEls = this.screenshareAudioElements.get(userId); if (audioEls === undefined) { @@ -187,10 +194,12 @@ export class AudioElements { savePref("outputVolume", clamped); this.outputVolumeMultiplier = clamped / 100; this.applyAllVolumes(); - const screenshareVolume = this.getScreenshareOutputVolume(); - for (const audioEls of this.screenshareAudioElements.values()) { + // Re-apply per-user screenshare volumes scaled by the new master value + // (BUG: previously overwrote them with just the master multiplier). + for (const [userId, audioEls] of this.screenshareAudioElements) { + const effective = this.getEffectiveScreenshareVolume(userId); for (const audioEl of audioEls) { - audioEl.volume = screenshareVolume; + audioEl.volume = effective; } } } @@ -198,10 +207,19 @@ export class AudioElements { // --- Screenshare audio --- setScreenshareAudioVolume(userId: number, volume: number): void { + const clamped = Math.max(0, Math.min(1, volume)); + // Always store, even before the audio track attaches — the stored value + // is applied in handleTrackSubscribedAudio when the element appears. + this.screenshareVolumeByUser.set(userId, clamped); const audioEls = this.screenshareAudioElements.get(userId); if (audioEls === undefined) return; - const clamped = Math.max(0, Math.min(1, volume)); - for (const el of audioEls) el.volume = clamped; + const effective = this.getEffectiveScreenshareVolume(userId); + for (const el of audioEls) el.volume = effective; + } + + /** Stored per-user screenshare volume (0-1, default 1) for slider init. */ + getScreenshareAudioVolume(userId: number): number { + return this.screenshareVolumeByUser.get(userId) ?? 1; } muteScreenshareAudio(userId: number, muted: boolean): void { @@ -242,9 +260,10 @@ export class AudioElements { this.screenshareAudioElements.clear(); } - /** Full cleanup including screenshare mute state — used on intentional leave. */ + /** Full cleanup including screenshare mute/volume state — used on intentional leave. */ cleanupAllAudioElementsFull(): void { this.cleanupAllAudioElements(); this.screenshareAudioMutedByUser.clear(); + this.screenshareVolumeByUser.clear(); } } diff --git a/Client/tauri-client/src/lib/livekitSession.ts b/Client/tauri-client/src/lib/livekitSession.ts index f8ab99d5..cbfd2e42 100644 --- a/Client/tauri-client/src/lib/livekitSession.ts +++ b/Client/tauri-client/src/lib/livekitSession.ts @@ -1590,6 +1590,10 @@ export class LiveKitSession { this._audioElements.setScreenshareAudioVolume(userId, volume); } + getScreenshareAudioVolume(userId: number): number { + return this._audioElements.getScreenshareAudioVolume(userId); + } + muteScreenshareAudio(userId: number, muted: boolean): void { this._audioElements.muteScreenshareAudio(userId, muted); } @@ -1697,6 +1701,7 @@ export const getLocalScreenshareStream = session.getLocalScreenshareStream.bind( export const getRemoteVideoStream = session.getRemoteVideoStream.bind(session); export const getSessionDebugInfo = session.getSessionDebugInfo.bind(session); export const setScreenshareAudioVolume = session.setScreenshareAudioVolume.bind(session); +export const getScreenshareAudioVolume = session.getScreenshareAudioVolume.bind(session); export const muteScreenshareAudio = session.muteScreenshareAudio.bind(session); export const getScreenshareAudioMuted = session.getScreenshareAudioMuted.bind(session); diff --git a/Client/tauri-client/tests/unit/audio-elements.test.ts b/Client/tauri-client/tests/unit/audio-elements.test.ts index dfdf4ced..1088ffcc 100644 --- a/Client/tauri-client/tests/unit/audio-elements.test.ts +++ b/Client/tauri-client/tests/unit/audio-elements.test.ts @@ -237,6 +237,43 @@ describe("AudioElements", () => { expect(audioEl.volume).toBe(1); }); + it("getScreenshareAudioVolume defaults to 1 and reflects stored value", () => { + expect(elements.getScreenshareAudioVolume(42)).toBe(1); + elements.setScreenshareAudioVolume(42, 0.4); + expect(elements.getScreenshareAudioVolume(42)).toBe(0.4); + }); + + it("volume chosen before the track attaches applies on attach", () => { + elements.setScreenshareAudioVolume(42, 0.25); + + const { track, audioEl } = createMockTrack("audio", "track-ss-early"); + const publication = { source: "screenShareAudio" }; + const participant = { identity: "user-42", setVolume: vi.fn() }; + elements.handleTrackSubscribedAudio(track as any, publication as any, participant as any); + + expect(audioEl.volume).toBe(0.25); + }); + + it("setOutputVolume preserves per-user screenshare volume", () => { + const audioEl = document.createElement("audio"); + (elements as any).screenshareAudioElements = new Map([[42, new Set([audioEl])]]); + + elements.setScreenshareAudioVolume(42, 0.5); + elements.setOutputVolume(50); + + expect(audioEl.volume).toBe(0.25); // 0.5 user * 0.5 master + }); + + it("setScreenshareAudioVolume scales by the master output multiplier", () => { + const audioEl = document.createElement("audio"); + (elements as any).screenshareAudioElements = new Map([[42, new Set([audioEl])]]); + + elements.setOutputVolume(50); + elements.setScreenshareAudioVolume(42, 0.8); + + expect(audioEl.volume).toBe(0.4); // 0.8 user * 0.5 master + }); + it("muteScreenshareAudio persists muted state", () => { elements.muteScreenshareAudio(42, true); expect(elements.getScreenshareAudioMuted(42)).toBe(true); diff --git a/Client/tauri-client/tests/unit/video-grid.test.ts b/Client/tauri-client/tests/unit/video-grid.test.ts index bcc2c903..ccc80b7e 100644 --- a/Client/tauri-client/tests/unit/video-grid.test.ts +++ b/Client/tauri-client/tests/unit/video-grid.test.ts @@ -7,11 +7,15 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; const mockMuteScreenshareAudio = vi.fn(); const mockSetScreenshareAudioVolume = vi.fn(); const mockSetUserVolume = vi.fn(); +const mockGetScreenshareAudioMuted = vi.fn((_userId?: unknown) => false); +const mockGetScreenshareAudioVolume = vi.fn((_userId?: unknown) => 1); vi.mock("@lib/livekitSession", () => ({ muteScreenshareAudio: (...args: unknown[]) => mockMuteScreenshareAudio(...args), setScreenshareAudioVolume: (...args: unknown[]) => mockSetScreenshareAudioVolume(...args), setUserVolume: (...args: unknown[]) => mockSetUserVolume(...args), + getScreenshareAudioMuted: (userId: unknown) => mockGetScreenshareAudioMuted(userId), + getScreenshareAudioVolume: (userId: unknown) => mockGetScreenshareAudioVolume(userId), })); // --------------------------------------------------------------------------- @@ -487,6 +491,45 @@ describe("VideoGrid", () => { expect(mockMuteScreenshareAudio).toHaveBeenCalledWith(88, false); }); + it("screenshare slider uses 0-100 range where 100 maps to volume 1.0", () => { + const config = makeTileConfig({ isSelf: false, audioUserId: 88, isScreenshare: true }); + grid.addStream(88, "screen", fakeStream(), config); + + const slider = container.querySelector(".tile-volume-slider") as HTMLInputElement; + expect(slider.max).toBe("100"); + expect(slider.value).toBe("100"); // stored default 1.0 → 100 + + slider.value = "50"; + slider.dispatchEvent(new Event("input")); + expect(mockSetScreenshareAudioVolume).toHaveBeenCalledWith(88, 0.5); + + slider.value = "100"; + slider.dispatchEvent(new Event("input")); + expect(mockSetScreenshareAudioVolume).toHaveBeenCalledWith(88, 1); + }); + + it("screenshare slider initializes from stored volume and mute state", () => { + mockGetScreenshareAudioVolume.mockReturnValueOnce(0.3); + mockGetScreenshareAudioMuted.mockReturnValueOnce(true); + const config = makeTileConfig({ isSelf: false, audioUserId: 88, isScreenshare: true }); + grid.addStream(88, "screen", fakeStream(), config); + + const slider = container.querySelector(".tile-volume-slider") as HTMLInputElement; + const muteBtn = container.querySelector(".tile-mute-btn") as HTMLButtonElement; + const overlay = container.querySelector(".video-tile-overlay"); + expect(slider.value).toBe("30"); + expect(muteBtn.getAttribute("aria-label")).toBe("Unmute"); + expect(overlay!.classList.contains("muted")).toBe(true); + }); + + it("mic slider keeps the 0-200 boost range", () => { + const config = makeTileConfig({ isSelf: false, audioUserId: 77, isScreenshare: false }); + grid.addStream(77, "dave", fakeStream(), config); + + const slider = container.querySelector(".tile-volume-slider") as HTMLInputElement; + expect(slider.max).toBe("200"); + }); + it("mute button unmutes with previous volume when currentVolume was non-zero", () => { const config = makeTileConfig({ isSelf: false, audioUserId: 77, isScreenshare: false }); grid.addStream(77, "dave", fakeStream(), config);