feat(packages): add speaches support for SileroVAD and Whisper STT as OpenAI API #872
Conversation
There was a problem hiding this comment.
Hey there @OriNachum, thanks for your PR! 🥇
I’ve just wrapped up my review, and everything looks well crafted. Found just few places where we could improve and resolve some of the issues that you are facing.
Appreciate your hard work and contribution! 🫶
| #--- | ||
| # name: faster-whisper:speaches | ||
| # group: audio | ||
| # depends: [ctranslate2:master-builder,faster-whisper, python:3.12, numpy] |
There was a problem hiding this comment.
Is there any reason to not use ctranslate2 here instead of builder? This impacts the image size and build time. We can easily reduce them.
| # docs: docs.md | ||
| # notes: run with cuda12.6 | ||
| #--- | ||
|
|
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # Remove any existing .venv directories | ||
| RUN find / -name ".venv" -type d -exec rm -rf {} + 2>/dev/null || true |
There was a problem hiding this comment.
Running find / as root inside a Docker RUN command can be slow. If you want to remove a .venv dir at a specific path, you can use:
RUN [ -d "/app/.venv" ] && rm -rf /app/.venv || true| # Build faster-whisper wheel | ||
| RUN git clone https://github.com/guillaumekln/faster-whisper && \ | ||
| cd faster-whisper && \ | ||
| sed -i \ | ||
| -e 's|^onnxruntime.*||g' \ | ||
| -e 's|^ctranslate2.*||g' \ | ||
| -e 's|^huggingface_hub.*||g' \ | ||
| -e 's|^tokenizers.*|tokenizers|g' \ | ||
| requirements.txt && \ | ||
| pip3 install --no-cache-dir --verbose -r requirements.txt && \ | ||
| python3 setup.py bdist_wheel && \ | ||
| cp dist/faster_whisper*.whl ${WHEEL_DIR}/ |
There was a problem hiding this comment.
You already included the faster-whisper in the depends, so it's wheel is already installed. Safe to remove I assume.
| FROM ${BASE_IMAGE} | ||
|
|
||
| ARG SPEACHES_ROOT="/opt/speaches" | ||
| ARG WHEEL_DIR="/opt/wheels" |
There was a problem hiding this comment.
@dusty-nv would we like to set JC default wheels dir and set this ENV in python package instead?
| RUN chmod +x ${SPEACHES_ROOT}/start-speaches-server.sh | ||
|
|
||
| # Expose port | ||
| EXPOSE 8000 |
There was a problem hiding this comment.
NTH: Make the port configurable by ARG
| COPY start-speaches-server.sh ${SPEACHES_ROOT}/start-speaches-server.sh | ||
| RUN chmod +x ${SPEACHES_ROOT}/start-speaches-server.sh |
There was a problem hiding this comment.
I doubt that anybody would build jetson-containers on Windows, so we are safe to assume that you can set (on Linux) start-speaches-server.sh permissions before copying the file into docker image as the Docker will typically preserve the permissions if you're working in a Unix-like environment.
| # Print locations of globally installed packages (for debugging) | ||
| RUN python3 -c "import sys, numpy, ctranslate2, faster_whisper; \ | ||
| print(f'Python path: {sys.path}'); \ | ||
| print(f'NumPy: {numpy.__file__}'); \ | ||
| print(f'ctranslate2: {ctranslate2.__file__}'); \ | ||
| print(f'faster_whisper: {faster_whisper.__file__}')" || \ | ||
| echo "Warning: Not all packages could be imported." |
There was a problem hiding this comment.
This is not needed and can be moved to tests.
| # requires: '>=34.1.0' | ||
| # test: [test.py] | ||
| # docs: docs.md | ||
| # notes: run with cuda12.6 |
There was a problem hiding this comment.
If you dare to create config.py for this package we could create a minimum requirement for this package and pin it to cu126
| @@ -0,0 +1,77 @@ | |||
| #--- | |||
| # name: faster-whisper:speaches | |||
There was a problem hiding this comment.
@dusty-nv I know that the initial package name comes from package root directory. Should we allow to use colon : within the package name in Dockerfile?
Note: I couldn't get the test to run.
It goes right into server startup.
When I run the test (From another docker) it works great.
Edit: Also, onnx for TTS not supported for now (onnx based)