Repository: discordjs/voice (ts)

Issue:
Play silence after resource has ended
Once a resource ends, it can sometimes have audio glitches when switching to a new resource since there are no filler silence packets between the two resource streams (they share different Opus encoder contexts).

To fix this, after a resource has finished playing on a player, the player should then play 5 frames of silence.

Pull request description:
feat!: add silence padding frames to AudioResources
**Please describe the changes this PR makes and why it should be merged:**

Resolves #119.

By default, resources will now output 5 silence frames after their underlying stream has finished. This means that playing two resources one after the other will give a 100ms delay between both, but will prevent interpolation glitches.

This can be configured:

```ts
createAudioResource(stream, { silencePaddingFrames: 0 }); // disable this feature
createAudioResource(stream, { silencePaddingFrames: 5 }); // default
createAudioResource(stream, { silencePaddingFrames: 10 }); // 200ms worth of silence after resource finishes
```

This is mostly transparent to the AudioPlayer, except for the `#stop()` method. This method will now, by default, cause the player's resource to start outputting its final silence frames before ending - this means the AudioPlayer will not immediately enter the Idle state, it will do so after these silence frames have finished playing. This can be configured with the force parameter (defaults to false):

```ts
// ignore resource's silence padding - stop immediately and enter Idle state
player.stop(true);

// if the resource has silence padding, allow that padding to play before entering the Idle state
player.stop();
player.stop(false);
```

**Status and versioning classification:**

- Code changes have been tested against the Discord API, or there are no code changes
- This PR changes the library's interface (methods or parameters added)
- This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

