Clone samples when reused with autoClose: false

src/cursors.ts

@@ -422,6 +422,19 @@ export abstract class SampleCursor<
 			.finally(() => lock.release());
 	}
 
+	transformSample() {
+		assert(this.currentRaw && !this.currentRaw.closed);
+
+		if (this.autoClose) {
+			return this.current ??= this.transform(this.currentRaw);
+		} else {
+			const clone = this.currentRaw.clone();
+			const transformed = this.transform(clone as Sample);
+
+			return this.current = transformed;
+		}
+	}
+
 	onDecoderSample(sample: Sample): void {
 		try {
 			if (this.debugInfo.enabled && this.debugInfo.throwDecoderError) {
@@ -446,7 +459,6 @@ export abstract class SampleCursor<
 				}
 			} else {
 				let given = false;
-				let transformed: TransformedSample | null = null;
 
 				for (let i = 0; i < this.pendingRequests.length; i++) {
 					const request = this.pendingRequests[i]!;
@@ -454,11 +466,10 @@ export abstract class SampleCursor<
 						break;
 					}
 
-					transformed ??= this.transform(sample);
+					this.setCurrentRaw(sample);
+					request.resolve(this.transformSample());
 
-					request.resolve(transformed);
 					this.pendingRequests.splice(i--, 1);
-					this.setCurrent(sample, transformed);
 					given = true;
 
 					if (request.successor) {
@@ -488,13 +499,14 @@ export abstract class SampleCursor<
 		this.queueDequeue = promiseWithResolvers();
 	}
 
-	setCurrent(newCurrentRaw: Sample | null, newCurrent: TransformedSample | null) {
+	setCurrentRaw(newCurrentRaw: Sample | null) {
-		if (this.autoClose && this.currentRaw && this.currentRaw !== newCurrentRaw) {
+		if (this.currentRaw === newCurrentRaw) {
-			this.currentRaw.close();
+			return;
 		}
 
+		this.currentRaw?.close();
 		this.currentRaw = newCurrentRaw;
-		this.current = newCurrent;
+		this.current = null;
 	}
 
 	getNextExpectedTimestamp() {
@@ -543,12 +555,12 @@ export abstract class SampleCursor<
 		this.nextIsFirst = !targetPacket;
 
 		if (!targetPacket) {
-			this.setCurrent(null, null);
+			this.setCurrentRaw(null);
 			return res.set(null);
 		}
 
-		if (this.currentRaw?.timestamp === targetPacket.timestamp) {
+		if (this.currentRaw?.timestamp === targetPacket.timestamp && !this.currentRaw.closed) {
-			return res.set(this.current);
+			return res.set(this.transformSample());
 		}
 
 		let setNewPump = true;
@@ -562,9 +574,8 @@ export abstract class SampleCursor<
 				this.queueDequeue = promiseWithResolvers();
 
 				if (targetPacket.timestamp <= nextSample.timestamp) {
-					const transformed = this.transform(nextSample);
+					this.setCurrentRaw(nextSample);
-					this.setCurrent(nextSample, transformed);
+					return res.set(this.transformSample());
-					return res.set(transformed);
 				} else {
 					nextSample.close();
 				}
@@ -711,13 +722,12 @@ export abstract class SampleCursor<
 			this.queueDequeue.resolve();
 			this.queueDequeue = promiseWithResolvers();
 
-			const transformed = this.transform(nextSample);
+			this.setCurrentRaw(nextSample);
-			this.setCurrent(nextSample, transformed);
+			return res.set(this.transformSample());
-			return res.set(transformed);
 		}
 
 		if (!this.pumpRunning) {
-			this.setCurrent(null, null);
+			this.setCurrentRaw(null);
 			return res.set(null); // None more after this, boy
 		}
 
@@ -865,7 +875,7 @@ export abstract class SampleCursor<
 			await this.stopPump();
 		}
 
-		this.setCurrent(null, null);
+		this.setCurrentRaw(null);
 		this.decoder?.close();
 		this.decoder = null;
 	}
@@ -944,8 +954,8 @@ export abstract class SampleCursor<
 				}
 			};
 
+			this.setCurrentRaw(null);
 			this.pendingRequests.forEach(uh);
-			this.setCurrent(null, null);
 		} catch (error) {
 			if (!this.decoder.closed && this.pendingRequests.length > 0) {
 				await this.decoder.flush();

test/browser/sample-cursor.test.ts

@@ -286,6 +286,71 @@ test('Sample cursor advancing, cold start', async () => {
 	expect(VideoSample._openSampleCount).toBe(0);
 });
 
+test('Sample cursor sample reuse', async () => {
+	using input = new Input({
+		source: new UrlSource('/trim-buck-bunny.mov'),
+		formats: ALL_FORMATS,
+	});
+
+	const videoTrack = (await input.getPrimaryVideoTrack())!;
+	const reader = new PacketReader(videoTrack);
+	const cursor1 = new VideoSampleCursor(reader);
+	cursor1.debugInfo.enabled = true;
+
+	const sample1 = await cursor1.seekToFirst();
+	const sample2 = await cursor1.seekToFirst();
+
+	expect(cursor1.debugInfo.pumpsStarted).toBe(1);
+	expect(sample1!.timestamp).toBe(sample2!.timestamp);
+	expect(sample1).toBe(sample2);
+
+	await cursor1.next();
+	expect(sample1!.closed).toBe(true);
+	expect(sample2!.closed).toBe(true);
+
+	const cursor2 = new VideoSampleCursor(reader, {
+		autoClose: false,
+	});
+
+	const sample3 = await cursor2.seekToFirst();
+	const sample4 = await cursor2.seekToFirst();
+	expect(sample3!.timestamp).toBe(sample4!.timestamp);
+	expect(sample3).not.toBe(sample4);
+
+	sample3!.close();
+	expect(sample3!.closed).toBe(true);
+	expect(sample4!.closed).toBe(false);
+	sample4!.close();
+
+	let count = 0;
+	const cursor3 = new VideoSampleCursor(reader, {
+		transform: () => count++,
+	});
+
+	await cursor3.seekToFirst();
+	await cursor3.seekToFirst();
+
+	expect(count).toBe(1);
+
+	count = 0;
+	const cursor4 = new VideoSampleCursor(reader, {
+		autoClose: false,
+		transform: sample => (sample.close(), count++),
+	});
+
+	await cursor4.seekToFirst();
+	await cursor4.seekToFirst();
+
+	expect(count).toBe(2);
+
+	await cursor1.close();
+	await cursor2.close();
+	await cursor3.close();
+	await cursor4.close();
+
+	expect(VideoSample._openSampleCount).toBe(0);
+});
+
 test('Sample cursor reset', async () => {
 	using input = new Input({
 		source: new UrlSource('/trim-buck-bunny.mov'),
@@ -704,6 +769,22 @@ test('Command queuing', async () => {
 
 	await cursor8.close();
 
+	const cursor9 = new VideoSampleCursor(reader, { autoClose: false });
+
+	const commands9 = [
+		cursor9.seekToFirst(),
+		cursor9.seekToFirst(),
+		cursor9.seekToFirst(),
+	];
+	expect(commands9.every(x => x instanceof Promise)).toBe(true);
+
+	for await (using sample of promiseIterateAll(commands9)) {
+		expect(sample!.timestamp).toBe(0);
+		expect(sample!.closed).toBe(false);
+	}
+
+	await cursor9.close();
+
 	expect(VideoSample._openSampleCount).toBe(0);
 });