show the progress rate of a recognition

ooyamatakehisa commented 1 year ago

Related Issue

close #184
107
296
close #222

What

進捗表示

https://github.com/openly-jp/voiscribe/assets/44559556/159963de-3d51-4078-b67f-3354013453bc

Recognizerのリファクタリング
- RecognitionManagerが各recognizedSpeechに対応するRecognizerを保持する。
- ViewにはRecognitionManegerのみを持たせて、各Recognizerは隠蔽する。
- いろんな箇所に同じ参照をもたせるのは極力避けたいので。
- もともとあった以下のpropertyは削除しました。
```
// `isRecognizing` becomes temporarily `false`
// from the end of a previous task in the `serialDispatchQueue`
// to the start of the next task
var isRecognizing = false
```
  かわりにRecognizerStateでRecognizerの状態を保持するようにした。コメントにあるように微妙に機能性は違うけど、まあほぼ同じだしいいかなと。
- streamingRecognize関数のfeasibilityCheckを削除
- 認識中止に行うCoreDataからrecognizedSpeechを削除するなどの作業をキューの最後に追加することで、認識中に変更対象のrecognizedSpeechがなくなるといったことが起きなくなるので、feasibilityCheckの必要がなくなる
- 284 も同様に一番最初にキューの先頭にloadのクロージャーを追加することでUIの描画と並列に裏では認識タスクとは直列にできるのではないかなと思った
- 今までrecognizingSpeechIdsというリストを書くビューで取り回して、認識中かどうかを判断していたが、そのあたりをRecognitionManagerに委譲_
Language周りのリファクタリング
- 今まで認識言語を表していたLanguage -> RecognitionLanguage、モデルの言語を表していたLang -> ModelLanguageに変更
- 今までWhisperModelの引数はモデルの言語を表すLang型の値を受け取っていたけど、モデル使う側からしたら認識モデルの言語事情(multiとか)とか知る必要ないと思ったので、引数はRecognitionLanguage型に変更。RecognitionLanguageがどのModelLanguageに対応するかもWhisperModel内に留められるべき知識だと思ったので。
- 今までRecognitionLanguageとModelLanguage両方がUserDefautlsとかに保存されていたが、ModelLanguageはRecognitionLanguageが定まれば決定するものなので、RecognitionLanguageのみを保存するように変更。

Memo

変更が結構大きいのでコミットごとに見たほうがいいかもです。

todo

一番最初のrecognizedSpeechはviewでcoredataの保存してるけど、以降のtranscriptionLineのcoredataへの保存はrecognizerの中でしている。一応recognizerモデルだし、保存はrecognizerの外に出したいが、力尽きたのでいつかやりましょう。ということで#107 は一応残しておきます。

ooyamatakehisa commented 1 year ago

ごめん微妙に動画バグってる（RecordDetailsで認識が進んでも新たなTranscriptionLineが追加されていない）ので修正するけど、コミット数多いからレビューはしていただいて結構です！

ooyamatakehisa commented 1 year ago

バグ修正終わり

ooyamatakehisa commented 1 year ago

@shibukazu このPRのdescriptionにも少し書いたけど、もうモデルのロードとかfreeModelとかすべてDispatchQueue(label: "recognize")の中にいれていい？そしたら

viewとは並列に動く
認識タスクとは直列にうごく

から色々チェックとかも減りそうだし、認識タスクの実行中にモデルのfreeとかが行われることを防げるし(これ)、

284

とかも解決できそう？

まあ今そもそもfreeModelされるのってchangeModelでモデル変更するときだけで、今は認識中はモデル変更できないようになってるから、実際大丈夫といえば大丈夫なのか。で、もしこういう変更とかするのをキューに入れるなら、認識中でもモデルを変更できるようにできるな。Viewではモデル変更されたようにして、実際にはモデル変更のタスクはまだキューの中に入ってるみたいなことになって結構ややこしくなるけど。

shibukazu commented 1 year ago

@shibukazu このPRのdescriptionにも少し書いたけど、もうモデルのロードとかfreeModelとかすべてDispatchQueue(label: "recognize")の中にいれていい？そしたら

viewとは並列に動く

認識タスクとは直列にうごく

から色々チェックとかも減りそうだし、認識タスクの実行中にモデルのfreeとかが行われることを防げるし(これ)、

284

とかも解決できそう？

まあ今そもそもfreeModelされるのってchangeModelでモデル変更するときだけで、今は認識中はモデル変更できないようになってるから、実際大丈夫といえば大丈夫なのか。で、もしこういう変更とかするのをキューに入れるなら、認識中でもモデルを変更できるようにできるな。Viewではモデル変更されたようにして、実際にはモデル変更のタスクはまだキューの中に入ってるみたいなことになって結構ややこしくなるけど。

これぜひやってほしいです認識中でもモデルが(View上では)変更できることに関しては特に問題はないと思います。

ooyamatakehisa commented 1 year ago

ごめん、キューの中にloadとか入れるのはやるけど、前の認識がおわってなくてもモデル変更できるようにするっていうのはやめとくわ。今ってどのモデルが選択されてるかってRecognitionManager.modeを@Publishedにしてることで、ロードされたタイミングで自動的に選択中モデルの表示も変わるようになってるけど、ビュー上だけで変更できるようにするってことはまた色々@State追加しないといけなさそうで結構めんどくさそうなので...

shibukazu commented 1 year ago

ごめん、キューの中にloadとか入れるのはやるけど、前の認識がおわってなくてもモデル変更できるようにするっていうのはやめとくわ。今ってどのモデルが選択されてるかってRecognitionManager.modeを@Publishedにしてることで、ロードされたタイミングで自動的に選択中モデルの表示も変わるようになってるけど、ビュー上だけで変更できるようにするってことはまた色々@State追加しないといけなさそうで結構めんどくさそうなので...

これってつまり、現状はView上での変更と実際のモデルの変更が結びついちゃってるってことですかね？であれば確かにめんどくさそうなのでキューの中にloadを入れてSequentiallyに実行されることを担保するだけで十分です！！

ooyamatakehisa commented 1 year ago

@shibukazu RecognitionManagerのmodelプロパティの値がかわったら@publishedによってビューの再レンダリングされるから結びついてるといえば結びついてるけど、まあDRYとは言わんけど似たようなどのモデルが選択されてるかみたいな情報をビューにも持たせるのは（上で俺が提案したみたいにビューのモデルの変更と実際のモデルの変更を分けない限りは）冗長な気がするからまあこれでいいかなと個人的には思う。ビューがモデル側（ビジネスロジック側）に依存するのはまあコントローラーがユースケースに依存するのと同じでいいかな的な。

と思ったけど、 #222 と少し関係してるけど、今ってロード自体は非同期で行われてるのに、モデル変更のビューの更新自体は同期的（変更が完了しないと表示は切り替わらない）で、なんかチグハグな気もするし、ロードに時間がかかるようなモデルはやっぱり実際にロードが完了するよりも先にビュー上では変わったほうきもするな。まあFuture Workですね。

とおもったけど、 https://github.com/openly-jp/voiscribe/issues/303#issuecomment-1606023718 ここに書いた通り簡単に対応できた。

openly-jp / voiscribe