マルチ転送フィルターの追加 #108

torikizi · 2024-12-19T01:57:02Z

既存の転送フィルターに name / priority を追加
マルチ転送フィルターを追加

This pull request introduces several enhancements to the Sora class and related protobuf definitions, primarily focusing on the forwarding filter functionality. The changes add new fields to the ForwardingFilter class and introduce a new ForwardingFilters class to handle multiple filters.

Enhancements to forwarding filter functionality:

Sora/Sora.cs: Added Name and Priority fields to the ForwardingFilter class and created a new ForwardingFilters class to manage a list of ForwardingFilter objects. Updated the Config class to include an optional ForwardingFilters property. Enhanced the Connect method to handle the new fields and iterate through multiple filters if provided. [1] [2] [3] [4] [5]

Protobuf updates:

proto/sora_conf_internal.proto: Added name and priority fields to the ForwardingFilter message. Introduced a new ForwardingFilters message to encapsulate multiple ForwardingFilter objects. Updated the ConnectConfig message to include the new ForwardingFilters field. [1] [2] [3]

C++ implementation updates:

src/sora.cpp: Updated the DoConnect method to handle the new name and priority fields in ForwardingFilter. Added logic to process the new ForwardingFilters message, iterating over multiple filters and setting their respective fields. [1] [2]

melpon · 2024-12-19T08:48:16Z

Sora/Sora.cs

        public ForwardingFilter ForwardingFilter;
+        public ForwardingFilters? ForwardingFilters;


public List<ForwardingFilter> ForwardingFilters で良さそう

ご指摘のとおり修正しました。

melpon · 2024-12-19T08:49:18Z

Sora/Sora.cs