Patch:
diff --git a/src/audio/AudioPlayer.ts b/src/audio/AudioPlayer.ts
index 9e10a93..ae8efcb 100644
--- a/src/audio/AudioPlayer.ts
+++ b/src/audio/AudioPlayer.ts
@@ -321,7 +321,7 @@ export class AudioPlayer extends TypedEmitter<AudioPlayerEvents> {
 	 * @throws Will throw if attempting to play an audio resource that has already ended, or is being played by another player.
 	 */
 	public play<T>(resource: AudioResource<T>) {
-		if (resource.playStream.readableEnded || resource.playStream.destroyed) {
+		if (resource.ended) {
 			throw new Error('Cannot play a resource that has already ended.');
 		}
 
@@ -432,15 +432,21 @@ export class AudioPlayer extends TypedEmitter<AudioPlayerEvents> {
 	}
 
 	/**
-	 * Stops playback of the current resource and destroys the resource. The player will transition to the Idle state.
+	 * Stops playback of the current resource and destroys the resource. The player will either transition to the Idle state,
+	 * or remain in its current state until the silence padding frames of the resource have been played.
 	 *
-	 * @returns true if the player was successfully stopped, otherwise false.
+	 * @param force - If true, will force the player to enter the Idle state even if the resource has silence padding frames.
+	 * @returns true if the player will come to a stop, otherwise false.
 	 */
-	public stop() {
+	public stop(force = false) {
 		if (this.state.status === AudioPlayerStatus.Idle) return false;
-		this.state = {
-			status: AudioPlayerStatus.Idle,
-		};
+		if (force || this.state.resource.silencePaddingFrames === 0) {
+			this.state = {
+				status: AudioPlayerStatus.Idle,
+			};
+		} else if (this.state.resource.silenceRemaining === -1) {
+			this.state.resource.silenceRemaining = this.state.resource.silencePaddingFrames;
+		}
 		return true;
 	}
 
@@ -454,7 +460,7 @@ export class AudioPlayer extends TypedEmitter<AudioPlayerEvents> {
 		if (state.status === AudioPlayerStatus.Idle || state.status === AudioPlayerStatus.Buffering) return false;
 
 		// If the stream has been destroyed or is no longer readable, then transition to the Idle state.
-		if (!state.resource.playStream.readable) {
+		if (!state.resource.readable) {
 			this.state = {
 				status: AudioPlayerStatus.Idle,
 			};
@@ -524,7 +530,7 @@ export class AudioPlayer extends TypedEmitter<AudioPlayerEvents> {
 				};
 				return;
 			} else if (this.behaviors.noSubscriber === NoSubscriberBehavior.Stop) {
-				this.stop();
+				this.stop(true);
 			}
 		}
 
diff --git a/src/audio/AudioResource.ts b/src/audio/AudioResource.ts
index 2336a79..ccbfca5 100644
--- a/src/audio/AudioResource.ts
+++ b/src/audio/AudioResource.ts
@@ -2,7 +2,7 @@ import { Edge, findPipeline, StreamType, TransformerType } from './TransformerGr
 import { once, pipeline, Readable } from 'stream';
 import { noop } from '../util/util';
 import { VolumeTransformer, opus } from 'prism-media';
-import type { AudioPlayer } from './AudioPlayer';
+import { AudioPlayer, SILENCE_FRAME } from './AudioPlayer';
 
 /**
  * Options that are set when creating a new audio resource.
@@ -27,6 +27,12 @@ interface CreateAudioResourceOptions<T> {
 	 * of the stream on-the-fly. However, this also increases the performance cost of playback. Defaults to `false`.
 	 */
 	inlineVolume?: boolean;
+
+	/**
+	 * The number of silence frames to append to the end of the resource's audio stream, to prevent interpolation glitches.
+	 * Defaults to 5.
+	 */
+	silencePaddingFrames?: number;
 }
 
 /**
@@ -79,10 +85,21 @@ export class AudioResource<T = unknown> {
 	 */
 	public started = false;
 
-	public constructor(edges: readonly Edge[], streams: readonly Readable[], metadata: T) {
+	/**
+	 * The number of silence frames to append to the end of the resource's audio stream, to prevent interpolation glitches.
+	 */
+	public readonly silencePaddingFrames: number;
+
+	/**
+	 * The number of remaining silence frames to play. If -1, the frames have not yet started playing.
+	 */
+	public silenceRemaining = -1;
+
+	public constructor(edges: readonly Edge[], streams: readonly Readable[], metadata: T, silencePaddingFrames: number) {
 		this.edges = edges;
 		this.playStream = streams.length > 1 ? (pipeline(streams, noop) as any as Readable) : streams[0];
 		this.metadata = metadata;
+		this.silencePaddingFrames = silencePaddingFrames;
 
 		for (const stream of streams) {
 			if (stream instanceof VolumeTransformer) {
@@ -97,6 +114,27 @@ export class AudioResource<T = unknown> {
 			.catch(noop);
 	}
 
+	/**
+	 * Whether this resource is readable. If the underlying resource is no longer readable, this will still return true
+	 * while there are silence padding frames left to play.
+	 */
+	public get readable() {
+		if (this.silenceRemaining === 0) return false;
+		const real = this.playStream.readable;
+		if (!real) {
+			if (this.silenceRemaining === -1) this.silenceRemaining = this.silencePaddingFrames;
+			return this.silenceRemaining !== 0;
+		}
+		return real;
+	}
+
+	/**
+	 * Whether this resource has ended or not.
+	 */
+	public get ended() {
+		return this.playStream.readableEnded || this.playStream.destroyed || this.silenceRemaining === 0;
+	}
+
 	/**
 	 * Attempts to read an Opus packet from the audio resource. If a packet is available, the playbackDuration
 	 * is incremented.
@@ -107,6 +145,12 @@ export class AudioResource<T = unknown> {
 	 * read from it.
 	 */
 	public read(): Buffer | null {
+		if (this.silenceRemaining === 0) {
+			return null;
+		} else if (this.silenceRemaining > 0) {
+			this.silenceRemaining--;
+			return SILENCE_FRAME;
+		}
 		const packet: Buffer | null = this.playStream.read();
 		if (packet) {
 			this.playbackDuration += 20;
@@ -197,10 +241,15 @@ export function createAudioResource<T>(
 	if (transformerPipeline.length === 0) {
 		if (typeof input === 'string') throw new Error(`Invalid pipeline constructed for string resource '${input}'`);
 		// No adjustments required
-		return new AudioResource<T>([], [input], (options.metadata ?? null) as T);
+		return new AudioResource<T>([], [input], (options.metadata ?? null) as T, options.silencePaddingFrames ?? 5);
 	}
 	const streams = transformerPipeline.map((edge) => edge.transformer(input));
 	if (typeof input !== 'string') streams.unshift(input);
 
-	return new AudioResource<T>(transformerPipeline, streams, (options.metadata ?? null) as T);
+	return new AudioResource<T>(
+		transformerPipeline,
+		streams,
+		(options.metadata ?? null) as T,
+		options.silencePaddingFrames ?? 5,
+	);
 }


Test patch:
diff --git a/src/audio/__tests__/AudioPlayer.test.ts b/src/audio/__tests__/AudioPlayer.test.ts
index 6c35510..6245262 100644
--- a/src/audio/__tests__/AudioPlayer.test.ts
+++ b/src/audio/__tests__/AudioPlayer.test.ts
@@ -1,6 +1,6 @@
 /* eslint-disable @typescript-eslint/dot-notation */
 import { AudioResource } from '../../audio/AudioResource';
-import { createAudioPlayer, AudioPlayerStatus, AudioPlayer } from '../AudioPlayer';
+import { createAudioPlayer, AudioPlayerStatus, AudioPlayer, SILENCE_FRAME } from '../AudioPlayer';
 import { Readable } from 'stream';
 import { addAudioPlayer, deleteAudioPlayer } from '../../DataStore';
 import { NoSubscriberBehavior } from '../..';
@@ -58,7 +58,7 @@ beforeEach(() => {
 });
 
 afterEach(() => {
-	player?.stop();
+	player?.stop(true);
 });
 
 describe('State transitions', () => {
@@ -71,7 +71,7 @@ describe('State transitions', () => {
 
 	test('Playing resource with pausing and resuming', async () => {
 		// Call AudioResource constructor directly to avoid analysing pipeline for stream
-		const resource = await started(new AudioResource([], [Readable.from(silence())], null));
+		const resource = await started(new AudioResource([], [Readable.from(silence())], null, 5));
 		player = createAudioPlayer();
 		expect(player.state.status).toBe(AudioPlayerStatus.Idle);
 
@@ -107,11 +107,11 @@ describe('State transitions', () => {
 	});
 
 	test('Playing to Stopping', async () => {
-		const resource = await started(new AudioResource([], [Readable.from(silence())], null));
+		const resource = await started(new AudioResource([], [Readable.from(silence())], null, 5));
 		player = createAudioPlayer();
 
 		// stop() shouldn't do anything in Idle state
-		expect(player.stop()).toBe(false);
+		expect(player.stop(true)).toBe(false);
 		expect(player.state.status).toBe(AudioPlayerStatus.Idle);
 
 		player.play(resource);
@@ -120,13 +120,14 @@ describe('State transitions', () => {
 		expect(deleteAudioPlayerMock).toBeCalledTimes(0);
 
 		expect(player.stop()).toBe(true);
-		expect(player.state.status).toBe(AudioPlayerStatus.Idle);
+		expect(player.state.status).toBe(AudioPlayerStatus.Playing);
 		expect(addAudioPlayerMock).toBeCalledTimes(1);
-		expect(deleteAudioPlayerMock).toBeCalledTimes(1);
+		expect(deleteAudioPlayerMock).toBeCalledTimes(0);
+		expect(resource.silenceRemaining).toBe(5);
 	});
 
 	test('Buffering to Playing', async () => {
-		const resource = new AudioResource([], [Readable.from(silence())], null);
+		const resource = new AudioResource([], [Readable.from(silence())], null, 5);
 		player = createAudioPlayer();
 
 		player.play(resource);
@@ -146,7 +147,7 @@ describe('State transitions', () => {
 				throw new Error('Voice connection should have been Signalling');
 			}
 
-			const resource = await started(new AudioResource([], [Readable.from(silence())], null));
+			const resource = await started(new AudioResource([], [Readable.from(silence())], null, 5));
 			player = createAudioPlayer({ behaviors: { noSubscriber: NoSubscriberBehavior.Pause } });
 			connection.subscribe(player);
 
@@ -167,7 +168,7 @@ describe('State transitions', () => {
 		});
 
 		test('NoSubscriberBehavior.Play', async () => {
-			const resource = await started(new AudioResource([], [Readable.from(silence())], null));
+			const resource = await started(new AudioResource([], [Readable.from(silence())], null, 5));
 			player = createAudioPlayer({ behaviors: { noSubscriber: NoSubscriberBehavior.Play } });
 
 			player.play(resource);
@@ -177,7 +178,7 @@ describe('State transitions', () => {
 		});
 
 		test('NoSubscriberBehavior.Stop', async () => {
-			const resource = await started(new AudioResource([], [Readable.from(silence())], null));
+			const resource = await started(new AudioResource([], [Readable.from(silence())], null, 5));
 			player = createAudioPlayer({ behaviors: { noSubscriber: NoSubscriberBehavior.Stop } });
 
 			player.play(resource);
@@ -202,7 +203,7 @@ describe('State transitions', () => {
 
 		const buffer = Buffer.from([1, 2, 4, 8]);
 		const resource = await started(
-			new AudioResource([], [Readable.from([buffer, buffer, buffer, buffer, buffer])], null),
+			new AudioResource([], [Readable.from([buffer, buffer, buffer, buffer, buffer])], null, 5),
 		);
 		player = createAudioPlayer();
 		connection.subscribe(player);
@@ -239,7 +240,60 @@ describe('State transitions', () => {
 		expect(prepareAudioPacket).toHaveBeenCalledTimes(6);
 		expect(prepareAudioPacket.mock.calls[5][0]).toEqual(silence().next().value);
 
+		player.stop(true);
+		expect(player.state.status).toBe(AudioPlayerStatus.Idle);
+		expect(connection.setSpeaking).toBeCalledTimes(1);
+		expect(connection.setSpeaking).toHaveBeenLastCalledWith(false);
+		expect(deleteAudioPlayerMock).toHaveBeenCalledTimes(1);
+	});
+
+	test('stop() causes resource to use silence padding frames', async () => {
+		const connection = createVoiceConnectionMock();
+		if (connection.state.status !== VoiceConnectionStatus.Signalling) {
+			throw new Error('Voice connection should have been Signalling');
+		}
+		connection.state = {
+			...connection.state,
+			status: VoiceConnectionStatus.Ready,
+			networking: null as any,
+		};
+
+		const buffer = Buffer.from([1, 2, 4, 8]);
+		const resource = await started(
+			new AudioResource([], [Readable.from([buffer, buffer, buffer, buffer, buffer])], null, 5),
+		);
+		player = createAudioPlayer();
+		connection.subscribe(player);
+
+		player.play(resource);
+		expect(player.state.status).toBe(AudioPlayerStatus.Playing);
+		expect(addAudioPlayerMock).toBeCalledTimes(1);
+		expect(player.checkPlayable()).toBe(true);
+
 		player.stop();
+
+		// Run through a few packet cycles
+		for (let i = 1; i <= 5; i++) {
+			player['_stepDispatch']();
+			expect(connection.dispatchAudio).toHaveBeenCalledTimes(i);
+
+			await wait(); // Wait for the stream
+
+			player['_stepPrepare']();
+			expect(connection.prepareAudioPacket).toHaveBeenCalledTimes(i);
+			expect(connection.prepareAudioPacket).toHaveBeenLastCalledWith(SILENCE_FRAME);
+			expect(player.state.status).toBe(AudioPlayerStatus.Playing);
+			if (player.state.status === AudioPlayerStatus.Playing) {
+				expect(player.state.playbackDuration).toStrictEqual(i * 20);
+			}
+		}
+		await wait();
+		expect(player.checkPlayable()).toBe(false);
+		const prepareAudioPacket = connection.prepareAudioPacket as unknown as jest.Mock<
+			typeof connection.prepareAudioPacket
+		>;
+		expect(prepareAudioPacket).toHaveBeenCalledTimes(5);
+
 		expect(player.state.status).toBe(AudioPlayerStatus.Idle);
 		expect(connection.setSpeaking).toBeCalledTimes(1);
 		expect(connection.setSpeaking).toHaveBeenLastCalledWith(false);
@@ -257,7 +311,7 @@ describe('State transitions', () => {
 			networking: null as any,
 		};
 
-		const resource = await started(new AudioResource([], [Readable.from([1])], null));
+		const resource = await started(new AudioResource([], [Readable.from([1])], null, 0));
 		resource.playStream.read();
 		player = createAudioPlayer({ behaviors: { maxMissedFrames: 5 } });
 		connection.subscribe(player);
@@ -291,7 +345,7 @@ describe('State transitions', () => {
 	});
 
 	test('checkPlayable() transitions to Idle for unreadable stream', async () => {
-		const resource = await started(new AudioResource([], [Readable.from([1])], null));
+		const resource = await started(new AudioResource([], [Readable.from([1])], null, 0));
 		player = createAudioPlayer();
 		player.play(resource);
 		expect(player.checkPlayable()).toBe(true);
@@ -307,7 +361,7 @@ describe('State transitions', () => {
 });
 
 test('play() throws when playing a resource that has already ended', async () => {
-	const resource = await started(new AudioResource([], [Readable.from([1])], null));
+	const resource = await started(new AudioResource([], [Readable.from([1])], null, 5));
 	player = createAudioPlayer();
 	player.play(resource);
 	expect(player.state.status).toBe(AudioPlayerStatus.Playing);
@@ -316,13 +370,13 @@ test('play() throws when playing a resource that has already ended', async () =>
 		await wait();
 	}
 	expect(resource.playStream.readableEnded).toBe(true);
-	player.stop();
+	player.stop(true);
 	expect(player.state.status).toBe(AudioPlayerStatus.Idle);
 	expect(() => player.play(resource)).toThrow();
 });
 
 test('Propagates errors from streams', async () => {
-	const resource = await started(new AudioResource([], [Readable.from(silence())], null));
+	const resource = await started(new AudioResource([], [Readable.from(silence())], null, 5));
 	player = createAudioPlayer();
 	player.play(resource);
 	expect(player.state.status).toBe(AudioPlayerStatus.Playing);
diff --git a/src/audio/__tests__/AudioResource.test.ts b/src/audio/__tests__/AudioResource.test.ts
index 69e298f..ea969a5 100644
--- a/src/audio/__tests__/AudioResource.test.ts
+++ b/src/audio/__tests__/AudioResource.test.ts
@@ -1,11 +1,23 @@
 import { opus, VolumeTransformer } from 'prism-media';
-import { PassThrough } from 'stream';
-import { createAudioResource, NO_CONSTRAINT, VOLUME_CONSTRAINT } from '../AudioResource';
+import { PassThrough, Readable } from 'stream';
+import { SILENCE_FRAME } from '../AudioPlayer';
+import { AudioResource, createAudioResource, NO_CONSTRAINT, VOLUME_CONSTRAINT } from '../AudioResource';
 import { Edge, findPipeline as _findPipeline, StreamType, TransformerType } from '../TransformerGraph';
 
 jest.mock('prism-media');
 jest.mock('../TransformerGraph');
 
+function wait() {
+	return new Promise((resolve) => process.nextTick(resolve));
+}
+
+async function started(resource: AudioResource) {
+	while (!resource.started) {
+		await wait();
+	}
+	return resource;
+}
+
 const findPipeline = _findPipeline as unknown as jest.MockedFunction<typeof _findPipeline>;
 
 beforeAll(() => {
@@ -89,4 +101,22 @@ describe('createAudioResource', () => {
 		expect(findPipeline).toHaveBeenCalledWith(StreamType.Arbitrary, NO_CONSTRAINT);
 		expect(resource.volume).toBeUndefined();
 	});
+
+	test('Appends silence frames when ended', async () => {
+		const stream = Readable.from(Buffer.from([1]));
+
+		const resource = new AudioResource([], [stream], null, 5);
+
+		await started(resource);
+		expect(resource.readable).toBe(true);
+		expect(resource.read()).toEqual(Buffer.from([1]));
+		for (let i = 0; i < 5; i++) {
+			await wait();
+			expect(resource.readable).toBe(true);
+			expect(resource.read()).toBe(SILENCE_FRAME);
+		}
+		await wait();
+		expect(resource.readable).toBe(false);
+		expect(resource.read()).toBe(null);
+	});
 });