Skip to content

Add support for streaming speech-to-text results - #242

Merged
jonatanklosko merged 4 commits into
mainfrom
jk-audio-stream
Sep 12, 2023
Merged

Add support for streaming speech-to-text results#242
jonatanklosko merged 4 commits into
mainfrom
jk-audio-stream

Conversation

@jonatanklosko

Copy link
Copy Markdown
Member

Large audio is split into multiple chunks, now we support stream: true, so that output chunks are emitted as soon as they are available.

Comment thread lib/bumblebee/audio.ex
Comment on lines +9 to +12
serving = speech_to_text_whisper(model_info, featurizer, tokenizer, generation_config, opts)
client_postprocessing = serving.client_postprocessing

Nx.Serving.client_postprocessing(serving, fn output_pair, info ->

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output format changed, so I'm wrapping to avoid a breaking change.

Comment thread lib/bumblebee/audio.ex

"""
@type speech_to_text_whisper_input :: Nx.t() | {:file, String.t()}
@type speech_to_text_whisper_output :: %{results: list(speech_to_text_whisper_result())}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed results: [...]. It could make sense for non-deterministic text generation with sampling, but for transcription we generally expect a specific output, so I don't think it will ever be useful.

Comment thread lib/bumblebee/audio/speech_to_text_whisper.ex
@jonatanklosko

Copy link
Copy Markdown
Member Author

@josevalim Nx should be good to go with respect to streaming, awesome job!

Comment thread lib/bumblebee/audio/speech_to_text_whisper.ex Outdated
Comment thread lib/bumblebee/audio.ex
end_timestamp_seconds: number() | nil
})
start_timestamp_seconds: number() | nil,
end_timestamp_seconds: number() | nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to wrap up our conversation, the issue with this is that, based on an option, the timestamp is either nil or number. However, callers of the code that only call this with timestamps: true, once we have the typesystem, will now have to potentially handle nil on their code, even though it can never be nil! That's because it is hard in a type system to have different result types based on an option.

There are a couple fixes. One is to say this is dynamic and (number or nil) (in the future, not now). The other option is to make it always a number and return -1 if timestamps is false. Another option is to have different functions. One with timestamps and one without. But this is what I meant, in a nutshell.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you are right it still depends on the option just in a different way. I'm not really a fan of -1, though it's the most straightforward option. Typing serving may be tricky on its own because what we call is Nx.Serving.batched_run and the return value depends on the serving name we pass?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there is still a lot for it to happen, but I thought I would start the discussion. If we keep this as is, then -1 is like the only viable option in the future if we want to keep a single type.

Another option is to have a speech_to_text_whisper function, which returns a string (as it did before) and have transcribe_whisper (which has the return type above without nils).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep nil and if we get to type serving outputs we can switch to -1. The issue with separate functions is that if we have another option like that then we have 2x2 functions :D

@jonatanklosko
jonatanklosko merged commit 391fcd0 into main Sep 12, 2023
@jonatanklosko
jonatanklosko deleted the jk-audio-stream branch September 12, 2023 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants