Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

soundfileのバージョンを0.12.0に上げてlibsndfileの依存を外す #747

Closed
Hiroshiba opened this issue Sep 15, 2023 · 6 comments · Fixed by #769
Closed

soundfileのバージョンを0.12.0に上げてlibsndfileの依存を外す #747

Hiroshiba opened this issue Sep 15, 2023 · 6 comments · Fixed by #769

Comments

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 15, 2023

内容

soundfileはlinux版だけlibsndfile1というライブラリが必要になっています。
これの影響でいろんなエラーなどが出たり、エディターでこのライブラリーを特別にインストールする必要があったりします。

soundfileのバージョン0.12.0からはこの制約がなくなり、ライブラリのインストールが必要なくなりました!(thx @sabonerune
#731 (comment)

soundfileのバージョンを上げてlibsndfile1の依存をなくせばこの課題は解決です。

Pros 良くなる点

依存が減る

Cons 悪くなる点

実現方法

バージョンを上げたあと、libsndfileに依存しているソースコードを全体検索して見つけ、依存を外していけば作業は完了だと思います。
あとは一度テストでビルドしてみて、音声がちゃんと合成できるかどうかを確認したらOKかなと。

合成できるかの確認は実際にGithub Workflowでビルドしたら自動的にテストが実行されるので、それがクリアできたらOKだと思います。

その他

close #212

@sabonerune
Copy link
Contributor

エンジンのsoundfileを0.12に更新するとlibsndfileバイナリにGPLが混入するかもしれない?

0.11.0でMP3対応に伴ってバイナリの依存にLAMEが追加
LAMEのライセンスにはこのようにある

*** IMPORTANT NOTE ***

The decoding functions provided in LAME use a version of the mpglib decoding
engine which is under the GPL. They may not be used by any program not
released under the GPL unless you obtain such permission from the MPG123
project (www.mpg123.de).
(yes, we know MPG123 is currently under the LGPL, but we use an older version
that was released under the former license and, until someone tweaks the
current MPG123 to suit some of LAME's specific needs, it'll continue being
licensed under the GPL)

https://lame.sourceforge.io/license.txt

LAMEのビルドスクリプトにはMP3のデコード無効化するオプションがある(デフォルトは恐らく有効)

--disable-decoder Exclude mpg123 decoder

https://sourceforge.net/p/lame/svn/HEAD/tree/tags/RELEASE__3_100/lame/configure

一方soundfileにバイナリを提供しているlibsndfile-binariesではLAMEのビルドオプションに--disable-decoderを付けていないのでGPLのコードが混入しているのでは?
https://github.com/bastibe/libsndfile-binaries/blob/d9887ef926bb11cf1a2526be4ab6f9dc690234c0/linux_build.sh#L66-L71

ちょっと自信がなくてsoundfileの方にissueを投稿していないのですが…
また、WIndowsバイナリの方はlibsndfileがビルドしたバイナリを使用しているようでどのようにビルドしたか自分には理解できなかったので確認できていません。

@Hiroshiba
Copy link
Member Author

Hiroshiba commented Oct 17, 2023

おっとなるほどです!!気づいてくださってありがとうございます!!!
issue立てるとしたらlibsndfile-binaries側ですかねぇ。

ただまあ、なんか調べてる感じLGPLな気がしました!!
めちゃめちゃ調べたんですが、まずLAME 3.100内に含まれるmpg123のコードmpg123.hにはLGPLだと書いてました。
mpg123のwikipedia見た感じ2006年にGPLからLGPLに変わったらしく、LAME 3.100内のmpg123のコードは2010年時点のものっぽいので合いそうです。
あと、LAME 3.100内のコードに含まれるLICENSEは、そのIMPORTANTの項目が書かれてませんでした。
LAME側もホームページ見る感じしっかりLGPLだと伝えてるっぽいので、なんかそのhttps://lame.sourceforge.io/license.txtだけ古い気がしました。

もしそうだったら少なくとも6年間放置されてる気がするので、LAME側に伝えてあげたいですね!

@sabonerune
Copy link
Contributor

確認ありがとうございます。
改めてLAMEのコードを確認したところLAME 3.90.1のmpglib内にあったGPLの記述が3.100ではLGPLに切り替わっていることを自分も確認できました。
恐らく大丈夫であるということで作業を続けます。

@Hiroshiba
Copy link
Member Author

プルリクエストがマージされてlibsndfileのインストールが不要になりました! 🎉 ( thx @sabonerune !! )

確かエディターの方でもエンジンをインストールした後にlibsndfileを持ってくる的な処理があった気がする(インストール時に持ってくる、だったかも)、そこのコードを変えられそうな気がします。
@sabonerune もしよかったらそっちの変更も挑戦してみませんか? 👀

もしかしたらシェルスクリプトの変更が必要になって来るかもなので、難しいとかあればこっちでやっちゃおうかなと思います!

@sabonerune
Copy link
Contributor

sabonerune commented Oct 18, 2023

ぱっと調べた感じLinuxのインストーラーにlibsndfile.soのチェックが、テストのワークフローにlibsndfileのインストールがありますね。

インストーラーの方は複数のバージョンに対応しているため単純に削除するのではなくインストールするバージョンが0.15以上の場合はチェックしない感じにすればいい気がします。

テストのワークフローはまだ0.14のエンジンを使っている感じみたいなのでまだ変更できなさそうな気がします。
こちらは0.15リリース後の作業ですね?

シェルスクリプトはあまり慣れていないのですが挑戦してみたいと思います。
(できなくて諦めたときはエディタの方にIssueを作ろうと思います)

@Hiroshiba
Copy link
Member Author

ぜひぜひ!!

インストーラーの方は複数のバージョンに対応しているため単純に削除するのではなくインストールするバージョンが0.15以上の場合はチェックしない感じにすればいい気がします。

たしかに・・・!!
(毎リリースでそのバージョンに対応したlinuxのインストーラーをreleasesにアップロードしておけば良かったなと思いました・・・。)

テストのワークフロー

あ、本当ですね!
Nemoの方はまだ0.15版作ってないのでどうしようもなさそう。将来消すの忘れそうですが、そんな問題じゃ無さそうな気がしました!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants