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.

Sign up for GitHub

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

libwebrtc を m124.6367.3.1 にあげる #98

Merged
merged 17 commits into from
May 13, 2024
Merged

libwebrtc を m124.6367.3.1 にあげる #98

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
90207d6
libwebrtc を m124.6367.3.1 にあげる
enm10k May 7, 2024
4861e13
webrtc::VideoDecoderFactory 初期化時に webrtc::Environment の指定が必要になった
enm10k May 7, 2024
01ad4ff
test のビルドが通るように修正する
enm10k May 7, 2024
ba5bb61
sora::SoraClientContext::Create に webrtc::Environment を渡す
enm10k May 7, 2024
78843b9
webrtc::Environment を渡さない初期化方法を維持する
enm10k May 7, 2024
cac00ea
libwebrtc から examples/unityplugin が削除されたので追従する
enm10k May 8, 2024
01f5ba6
試行錯誤した際のコメントを消す
enm10k May 8, 2024
7e8622b
SoraClientContext で保持する webrtc::Environment は private で良かった
enm10k May 8, 2024
3e789b1
SoraClientContext で webrtc::Environment を保持するのを止める
enm10k May 10, 2024
ba0da74
互換性を維持するために webrtc::Environment を最後の変数として追加し、デフォルト値を指定する
enm10k May 10, 2024
9d5eb9b
最終的に webrtc::CreateEnvironment を実行する回数が変わらないので、 webrtc::Environment を…
enm10k May 10, 2024
ecec7db
不要な差分を元に戻す
enm10k May 10, 2024
d95ab15
CHANGES.md を更新する
enm10k May 10, 2024
2ae98b7
参照でキャプチャーするのをやめる
enm10k May 10, 2024
fdb7947
CHANGES.md を修正する
enm10k May 10, 2024
7a21aa4
Merge remote-tracking branch 'origin/develop' into feature/update-lib…
enm10k May 10, 2024
35b1eb6
yaml のエラーを直す
melpon May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion VERSION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
SORA_CPP_SDK_VERSION=2024.6.1
WEBRTC_BUILD_VERSION=m123.6312.3.5
WEBRTC_BUILD_VERSION=m124.6367.3.1
BOOST_VERSION=1.85.0
CMAKE_VERSION=3.28.1
CUDA_VERSION=11.8.0-1
Expand Down
4 changes: 2 additions & 2 deletions examples/VERSION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
SORA_CPP_SDK_VERSION=2024.6.1
WEBRTC_BUILD_VERSION=m123.6312.3.5
BOOST_VERSION=1.85.0
WEBRTC_BUILD_VERSION=m124.6367.3.1
BOOST_VERSION=1.84.0
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@enm10k enm10k May 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

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

Copy link
Member

Choose a reason for hiding this comment

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

1fa19e0

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

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

CMAKE_VERSION=3.28.1
SDL2_VERSION=2.30.3
CLI11_VERSION=v2.4.2
6 changes: 6 additions & 0 deletions include/sora/sora_client_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define SORA_SORA_CLIENT_CONTEXT_H_

