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.

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

Integrate new video track switch off interface #208

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 16 additions & 16 deletions VideoApp/Video-InternalTests/Mocks/MockAppSettingsStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -128,25 +128,25 @@ class MockAppSettingsStore: AppSettingsStoreWriting {
}
}

var invokedAreInsightsEnabledSetter = false
var invokedAreInsightsEnabledSetterCount = 0
var invokedAreInsightsEnabled: Bool?
var invokedAreInsightsEnabledList = [Bool]()
var invokedAreInsightsEnabledGetter = false
var invokedAreInsightsEnabledGetterCount = 0
var stubbedAreInsightsEnabled: Bool! = false

var areInsightsEnabled: Bool {
var invokedIsInsightsEnabledSetter = false
var invokedIsInsightsEnabledSetterCount = 0
var invokedIsInsightsEnabled: Bool?
var invokedIsInsightsEnabledList = [Bool]()
var invokedIsInsightsEnabledGetter = false
var invokedIsInsightsEnabledGetterCount = 0
var stubbedIsInsightsEnabled: Bool! = false

var isInsightsEnabled: Bool {
set {
invokedAreInsightsEnabledSetter = true
invokedAreInsightsEnabledSetterCount += 1
invokedAreInsightsEnabled = newValue
invokedAreInsightsEnabledList.append(newValue)
invokedIsInsightsEnabledSetter = true
invokedIsInsightsEnabledSetterCount += 1
invokedIsInsightsEnabled = newValue
invokedIsInsightsEnabledList.append(newValue)
}
get {
invokedAreInsightsEnabledGetter = true
invokedAreInsightsEnabledGetterCount += 1
return stubbedAreInsightsEnabled
invokedIsInsightsEnabledGetter = true
invokedIsInsightsEnabledGetterCount += 1
return stubbedIsInsightsEnabled
}
}

Expand Down
50 changes: 25 additions & 25 deletions VideoApp/VideoApp.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@
DC5CC39F249C0F5900355CC6 /* TestSecrets.json in Resources */ = {isa = PBXBuildFile; fileRef = DC9A4C9123D161A200D37CEC /* TestSecrets.json */; };
DC5CC3A0249C0F5900355CC6 /* TestSecrets.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC0446A2239716860072F597 /* TestSecrets.swift */; };
DC5CC3A1249C0F5900355CC6 /* TestSecretsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC0446A0239716530072F597 /* TestSecretsStore.swift */; };
DC674A31285D31190010A15B /* TwilioVideo in Frameworks */ = {isa = PBXBuildFile; productRef = DC674A30285D31190010A15B /* TwilioVideo */; };
DC674A33285D31580010A15B /* TwilioVideo in Frameworks */ = {isa = PBXBuildFile; productRef = DC674A32285D31580010A15B /* TwilioVideo */; };
DC699C7C282DB045009D3C1A /* FirebaseAuth in Frameworks */ = {isa = PBXBuildFile; productRef = DC699C7B282DB045009D3C1A /* FirebaseAuth */; };
DC699C7E282DB045009D3C1A /* FirebaseCrashlytics in Frameworks */ = {isa = PBXBuildFile; productRef = DC699C7D282DB045009D3C1A /* FirebaseCrashlytics */; };
DC699C80282DB07D009D3C1A /* FirebaseAnalyticsSwift in Frameworks */ = {isa = PBXBuildFile; productRef = DC699C7F282DB07D009D3C1A /* FirebaseAnalyticsSwift */; };
Expand Down Expand Up @@ -257,8 +259,6 @@
DCB3BAD8282DA2C400AF7072 /* AppCenterDistribute in Frameworks */ = {isa = PBXBuildFile; productRef = DCB3BAD7282DA2C400AF7072 /* AppCenterDistribute */; };
DCB3BADB282DA30B00AF7072 /* KeychainAccess in Frameworks */ = {isa = PBXBuildFile; productRef = DCB3BADA282DA30B00AF7072 /* KeychainAccess */; };
DCB3BADD282DA31400AF7072 /* KeychainAccess in Frameworks */ = {isa = PBXBuildFile; productRef = DCB3BADC282DA31400AF7072 /* KeychainAccess */; };
DCB3BAE0282DA34400AF7072 /* TwilioVideo in Frameworks */ = {isa = PBXBuildFile; productRef = DCB3BADF282DA34400AF7072 /* TwilioVideo */; };
DCB3BAE2282DA35100AF7072 /* TwilioVideo in Frameworks */ = {isa = PBXBuildFile; productRef = DCB3BAE1282DA35100AF7072 /* TwilioVideo */; };
DCB64B502409976F00E090BE /* APIRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = DCB64B4E2409976F00E090BE /* APIRequest.swift */; };
DCB64B512409976F00E090BE /* APIRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = DCB64B4E2409976F00E090BE /* APIRequest.swift */; };
DCB64B562409BC4E00E090BE /* APIConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = DCB64B542409BC4E00E090BE /* APIConfig.swift */; };
Expand Down Expand Up @@ -681,11 +681,11 @@
files = (
DCB3BAD6282DA2B300AF7072 /* AppCenterDistribute in Frameworks */,
DC699C87282DB4E9009D3C1A /* GoogleSignIn in Frameworks */,
DC674A31285D31190010A15B /* TwilioVideo in Frameworks */,
DC699C97282EB231009D3C1A /* FirebaseAnalytics in Frameworks */,
DC699C7C282DB045009D3C1A /* FirebaseAuth in Frameworks */,
DCB3BAD0282DA20000AF7072 /* Alamofire in Frameworks */,
DC699C7E282DB045009D3C1A /* FirebaseCrashlytics in Frameworks */,
DCB3BAE0282DA34400AF7072 /* TwilioVideo in Frameworks */,
DCB3BADB282DA30B00AF7072 /* KeychainAccess in Frameworks */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand All @@ -696,8 +696,8 @@
files = (
DCB3BAD8282DA2C400AF7072 /* AppCenterDistribute in Frameworks */,
DC699C89282DB536009D3C1A /* GoogleSignIn in Frameworks */,
DC674A33285D31580010A15B /* TwilioVideo in Frameworks */,
DCB3BAD3282DA23200AF7072 /* Alamofire in Frameworks */,
DCB3BAE2282DA35100AF7072 /* TwilioVideo in Frameworks */,
DC699C82282DB084009D3C1A /* FirebaseCrashlytics in Frameworks */,
DCB3BADD282DA31400AF7072 /* KeychainAccess in Frameworks */,
DC699C84282DB08B009D3C1A /* FirebaseAuth in Frameworks */,
Expand Down Expand Up @@ -1577,11 +1577,11 @@
DCB3BACF282DA20000AF7072 /* Alamofire */,
DCB3BAD5282DA2B300AF7072 /* AppCenterDistribute */,
DCB3BADA282DA30B00AF7072 /* KeychainAccess */,
DCB3BADF282DA34400AF7072 /* TwilioVideo */,
DC699C7B282DB045009D3C1A /* FirebaseAuth */,
DC699C7D282DB045009D3C1A /* FirebaseCrashlytics */,
DC699C86282DB4E9009D3C1A /* GoogleSignIn */,
DC699C96282EB231009D3C1A /* FirebaseAnalytics */,
DC674A30285D31190010A15B /* TwilioVideo */,
);
productName = VideoApp;
productReference = 4B00124E1FBA52C4004A587E /* Video-Internal.app */;
Expand All @@ -1604,11 +1604,11 @@
DCB3BAD2282DA23200AF7072 /* Alamofire */,
DCB3BAD7282DA2C400AF7072 /* AppCenterDistribute */,
DCB3BADC282DA31400AF7072 /* KeychainAccess */,
DCB3BAE1282DA35100AF7072 /* TwilioVideo */,
DC699C7F282DB07D009D3C1A /* FirebaseAnalyticsSwift */,
DC699C81282DB084009D3C1A /* FirebaseCrashlytics */,
DC699C83282DB08B009D3C1A /* FirebaseAuth */,
DC699C88282DB536009D3C1A /* GoogleSignIn */,
DC674A32285D31580010A15B /* TwilioVideo */,
);
productName = VideoApp;
productReference = 4B00128B1FBA52E5004A587E /* Video-Community.app */;
Expand Down Expand Up @@ -1732,11 +1732,11 @@
DCB3BACE282DA20000AF7072 /* XCRemoteSwiftPackageReference "Alamofire" */,
DCB3BAD4282DA2B300AF7072 /* XCRemoteSwiftPackageReference "appcenter-sdk-apple" */,
DCB3BAD9282DA30B00AF7072 /* XCRemoteSwiftPackageReference "KeychainAccess" */,
DCB3BADE282DA34400AF7072 /* XCRemoteSwiftPackageReference "twilio-video-ios" */,
DC699C78282DB045009D3C1A /* XCRemoteSwiftPackageReference "firebase-ios-sdk" */,
DC699C85282DB4E9009D3C1A /* XCRemoteSwiftPackageReference "GoogleSignIn-iOS" */,
DC699C8A282EAD46009D3C1A /* XCRemoteSwiftPackageReference "Quick" */,
DC699C8F282EAE21009D3C1A /* XCRemoteSwiftPackageReference "Nimble" */,
DC674A2F285D31190010A15B /* XCRemoteSwiftPackageReference "twilio-video-ios-internal-package" */,
);
productRefGroup = 241282381E36A6AB002198BE /* Products */;
projectDirPath = "";
Expand Down Expand Up @@ -2762,6 +2762,14 @@
/* End XCConfigurationList section */

