-
Notifications
You must be signed in to change notification settings - Fork 112
Port data streams over to rust livekit-ffi version #697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d06f0f2
e3c58f7
314d093
004f531
0563c99
8e94ed3
68cb125
02f4b00
d476d92
5e65b29
230cda6
511174c
c9408f8
82eae6a
d00a938
714aacd
1ef0380
9124f4f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@livekit/rtc-node': patch | ||
| --- | ||
|
|
||
| Convert data streams to use livekit-ffi exposed data streams interface |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,6 @@ | |
| // SPDX-License-Identifier: Apache-2.0 | ||
| import type { DataStream_Chunk } from '@livekit/rtc-ffi-bindings'; | ||
| import { log } from '../log.js'; | ||
| import { bigIntToNumber } from '../utils.js'; | ||
| import type { BaseStreamInfo, ByteStreamInfo, TextStreamInfo } from './types.js'; | ||
|
|
||
| abstract class BaseStreamReader<T extends BaseStreamInfo> { | ||
|
|
@@ -15,6 +14,12 @@ abstract class BaseStreamReader<T extends BaseStreamInfo> { | |
|
|
||
| protected bytesReceived: number; | ||
|
|
||
| private closed = false; | ||
|
|
||
| // The reader held by an in-progress iteration. Cancelling has to go through | ||
| // it, since it holds the stream's lock. | ||
| private activeReader: ReadableStreamDefaultReader<DataStream_Chunk> | null = null; | ||
|
|
||
| get info() { | ||
| return this._info; | ||
| } | ||
|
|
@@ -26,7 +31,68 @@ abstract class BaseStreamReader<T extends BaseStreamInfo> { | |
| this.bytesReceived = 0; | ||
| } | ||
|
|
||
| protected abstract handleChunkReceived(chunk: DataStream_Chunk): void; | ||
| protected handleChunkReceived(chunk: DataStream_Chunk) { | ||
| this.bytesReceived += chunk.content!.byteLength; | ||
| const currentProgress = this.totalByteSize | ||
| ? this.bytesReceived / this.totalByteSize | ||
| : undefined; | ||
| this.onProgress?.(currentProgress); | ||
| } | ||
|
|
||
| /** Takes the stream's reader for an iteration, remembering it so that | ||
| * close() can cancel a read that is still in flight. */ | ||
| protected acquireReader(): ReadableStreamDefaultReader<DataStream_Chunk> { | ||
| const reader = this.reader.getReader(); | ||
| this.activeReader = reader; | ||
| return reader; | ||
| } | ||
|
|
||
| /** Releases the stream lock after an iteration ended on its own — because a | ||
| * read reported end-of-stream, or because the stream itself errored. Either | ||
| * way the stream is in a terminal state and the FFI subscription behind it | ||
| * was already dropped by that terminal event, so there is nothing to cancel. | ||
| * | ||
| * An error raised *around* a read rather than by it — a chunk that failed to | ||
| * process — leaves the stream live, so it has to go through close() instead. */ | ||
| protected finishIteration(reader: ReadableStreamDefaultReader<DataStream_Chunk>) { | ||
| this.closed = true; | ||
| if (this.activeReader === reader) { | ||
| this.activeReader = null; | ||
| } | ||
| reader.releaseLock(); | ||
| } | ||
|
|
||
| /** | ||
| * Stops receiving this stream and releases the resources behind it. | ||
| * | ||
| * A reader is subscribed to FFI events from the moment it is handed to a | ||
| * stream handler, and only unsubscribes once a read consumes the stream's | ||
| * end-of-stream event. So a reader that is abandoned part-way through, or | ||
| * that a handler never reads at all, has to be closed here or its | ||
| * subscription lives for the rest of the process. Reading to completion — or | ||
| * breaking out of a `for await` — closes the reader for you, and closing an | ||
| * already-closed reader is a no-op. | ||
| */ | ||
| async close(): Promise<void> { | ||
| if (this.closed) { | ||
| return; | ||
| } | ||
| this.closed = true; | ||
| const active = this.activeReader; | ||
| this.activeReader = null; | ||
| try { | ||
| if (active) { | ||
| await active.cancel(); | ||
| active.releaseLock(); | ||
| } else { | ||
| await this.reader.cancel(); | ||
| } | ||
|
Comment on lines
+83
to
+89
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Closing an incoming stream while a loop is still reading it makes the loop fail with an error The stream is unlocked ( Impact: An application that stops an incoming data stream from inside its own read loop sees a spurious error thrown instead of a clean end of the loop. Mechanism: releaseLock() detaches the reader that the in-progress async iterator still uses
Example that hits it: for await (const chunk of reader) {
handle(chunk);
if (enough) await reader.close(); // no `break`
}A fix could either avoid releasing the lock in Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| } catch (error: unknown) { | ||
| // The stream was already closed or errored (e.g. the room disconnected | ||
| // mid-stream), in which case its subscription is already gone. | ||
| log.debug('error closing stream reader: %s', error); | ||
| } | ||
| } | ||
|
|
||
| onProgress?: (progress: number | undefined) => void; | ||
|
|
||
|
|
@@ -37,41 +103,48 @@ abstract class BaseStreamReader<T extends BaseStreamInfo> { | |
| * A class to read chunks from a ReadableStream and provide them in a structured format. | ||
| */ | ||
| export class ByteStreamReader extends BaseStreamReader<ByteStreamInfo> { | ||
| protected handleChunkReceived(chunk: DataStream_Chunk) { | ||
| this.bytesReceived += chunk.content!.byteLength; | ||
| const currentProgress = this.totalByteSize | ||
| ? this.bytesReceived / this.totalByteSize | ||
| : undefined; | ||
| this.onProgress?.(currentProgress); | ||
| } | ||
|
|
||
| [Symbol.asyncIterator]() { | ||
| const reader = this.reader.getReader(); | ||
| const reader = this.acquireReader(); | ||
|
|
||
| return { | ||
| next: async (): Promise<IteratorResult<Uint8Array>> => { | ||
| let read: Awaited<ReturnType<typeof reader.read>>; | ||
| try { | ||
| const { done, value } = await reader.read(); | ||
| if (done) { | ||
| // Release the lock when the stream is exhausted so the | ||
| // underlying ReadableStream can be garbage-collected. | ||
| reader.releaseLock(); | ||
| return { done: true, value: undefined as unknown }; | ||
| } else { | ||
| this.handleChunkReceived(value); | ||
| return { done: false, value: value.content! }; | ||
| } | ||
| read = await reader.read(); | ||
| } catch (error: unknown) { | ||
| // Release the lock on error so it doesn't stay held when the | ||
| // consumer never calls return() (e.g. breaking out of for-await). | ||
| reader.releaseLock(); | ||
| log.error('error processing stream update: %s', error); | ||
| // The stream errored, which is terminal: release the lock so it | ||
| // doesn't stay held when the consumer never calls return() (a | ||
| // rejecting next() doesn't trigger it). | ||
| this.finishIteration(reader); | ||
| log.error('error reading stream: %s', error); | ||
| // Propagate abnormal termination (e.g. remote abort, payload over | ||
| // the receiver's size limit) instead of presenting the truncated | ||
| // payload as a clean EOF. | ||
| throw error; | ||
| } | ||
|
|
||
| if (read.done) { | ||
| // Release the lock when the stream is exhausted so the | ||
| // underlying ReadableStream can be garbage-collected. | ||
| this.finishIteration(reader); | ||
| return { done: true, value: undefined as unknown }; | ||
| } | ||
|
|
||
| try { | ||
| this.handleChunkReceived(read.value); | ||
| return { done: false, value: read.value.content! }; | ||
| } catch (error: unknown) { | ||
| // The chunk arrived fine but handling it threw (e.g. a consumer's | ||
| // onProgress callback). The stream is still live, so it has to be | ||
| // cancelled to release its FFI subscription. | ||
| await this.close(); | ||
| log.error('error processing stream update: %s', error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| return(): IteratorResult<Uint8Array> { | ||
| reader.releaseLock(); | ||
| return: async (): Promise<IteratorResult<Uint8Array>> => { | ||
| await this.close(); | ||
| return { done: true, value: undefined }; | ||
| }, | ||
| }; | ||
|
|
@@ -90,77 +163,54 @@ export class ByteStreamReader extends BaseStreamReader<ByteStreamInfo> { | |
| * A class to read chunks from a ReadableStream and provide them in a structured format. | ||
| */ | ||
| export class TextStreamReader extends BaseStreamReader<TextStreamInfo> { | ||
| private receivedChunks: Map<number, DataStream_Chunk>; | ||
|
|
||
| /** | ||
| * A TextStreamReader instance can be used as an AsyncIterator that returns the entire string | ||
| * that has been received up to the current point in time. | ||
| */ | ||
| constructor( | ||
| info: TextStreamInfo, | ||
| stream: ReadableStream<DataStream_Chunk>, | ||
| totalChunkCount?: number, | ||
| ) { | ||
| super(info, stream, totalChunkCount); | ||
| this.receivedChunks = new Map(); | ||
| } | ||
|
|
||
| protected handleChunkReceived(chunk: DataStream_Chunk) { | ||
| const index = bigIntToNumber(chunk.chunkIndex!); | ||
| const previousChunkAtIndex = this.receivedChunks.get(index!); | ||
| if (previousChunkAtIndex && previousChunkAtIndex.version! > chunk.version!) { | ||
| // we have a newer version already, dropping the old one | ||
| return; | ||
| } | ||
| this.receivedChunks.set(index, chunk); | ||
| const currentProgress = this.totalByteSize | ||
| ? this.receivedChunks.size / this.totalByteSize | ||
| : undefined; | ||
| this.onProgress?.(currentProgress); | ||
| } | ||
|
|
||
| /** | ||
| * Async iterator implementation to allow usage of `for await...of` syntax. | ||
| * Yields structured chunks from the stream. | ||
| * | ||
| */ | ||
| [Symbol.asyncIterator]() { | ||
| const reader = this.reader.getReader(); | ||
| const reader = this.acquireReader(); | ||
| const decoder = new TextDecoder(); | ||
| const receivedChunks = this.receivedChunks; | ||
|
|
||
| return { | ||
| next: async (): Promise<IteratorResult<string>> => { | ||
| let read: Awaited<ReturnType<typeof reader.read>>; | ||
| try { | ||
| const { done, value } = await reader.read(); | ||
| if (done) { | ||
| // Release the lock when the stream is exhausted so the | ||
| // underlying ReadableStream can be garbage-collected. | ||
| reader.releaseLock(); | ||
| // Clear received chunks so the buffered data can be GC'd. | ||
| receivedChunks.clear(); | ||
| return { done: true, value: undefined }; | ||
| } else { | ||
| this.handleChunkReceived(value); | ||
| return { | ||
| done: false, | ||
| value: decoder.decode(value.content!), | ||
| }; | ||
| } | ||
| read = await reader.read(); | ||
| } catch (error: unknown) { | ||
| // Release the lock on error so it doesn't stay held when the | ||
| // consumer never calls return() (e.g. breaking out of for-await). | ||
| reader.releaseLock(); | ||
| receivedChunks.clear(); | ||
| log.error('error processing stream update: %s', error); | ||
| // The stream errored, which is terminal: release the lock so it | ||
| // doesn't stay held when the consumer never calls return() (a | ||
| // rejecting next() doesn't trigger it). | ||
| this.finishIteration(reader); | ||
| log.error('error reading stream: %s', error); | ||
| // Propagate abnormal termination (e.g. remote abort, payload over | ||
| // the receiver's size limit) instead of presenting the truncated | ||
| // payload as a clean EOF. | ||
| throw error; | ||
| } | ||
|
|
||
| if (read.done) { | ||
| // Release the lock when the stream is exhausted so the | ||
| // underlying ReadableStream can be garbage-collected. | ||
| this.finishIteration(reader); | ||
| return { done: true, value: undefined }; | ||
| } | ||
|
|
||
| try { | ||
| this.handleChunkReceived(read.value); | ||
| return { done: false, value: decoder.decode(read.value.content!) }; | ||
| } catch (error: unknown) { | ||
| // The chunk arrived fine but handling it threw (e.g. a consumer's | ||
| // onProgress callback). The stream is still live, so it has to be | ||
| // cancelled to release its FFI subscription. | ||
| await this.close(); | ||
| log.error('error processing stream update: %s', error); | ||
| throw error; | ||
| } | ||
| }, | ||
|
|
||
| return(): IteratorResult<string> { | ||
| reader.releaseLock(); | ||
| // Clear received chunks so the buffered data can be GC'd. | ||
| receivedChunks.clear(); | ||
| return: async (): Promise<IteratorResult<string>> => { | ||
| await this.close(); | ||
| return { done: true, value: undefined }; | ||
| }, | ||
| }; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.