// WebRTC
#include <api/environment/environment_factory.h>
#include <api/peer_connection_interface.h>
#include <media/engine/webrtc_media_engine.h>
#include <pc/connection_context.h>
Expand Down Expand Up @@ -55,6 +56,10 @@ struct SoraClientContextConfig {
// auto client = std::make_shared<MyClient>(context);
class SoraClientContext {
public:
static std::shared_ptr<SoraClientContext> Create(
const SoraClientContextConfig& config,
webrtc::Environment& env);

Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

static std::shared_ptr<SoraClientContext> Create(
const SoraClientContextConfig& config);

Expand All @@ -76,6 +81,7 @@ class SoraClientContext {
? config_.get_android_application_context(env)
: nullptr;
}
std::shared_ptr<webrtc::Environment> env;

private:
SoraClientContextConfig config_;
Expand Down
9 changes: 6 additions & 3 deletions include/sora/sora_video_decoder_factory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <vector>

// WebRTC
#include <api/environment/environment.h>
#include <api/video/video_codec_type.h>
#include <api/video_codecs/video_decoder_factory.h>

Expand Down Expand Up @@ -52,7 +53,8 @@ class SoraVideoDecoderFactory : public webrtc::VideoDecoderFactory {

std::vector<webrtc::SdpVideoFormat> GetSupportedFormats() const override;

std::unique_ptr<webrtc::VideoDecoder> CreateVideoDecoder(
std::unique_ptr<webrtc::VideoDecoder> Create(
const webrtc::Environment& env,
const webrtc::SdpVideoFormat& format) override;

private:
Expand All @@ -62,10 +64,11 @@ class SoraVideoDecoderFactory : public webrtc::VideoDecoderFactory {

// ハードウェアデコーダを出来るだけ使おうとして、見つからなければソフトウェアデコーダを使う設定を返す
SoraVideoDecoderFactoryConfig GetDefaultVideoDecoderFactoryConfig(
webrtc::Environment& env,
std::shared_ptr<CudaContext> cuda_context = nullptr,
void* env = nullptr);
void* jni_env = nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

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

// ソフトウェアデコーダのみを使う設定を返す
SoraVideoDecoderFactoryConfig GetSoftwareOnlyVideoDecoderFactoryConfig();
SoraVideoDecoderFactoryConfig GetSoftwareOnlyVideoDecoderFactoryConfig(webrtc::Environment& env);

} // namespace sora

Expand Down
23 changes: 15 additions & 8 deletions src/sora_client_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include <api/audio_codecs/builtin_audio_encoder_factory.h>
#include <api/create_peerconnection_factory.h>
#include <api/enable_media.h>
#include <api/environment/environment_factory.h>
#include <api/rtc_event_log/rtc_event_log_factory.h>
#include <api/task_queue/default_task_queue_factory.h>
#include <call/call_config.h>
Expand Down Expand Up @@ -41,7 +40,8 @@ SoraClientContext::~SoraClientContext() {
}

std::shared_ptr<SoraClientContext> SoraClientContext::Create(
const SoraClientContextConfig& config) {
const SoraClientContextConfig& config,
webrtc::Environment& env) {
rtc::InitializeSSL();

std::shared_ptr<SoraClientContext> c = std::make_shared<SoraClientContext>();
Expand All @@ -53,6 +53,7 @@ std::shared_ptr<SoraClientContext> SoraClientContext::Create(
c->worker_thread_->Start();
c->signaling_thread_ = rtc::Thread::Create();
c->signaling_thread_->Start();
c->env = std::make_shared<webrtc::Environment>(env);

webrtc::PeerConnectionFactoryDependencies dependencies;
dependencies.network_thread = c->network_thread_.get();
Expand All @@ -63,7 +64,7 @@ std::shared_ptr<SoraClientContext> SoraClientContext::Create(
absl::make_unique<webrtc::RtcEventLogFactory>(
dependencies.task_queue_factory.get());

void* env = sora::GetJNIEnv();
void* jni_env = sora::GetJNIEnv();

dependencies.adm = c->worker_thread_->BlockingCall([&] {
sora::AudioDeviceModuleConfig config;
Expand Down Expand Up @@ -92,7 +93,7 @@ std::shared_ptr<SoraClientContext> SoraClientContext::Create(
{
auto config = c->config_.use_hardware_encoder
? sora::GetDefaultVideoEncoderFactoryConfig(
cuda_context, env, c->config_.openh264)
cuda_context, jni_env, c->config_.openh264)
: sora::GetSoftwareOnlyVideoEncoderFactoryConfig(
c->config_.openh264);
config.use_simulcast_adapter = true;
Expand All @@ -102,10 +103,10 @@ std::shared_ptr<SoraClientContext> SoraClientContext::Create(
absl::make_unique<sora::SoraVideoEncoderFactory>(std::move(config));
}
{
auto config =
c->config_.use_hardware_encoder
? sora::GetDefaultVideoDecoderFactoryConfig(cuda_context, env)
: sora::GetSoftwareOnlyVideoDecoderFactoryConfig();
auto config = c->config_.use_hardware_encoder
? sora::GetDefaultVideoDecoderFactoryConfig(
env, cuda_context, jni_env)
: sora::GetSoftwareOnlyVideoDecoderFactoryConfig(env);
dependencies.video_decoder_factory =
absl::make_unique<sora::SoraVideoDecoderFactory>(std::move(config));
}
Expand Down Expand Up @@ -136,4 +137,10 @@ std::shared_ptr<SoraClientContext> SoraClientContext::Create(
return c;
}

std::shared_ptr<SoraClientContext> SoraClientContext::Create(
const SoraClientContextConfig& config) {
auto env = webrtc::CreateEnvironment();
return Create(config, env);
}

} // namespace sora
1 change: 1 addition & 0 deletions src/sora_peer_connection_factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class PeerConnectionFactoryWithContext : public webrtc::PeerConnectionFactory {
PeerConnectionFactoryWithContext(
webrtc::PeerConnectionFactoryDependencies dependencies)
: PeerConnectionFactoryWithContext(
// TODO(enm10k): environment を外部から設定できるようにする
webrtc::ConnectionContext::Create(webrtc::CreateEnvironment(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

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

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

上記の部分について補足をすると、現在の 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

&dependencies),
&dependencies) {}
Expand Down
27 changes: 15 additions & 12 deletions src/sora_video_decoder_factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

// WebRTC
#include <absl/strings/match.h>
#include <api/environment/environment.h>
#include <api/video_codecs/sdp_video_format.h>
#include <media/base/codec.h>
#include <media/base/media_constants.h>
Expand Down Expand Up @@ -67,8 +68,8 @@ SoraVideoDecoderFactory::GetSupportedFormats() const {
return r;
}

std::unique_ptr<webrtc::VideoDecoder>
SoraVideoDecoderFactory::CreateVideoDecoder(
std::unique_ptr<webrtc::VideoDecoder> SoraVideoDecoderFactory::Create(
const webrtc::Environment& env,
const webrtc::SdpVideoFormat& format) {
webrtc::VideoCodecType specified_codec =
webrtc::PayloadStringToCodecType(format.name);
Expand All @@ -84,10 +85,10 @@ SoraVideoDecoderFactory::CreateVideoDecoder(
std::vector<webrtc::SdpVideoFormat> supported_formats = formats_[n++];

if (enc.factory != nullptr) {
create_video_decoder =
[factory = enc.factory.get()](const webrtc::SdpVideoFormat& format) {
return factory->CreateVideoDecoder(format);
};
create_video_decoder = [factory = enc.factory.get(),
&env](const webrtc::SdpVideoFormat& format) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2ae98b7 で修正しました

return factory->Create(env, format);
};
} else if (enc.create_video_decoder != nullptr) {
create_video_decoder = enc.create_video_decoder;
}
Expand All @@ -108,20 +109,21 @@ SoraVideoDecoderFactory::CreateVideoDecoder(
}

SoraVideoDecoderFactoryConfig GetDefaultVideoDecoderFactoryConfig(
webrtc::Environment& env,
std::shared_ptr<CudaContext> cuda_context,
void* env) {
auto config = GetSoftwareOnlyVideoDecoderFactoryConfig();
void* jni_env) {
auto config = GetSoftwareOnlyVideoDecoderFactoryConfig(env);

#if defined(__APPLE__)
config.decoders.insert(config.decoders.begin(),
VideoDecoderConfig(CreateMacVideoDecoderFactory()));
#endif

#if defined(SORA_CPP_SDK_ANDROID)
if (env != nullptr) {
if (jni_env != nullptr) {
config.decoders.insert(config.decoders.begin(),
VideoDecoderConfig(CreateAndroidVideoDecoderFactory(
static_cast<JNIEnv*>(env))));
static_cast<JNIEnv*>(jni_env))));
}
#endif

Expand Down Expand Up @@ -249,11 +251,12 @@ SoraVideoDecoderFactoryConfig GetDefaultVideoDecoderFactoryConfig(
return config;
}

SoraVideoDecoderFactoryConfig GetSoftwareOnlyVideoDecoderFactoryConfig() {
SoraVideoDecoderFactoryConfig GetSoftwareOnlyVideoDecoderFactoryConfig(
webrtc::Environment& env) {
SoraVideoDecoderFactoryConfig config;
config.decoders.push_back(VideoDecoderConfig(
webrtc::kVideoCodecVP8,
[](auto format) { return webrtc::VP8Decoder::Create(); }));
[&env](auto format) { return webrtc::CreateVp8Decoder(env); }));
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2ae98b7 で修正しました

config.decoders.push_back(VideoDecoderConfig(
webrtc::kVideoCodecVP9,
[](auto format) { return webrtc::VP9Decoder::Create(); }));
Expand Down
1 change: 0 additions & 1 deletion test/android/app/src/main/cpp/jni_onload.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#undef JNIEXPORT
#define JNIEXPORT __attribute__((visibility("default")))

#include "examples/unityplugin/class_reference_holder.h"
#include "rtc_base/logging.h"
#include "rtc_base/ssl_adapter.h"
#include "sdk/android/native_api/jni/class_loader.h"
Expand Down