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

Jump to bottom

offer メッセージの simulcast を利用する #129

Merged
merged 3 commits into from
Aug 15, 2024
Merged

offer メッセージの simulcast を利用する #129

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45d97a6
offer メッセージの Simulcast を利用する
miosakuma Jul 18, 2024
32a0522
変更履歴を更新する
miosakuma Aug 6, 2024
2a84da6
変更履歴の文章を見直す
miosakuma Aug 6, 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
1 change: 1 addition & 0 deletions sora-android-sdk/src/main/kotlin/jp/shiguredo/sora/sdk/channel/SoraMediaChannel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,7 @@ class SoraMediaChannel @JvmOverloads constructor(
mediaOption = mediaOption
),
mediaOption = mediaOption,
simulcastEnabled = offerMessage.simulcast,
dataChannelConfigs = offerMessage.dataChannels,
listener = peerListener
)
Expand Down
7 changes: 4 additions & 3 deletions sora-android-sdk/src/main/kotlin/jp/shiguredo/sora/sdk/channel/rtc/PeerChannel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ class PeerChannelImpl(
private val appContext: Context,
private val networkConfig: PeerNetworkConfig,
private val mediaOption: SoraMediaOption,
private val simulcastEnabled: Boolean = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

この simulcastEnabled が offer message から入ってきた値を表すのであれば、offerSimulcastEnabled という命名の方が良さそうに思いましたが、どうでしょうか？

offer message であることを意識した命名は、他の箇所にもあったのでリンク貼っておきます。

sora-android-sdk/sora-android-sdk/src/main/kotlin/jp/shiguredo/sora/sdk/channel/SoraMediaChannel.kt

Lines 107 to 109 in 8360fc7

// DataChannel 経由のシグナリングが有効なら true
// Sora から渡された値 (= offer メッセージ) を参照して更新している
private var offerDataChannelSignaling: Boolean = false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PeerChannel クラスは offer かどうかを意識する必要がないためこの名称にしています。
offer の値を渡すかどうかを判断するのは SoraMediaChannel クラスであるべきという考えです。

PeerChannel は以下リンクのように各所から必要な時に用途に応じて呼び出せるものだと考えています。

sora-android-sdk/sora-android-sdk/src/main/kotlin/jp/shiguredo/sora/sdk/channel/SoraMediaChannel.kt

Lines 720 to 728 in 45d97a6

val clientOfferPeer = PeerChannelImpl(
appContext = context,
networkConfig = PeerNetworkConfig(
serverConfig = OfferConfig(
iceServers = emptyList(),
iceTransportPolicy = ""
),
mediaOption = mediaOption
),

offerDataChannelSignaling は SoraMediaChannel 内の変数なので名称を分けるのが正しいと思います。

Copy link
Contributor

Choose a reason for hiding this comment

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

ありがとうございます。変数の命名についての考え方は理解できました。

ちょっと上のコメントの補足をすると
僕が当初、「この simulcastEnabled が offer message から入ってきた値を表すのであれば」と書いたのは、この変数が offer message に含まれている simulcast の値以外が入ることを期待していないのではないか、と思ったからでした。

dataChannelConfigs: List<Map<String, Any>>? = null,
private var listener: PeerChannel.Listener?,
private var useTracer: Boolean = false
Expand All @@ -104,7 +105,7 @@ class PeerChannelImpl(
}
}

private val componentFactory = RTCComponentFactory(mediaOption, listener)
private val componentFactory = RTCComponentFactory(mediaOption, simulcastEnabled, listener)

private var conn: PeerConnection? = null

Expand Down Expand Up @@ -309,7 +310,7 @@ class PeerChannelImpl(

return setRemoteDescription(offerSDP).flatMap {
// active: false が無効化されてしまう問題に対応
if (mediaOption.simulcastEnabled && mediaOption.videoUpstreamEnabled) {
if (simulcastEnabled && mediaOption.videoUpstreamEnabled) {
videoSender?.let { updateSenderOfferEncodings(it) }
}
return@flatMap createAnswer()
Expand Down Expand Up @@ -359,7 +360,7 @@ class PeerChannelImpl(
}
} ?: SoraLogger.d(TAG, "mid for video not found")

if (mediaOption.simulcastEnabled && mediaOption.videoUpstreamEnabled) {
if (simulcastEnabled && mediaOption.videoUpstreamEnabled) {
videoSender?.let { updateSenderOfferEncodings(it) }
}

Expand Down
3 changes: 2 additions & 1 deletion sora-android-sdk/src/main/kotlin/jp/shiguredo/sora/sdk/channel/rtc/RTCComponentFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import org.webrtc.audio.JavaAudioDeviceModule

class RTCComponentFactory(
private val mediaOption: SoraMediaOption,
private val simulcastEnabled: Boolean,
private val listener: PeerChannel.Listener?
) {
companion object {
Expand All @@ -36,7 +37,7 @@ class RTCComponentFactory(
val encoderFactory = when {
mediaOption.videoEncoderFactory != null ->
mediaOption.videoEncoderFactory!!
mediaOption.simulcastEnabled ->
simulcastEnabled ->
SimulcastVideoEncoderFactoryWrapper(
mediaOption.videoUpstreamContext,
resolutionAdjustment = mediaOption.hardwareVideoEncoderResolutionAdjustment,
Expand Down
1 change: 1 addition & 0 deletions sora-android-sdk/src/main/kotlin/jp/shiguredo/sora/sdk/channel/signaling/message/Catalog.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ data class OfferMessage(
@SerializedName("client_id") val clientId: String,
@SerializedName("bundle_id") val bundleId: String? = null,
@SerializedName("connection_id") val connectionId: String,
@SerializedName("simulcast") val simulcast: Boolean = false,
@SerializedName("metadata") val metadata: Any?,
@SerializedName("config") val config: OfferConfig? = null,
@SerializedName("mid") val mid: Map<String, String>? = null,
Expand Down