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

FCE-423, FCE-298: Propose updated way of configuring video quality in SDKs; Re-think default activeEncodings settings in SDK; FCE-1032: Fix stopCapture #281

Merged
merged 11 commits into from
Jan 15, 2025
Merged

FCE-423, FCE-298: Propose updated way of configuring video quality in SDKs; Re-think default activeEncodings settings in SDK; FCE-1032: Fix stopCapture #281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1314717
remove encoding from typescript
MiloszFilimowski Dec 20, 2024
9cb6cf4
remove ios config
MiloszFilimowski Dec 20, 2024
4b38d62
add fix todo
MiloszFilimowski Dec 20, 2024
7cbd506
remove android config
MiloszFilimowski Dec 20, 2024
0fc8b05
Fix thread issue
MiloszFilimowski Jan 7, 2025
8403865
update max bitrate settings
MiloszFilimowski Jan 13, 2025
8088bdf
fix ios stopCapture
MiloszFilimowski Jan 13, 2025
be24d31
WIP
MiloszFilimowski Jan 13, 2025
ee10165
Merge remote-tracking branch 'origin/main' into mfilimowski/FCE-423-u…
MiloszFilimowski Jan 14, 2025
c2a6f95
fix crash on stop capture
MiloszFilimowski Jan 14, 2025
04ca414
remove not needed call
MiloszFilimowski Jan 15, 2025
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
80 changes: 0 additions & 80 deletions examples/fishjam-chat/components/LetterButton.tsx
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions examples/fishjam-chat/components/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import Button from './Button';
import DismissKeyboard from './DismissKeyboard';
import InCallButton from './InCallButton';
import LetterButton from './LetterButton';
import NoCameraView from './NoCameraView';
import SoundOutputDevicesBottomSheet from './SoundOutputDevicesBottomSheet';
import TextInput from './TextInput';
Expand All @@ -11,7 +10,6 @@ export {
Button,
DismissKeyboard,
InCallButton,
LetterButton,
NoCameraView,
SoundOutputDevicesBottomSheet,
TextInput,
Expand Down
5 changes: 0 additions & 5 deletions packages/ios-client/Sources/FishjamClient/FishjamClientInternal.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,6 @@ class FishjamClientInternal {

func leave(onLeave: (() -> Void)? = nil) {
rtcEngineCommunication.disconnect()
for track in localEndpoint.tracks.values {
if let track = track as? LocalTrack {
track.stop()
}
}
peerConnectionManager.close()
localEndpoint = Endpoint(id: "")
remoteEndpointsMap = [:]
Expand Down
29 changes: 4 additions & 25 deletions packages/react-native-client/android/src/main/java/io/fishjam/reactnative/RNFishjamClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.fishjamcloud.client.models.RTCOutboundStats
import com.fishjamcloud.client.models.ReconnectConfig
import com.fishjamcloud.client.models.SimulcastConfig
import com.fishjamcloud.client.models.TrackBandwidthLimit
import com.fishjamcloud.client.models.TrackEncoding
import com.fishjamcloud.client.models.VideoParameters
import com.twilio.audioswitch.AudioDevice
import expo.modules.kotlin.AppContext
Expand Down Expand Up @@ -74,7 +75,6 @@ class RNFishjamClient(

var screenShareQuality: String? = null
var screenShareSimulcastConfig: SimulcastConfig = SimulcastConfig()
var screenShareMaxBandwidth: TrackBandwidthLimit = TrackBandwidthLimit.BandwidthLimit(0)

var screenShareMetadata: Map<String, Any> = mutableMapOf()

Expand Down Expand Up @@ -151,27 +151,13 @@ class RNFishjamClient(

private fun getSimulcastConfigFromOptions(simulcastConfigMap: io.fishjam.reactnative.SimulcastConfig): SimulcastConfig {
val simulcastEnabled = simulcastConfigMap.enabled
val activeEncodings = simulcastConfigMap.activeEncodings.map { e -> e.toTrackEncoding() }
val activeEncodings = listOf(TrackEncoding.L, TrackEncoding.M, TrackEncoding.H)
return SimulcastConfig(
enabled = simulcastEnabled,
activeEncodings = activeEncodings
)
}

private fun getMaxBandwidthFromOptions(
maxBandwidthMap: Map<String, Int>?,
maxBandwidthInt: Int
): TrackBandwidthLimit {
if (maxBandwidthMap != null) {
val maxBandwidthSimulcast = mutableMapOf<String, TrackBandwidthLimit.BandwidthLimit>()
maxBandwidthMap.forEach {
maxBandwidthSimulcast[it.key] = TrackBandwidthLimit.BandwidthLimit(it.value)
}
return TrackBandwidthLimit.SimulcastBandwidthLimit(maxBandwidthSimulcast)
}
return TrackBandwidthLimit.BandwidthLimit(maxBandwidthInt)
}

private fun create() {
audioSwitchManager = AudioSwitchManager(appContext?.reactContext!!)
fishjamClient =
Expand All @@ -182,8 +168,6 @@ class RNFishjamClient(
}

private fun getVideoParametersFromOptions(createOptions: CameraConfig): VideoParameters {
val videoMaxBandwidth =
getMaxBandwidthFromOptions(createOptions.maxBandwidthMap, createOptions.maxBandwidthInt)
var videoParameters =
when (createOptions.quality) {
"QVGA169" -> VideoParameters.presetQVGA169
Expand All @@ -202,7 +186,7 @@ class RNFishjamClient(
videoParameters.copy(
dimensions = if (createOptions.flipDimensions) videoParameters.dimensions.flip() else videoParameters.dimensions,
simulcastConfig = getSimulcastConfigFromOptions(createOptions.simulcastConfig),
maxBitrate = videoMaxBandwidth
maxBitrate = videoParameters.maxBitrate
)
return videoParameters
}
Expand Down Expand Up @@ -431,11 +415,6 @@ class RNFishjamClient(
this.screenShareQuality = screenShareOptions.quality
this.screenShareSimulcastConfig =
getSimulcastConfigFromOptions(screenShareOptions.simulcastConfig)
this.screenShareMaxBandwidth =
getMaxBandwidthFromOptions(
screenShareOptions.maxBandwidthMap,
screenShareOptions.maxBandwidthInt
)

if (!isScreenShareOn) {
ensureConnected()
Expand Down Expand Up @@ -769,7 +748,7 @@ class RNFishjamClient(
return videoParameters.copy(
dimensions = dimensions,
simulcastConfig = screenShareSimulcastConfig,
maxBitrate = screenShareMaxBandwidth
maxBitrate = videoParameters.maxBitrate
)
}

Expand Down
16 changes: 0 additions & 16 deletions ...react-native-client/android/src/main/java/io/fishjam/reactnative/RNFishjamClientModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ import kotlinx.coroutines.withContext
class SimulcastConfig : Record {
@Field
val enabled: Boolean = false

@Field
val activeEncodings: List<String> = emptyList()
}

class CameraConfig : Record {
Expand All @@ -34,13 +31,6 @@ class CameraConfig : Record {
@Field
val simulcastConfig: SimulcastConfig = SimulcastConfig()

// expo-modules on Android don't support Either type
@Field
val maxBandwidthMap: Map<String, Int> = emptyMap()

@Field
val maxBandwidthInt: Int = 0

@Field
val cameraEnabled: Boolean = true

Expand All @@ -57,12 +47,6 @@ class ScreenShareOptions : Record {

@Field
val simulcastConfig: SimulcastConfig = SimulcastConfig()

@Field
val maxBandwidthMap: Map<String, Int> = emptyMap()

@Field
val maxBandwidthInt: Int = 0
}

class ReconnectConfig : Record {
Expand Down
28 changes: 5 additions & 23 deletions packages/react-native-client/ios/RNFishjamClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class RNFishjamClient: FishjamClientListener {
var videoSimulcastConfig: SimulcastConfig = SimulcastConfig()

var screenShareSimulcastConfig: SimulcastConfig = SimulcastConfig()
var screenShareMaxBandwidth: TrackBandwidthLimit = .BandwidthLimit(0)

var cameraId: String? = nil

Expand Down Expand Up @@ -64,37 +63,24 @@ class RNFishjamClient: FishjamClientListener {
}

private func getSimulcastConfigFromOptions(simulcastConfig: RNSimulcastConfig) throws -> SimulcastConfig {
var activeEncodings: [TrackEncoding] = []

for encoding in simulcastConfig.activeEncodings {
if let fishjamEncoding = try? TrackEncoding(encoding) {
activeEncodings.append(fishjamEncoding)
}
}
// iOS has a limit of 3 hardware encoders
// 3 simulcast layers + 1 screen share layer = 4, which is too much
Comment on lines +66 to +67
Copy link
Member

Choose a reason for hiding this comment

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

I was just thinking - do we still know that this limit applies on latest iOS version? If not, maybe we should investigate it again 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created an issue!
https://linear.app/swmansion/issue/FCE-1033

// so we limit simulcast layers to 2
let activeEncodings: [TrackEncoding] = [.l, .h]

return SimulcastConfig(
enabled: simulcastConfig.enabled,
activeEncodings: activeEncodings
)
}

private func getMaxBandwidthFromOptions(maxBandwidth: RNTrackBandwidthLimit) -> TrackBandwidthLimit {
if let maxBandwidth: Int = maxBandwidth.get() {
return .BandwidthLimit(maxBandwidth)
} else if let maxBandwidth: [String: Int] = maxBandwidth.get() {
return .SimulcastBandwidthLimit(maxBandwidth)
}
return .BandwidthLimit(0)
}

func create() {
RNFishjamClient.fishjamClient = FishjamClient(listener: self)
}

func getVideoParametersFromOptions(connectionOptions: CameraConfig) throws -> VideoParameters {
let videoQuality = connectionOptions.quality
let flipDimensions = connectionOptions.flipDimensions
let videoBandwidthLimit = getMaxBandwidthFromOptions(maxBandwidth: connectionOptions.maxBandwidth)
let simulcastConfig = try getSimulcastConfigFromOptions(simulcastConfig: connectionOptions.simulcastConfig)

let preset: VideoParameters = {
Expand Down Expand Up @@ -125,7 +111,7 @@ class RNFishjamClient: FishjamClientListener {
}()
let videoParameters = VideoParameters(
dimensions: flipDimensions ? preset.dimensions.flipped : preset.dimensions,
maxBandwidth: videoBandwidthLimit,
maxBandwidth: preset.maxBandwidth,
simulcastConfig: simulcastConfig
)
return videoParameters
Expand Down Expand Up @@ -398,8 +384,6 @@ class RNFishjamClient: FishjamClientListener {
let simulcastConfig = try getSimulcastConfigFromOptions(simulcastConfig: screenShareOptions.simulcastConfig)

screenShareSimulcastConfig = simulcastConfig
screenShareMaxBandwidth = getMaxBandwidthFromOptions(
maxBandwidth: screenShareOptions.maxBandwidth)
let screenShareMetadata = screenShareOptions.screenShareMetadata.toMetadata()
let videoParameters = getScreenShareVideoParameters(options: screenShareOptions)
RNFishjamClient.fishjamClient!.prepareForScreenBroadcast(
Expand Down Expand Up @@ -453,7 +437,6 @@ class RNFishjamClient: FishjamClientListener {
let simulcastConfig = try getSimulcastConfigFromOptions(simulcastConfig: screenShareOptions.simulcastConfig)

screenShareSimulcastConfig = simulcastConfig
screenShareMaxBandwidth = getMaxBandwidthFromOptions(maxBandwidth: screenShareOptions.maxBandwidth)
let metadata = screenShareOptions.screenShareMetadata.toMetadata()
let videoParameters = getScreenShareVideoParameters(options: screenShareOptions)
let screenAppTrack = RNFishjamClient.fishjamClient!.createScreenAppTrack(
Expand Down Expand Up @@ -762,7 +745,6 @@ class RNFishjamClient: FishjamClientListener {
}
return VideoParameters(
dimensions: preset.dimensions.flipped,
maxBandwidth: screenShareMaxBandwidth,
maxFps: preset.maxFps,
simulcastConfig: screenShareSimulcastConfig
)
Expand Down
9 changes: 0 additions & 9 deletions packages/react-native-client/ios/RNFishjamClientModule.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ import ExpoModulesCore
struct RNSimulcastConfig: Record {
@Field
var enabled: Bool = false

@Field
var activeEncodings: [String] = []
}

struct CameraConfig: Record {
Expand All @@ -21,9 +18,6 @@ struct CameraConfig: Record {
@Field
var simulcastConfig: RNSimulcastConfig = RNSimulcastConfig()

@Field
var maxBandwidth: RNTrackBandwidthLimit = RNTrackBandwidthLimit(0)

@Field
var cameraEnabled: Bool = true

Expand All @@ -48,9 +42,6 @@ struct ScreenShareOptions: Record {

@Field
var simulcastConfig: RNSimulcastConfig = RNSimulcastConfig()

@Field
var maxBandwidth: RNTrackBandwidthLimit = RNTrackBandwidthLimit(0)
}

struct ReconnectConfig: Record {
Expand Down
21 changes: 0 additions & 21 deletions packages/react-native-client/src/common/webRTC.ts
View file
Open in desktop

This file was deleted.

8 changes: 4 additions & 4 deletions packages/react-native-client/src/hooks/useAppScreenShare.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { ScreenShareOptions } from './useScreenShare';
import { Platform } from 'react-native';
import { useFishjamEventState } from './internal/useFishjamEventState';

const defaultSimulcastConfig = () => ({
enabled: false,
activeEncodings: [],
});
const defaultSimulcastConfig = () =>
({
enabled: false,
}) satisfies SimulcastConfig;
Comment on lines +10 to +13
Copy link
Member

Choose a reason for hiding this comment

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

There is philosophical question: should satisfies be used, or just type variable as () => SimulcastConfig 🤔

(in the end, it does not really matter here)

Copy link
Collaborator Author

@MiloszFilimowski MiloszFilimowski Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

In this case, it doesn't really matter. The advantage of using satisfies instead of a type is that you preserve the actual value.
So if you type is

type Something = { something: string}

and you create an object

const test = { something: 'test'} satisfies Something

You still retail the autocompletion when using test.something as "test", whereas with type annotation it's just string.

So in our case the user would now that the default value is always false.


let screenShareSimulcastConfig: SimulcastConfig = defaultSimulcastConfig();

Expand Down
Loading
Loading