libwebrtc を m124.6367.3.1 にあげる #98

enm10k · 2024-05-07T07:01:41Z

変更内容

libwebrtc を m124 に更新しました

VideoDecoderFactory から CreateVideoDecoder が削除され、 webrtc::Environment を引数にとる Create を使って初期化するようになりました
- 参照: https://source.chromium.org/chromium/_/webrtc/src/+/38c1ab1e6c3e72176cc051d0174cfd0a2a639e8a
libwebrtc から examples/unityplugin ディレクトリが削除されたため、 test/android のビルドが通らなくなっていたので修正しました
- https://source.chromium.org/chromium/_/webrtc/src/+/97df932ecc58ac9ee49192992c22995809145cbf
- include していたヘッダー・ファイルを削除したところビルドが通り、 test/android も動作しました

残作業

README.md を書く

enm10k · 2024-05-08T05:24:17Z

src/sora_peer_connection_factory.cpp

+            // TODO(enm10k): environment を外部から設定できるようにする
            webrtc::ConnectionContext::Create(webrtc::CreateEnvironment(),


@melpon
既存の API を維持しつつ、新しい API を追加して webrtc::Environment を外部から渡せるようにしたかったのですが、上手くいきませんでした
良い対応方法があればアドバイスをいただきたいです

これってそもそも複数作ったらいけないんですかね？いろんな場所で webrtc::CreateEnvironment() して動くならそれで良さそうな気も

これってそもそも複数作ったらいけないんですかね？

自信が無いですが、クラスによっては? 複数作っても問題なさそうです

いろんな場所で webrtc::CreateEnvironment() して動くならそれで良さそうな気も

Sora C++ SDK の内部でで webrtc::CreateEnvironment() するのでも動くはずです
最近 libwebrtc の色んなクラスが webrtc::Environment を引数にとるようになってきているので、将来的には外部から渡せた方が良いのかなと思っていますが、本当にそうなってからの対応で良いのかもしれませんね

... 将来的には外部から渡せた方が良いのかなと思っています

上記の部分について補足をすると、現在の webrtc::Environment のメンバーを見る限り、 field trials を Sora C++ SDK のユーザーが設定したい場合を除いて、 webrtc::Environment を外部から受け取る必要はなさそうです
参照: https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/api/environment/environment.h;l=118-123;drc=db329edf40185945b3ead6fccaf0c2ce780c72b9

enm10k · 2024-05-08T05:34:28Z

include/sora/sora_client_context.h

@@ -84,6 +89,7 @@ class SoraClientContext {
  std::unique_ptr<rtc::Thread> signaling_thread_;
  rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> factory_;
  rtc::scoped_refptr<webrtc::ConnectionContext> connection_context_;
+  std::shared_ptr<webrtc::Environment> env_;


@melpon
(webrtc::Environement を引数にとらない) 既存の API を維持しようとしたところ、ライフタイムの問題が発生したため、 webrtc::Environment をこのクラスで保持しています
実装方針として問題ないか相談したいです

shared_ptr にする必要は無さそう

melpon · 2024-05-08T06:07:52Z

include/sora/sora_client_context.h

@@ -84,6 +89,7 @@ class SoraClientContext {
  std::unique_ptr<rtc::Thread> signaling_thread_;
  rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> factory_;
  rtc::scoped_refptr<webrtc::ConnectionContext> connection_context_;
+  std::shared_ptr<webrtc::Environment> env_;


shared_ptr にする必要は無さそう

melpon · 2024-05-08T06:10:32Z

include/sora/sora_client_context.h

+  static std::shared_ptr<SoraClientContext> Create(
+      const SoraClientContextConfig& config,
+      webrtc::Environment& env);
+


Create に引数追加しなくても、SoraClientContextConfig に webrtc::Environment 入れてしまえば良さそうに見えます。

ただ、そもそも外から指定可能にする意味も無さそうに見えるので、SoraClientContext のメンバに env_ 持たせて内部で webrtc::CreateEnvironment すれば良さそう。

melpon · 2024-05-08T06:21:08Z

src/sora_peer_connection_factory.cpp

+            // TODO(enm10k): environment を外部から設定できるようにする
            webrtc::ConnectionContext::Create(webrtc::CreateEnvironment(),


これってそもそも複数作ったらいけないんですかね？いろんな場所で webrtc::CreateEnvironment() して動くならそれで良さそうな気も

melpon · 2024-05-08T06:49:38Z

include/sora/sora_video_decoder_factory.h

    std::shared_ptr<CudaContext> cuda_context = nullptr,
-    void* env = nullptr);
+    void* jni_env = nullptr);


１番目に引数を追加すると破壊的変更になるので、ここらは内部で webrtc::CreateEnvironment を呼んで生成するか、一番うしろに引数を追加してデフォルトを webrtc::CreateEnvironment にしないといけないですね

…引き回すのを止める

enm10k · 2024-05-10T07:59:32Z

@melpon
修正したので再度ご確認いただきたいです

以下の理由で webrtc::Environment を引き回すのを止めました

最終的に webrtc::CreateEnvironment を実行する回数が (ほとんど) 変わらない
webrtc::Environment を引き回そうとすると変更箇所が多くなるので、本当に必要になるまで保留したい

melpon · 2024-05-10T14:33:49Z

src/sora_video_decoder_factory.cpp

  SoraVideoDecoderFactoryConfig config;
  config.decoders.push_back(VideoDecoderConfig(
      webrtc::kVideoCodecVP8,
-      [](auto format) { return webrtc::VP8Decoder::Create(); }));
+      [&env](auto format) { return webrtc::CreateVp8Decoder(env); }));


