refactor and pybind of OnlineWebsocketServer #1943
Conversation
|
Hi @csukuangfj, |
|
Have you compiled and tested it locally? Does it build successfully and also run successfully for you? |
|
yes, it is functional in my local testing with CUDA provider, when I raised PR, observed that for some android build, it was failing with '#include "asio"', |
In that case, I suggest that you move the note that |
|
I have moved the WebSocket file to python/src object file and cleaned it, let me know if it is good enough |
| tokens: str = "/path/to/tokens.txt", | ||
| encoder: str = "/path/to/encoder.onnx", | ||
| decoder: str = "/path/to/decoder.onnx", | ||
| joiner: str = "/path/to/joiner.onnx", |
There was a problem hiding this comment.
The default values are not useful. Could you remove the default values?
| return " ".join([f"--{k.replace('_', '-')}={v}" for k, v in d.items()]) | ||
|
|
||
| class OnlineWebSocketServer(object): | ||
| def __init__(self, |
There was a problem hiding this comment.
I suggest that you pass OnlineRecognizerConfig to the constructor of this class.
This class's constructor has toooo many arguments.
| self.server_args = server_args | ||
| start_server(dict_to_string(self.server_args)) | ||
|
|
||
| def __init__(self, server_args : List[str]) : |
There was a problem hiding this comment.
Why does this class have two __init__ methods?
| // | ||
| // Copyright (c) 2025 Uniphore (Author: Manickavela A) | ||
|
|
||
| #ifndef SHERPA_ONNX_PYTHON_CSRC_ONLINE_WEBSOCKET_SERVER_APP_H_ |
There was a problem hiding this comment.
Please change the header guard to match the filename.
HINT: There is no app in the filename.
|
|
||
| namespace sherpa_onnx { | ||
|
|
||
| void StartServerWrapper(py::list args) { |
There was a problem hiding this comment.
I suggest that you refactor the OnlineWebSocketServer so that it can accept a pointer to an OnlineRecognizer in its constructor.
In this way, it can simplify how you construct an OnlineWebsocketServer.
You can reuse the methods form online_recognizer.py to construct an OnlineRecognizer and pass it to OnlineWebSocketServer.
|
Thanks for the suggestions, I will address them this week. |
Hi @csukuangfj ,
I have refactored the previous implementation,
Below code base is functional tested and cleaned up
I think you can suggest more for naming for more clarity
or clean up.
overall summary,
-- refactoring online-websocket-server
-- pybind interface
-- signal termination, graceful shut down
Thanks
closed #1931