Closed nekomimimi closed 5 months ago
すみません、大変お待たせしました!!! ほぼLGTMです!!
ほぼというのは、ちょっとコードを共通化できそうだったので提案になります!
例えばなのですが、Create Framework
とChange @rpath
それぞれで、simとaarch64の2種をまとめて1ブロックずつにするのはどうでしょう?
特にChange @rpath
の方は-sim
と-aarch64
の違いだけしか無いと思うので、for文でfor arch in "sim" "aarch64"
とすれば処理コードを共通化できそうに思いました!
yamlファイルに書くのが大変であれば、別途.bash
コードを/build_util/
に置く形でもOKです。(むしろそっちのほうがメンテナンスしやすい・・・。)
bashスクリプトがそこそこ書けないと結構大変なのですが、見たところしっかりしたコードだったのでお伺いしてみた次第です。
もしよければ・・・!!
はい。bashスクリプトを書く方向で直してみたいと思います!
ありがとうございます!!!ぜひ!!!
Bashスクリプト化して、ループによる共通化をし、動作確認しました〜! (onnxruntime-builderのほうも同様の修正をした方がいいのかな?)
指摘の通り直す方向で行きたいと思います。 onnxruntime-builderの方は、こっちが固まってから行きたいと思います〜
レビュー頂いた箇所の修正が完了したのでお知らせします〜
@PickledChair @qryxip @HyodaKazuaki もしよかったらレビューいただけると心強いです・・・! 🙇
コードチェック&動作チェックできました、LGTMです! App Store提出まで検証していただきありがとうございます :bow:
PR&レビュー&検証ありがとうございます! マージします!
内容
iOS版がApp Storeへの申請が通らなかったため、 現状、xcframeworkの中に直接、動的ライブラリが入っていたが、 xcframeworkの中にframeworkを作り、その中に動的ライブラリを入れた。
関連 Issue
ref #715
その他
1.onnixruntime側の修正とセットです。 https://github.com/VOICEVOX/onnxruntime-builder/pull/25
2.利用する時のヘッダーファイルがデフォルトでは
include "voicevox_core.h"
でしたが、
include <voicevox_core/voicevox_core.h>
に変わります。 (iOSのframeworkでの標準的な位置となります)
3.テストはsample.vvmで簡単に動作を確認し、AppStoreに申請して受理されることを確認しました。