+            var ffs = new SoraConf.Internal.ForwardingFilters();
+            foreach (var filter in config.ForwardingFilters.Filters)
+            {
+                var ff = new SoraConf.Internal.ForwardingFilter();


このあたりのロジックは ForwardingFilter に対する処理と共通化できるはず

c55e160 で共通化しました。

melpon · 2024-12-19T08:51:14Z

proto/sora_conf_internal.proto

@@ -80,6 +86,7 @@ message ConnectConfig {
    string audio_streaming_language_code = 48;
    string signaling_notify_metadata = 49;
    optional ForwardingFilter forwarding_filter = 51;
+    optional ForwardingFilters forwarding_filters = 52;


optional repeated ForwardingFilter forwarding_filters とは書けないんでしたっけ。できないならそのままで良いです。

書けないようです。ビルドエラーとなりました。

melpon · 2024-12-19T08:52:03Z

src/sora.cpp

+    if (cc.has_forwarding_filters()) {
+      std::vector<sora::SoraSignalingConfig::ForwardingFilter> filters;
+      for (const auto& filter : cc.forwarding_filters.filters) {
+        sora::SoraSignalingConfig::ForwardingFilter ff;


このあたりのロジックは forwarding_filter に対する処理と共通化できるはず

c55e160 にて共通化しました。

melpon · 2024-12-20T07:07:31Z

Sora/Sora.cs

@@ -85,7 +86,10 @@ public class Rule
        public string? Version;
        public string? Metadata;
    }
-
+    public class ForwardingFilters


これはもう不要なはず

修正しました。

melpon · 2024-12-20T07:08:50Z

Sora/Sora.cs

@@ -522,6 +507,48 @@ public void Disconnect()
        sora_disconnect(p);
    }

+    private SoraConf.Internal.ForwardingFilter ConvertToInternalFilter(ForwardingFilter filter)


forwarding filter で１個の機能なので、ConvertToInternalForwardingFilter の方が良さそう

あとこの関数は static で良いはず

名前の変更と static へ変更しました

melpon · 2024-12-20T07:10:44Z

Sora/Sora.cs

+            cc.SetForwardingFilter(ConvertToInternalFilter(config.ForwardingFilter));
+        }
+
+        if (config.ForwardingFilters != null && config.ForwardingFilters.Count > 0)


config.ForwardingFilters.Count == 0 の時に空の SoraConf.Internal.ForwardingFilters を設定しないといけないので、config.ForwardingFilters.Count > 0 の条件は外した方が良いです。

ご指摘の通りに修正しました。

melpon · 2024-12-20T07:16:01Z

src/sora.cpp

@@ -761,6 +740,48 @@ bool Sora::InitADM(rtc::scoped_refptr<webrtc::AudioDeviceModule> adm,
  return true;
 }

+sora::SoraSignalingConfig::ForwardingFilter Sora::convertForwardingFilter(


convertForwardingFilter だと、sora::SoraSignalingConfig::ForwardingFilter を sora_conf::internal::ForwardingFilter に変換する関数に読めるので、convertToForwardingFilter の方が良さそう

全体的に関数名は UpperCamelCase で書いてるので ConvertToForwardingFilter にした方が良さそう

ConvertToForwardingFilter に修正しました

melpon · 2024-12-20T07:16:19Z

src/sora.h

@@ -80,6 +80,10 @@ class Sora : public std::enable_shared_from_this<Sora>,
  void* GetAndroidApplicationContext(void* env);
  static sora_conf::ErrorCode ToErrorCode(sora::SoraSignalingErrorCode ec);

+ private:
+  sora::SoraSignalingConfig::ForwardingFilter convertForwardingFilter(


この関数は static にできるはず

cpp の修正と合わせて static に変更しました。

melpon · 2024-12-20T11:52:56Z

Sora/Sora.cs

@@ -85,7 +86,7 @@ public class Rule
        public string? Version;
        public string? Metadata;
    }
-
+    public List<ForwardingFilter> Filters = new List<ForwardingFilter>();


これ要らないですよね…？

失礼しました。ご指摘の通り不要でした。

melpon

よさそう

torikizi added 9 commits December 17, 2024 19:26

転送フィルターを複数指定可能にし name / priority を追加

0a6b657

proto の順番に誤りがあったので修正

874c8fe

has_forwarding_filters() の内容修正

8292815

List を用意すれば良さそう

a515d82

name / priority を送信するように修正

dbeea61

マルチ転送フィルター用に設定を見直す

0d3cb34

public ForwardingFilters に修正して他のメンバも見直す

6aa7560

タイポ修正

219a3df

Internal を外す

5cf82fe

torikizi changed the title ~~[WIP~~ [WIP] マルチ転送フィルターの追加 Dec 19, 2024

シンプルな構成に見直し

16a605c

torikizi changed the title ~~[WIP] マルチ転送フィルターの追加~~ マルチ転送フィルターの追加 Dec 19, 2024

変更履歴を更新

3c9e699

torikizi requested review from melpon and miosakuma December 19, 2024 07:58

melpon reviewed Dec 19, 2024

View reviewed changes

転送フィルターの設定方法をまとめる

c55e160

melpon reviewed Dec 20, 2024

View reviewed changes

共通化して不要になった部分を整える

d88a3cd

melpon reviewed Dec 20, 2024

View reviewed changes

不要な定義が残っていた

0e8f309

melpon approved these changes Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

マルチ転送フィルターの追加 #108

マルチ転送フィルターの追加 #108

torikizi commented Dec 19, 2024 •

edited

Loading

melpon Dec 19, 2024

torikizi Dec 20, 2024

melpon Dec 19, 2024

torikizi Dec 20, 2024

melpon Dec 19, 2024

torikizi Dec 20, 2024

melpon Dec 19, 2024

torikizi Dec 20, 2024 •

edited

Loading

melpon Dec 20, 2024

torikizi Dec 20, 2024

melpon Dec 20, 2024

melpon Dec 20, 2024

torikizi Dec 20, 2024

melpon Dec 20, 2024

torikizi Dec 20, 2024

melpon Dec 20, 2024

torikizi Dec 20, 2024

melpon Dec 20, 2024

torikizi Dec 20, 2024

melpon Dec 20, 2024

torikizi Dec 20, 2024

melpon left a comment

		public ForwardingFilter ForwardingFilter;
		public ForwardingFilters? ForwardingFilters;

マルチ転送フィルターの追加 #108

Are you sure you want to change the base?

マルチ転送フィルターの追加 #108

Conversation

torikizi commented Dec 19, 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

torikizi Dec 20, 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

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

melpon left a comment

Choose a reason for hiding this comment

torikizi commented Dec 19, 2024 •

edited

Loading

torikizi Dec 20, 2024 •

edited

Loading