-
Notifications
You must be signed in to change notification settings - Fork 285
Shubhra/ajs 137 add resampler to input #541
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -3,9 +3,12 @@ | |
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
| import { AudioResampler } from '@livekit/rtc-node'; | ||
| import { AudioFrame } from '@livekit/rtc-node'; | ||
| import { delay } from '@std/async'; | ||
| import { EventEmitter, once } from 'node:events'; | ||
| import type { ReadableStream } from 'node:stream/web'; | ||
| import { TransformStream, type TransformStreamDefaultController } from 'node:stream/web'; | ||
| import { v4 as uuidv4 } from 'uuid'; | ||
| import { log } from './log.js'; | ||
|
|
||
|
|
@@ -587,3 +590,36 @@ export function isImmutableArray(array: unknown): boolean { | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| return typeof array === 'object' && !!(array as any)[READONLY_SYMBOL]; | ||
| } | ||
|
|
||
| /** | ||
| * Resamples an audio stream to a target sample rate. | ||
| * | ||
| * WARINING: The input stream will be locked until the resampled stream is closed. | ||
| * | ||
| * @param stream - The input stream to resample. | ||
| * @param outputRate - The target sample rate. | ||
| * @returns A new stream with the resampled audio. | ||
| */ | ||
| export function resampleStream({ | ||
| stream, | ||
| outputRate, | ||
| }: { | ||
| stream: ReadableStream<AudioFrame>; | ||
| outputRate: number; | ||
| }): ReadableStream<AudioFrame> { | ||
| let resampler: AudioResampler | null = null; | ||
| const transformStream = new TransformStream<AudioFrame, AudioFrame>({ | ||
| transform(chunk: AudioFrame, controller: TransformStreamDefaultController<AudioFrame>) { | ||
| if (!resampler) { | ||
| resampler = new AudioResampler(chunk.sampleRate, outputRate); | ||
| } | ||
| for (const frame of resampler.push(chunk)) { | ||
| controller.enqueue(frame); | ||
| } | ||
| for (const frame of resampler.flush()) { | ||
| controller.enqueue(frame); | ||
| } | ||
| }, | ||
| }); | ||
| return stream.pipeThrough(transformStream); | ||
|
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. note that this will lock the input stream argument. When doing resource cleanup, we need to keep this in mind to avoid potential locking issue.
Contributor
Author
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. this a good callout. I'll add a comment for the function.
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. I wonder if we make it a class like "ResampledStream" and we expose method for unlock, cancel, just like deferred readable stream. I think instead of using
Contributor
Author
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. We can just use the streampipe options?
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. I haven't tried it yet, but feel free to try it. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import { | |
| import type { ReadableStream } from 'node:stream/web'; | ||
| import { log } from '../../log.js'; | ||
| import { DeferredReadableStream } from '../../stream/deferred_stream.js'; | ||
| import { resampleStream } from '../../utils.js'; | ||
|
|
||
| export class ParticipantAudioInputStream { | ||
| private room: Room; | ||
|
|
@@ -83,7 +84,12 @@ export class ParticipantAudioInputStream { | |
| } | ||
| this.publication = publication; | ||
| this.logger.debug({ track, publication, participant }, 'track subscribed'); | ||
| this.deferredStream.setSource(this.createStream(track)); | ||
| this.deferredStream.setSource( | ||
| resampleStream({ | ||
|
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. ^ same, we need to take care of un-
Contributor
Author
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. What do you mean "un-pipeThrough"? If you call
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. If we cancel the
Contributor
Author
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. This is a great point. Let me look through the code base and see what makes sense.
Contributor
Author
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.
you can do this via
Contributor
Author
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. readable = stream.pipeThrough({ writable, readable }[, { preventClose, preventAbort, preventCancel, signal }])
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.
Contributor
Author
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. Yeah I think you're right. Going to test it with restaurant agent and handoff to make sure it works okay. Then I'll let you know.
Contributor
Author
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. hand off worked correctly so I think we don't need it here. |
||
| stream: this.createStream(track), | ||
| outputRate: this.sampleRate, | ||
| }), | ||
| ); | ||
| return true; | ||
| }; | ||
|
|
||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will probably need this in the plugins. That's why I kept it a separate utility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a brief function spec to this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like a JS doc you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, just a one or two line brief description would be fine