mirror of
https://github.com/J3vb/OwnCord.git
synced 2026-09-03 03:50:00 +03:00
fix(client): make the screenshare volume slider actually change volume
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwtnpHAoSFr1ZibQgQkNQK
This commit is contained in:
@@ -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);
|
||||
}
|
||||
|
||||
@@ -31,6 +31,9 @@ export class AudioElements {
|
||||
private screenshareAudioElements = new Map<number, Set<HTMLAudioElement>>();
|
||||
/** Persisted mute state for screenshare audio so replacement tracks inherit UI state. */
|
||||
private screenshareAudioMutedByUser = new Map<number, boolean>();
|
||||
/** 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<number, number>();
|
||||
|
||||
/** 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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user