Skip to content

Conversation

gbaian10
Copy link

相關 issue: #2208 #2212


由於改動有點多,所以先只改一個檔案並發個草稿 PR,看看能不能接受此改動。
如果能接受,之後將會將類似此改動套用到以下檔案

  • speaker-identification.py
  • speaker-identification-with-vad.py
  • speaker-identification-with-vad-non-streaming-asr.py
  • speaker-identification-with-vad-non-streaming-asr-alsa.py
  • speaker-identification-with-vad-dynamic.py

以上都是在 python-api-examples 資料夾下的檔案


這次草稿以 speaker-identification.py 為修改範例,主要邏輯基本上全部維持一樣,主要重構的方向有

  1. 補齊 Type Hint,讓使用者在 IDE 使用以及閱讀程式碼都有更好的體驗
  2. 將專屬該檔案命令列部分使用的程式碼分離至下半部分,上半部的 code 幾乎都是對 sherpa-onnx 的使用封裝 (目前只包成 func)。 這五個檔案在這部分應該都會是類似的。 命令列部分(專門執行該檔案 example 的 code 放到了下半部)
  3. 包裝了使用 sounddevice 的錄音程式碼成一個 class,使閱讀更容易

因為我本身不太熟這些看起來像是 C 語言(?)包裝的物件怎使用,這些物件能使用的方法除了看 example 之外很難知道
例如以下 code

stream = extractor.create_stream()
stream.accept_waveform(sample_rate=sample_rate, waveform=samples)
stream.input_finished()

我自己是期望對於這些物件,例如: sherpa_onnx.SpeakerEmbeddingExtractor 或者 sherpa_onnx.SpeakerEmbeddingManager,都能移動到此 package 的一個類似 core 的 module,而另外包一個應用層的程式碼,例如此 PR 的上半部給使用者更直觀的使用。

from sherpa_onnx.core import SpeakerEmbeddingExtractor

當然為了向前兼容,使用者依然可以直接操縱這些 core 的物件來寫 code,這樣也不會影響到已經正在使用現有套件專案的人

@@ -1,5 +1,3 @@
#!/usr/bin/env python3
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove this line.

Copy link
Author

Choose a reason for hiding this comment

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

Restored

# %%
# The following code is required for command line interface.
# If you only need the packaged functions, you can use only the code above
import argparse
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please put all imports at the beginning.

Copy link
Author

Choose a reason for hiding this comment

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

The code has been moved to the top.

@gbaian10
Copy link
Author

It has been revised according to the requirements.
Are there any other suggested directions for improvement?

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