参照を渡すのは未定義動作なので env を渡して下さい。

2ae98b7 で修正しました

melpon · 2024-05-10T14:34:16Z

src/sora_video_decoder_factory.cpp

-            return factory->CreateVideoDecoder(format);
-          };
+      create_video_decoder = [factory = enc.factory.get(),
+                              &env](const webrtc::SdpVideoFormat& format) {


参照でも動作するとは思いますけど、参照にする理由もないので env にして下さい。

2ae98b7 で修正しました

melpon · 2024-05-10T14:35:00Z

CHANGES.md

@@ -11,14 +11,15 @@

 ## develop

+- [CHANGE] libwebrtc を m124.6367.3.1 にあげる
+  - 継承元のクラスの変更に追従するため、 SoraVideoDecoderFactory の初期化に利用する関数を CreateVideoDecoder から Create に変更


CHANGE なのはこの部分だけで、他の部分は違うと思うので、分けたほうが良いです

fdb7947 で対応しました

melpon · 2024-05-10T14:35:23Z

examples/VERSION

-WEBRTC_BUILD_VERSION=m123.6312.3.5
-BOOST_VERSION=1.85.0
+WEBRTC_BUILD_VERSION=m124.6367.3.1
+BOOST_VERSION=1.84.0


Boost のバージョンが下がってる…？

こちらですが、現在 Sora C++ SDK でリリースされている最新の Boost のバージョンが 1.84.0 なため、 --sora-dir . を指定しない場合ビルドが落ちるようです (= develop のsサンプル BOOST_VERSION を誤ってあげてしまっている)

この PR のマージにもう少し時間がかかるようであれば、 develop ブランチを直接修正します

1fa19e0

あやまってあげてしまってるってのがよくわからんのだが。

サンプルのビルド時は Sora C++ SDK のリリースに含まれる Boost のバイナリを取得してビルドする挙動がデフォルトとなっています。
そのため、 SDK とサンプルの Boost のバージョンを同時に上げられないようです。
(SDK の Boost のバージョンを上げてリリースした後、サンプルの Boost のバージョンをあげる必要がある)

0997648 でサンプルの BOOST_VERSION を 1.84.0 に戻しました

…webrtc-to-m124

enm10k · 2024-05-10T15:40:58Z

@melpon 指摘いただいた箇所を修正しました

melpon

よさそう

enm10k · 2024-05-13T07:04:46Z

ありがとうございます。
マージします。

enm10k added 2 commits May 7, 2024 16:16

libwebrtc を m124.6367.3.1 にあげる

90207d6

webrtc::VideoDecoderFactory 初期化時に webrtc::Environment の指定が必要になった

4861e13

enm10k force-pushed the feature/update-libwebrtc-to-m124 branch from b927602 to 4861e13 Compare May 7, 2024 09:46

test のビルドが通るように修正する

01ad4ff

enm10k force-pushed the feature/update-libwebrtc-to-m124 branch from 1e78ecf to 01ad4ff Compare May 7, 2024 11:03

sora::SoraClientContext::Create に webrtc::Environment を渡す

ba5bb61

enm10k force-pushed the feature/update-libwebrtc-to-m124 branch from 477ae4c to 7061b78 Compare May 7, 2024 12:20

enm10k added 2 commits May 8, 2024 10:45

webrtc::Environment を渡さない初期化方法を維持する

78843b9

libwebrtc から examples/unityplugin が削除されたので追従する

cac00ea

enm10k force-pushed the feature/update-libwebrtc-to-m124 branch from 7061b78 to cac00ea Compare May 8, 2024 01:48

試行錯誤した際のコメントを消す

01f5ba6

enm10k marked this pull request as ready for review May 8, 2024 05:21

enm10k requested a review from melpon May 8, 2024 05:21

enm10k commented May 8, 2024

View reviewed changes

SoraClientContext で保持する webrtc::Environment は private で良かった

7e8622b

enm10k commented May 8, 2024

View reviewed changes

melpon reviewed May 8, 2024

View reviewed changes

enm10k added 3 commits May 10, 2024 12:20

SoraClientContext で webrtc::Environment を保持するのを止める

3e789b1

互換性を維持するために webrtc::Environment を最後の変数として追加し、デフォルト値を指定する

ba0da74

最終的に webrtc::CreateEnvironment を実行する回数が変わらないので、 webrtc::Environment を…

9d5eb9b

…引き回すのを止める

enm10k force-pushed the feature/update-libwebrtc-to-m124 branch from 95bd6d4 to 9d5eb9b Compare May 10, 2024 06:28

不要な差分を元に戻す

ecec7db

enm10k force-pushed the feature/update-libwebrtc-to-m124 branch from f263614 to ecec7db Compare May 10, 2024 07:24

CHANGES.md を更新する

d95ab15

melpon reviewed May 10, 2024

View reviewed changes

enm10k added 3 commits May 10, 2024 23:46

参照でキャプチャーするのをやめる

2ae98b7

CHANGES.md を修正する

fdb7947

Merge remote-tracking branch 'origin/develop' into feature/update-lib…

7a21aa4

…webrtc-to-m124

yaml のエラーを直す

35b1eb6

miosakuma requested a review from melpon May 13, 2024 06:56

melpon approved these changes May 13, 2024

View reviewed changes

enm10k merged commit ac936f5 into develop May 13, 2024
9 checks passed

enm10k deleted the feature/update-libwebrtc-to-m124 branch May 13, 2024 07:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

libwebrtc を m124.6367.3.1 にあげる #98

libwebrtc を m124.6367.3.1 にあげる #98

enm10k commented May 7, 2024 •

edited

Loading

enm10k May 8, 2024

melpon May 8, 2024

enm10k May 8, 2024

enm10k May 8, 2024

enm10k May 8, 2024

melpon May 8, 2024

melpon May 8, 2024

melpon May 8, 2024

melpon May 8, 2024

melpon May 8, 2024

enm10k commented May 10, 2024 •

edited

Loading

melpon May 10, 2024

enm10k May 10, 2024

melpon May 10, 2024

enm10k May 10, 2024

melpon May 10, 2024

enm10k May 10, 2024

melpon May 10, 2024

enm10k May 10, 2024 •

edited

Loading

voluntas May 10, 2024

enm10k May 10, 2024

enm10k commented May 10, 2024

melpon left a comment

enm10k commented May 13, 2024

		// TODO(enm10k): environment を外部から設定できるようにする
		webrtc::ConnectionContext::Create(webrtc::CreateEnvironment(),

libwebrtc を m124.6367.3.1 にあげる #98

libwebrtc を m124.6367.3.1 にあげる #98

Conversation

enm10k commented May 7, 2024 • edited Loading

変更内容

残作業

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enm10k commented May 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enm10k May 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enm10k commented May 10, 2024

melpon left a comment

Choose a reason for hiding this comment

enm10k commented May 13, 2024

enm10k commented May 7, 2024 •

edited

Loading

enm10k commented May 10, 2024 •

edited

Loading

enm10k May 10, 2024 •

edited

Loading