/* Begin XCRemoteSwiftPackageReference section */
DC674A2F285D31190010A15B /* XCRemoteSwiftPackageReference "twilio-video-ios-internal-package" */ = {
isa = XCRemoteSwiftPackageReference;
repositoryURL = "[email protected]:twilio/twilio-video-ios-internal-package.git";
requirement = {
kind = exactVersion;
version = "6.0.0-rc3";
};
};
DC699C78282DB045009D3C1A /* XCRemoteSwiftPackageReference "firebase-ios-sdk" */ = {
isa = XCRemoteSwiftPackageReference;
repositoryURL = "https://github.com/firebase/firebase-ios-sdk.git";
Expand Down Expand Up @@ -2818,17 +2826,19 @@
minimumVersion = 4.0.0;
};
};
DCB3BADE282DA34400AF7072 /* XCRemoteSwiftPackageReference "twilio-video-ios" */ = {
isa = XCRemoteSwiftPackageReference;
repositoryURL = "https://github.com/twilio/twilio-video-ios";
requirement = {
kind = upToNextMajorVersion;
minimumVersion = 5.0.0;
};
};
/* End XCRemoteSwiftPackageReference section */

/* Begin XCSwiftPackageProductDependency section */
DC674A30285D31190010A15B /* TwilioVideo */ = {
isa = XCSwiftPackageProductDependency;
package = DC674A2F285D31190010A15B /* XCRemoteSwiftPackageReference "twilio-video-ios-internal-package" */;
productName = TwilioVideo;
};
DC674A32285D31580010A15B /* TwilioVideo */ = {
isa = XCSwiftPackageProductDependency;
package = DC674A2F285D31190010A15B /* XCRemoteSwiftPackageReference "twilio-video-ios-internal-package" */;
productName = TwilioVideo;
};
DC699C7B282DB045009D3C1A /* FirebaseAuth */ = {
isa = XCSwiftPackageProductDependency;
package = DC699C78282DB045009D3C1A /* XCRemoteSwiftPackageReference "firebase-ios-sdk" */;
Expand Down Expand Up @@ -2924,16 +2934,6 @@
package = DCB3BAD9282DA30B00AF7072 /* XCRemoteSwiftPackageReference "KeychainAccess" */;
productName = KeychainAccess;
};
DCB3BADF282DA34400AF7072 /* TwilioVideo */ = {
isa = XCSwiftPackageProductDependency;
package = DCB3BADE282DA34400AF7072 /* XCRemoteSwiftPackageReference "twilio-video-ios" */;
productName = TwilioVideo;
};
DCB3BAE1282DA35100AF7072 /* TwilioVideo */ = {
isa = XCSwiftPackageProductDependency;
package = DCB3BADE282DA34400AF7072 /* XCRemoteSwiftPackageReference "twilio-video-ios" */;
productName = TwilioVideo;
};
/* End XCSwiftPackageProductDependency section */
};
rootObject = 2412822F1E36A6AB002198BE /* Project object */;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,12 @@
}
},
{
"identity" : "twilio-video-ios",
"identity" : "twilio-video-ios-internal-package",
"kind" : "remoteSourceControl",
"location" : "https://github.com/twilio/twilio-video-ios",
"location" : "git@github.com:twilio/twilio-video-ios-internal-package.git",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Must use public package before merging to main branch.

"state" : {
"revision" : "21805ce89585481405bb914922be288d47c1fc62",
"version" : "5.1.1"
"revision" : "1e0748fa89c59f9a465e30d12f4261dbdc29b53f",
"version" : "6.0.0-rc3"
}
}
],
Expand Down
4 changes: 2 additions & 2 deletions VideoApp/VideoApp/Stores/AppSettings/AppSettingsStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ protocol AppSettingsStoreWriting: LaunchStore {
var videoSize: VideoSize { get set }
var userIdentity: String { get set }
var isTURNMediaRelayOn: Bool { get set }
var areInsightsEnabled: Bool { get set }
var isInsightsEnabled: Bool { get set }
var coreSDKLogLevel: SDKLogLevel { get set }
var platformSDKLogLevel: SDKLogLevel { get set }
var signalingSDKLogLevel: SDKLogLevel { get set }
Expand All @@ -43,7 +43,7 @@ class AppSettingsStore: AppSettingsStoreWriting {
@Storage(key: makeKey("videoSize"), defaultValue: .vga) var videoSize: VideoSize
@Storage(key: makeKey("userIdentity"), defaultValue: "") var userIdentity: String
@Storage(key: makeKey("isTURNMediaRelayOn"), defaultValue: false) var isTURNMediaRelayOn: Bool
@Storage(key: makeKey("areInsightsEnabled"), defaultValue: true) var areInsightsEnabled: Bool
@Storage(key: makeKey("isInsightsEnabled"), defaultValue: true) var isInsightsEnabled: Bool
@Storage(key: makeKey("coreSDKLogLevel"), defaultValue: SDKLogLevel.info) var coreSDKLogLevel: SDKLogLevel
@Storage(key: makeKey("platformSDKLogLevel"), defaultValue: SDKLogLevel.info) var platformSDKLogLevel: SDKLogLevel
@Storage(key: makeKey("signalingSDKLogLevel"), defaultValue: SDKLogLevel.error) var signalingSDKLogLevel: SDKLogLevel
Expand Down
6 changes: 3 additions & 3 deletions VideoApp/VideoApp/Stores/Auth/InternalAuthStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ class InternalAuthStore: NSObject, AuthStoreWriting {
private extension TwilioEnvironment {
var host: String {
switch self {
case .production: return "twilio-video-react.appspot.com"
case .staging: return "stage-dot-twilio-video-react.appspot.com"
case .development: return "dev-dot-twilio-video-react.appspot.com"
case .production: return "large-rooms-dot-twilio-video-react.appspot.com"
case .staging: return "large-rooms-stage-dot-twilio-video-react.appspot.com"
case .development: return "large-rooms-dev-dot-twilio-video-react.appspot.com"
Copy link
Contributor Author

@timrozum timrozum Jun 21, 2022

Choose a reason for hiding this comment

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

To test the app, switch environment to development in settings > internal > environment.

I'm thinking about how I'm going to distribute this app for testing. I wonder if I should just hardcode environment to development for now. I may do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also this will need to be reverted before merging to main branch.

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,20 @@ struct ParticipantView: View {
.padding()

if viewModel.cameraTrack != nil {
/// Use opacity to hide the video when the server switches the track off due to bandwidth constraints. This will keep
/// the view in the hierarchy which will signal to the server that the UI wants to render this video. The server will
/// switch the track on when bandwidth constraints allow. If the video was completely removed from the hierarchy
/// the server would never switch the track on.
ZStack {
if !viewModel.shouldFillCameraVideo {
Color.black // For black bars
}
Color.black

/// Use opacity to hide the video when the server switches the track off due to bandwidth constraints.
/// This will keep the view in the hierarchy which will signal to the server that the UI wants to render
/// this video. The server will switch the track on when bandwidth constraints allow. If the video was
/// completely removed from the hierarchy the server would never switch the track on.
SwiftUIVideoView(
videoTrack: $viewModel.cameraTrack,
shouldMirror: $viewModel.shouldMirrorCameraVideo,
fill: viewModel.shouldFillCameraVideo
)
.opacity(viewModel.isCameraTrackSwitchedOffByServer ? 0 : 1)
}
.opacity(viewModel.isCameraTrackSwitchedOff ? 0 : 1)
}

VStack {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ struct ParticipantViewModel {
var dominantSpeakerStartTime: Date = .distantPast
var isDominantSpeaker = false
var cameraTrack: VideoTrack?
var isCameraTrackSwitchedOff = false
var isCameraTrackSwitchedOffByServer = false
var shouldMirrorCameraVideo = false
var shouldFillCameraVideo = false
var networkQualityLevel: NetworkQualityLevel = .unknown
Expand Down Expand Up @@ -58,7 +58,7 @@ struct ParticipantViewModel {
isDominantSpeaker = participant.isDominantSpeaker
dominantSpeakerStartTime = participant.dominantSpeakerStartTime
cameraTrack = participant.cameraTrack
isCameraTrackSwitchedOff = participant.isCameraTrackSwitchedOff
isCameraTrackSwitchedOffByServer = participant.isCameraTrackSwitchedOffByServer
shouldFillCameraVideo = true
networkQualityLevel = participant.networkQualityLevel
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,19 @@ class RemoteParticipantManager: NSObject {
return track.isTrackSubscribed && track.isTrackEnabled
}
var cameraTrack: VideoTrack? {
participant.videoTrack(TrackName.camera)
guard
let track = participant.videoTrack(TrackName.camera),
!track.isSwitchedOffByUser
else {
return nil
}

return participant.videoTrack(TrackName.camera)
}
var isCameraTrackSwitchedOff: Bool {
participant.videoTrack(TrackName.camera)?.isSwitchedOff ?? false
var isCameraTrackSwitchedOffByServer: Bool {
guard let track = participant.videoTrack(TrackName.camera) else { return false }

return track.isSwitchedOff && !track.isSwitchedOffByUser
}
var presentationTrack: VideoTrack? {
participant.videoTrack(TrackName.screen)
Expand Down Expand Up @@ -96,7 +105,11 @@ extension RemoteParticipantManager: RemoteParticipantDelegate {
delegate?.participantDidChange(self)
}

func remoteParticipantSwitchedOffVideoTrack(participant: RemoteParticipant, track: RemoteVideoTrack) {
func remoteParticipantSwitchedOffVideoTrack(
participant: RemoteParticipant,
track: RemoteVideoTrack,
reason: Track.SwitchOffReason
) {
Comment on lines +108 to +112
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now I only had to update the delegate function interface. We were not previously checking disabled state for the camera track which I think was a bug. We should have been checking it and updating the UI when it was disabled. But now it just works with the new and improved track switch off API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a later PR we will probably look at the switch off reason to distinguish in the UI when the user turned off the camera from when the server switched the track off. We are working on new UX designs for this.

delegate?.participantDidChange(self)
}

Expand Down Expand Up @@ -138,8 +151,24 @@ extension RemoteParticipantManager: RemoteParticipantDelegate {
}
}

extension RemoteParticipant {
private extension RemoteParticipant {
func videoTrack(_ trackName: String) -> RemoteVideoTrack? {
remoteVideoTracks.first { $0.trackName.contains(trackName) }?.remoteTrack
}
}

private extension RemoteVideoTrack {
var isSwitchedOffByUser: Bool {
guard let switchOffReason = switchOffReason else { return false }

switch switchOffReason {
case .disabledByPublisher:
return true
case .maxBandwidthReached, .maxTracksReached, .mediaStreamTrackChanging, .networkCongestion, .disabledBySubscriber:
return false
default:
/// Waiting on an SDK fix so that we don't need this default case
return false
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@ class ConnectOptionsFactory: NSObject {
}
}

builder.defaultRspVersion = 3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will need to be reverted before merging to the main branch.

builder.roomName = roomName
builder.uuid = uuid
builder.audioTracks = audioTracks
builder.videoTracks = videoTracks
builder.isDominantSpeakerEnabled = true
builder.isNetworkQualityEnabled = true
builder.areInsightsEnabled = self.appSettingsStore.areInsightsEnabled
builder.isInsightsEnabled = self.appSettingsStore.isInsightsEnabled
builder.networkQualityConfiguration = NetworkQualityConfiguration(
localVerbosity: .minimal,
remoteVerbosity: .minimal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class AdvancedSettingsViewModel: SettingsViewModel {
),
.toggle(
title: "Insights",
isOn: appSettingsStore.areInsightsEnabled,
updateHandler: { self.appSettingsStore.areInsightsEnabled = $0 }
isOn: appSettingsStore.isInsightsEnabled,
updateHandler: { self.appSettingsStore.isInsightsEnabled = $0 }
)
]
),
Expand Down