feat: implement media echo fix and YouTube screenshare design

- Introduced a new `ScreenShareController` to manage YouTube screenshare functionality.
- Updated `DiscordPlayer` to track ownership of audio streams, preventing conflicts between music playback and screenshare.
- Added error handling for various states including voice connection checks and media busy states.
- Created unit tests for `ScreenShareController` and `DiscordPlayer` ownership rules to ensure correct functionality.
- Added documentation for the new media echo fix and screenshare design.

tests/media/mediaController.test.ts

@@ -5,6 +5,7 @@ import type {
   MusicPlayback,
   MusicPlayer,
   ResolvedMediaSource,
+  ScreenShareController,
 } from "../../src/media/mediaTypes";
 
 function deferred() {
@@ -190,7 +191,62 @@ describe("MediaController", () => {
     const state = await controller.stop();
 
     expect(stop).toHaveBeenCalled();
-    expect(state).toEqual({ playing: false, current: null, queue: [] });
+    expect(state).toEqual({
+      playing: false,
+      activeMode: null,
+      current: null,
+      queue: [],
+    });
+  });
+
+  it("starts screen share mode without resolving music source", async () => {
+    const screenPlayback = deferred();
+    const screenController: ScreenShareController = {
+      isActive: vi.fn(() => false),
+      start: vi.fn(async () => ({
+        done: screenPlayback.promise,
+        stop: vi.fn(),
+      })),
+    };
+    const resolveMediaSource = vi.fn(async (input) => source(input));
+    const controller = new MediaController({
+      isVoiceConnected: () => true,
+      isBrowserStreaming: () => false,
+      resolveMediaSource,
+      musicPlayer: { play: vi.fn() },
+      screenController,
+    });
+
+    const state = await controller.queue("https://youtu.be/video", {
+      mode: "screen",
+    });
+
+    expect(screenController.start).toHaveBeenCalledWith(
+      "https://youtu.be/video",
+    );
+    expect(resolveMediaSource).not.toHaveBeenCalled();
+    expect(state).toMatchObject({ playing: true, activeMode: "screen" });
+  });
+
+  it("rejects music while screen share is active", async () => {
+    const screenController: ScreenShareController = {
+      isActive: vi.fn(() => true),
+      start: vi.fn(),
+    };
+    const controller = new MediaController({
+      isVoiceConnected: () => true,
+      isBrowserStreaming: () => false,
+      resolveMediaSource: async (input) => source(input),
+      musicPlayer: { play: vi.fn() },
+      screenController,
+    });
+
+    await expect(
+      controller.queue("https://example.com/song.mp3"),
+    ).rejects.toMatchObject({
+      code: "MEDIA_BUSY",
+      statusCode: 409,
+    } satisfies Partial<AppError>);
   });
 
   it("emits state changes", async () => {
@@ -200,7 +256,10 @@ describe("MediaController", () => {
       isBrowserStreaming: () => false,
       resolveMediaSource: async (input) => source(input),
       musicPlayer: {
-        play: vi.fn(() => ({ done: new Promise(() => {}), stop: vi.fn() })),
+        play: vi.fn(() => ({
+          done: new Promise<void>(() => {}),
+          stop: vi.fn(),
+        })),
       },
       onStateChange,
     });

tests/media/mediaQueue.test.ts

@@ -20,7 +20,7 @@ describe("MediaQueue", () => {
       () => 1700000000000,
     );
 
-    const item = queue.add(source(), "tester");
+    const item = queue.add(source(), "music", "tester");
 
     expect(item).toMatchObject({
       id: "item-1",
@@ -40,7 +40,7 @@ describe("MediaQueue", () => {
       () => "item-1",
       () => 1700000000000,
     );
-    const item = queue.add(source(), "tester");
+    const item = queue.add(source(), "music", "tester");
 
     expect(queue.startNext()).toEqual({ ...item, status: "playing" });
     expect(queue.snapshot()).toEqual({
@@ -55,8 +55,8 @@ describe("MediaQueue", () => {
       () => `item-${++id}`,
       () => 1700000000000,
     );
-    queue.add(source({ title: "first" }), "tester");
-    queue.add(source({ title: "second" }), "tester");
+    queue.add(source({ title: "first" }), "music", "tester");
+    queue.add(source({ title: "second" }), "music", "tester");
     queue.startNext();
 
     queue.completeCurrent();
@@ -71,7 +71,7 @@ describe("MediaQueue", () => {
       () => "item-1",
       () => 1700000000000,
     );
-    const item = queue.add(source(), "tester");
+    const item = queue.add(source(), "music", "tester");
     queue.startNext();
 
     expect(queue.failCurrent()).toEqual({ ...item, status: "failed" });
@@ -83,7 +83,7 @@ describe("MediaQueue", () => {
       () => "item-1",
       () => 1700000000000,
     );
-    queue.add(source(), "tester");
+    queue.add(source(), "music", "tester");
     queue.startNext();
 
     queue.clear();

tests/media/musicPlayer.test.ts

@@ -1,7 +1,14 @@
+import type { spawn as nodeSpawn } from "node:child_process";
+
+type Spawn = typeof nodeSpawn;
+
 import { EventEmitter } from "node:events";
 import { PassThrough } from "node:stream";
 import { describe, expect, it, vi } from "vitest";
-import type { DiscordAudioPlayer } from "../../src/media/mediaTypes";
+import type {
+  DiscordAudioPlayer,
+  DiscordPlayerOwner,
+} from "../../src/media/mediaTypes";
 import { createMusicPlayer } from "../../src/media/musicPlayer";
 
 class FakeProcess extends EventEmitter {
@@ -22,9 +29,15 @@ describe("createMusicPlayer", () => {
     const discordPlayer: DiscordAudioPlayer = {
       isConnected: () => true,
       playStream: vi.fn(),
+      getOwner: vi.fn((): DiscordPlayerOwner => "none"),
+      pause: vi.fn(),
+      unpause: vi.fn(() => true),
       stop: vi.fn(),
     };
-    const player = createMusicPlayer({ spawn, discordPlayer });
+    const player = createMusicPlayer({
+      spawn: spawn as unknown as Spawn,
+      discordPlayer,
+    });
 
     const playback = player.play({
       source: "https://example.com/song.mp3",
@@ -55,7 +68,7 @@ describe("createMusicPlayer", () => {
       ],
       { stdio: ["ignore", "pipe", "pipe"] },
     );
-    expect(discordPlayer.playStream).toHaveBeenCalledWith(proc.stdout);
+    expect(discordPlayer.playStream).toHaveBeenCalledWith(proc.stdout, "music");
   });
 
   it("rejects playback when Discord is not connected", () => {
@@ -63,9 +76,15 @@ describe("createMusicPlayer", () => {
     const discordPlayer: DiscordAudioPlayer = {
       isConnected: () => false,
       playStream: vi.fn(),
+      getOwner: vi.fn((): DiscordPlayerOwner => "none"),
+      pause: vi.fn(),
+      unpause: vi.fn(() => true),
       stop: vi.fn(),
     };
-    const player = createMusicPlayer({ spawn, discordPlayer });
+    const player = createMusicPlayer({
+      spawn: spawn as unknown as Spawn,
+      discordPlayer,
+    });
 
     expect(() =>
       player.play({
@@ -77,15 +96,44 @@ describe("createMusicPlayer", () => {
     expect(spawn).not.toHaveBeenCalled();
   });
 
+  it("releases ownership on normal ffmpeg close", async () => {
+    const proc = new FakeProcess();
+    const discordPlayer: DiscordAudioPlayer = {
+      isConnected: () => true,
+      playStream: vi.fn(),
+      getOwner: vi.fn((): DiscordPlayerOwner => "none"),
+      pause: vi.fn(),
+      unpause: vi.fn(() => true),
+      stop: vi.fn(),
+    };
+    const player = createMusicPlayer({
+      spawn: vi.fn(() => proc) as unknown as Spawn,
+      discordPlayer,
+    });
+
+    const playback = player.play({
+      source: "/tmp/song.ogg",
+      title: "song.ogg",
+      kind: "local",
+    });
+    // simulate normal close
+    proc.emit("close", 0);
+    await playback.done;
+    expect(discordPlayer.stop).toHaveBeenCalledWith("music");
+  });
+
   it("kills ffmpeg and stops Discord playback once", () => {
     const proc = new FakeProcess();
     const discordPlayer: DiscordAudioPlayer = {
       isConnected: () => true,
       playStream: vi.fn(),
+      getOwner: vi.fn((): DiscordPlayerOwner => "none"),
+      pause: vi.fn(),
+      unpause: vi.fn(() => true),
       stop: vi.fn(),
     };
     const player = createMusicPlayer({
-      spawn: vi.fn(() => proc),
+      spawn: vi.fn(() => proc) as unknown as Spawn,
       discordPlayer,
     });
 

tests/media/screenShareController.test.ts

@@ -0,0 +1,94 @@
+import { PassThrough } from "node:stream";
+import { describe, expect, it, vi } from "vitest";
+import { AppError } from "../../src/errors";
+import type { DiscordPlayerOwner } from "../../src/media/mediaTypes";
+import { createScreenShareController } from "../../src/media/screenShareController";
+
+function createDependencies() {
+  const output = new PassThrough();
+  return {
+    getVoiceStatus: vi.fn(() => ({
+      connected: true,
+      activeGuildId: "guild-1" as string | null,
+      activeChannelId: "channel-1" as string | null,
+    })),
+    getPlayerOwner: vi.fn((): DiscordPlayerOwner => "none"),
+    getDirectVideoUrl: vi.fn(async () => "https://cdn.example.com/video.mp4"),
+    prepareStream: vi.fn(() => ({
+      command: { kill: vi.fn() },
+      output,
+    })),
+    playStream: vi.fn(() => new Promise<void>(() => {})),
+    streamer: { id: "streamer" },
+  };
+}
+
+describe("createScreenShareController", () => {
+  it("starts a YouTube Go Live stream", async () => {
+    const dependencies = createDependencies();
+    const controller = createScreenShareController(dependencies);
+
+    const playback = await controller.start("https://youtu.be/video");
+
+    expect(dependencies.getDirectVideoUrl).toHaveBeenCalledWith(
+      "https://youtu.be/video",
+    );
+    expect(dependencies.prepareStream).toHaveBeenCalledWith(
+      "https://cdn.example.com/video.mp4",
+      expect.objectContaining({ includeAudio: true }),
+    );
+    expect(dependencies.playStream).toHaveBeenCalledWith(
+      dependencies.prepareStream.mock.results[0].value.output,
+      dependencies.streamer,
+      { type: "go-live" },
+    );
+    expect(controller.isActive()).toBe(true);
+    playback.stop();
+    expect(controller.isActive()).toBe(false);
+  });
+
+  it("rejects when voice is not connected", async () => {
+    const dependencies = createDependencies();
+    dependencies.getVoiceStatus.mockReturnValue({
+      connected: false,
+      activeGuildId: null,
+      activeChannelId: null,
+    });
+    const controller = createScreenShareController(dependencies);
+
+    await expect(
+      controller.start("https://youtu.be/video"),
+    ).rejects.toMatchObject({
+      code: "VOICE_NOT_CONNECTED",
+      statusCode: 409,
+    } satisfies Partial<AppError>);
+  });
+
+  it("rejects when music owns the shared player", async () => {
+    const dependencies = createDependencies();
+    dependencies.getPlayerOwner.mockReturnValue("music");
+    const controller = createScreenShareController(dependencies);
+
+    await expect(
+      controller.start("https://youtu.be/video"),
+    ).rejects.toMatchObject({
+      code: "MEDIA_BUSY",
+      statusCode: 409,
+    } satisfies Partial<AppError>);
+  });
+
+  it("wraps stream startup failures", async () => {
+    const dependencies = createDependencies();
+    dependencies.playStream.mockImplementation(() => {
+      throw new Error("go live failed");
+    });
+    const controller = createScreenShareController(dependencies);
+
+    await expect(
+      controller.start("https://youtu.be/video"),
+    ).rejects.toMatchObject({
+      code: "SCREEN_STREAM_FAILED",
+      statusCode: 500,
+    } satisfies Partial<AppError>);
+  });
+});

tests/media/ytdlp.test.ts

@@ -43,7 +43,8 @@ describe("createYtDlp", () => {
 
   it("reads direct audio URL", async () => {
     const proc = new FakeProcess();
-    const ytdlp = createYtDlp({ spawn: vi.fn(() => proc) });
+    const spawn = vi.fn(() => proc);
+    const ytdlp = createYtDlp({ spawn });
 
     const result = ytdlp.getDirectAudioUrl("https://youtu.be/video");
     proc.stdout.write("https://audio.example.com/stream\n");
@@ -51,6 +52,45 @@ describe("createYtDlp", () => {
     proc.emit("close", 0);
 
     await expect(result).resolves.toBe("https://audio.example.com/stream");
+    expect(spawn).toHaveBeenCalledWith(
+      "yt-dlp",
+      [
+        "https://youtu.be/video",
+        "--get-url",
+        "--format",
+        "bestaudio[protocol^=http]/bestaudio/best",
+        "--no-playlist",
+        "--no-warnings",
+        "--quiet",
+      ],
+      { stdio: ["ignore", "pipe", "pipe"] },
+    );
+  });
+
+  it("reads direct video URL", async () => {
+    const proc = new FakeProcess();
+    const spawn = vi.fn(() => proc);
+    const ytdlp = createYtDlp({ spawn });
+
+    const result = ytdlp.getDirectVideoUrl("https://youtu.be/video");
+    proc.stdout.write("https://video.example.com/stream\n");
+    proc.stdout.end();
+    proc.emit("close", 0);
+
+    await expect(result).resolves.toBe("https://video.example.com/stream");
+    expect(spawn).toHaveBeenCalledWith(
+      "yt-dlp",
+      [
+        "https://youtu.be/video",
+        "--get-url",
+        "--format",
+        "bestvideo[protocol^=http]+bestaudio[protocol^=http]/best[protocol^=http]/best",
+        "--no-playlist",
+        "--no-warnings",
+        "--quiet",
+      ],
+      { stdio: ["ignore", "pipe", "pipe"] },
+    );
   });
 
   it("rejects when yt-dlp exits non-zero", async () => {

tests/player.test.ts

@@ -0,0 +1,82 @@
+import { Readable } from "node:stream";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+// Mock @discordjs/voice
+vi.mock("@discordjs/voice", () => {
+  const mockPlayer = {
+    play: vi.fn(),
+    pause: vi.fn(),
+    unpause: vi.fn().mockReturnValue(true),
+    stop: vi.fn(),
+    on: vi.fn(),
+    state: { status: "idle" },
+  };
+  const mockConnection = {
+    subscribe: vi.fn().mockReturnValue({}),
+  };
+  return {
+    AudioPlayerStatus: { Idle: "idle", Playing: "playing", Paused: "paused" },
+    createAudioPlayer: vi.fn(() => mockPlayer),
+    createAudioResource: vi.fn(() => ({})),
+    StreamType: { OggOpus: "OggOpus" },
+    AudioPlayer: vi.fn(),
+    VoiceConnection: vi.fn(),
+    __mockPlayer: mockPlayer,
+    __mockConnection: mockConnection,
+  };
+});
+
+// Import after mocks
+import { DiscordPlayer } from "../src/player";
+
+describe("DiscordPlayer", () => {
+  let player: DiscordPlayer;
+  const dummyStream = new Readable();
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    player = new DiscordPlayer();
+  });
+
+  describe("ownership", () => {
+    it("starts with owner none", () => {
+      expect(player.getOwner()).toBe("none");
+    });
+
+    it("playStream with owner sets owner", () => {
+      player.playStream(dummyStream, "music");
+      expect(player.getOwner()).toBe("music");
+    });
+
+    it("browser bridge cannot override music owner", () => {
+      player.playStream(dummyStream, "music");
+      expect(() => player.playStream(dummyStream, "browser-bridge")).toThrow(
+        "Discord audio player is owned by music",
+      );
+    });
+
+    it("same owner can replace stream without error", () => {
+      player.playStream(dummyStream, "music");
+      expect(() => player.playStream(dummyStream, "music")).not.toThrow();
+      expect(player.getOwner()).toBe("music");
+    });
+
+    it("matching owner stop releases ownership", () => {
+      player.playStream(dummyStream, "music");
+      player.stop("music");
+      expect(player.getOwner()).toBe("none");
+    });
+
+    it("non-owner stop is ignored", () => {
+      player.playStream(dummyStream, "music");
+      player.stop("browser-bridge");
+      expect(player.getOwner()).toBe("music");
+    });
+
+    it("stop without owner releases ownership", () => {
+      player.playStream(dummyStream, "music");
+      player.stop();
+      expect(player.getOwner()).toBe("none");
+    });
+  });
+});

tests/recorder.test.ts

@@ -0,0 +1,49 @@
+import { EventEmitter } from "node:events";
+import { describe, expect, it, vi } from "vitest";
+
+const speaking = new EventEmitter();
+const subscribe = vi.fn();
+const joinVoiceChannel = vi.fn(() => ({
+  receiver: {
+    speaking,
+    subscriptions: new Map(),
+    subscribe,
+  },
+  on: vi.fn(),
+  destroy: vi.fn(),
+}));
+
+vi.mock("@discordjs/voice", async () => {
+  const actual =
+    await vi.importActual<typeof import("@discordjs/voice")>(
+      "@discordjs/voice",
+    );
+  return {
+    ...actual,
+    joinVoiceChannel,
+    entersState: vi.fn(async () => undefined),
+  };
+});
+
+describe("startRecording", () => {
+  it("does not subscribe to the bot user's own audio", async () => {
+    const { startRecording } = await import("../src/recorder");
+    const client = {
+      user: { id: "bot-user" },
+    };
+    const channel = {
+      id: "voice-channel",
+      name: "Voice",
+      guild: {
+        id: "guild",
+        voiceAdapterCreator: {},
+      },
+    };
+
+    await startRecording(client as never, channel as never);
+    speaking.emit("start", "bot-user");
+    await new Promise((resolve) => setImmediate(resolve));
+
+    expect(subscribe).not.toHaveBeenCalled();
+  });
+});

tests/routes/mediaRoutes.test.ts

@@ -14,7 +14,12 @@ function getHandler(
 describe("createMediaRoutes", () => {
   it("returns media status", async () => {
     const controller = {
-      getState: vi.fn(() => ({ playing: false, current: null, queue: [] })),
+      getState: vi.fn(() => ({
+        playing: false,
+        activeMode: null,
+        current: null,
+        queue: [],
+      })),
       queue: vi.fn(),
       skip: vi.fn(),
       stop: vi.fn(),
@@ -30,13 +35,14 @@ describe("createMediaRoutes", () => {
 
     expect(json).toHaveBeenCalledWith({
       playing: false,
+      activeMode: null,
       current: null,
       queue: [],
     });
   });
 
   it("queues a source", async () => {
-    const state = { playing: true, current: null, queue: [] };
+    const state = { playing: true, activeMode: null, current: null, queue: [] };
     const controller = {
       getState: vi.fn(),
       queue: vi.fn(async () => state),
@@ -58,10 +64,71 @@ describe("createMediaRoutes", () => {
 
     expect(controller.queue).toHaveBeenCalledWith(
       "https://example.com/song.mp3",
+      { mode: "music" },
     );
     expect(json).toHaveBeenCalledWith(state);
   });
 
+  it("queues a screen source", async () => {
+    const state = {
+      playing: true,
+      activeMode: "screen" as const,
+      current: null,
+      queue: [],
+    };
+    const controller = {
+      getState: vi.fn(),
+      queue: vi.fn(async () => state),
+      skip: vi.fn(),
+      stop: vi.fn(),
+    };
+    const handler = getHandler(
+      createMediaRoutes(controller),
+      "/media/queue",
+      "post",
+    );
+    const json = vi.fn();
+
+    await handler?.(
+      { body: { source: "https://youtu.be/video", mode: "screen" } } as Request,
+      { json } as unknown as Response,
+      vi.fn(),
+    );
+
+    expect(controller.queue).toHaveBeenCalledWith("https://youtu.be/video", {
+      mode: "screen",
+    });
+    expect(json).toHaveBeenCalledWith(state);
+  });
+
+  it("passes invalid mode errors to Express", async () => {
+    const controller = {
+      getState: vi.fn(),
+      queue: vi.fn(),
+      skip: vi.fn(),
+      stop: vi.fn(),
+    };
+    const handler = getHandler(
+      createMediaRoutes(controller),
+      "/media/queue",
+      "post",
+    );
+    const next = vi.fn();
+
+    await handler?.(
+      {
+        body: { source: "https://example.com/song.mp3", mode: "video" },
+      } as Request,
+      { json: vi.fn() } as unknown as Response,
+      next,
+    );
+
+    expect(next.mock.calls[0][0]).toMatchObject({
+      code: "INVALID_MEDIA_MODE",
+      statusCode: 400,
+    });
+  });
+
   it("passes missing source errors to Express", async () => {
     const controller = {
       getState: vi.fn(),