From aa440eda69c533fd3a2b7a8f138897255d97228e Mon Sep 17 00:00:00 2001 From: asepharyana Date: Thu, 6 Aug 2026 21:26:50 +0700 Subject: [PATCH] =?UTF-8?q?fix(gateway):=20music/screen=20playback=20heads?= =?UTF-8?q?=20=E2=80=94=20read=20yt-dlp=20headers=20from=20stderr,=20drop?= =?UTF-8?q?=20no-simulate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Music playback produced no audio: with `-o -` yt-dlp streams media on stdout and emits its `--print` title/duration headers on stderr, but resolveMediaUrl read them from stdout — stripping two binary 'lines' off the WebM container and corrupting the stream (player 'playing' but silent). Now headers are read from stderr and the stdout media stream is returned untouched. Screenshare was failing with EACCES: getDirectScreenInput used --no-simulate, making yt-dlp write .f*.part files into the read-only Nix store CWD. Dropped it — simulate mode still returns requested_formats[].url in the JSON. Adds tests/mediaResolve.test.ts (stderr-header + untouched-stream regression). --- .../modules/voice-recording/mediaSource.ts | 190 +++++++++--------- .../tests/mediaResolve.test.ts | 133 ++++++++++++ 2 files changed, 228 insertions(+), 95 deletions(-) create mode 100644 services/discord-gateway/tests/mediaResolve.test.ts diff --git a/services/discord-gateway/src/modules/voice-recording/mediaSource.ts b/services/discord-gateway/src/modules/voice-recording/mediaSource.ts index 42fcf71..0b5b65b 100644 --- a/services/discord-gateway/src/modules/voice-recording/mediaSource.ts +++ b/services/discord-gateway/src/modules/voice-recording/mediaSource.ts @@ -62,90 +62,67 @@ function parseSeconds(value: string): number { */ const MAX_HEADER_BUFFER = 65536; // 64KB safety limit for metadata headers -function readFirstTwoLines( - stdout: Readable, +/** + * Read the title + duration header lines from yt-dlp's STDERR. + * + * When yt-dlp streams media to stdout (`-o -`) it redirects its `--print` + * output to STDERR so the media stream on stdout stays clean. The first two + * meaningful lines on stderr are then the title and (before_dl) duration. + * + * Blank lines and `[...]` info prefixes are skipped. An `ERROR:` line is + * reported via `onError` so a failing download surfaces as a resolution error + * instead of a silent empty stream. + */ +function readStderrHeader( + stderr: Readable, + onError: (message: string) => void, maxBufferSize: number = MAX_HEADER_BUFFER, -): Promise<{ - title: string; - duration: number; - remaining: Readable; -}> { - return new Promise((resolve, reject) => { - const passThrough = new PassThrough(); - - let buffer = Buffer.alloc(0); +): Promise<{ title: string; duration: number }> { + return new Promise((resolve) => { + let buffer = ""; let title = ""; - let stage: "title" | "duration" | "done" = "title"; + let duration = 0; + let done = false; - function cleanup() { - stdout.removeListener("data", onData); - stdout.removeListener("error", onError); - stdout.removeListener("end", onEnd); - } + const finish = () => { + if (done) return; + done = true; + stderr.removeListener("data", onData); + resolve({ title, duration }); + }; - function onData(chunk: Buffer) { - if (stage === "done") return; - buffer = Buffer.concat([buffer, chunk]); + const onData = (chunk: Buffer) => { + if (done) return; + buffer += chunk.toString("utf8"); if (buffer.length > maxBufferSize) { - cleanup(); - reject(new Error(`Metadata header exceeded ${maxBufferSize} bytes`)); + finish(); return; } - processBuffer(); - } + while (!done) { + const nl = buffer.indexOf("\n"); + if (nl === -1) break; // need more data + const line = buffer.slice(0, nl).trim(); + buffer = buffer.slice(nl + 1); - function processBuffer() { - while (buffer.length > 0 && stage !== "done") { - const nl = buffer.indexOf(0x0a); // '\n' byte - if (nl === -1) break; // Need more data - - const line = buffer.subarray(0, nl).toString("utf8").trim(); - buffer = buffer.subarray(nl + 1); - - if (stage === "title") { + if (line.length === 0) continue; // blank line + if (line.startsWith("[")) continue; // "[info] ..." — not a header + if (line.startsWith("ERROR")) { + onError(line); + finish(); + return; + } + if (!title) { title = line; - stage = "duration"; - } else if (stage === "duration") { - const duration = parseSeconds(line); - stage = "done"; - cleanup(); - - // Write any buffered data that follows the second newline - if (buffer.length > 0) { - passThrough.write(buffer); - } - - // Pipe the remainder of stdout into the pass-through - stdout.pipe(passThrough); - - resolve({ title, duration, remaining: passThrough }); + } else { + duration = parseSeconds(line); + finish(); return; } } - } + }; - function onError(err: Error) { - if (stage !== "done") { - cleanup(); - reject(err); - } - } - - function onEnd() { - if (stage !== "done") { - cleanup(); - reject( - new Error( - `yt-dlp stdout ended before metadata could be read. ` + - `Stage: ${stage}, partial title: "${title}"`, - ), - ); - } - } - - stdout.on("data", onData); - stdout.on("error", onError); - stdout.on("end", onEnd); + stderr.on("data", onData); + stderr.on("end", finish); }); } @@ -163,8 +140,10 @@ function buildNotInstalledError(): Error { /** * Resolve a media URL (YouTube, Spotify, etc.) to a playable audio stream. * - * Spawns `yt-dlp`, extracts the title and duration from the first two stdout - * lines, then pipes the remaining raw audio data into a Readable stream. + * Spawns `yt-dlp` with `-o -` so the raw audio bytes stream on stdout. Since + * stdout is the media sink, yt-dlp emits its `--print before_dl:title` / + * `before_dl:duration` header lines on STDERR — the title + duration are read + * from there and the stdout media stream is returned untouched. * * The returned stream uses `StreamType.Arbitrary` — suitable for * `DiscordPlayer.playStream()` with `inputType: StreamType.Arbitrary`. @@ -181,10 +160,10 @@ export function resolveMediaUrl( const args = [ "-f", format, - "--audio-format", - "best", "-o", "-", + "--no-progress", + "--no-warnings", "--print", "before_dl:title", "--print", @@ -195,11 +174,17 @@ export function resolveMediaUrl( logger.info({ url }, "Spawning yt-dlp for media resolution"); const proc = spawn("yt-dlp", args, { - stdio: ["pipe", "pipe", "pipe"], + stdio: ["ignore", "pipe", "pipe"], }); activeProcesses.add(proc); + // With `-o -` yt-dlp streams the raw audio on stdout and moves its + // `--print` headers to stderr — pipe stdout immediately so the child + // never blocks on a full pipe while we wait for the headers on stderr. + const mediaStream = new PassThrough(); + proc.stdout.pipe(mediaStream); + let stderrBuf = ""; let resolved = false; @@ -209,9 +194,23 @@ export function resolveMediaUrl( if (resolved) return; resolved = true; activeProcesses.delete(proc); + mediaStream.destroy(); reject(err); }; + const resolveOnce = (info: MediaInfo) => { + if (resolved) return; + resolved = true; + activeProcesses.delete(proc); + resolve({ + stream: mediaStream, + type: StreamType.Arbitrary, + title: info.title, + duration: info.duration, + info, + }); + }; + // -- spawn error (ENOENT etc.) ---------------------------------------- proc.on("error", (err: NodeJS.ErrnoException) => { @@ -222,31 +221,25 @@ export function resolveMediaUrl( } }); - // -- stderr (capture for diagnostics, capped at 4KB) ---------------------------------- + // -- stderr: title + duration headers --------------------------------- + // Capture raw stderr too, for the exit-diagnostics in the close handler. const _MAX_STDERR = 4096; if (proc.stderr) { proc.stderr.on("data", (chunk: Buffer) => { - stderrBuf += chunk.toString("utf8"); + if (stderrBuf.length < _MAX_STDERR) { + stderrBuf += chunk + .toString("utf8") + .slice(0, _MAX_STDERR - stderrBuf.length); + } }); } - // -- stdout: parse header, then stream audio --------------------------- - - readFirstTwoLines(proc.stdout) - .then(({ title, duration, remaining }) => { - if (resolved) return; - resolved = true; - activeProcesses.delete(proc); - - const info: MediaInfo = { title, duration }; - resolve({ - stream: remaining, - type: StreamType.Arbitrary, - title, - duration, - info, - }); + readStderrHeader(proc.stderr, (message) => { + failOnce(new Error(message)); + }) + .then(({ title, duration }) => { + resolveOnce({ title: title || url, duration }); }) .catch((err: Error) => { failOnce(err); @@ -264,6 +257,10 @@ export function resolveMediaUrl( failOnce(new Error(`yt-dlp exited with code ${code}${detail}`)); } else if (signal) { failOnce(new Error(`yt-dlp was killed by signal ${signal}`)); + } else { + // Exited cleanly but the header lines never surfaced (e.g. a direct + // file URL with no duration) — keep the media stream alive anyway. + resolveOnce({ title: url, duration: 0 }); } }); @@ -312,7 +309,10 @@ export function getDirectScreenInput(url: string): Promise { "--no-playlist", "--no-warnings", "--quiet", - "--no-simulate", + // NOTE: deliberately NOT --no-simulate. Simulate mode still resolves the + // requested format URLs into the JSON (requested_formats[].url), and it + // avoids yt-dlp writing .part files into the process CWD — which is the + // read-only Nix store dir for the deployed gateway (EACCES). ]; logger.info({ url }, "Spawning yt-dlp for screen share input resolution"); diff --git a/services/discord-gateway/tests/mediaResolve.test.ts b/services/discord-gateway/tests/mediaResolve.test.ts new file mode 100644 index 0000000..60e3895 --- /dev/null +++ b/services/discord-gateway/tests/mediaResolve.test.ts @@ -0,0 +1,133 @@ +// ═══════════════════════════════════════════════════════════════════════════════ +// resolveMediaUrl (music playback) resolution tests +// +// Guards the yt-dlp stdout-vs-stderr contract that broke music playback: +// - with `-o -` yt-dlp streams media on STDOUT and emits `--print` +// title/duration headers on STDERR +// - resolveMediaUrl must read headers from stderr and return the stdout +// media stream UNTOUCHED (the old code stripped binary "lines" from the +// WebM container, corrupting it → silent playback) +// +// yt-dlp is faked via a PATH shim; no network or real binaries needed. +// ═══════════════════════════════════════════════════════════════════════════════ + +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { resolveMediaUrl } from "../src/modules/voice-recording/mediaSource.js"; + +// ─── fake bin dir ────────────────────────────────────────────────────────────── +let fakeBinDir: string | null = null; +const realPath = process.env.PATH; + +/** WebM-ish payload: EBML magic + a few bytes that must survive untouched. */ +const MEDIA_PAYLOAD = Buffer.concat([ + Buffer.from([0x1a, 0x45, 0xdf, 0xa3, 0x9f]), // EBML magic + Buffer.from("fake-webm-cluster-data\nwith-newlines\n", "utf8"), + Buffer.from([0x00, 0x11, 0x22, 0x33, 0x0a, 0xff, 0xee]), +]); + +beforeAll(() => { + fakeBinDir = mkdtempSync(join(tmpdir(), "gmw-fake-media-resolve-")); + + // Fake yt-dlp: reads GMW_FAKE_STDERR (what to write on stderr) and + // GMW_FAKE_STDOUT (what to write on stdout, default = MEDIA_PAYLOAD). + // Written with string concat so bash `${...}` isn't TS-interpolated. + const ytShim = + "#!/usr/bin/env bash\n" + + 'if [ -f "$GMW_FAKE_STDERR" ]; then cat "$GMW_FAKE_STDERR" >&2; fi\n' + + 'if [ -f "$GMW_FAKE_STDOUT" ]; then cat "$GMW_FAKE_STDOUT"; fi\n' + + "exit " + + (process.env.GMW_FAKE_EXIT ?? "0") + + "\n"; + writeFileSync(join(fakeBinDir, "yt-dlp"), ytShim); + chmodSync(join(fakeBinDir, "yt-dlp"), 0o755); + + process.env.PATH = `${fakeBinDir}:${process.env.PATH}`; +}); + +afterAll(() => { + if (fakeBinDir) { + rmSync(fakeBinDir, { recursive: true, force: true }); + } + process.env.PATH = realPath; +}); + +// ─── helpers ─────────────────────────────────────────────────────────────────── +function writeTemp(name: string, content: Buffer | string): string { + const p = join( + tmpdir(), + `gmw-resolve-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}-${name}`, + ); + writeFileSync(p, content); + return p; +} + +function collect(stream: NodeJS.ReadableStream): Promise { + return new Promise((resolve, reject) => { + const chunks: Buffer[] = []; + stream.on("data", (c: Buffer) => chunks.push(c)); + stream.on("error", reject); + stream.on("end", () => resolve(Buffer.concat(chunks))); + }); +} + +// ─── tests ───────────────────────────────────────────────────────────────────── +describe("resolveMediaUrl", () => { + it("reads title+duration from STDERR and returns the untouched stdout media", async () => { + const stderrPath = writeTemp("stderr", "【Test】Song Title\n184.5\n"); + const stdoutPath = writeTemp("stdout", MEDIA_PAYLOAD); + process.env.GMW_FAKE_STDERR = stderrPath; + process.env.GMW_FAKE_STDOUT = stdoutPath; + process.env.GMW_FAKE_EXIT = "0"; + + const resolution = await resolveMediaUrl("https://youtu.be/abc"); + + expect(resolution.title).toBe("【Test】Song Title"); + expect(resolution.duration).toBeCloseTo(184.5, 1); + + const bytes = await collect(resolution.stream); + expect(bytes).toEqual(MEDIA_PAYLOAD); // byte-for-byte intact + }); + + it("skips [info] prefix lines before the title header", async () => { + const stderrPath = writeTemp( + "stderr", + "[info] Downloading webpage\nTitle After Info\n123\n", + ); + const stdoutPath = writeTemp("stdout", MEDIA_PAYLOAD); + process.env.GMW_FAKE_STDERR = stderrPath; + process.env.GMW_FAKE_STDOUT = stdoutPath; + process.env.GMW_FAKE_EXIT = "0"; + + const resolution = await resolveMediaUrl("https://youtu.be/abc"); + + expect(resolution.title).toBe("Title After Info"); + expect(resolution.duration).toBe(123); + }); + + it("rejects when yt-dlp reports an ERROR on stderr", async () => { + const stderrPath = writeTemp( + "stderr", + "ERROR: [youtube] xyz: This video is unavailable\n", + ); + process.env.GMW_FAKE_STDERR = stderrPath; + delete process.env.GMW_FAKE_STDOUT; + process.env.GMW_FAKE_EXIT = "1"; + + await expect(resolveMediaUrl("https://youtu.be/abc")).rejects.toThrow( + /This video is unavailable/, + ); + }); + + it("rejects when yt-dlp is not installed", async () => { + const oldPath = process.env.PATH; + process.env.PATH = "/usr/bin:/bin"; // no fake yt-dlp + await expect(resolveMediaUrl("https://youtu.be/abc")).rejects.toThrow( + /yt-dlp is not installed/, + ); + process.env.PATH = oldPath; + }); +});