From 8de25036b8d0500dc3b5ffe0b0602a86ad1c14a6 Mon Sep 17 00:00:00 2001
From: Vanilagy <1696106+Vanilagy@users.noreply.github.com>
Date: Fri, 9 May 2025 15:31:18 +0200
Subject: [PATCH] Automatically close new VideoFrames created by
 toCanvasImageSource, reduce clones in mediaSamplesAtTimestamps

---
 dev/demux.html                    | 20 +++++++++++++++-----
 docs/guide/packets-and-samples.md |  4 ++++
 src/media-sink.ts                 | 16 +++++++---------
 src/sample.ts                     | 12 ++++++++++--
 todo.txt                          |  3 +--
 5 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/dev/demux.html b/dev/demux.html
index b0a79a2..9e503ee 100644
--- a/dev/demux.html
+++ b/dev/demux.html
@@ -60,7 +60,16 @@
 			6.933333333333334,
 			7.039999999999999,
 			7.1466666666666665,
-			7.253333333333333
+			7.253333333333333,
+			7.253333333333333,
+			7.253333333333333,
+			7.253333333333333,
+			7.253333333333333,
+			7.253333333333333,
+			7.253333333333333,
+			7.253333333333333,
+			7.253333333333333,
+			7.253333333333333,
 		];
 
 		console.log(timestamps);
@@ -88,21 +97,22 @@
 		*/
 
 		
-		/*
+		
 		
 
-		for (let i = 0; i < 1000; i++) {
+		for (let i = 0; i < 40; i++) {
 			const count = Math.floor(50 * Math.random());
 			const timestamps = Array.from({ length: count }, () => Math.random() * 20).sort((a, b) => a-b);
 
-			for await (const wrappedCanvas of sink.canvasesAtTimestamps(timestamps)) {
+			for await (const wrappedCanvas of sink.samplesAtTimestamps(timestamps)) {
 				//console.log(wrappedCanvas);
+				wrappedCanvas.close()
 			}
 			console.log("don")
 		}
 
 		console.log("DONE!")
-		*/
+		
 		
 			
 
diff --git a/docs/guide/packets-and-samples.md b/docs/guide/packets-and-samples.md
index 4dc2181..04fbc7f 100644
--- a/docs/guide/packets-and-samples.md
+++ b/docs/guide/packets-and-samples.md
@@ -328,6 +328,10 @@ videoSample.toCanvasImageSource(); // => VideoFrame | OffscreenCanvas;
 
 This method returns a valid `CanvasImageSource` you can use with [drawImage](https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/drawImage).
 
+::: warning
+If this method returns a `VideoFrame`, you should use that frame immediately. This is because any internally-created video frames will automatically be closed in the next microtask.
+:::
+
 ---
 
 Sometimes you may want direct access to the underlying pixel data. To do this, `VideoSample` allows you to copy this data into an `ArrayBuffer`.
diff --git a/src/media-sink.ts b/src/media-sink.ts
index fc6e875..a5c393c 100644
--- a/src/media-sink.ts
+++ b/src/media-sink.ts
@@ -461,7 +461,6 @@ export abstract class BaseMediaSampleSink<
 		// the consumer.
 		let outOfBandError = null as Error | null;
 
-		let lastUsedSample = null as MediaSample | null;
 		const pushToQueue = (sample: MediaSample | null) => {
 			sampleQueue.push(sample);
 			onQueueNotEmpty();
@@ -479,19 +478,20 @@ export abstract class BaseMediaSampleSink<
 					return;
 				}
 
-				let sampleUsed = false;
+				let sampleUses = 0;
 				while (
 					timestampsOfInterest.length > 0
 					&& sample.timestamp - timestampsOfInterest[0]! > -1e-10 // Give it a little epsilon
 				) {
-					pushToQueue(sample.clone() as MediaSample);
+					sampleUses++;
 					timestampsOfInterest.shift();
-					sampleUsed = true;
 				}
 
-				if (sampleUsed) {
-					lastUsedSample?.close();
-					lastUsedSample = sample;
+				if (sampleUses > 0) {
+					for (let i = 0; i < sampleUses; i++) {
+						// Clone the sample if we need to emit it multiple times
+						pushToQueue((i < sampleUses - 1 ? sample.clone() : sample) as MediaSample);
+					}
 				} else {
 					sample.close();
 				}
@@ -601,7 +601,6 @@ export abstract class BaseMediaSampleSink<
 				}
 
 				await flushDecoder();
-				lastUsedSample?.close();
 			}
 			decoder.close();
 
@@ -641,7 +640,6 @@ export abstract class BaseMediaSampleSink<
 				for (const sample of sampleQueue) {
 					sample?.close();
 				}
-				lastUsedSample?.close();
 
 				return { value: undefined, done: true };
 			},
diff --git a/src/sample.ts b/src/sample.ts
index b1abb1e..f28f6eb 100644
--- a/src/sample.ts
+++ b/src/sample.ts
@@ -414,7 +414,12 @@ export class VideoSample {
 		context.restore();
 	}
 
-	/** Converts this video sample to a CanvasImageSource for drawing to a canvas. */
+	/**
+	 * Converts this video sample to a CanvasImageSource for drawing to a canvas.
+	 *
+	 * You must use the value returned by this method immediately, as any VideoFrame created internally will
+	 * automatically be closed in the next microtask.
+	 */
 	toCanvasImageSource() {
 		if (this._closed) {
 			throw new Error('VideoSample is closed.');
@@ -424,7 +429,10 @@ export class VideoSample {
 
 		if (this._data instanceof Uint8Array) {
 			// Requires VideoFrame to be defined
-			return this.toVideoFrame();
+			const videoFrame = this.toVideoFrame();
+			queueMicrotask(() => videoFrame.close()); // Let's automatically close the frame in the next microtask
+
+			return videoFrame;
 		} else {
 			return this._data;
 		}
diff --git a/todo.txt b/todo.txt
index adfe1d4..0bb2534 100644
--- a/todo.txt
+++ b/todo.txt
@@ -1,4 +1,3 @@
 - https://github.com/Vanilagy/mp4-muxer/issues/83 tell him it's possible now
 - textsubtitlesource, chunked piping
-- More efficient MP3 loading when reading sequentially
-- toCanvasImageSource might return a new videoframe; how to handle this?
\ No newline at end of file
+- More efficient MP3 loading when reading sequentially
\ No newline at end of file