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

Remove Unwanted IPS from SDP #358

Merged
merged 4 commits into from
Nov 18, 2024
Merged

Remove Unwanted IPS from SDP #358

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7a86bf7
- Remove Unwanted IPS from SDP
isaacakakpo1 Nov 15, 2024
e06c3d5
- Fix Answer Test
isaacakakpo1 Nov 15, 2024
011da73
- Deprecated `callStateLiveData` and introduced callStateFlow as a Fl…
isaacakakpo1 Nov 17, 2024
116eed0
- Set Initial callState for Call
isaacakakpo1 Nov 18, 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 telnyx_rtc/src/main/java/com/telnyx/webrtc/sdk/Call.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ data class Call(

init {
callStateLiveData.postValue(CallState.CONNECTING)

// Ensure that loudSpeakerLiveData is correct based on possible options provided from client.
loudSpeakerLiveData.postValue(audioManager.isSpeakerphoneOn)
}
Expand Down
39 changes: 27 additions & 12 deletions telnyx_rtc/src/main/java/com/telnyx/webrtc/sdk/TelnyxClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,13 @@ class TelnyxClient(
// reset audio mode to communication
speakerState?.let { setSpeakerMode(it) }


callStateLiveData.postValue(CallState.ACTIVE)
callStateLiveData.value = CallState.ACTIVE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this right?

In line 188 we are using postValue, which is thread-safe, and in the next line, we are using .value, which is not thread-safe, and they perform almost the same operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah they are the same, the issue is callStateLiveData.postValue(CallState.ACTIVE) doesent set the value somehow (it just updates the observers) and I have to observe the value from the TelnyxClient.kt class might I don't want to do. I could create a separate variablle for CallState.ACTIVE but then there's not single souce of truth.

For thread safety
@Override public void setValue(T value) { super.setValue(value); }

I believe it's just a setter. 
Now that you mkention though, I think there's a `getState()` method that might have the updated value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I am actually using it :
if (call.getCallState().value != CallState.ACTIVE) but somehow it doesn't print the latest value. Let me see if I can make it work

Copy link
Contributor Author

@isaacakakpo1 isaacakakpo1 Nov 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

IMPORTANT NOTE: I just noticed that if you're in main thread and use postValue it will take a few more millis compared to setValue to dispatch the data. I had this happen when I fill a layout manually, with setValue it was instant but not with post.

-- Probably this is why it still has the previous value. I may have to observe then

https://stackoverflow.com/a/51299672/24361182

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the mutable livedata with a flow and deprecated livedata. This was in Todo for sometime and I think I will want to deprecate all mutable livedata infavour of flow because it's threadsafe.

client.callOngoing()

}
}
this.addToCalls(acceptCall)
Timber.d("Event-Check Active ${ calls[callId]?.getCallState()?.value}")
return acceptCall
}

Expand All @@ -214,14 +214,19 @@ class TelnyxClient(
val inviteCallId: UUID = UUID.randomUUID()

callId = inviteCallId
val call = this

// Create new peer
peerConnection = Peer(
context, client, providedTurn, providedStun, callId.toString(),
object : PeerConnectionObserver() {
override fun onIceCandidate(p0: IceCandidate?) {
super.onIceCandidate(p0)
peerConnection?.addIceCandidate(p0)
Timber.d("Event-IceCandidate Generated")
if (call.getCallState().value != CallState.ACTIVE){
Copy link
Collaborator

Choose a reason for hiding this comment

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

A little change here. Please add a space between parentheses and braces ){

peerConnection?.addIceCandidate(p0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should check if peerConnection exist before printing the log that the ICE candidate was added

Timber.d("Event-IceCandidate Added")
}
}
}
)
Expand Down Expand Up @@ -311,7 +316,7 @@ class TelnyxClient(
* @param call, and instance of [Call]
*/
internal fun addToCalls(call: Call) {
calls.getOrPut(call.callId) { call }
calls[call.callId] = call
}

/**
Expand Down Expand Up @@ -1274,6 +1279,7 @@ class TelnyxClient(
peerConnection?.onRemoteSessionReceived(sdp)

callStateLiveData.postValue(CallState.ACTIVE)
callStateLiveData.value = CallState.ACTIVE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as before
In line 1281 we are using postValue, which is thread-safe, and in the next line, we are using .value, which is not thread-safe, and they perform almost the same operation.


val answerResponse = AnswerResponse(
UUID.fromString(callId),
Expand Down Expand Up @@ -1309,6 +1315,7 @@ class TelnyxClient(
)
)
callStateLiveData.postValue(CallState.ACTIVE)
callStateLiveData.value = CallState.ACTIVE
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment as before

}

else -> {
Expand All @@ -1320,6 +1327,9 @@ class TelnyxClient(
client.callOngoing()
client.stopMediaPlayer()
}
answeredCall?.let {
addToCalls(it)
}

}

Expand Down Expand Up @@ -1405,6 +1415,7 @@ class TelnyxClient(

// Set global callID
callId = offerCallId
val call = this

//retrieve custom headers
val customHeaders =
Expand All @@ -1414,8 +1425,12 @@ class TelnyxClient(
object : PeerConnectionObserver() {
override fun onIceCandidate(p0: IceCandidate?) {
super.onIceCandidate(p0)
peerConnection?.addIceCandidate(p0)
Timber.d("On IceCandidate Added")
Timber.d("Event-IceCandidate Generated")
Timber.d("Event-IceCandidate ${calls[callId]?.getCallState()?.value}")
if (calls[callId]?.getCallState()?.value != CallState.ACTIVE){
peerConnection?.addIceCandidate(p0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

check if peerConnection exist before printing the log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Peer connection should always exit on line 1425 peerConnection = Peer(

Timber.d("Event-IceCandidate Added")
}
}

override fun onRenegotiationNeeded() {
Expand Down Expand Up @@ -1540,8 +1555,6 @@ class TelnyxClient(
}

override fun onAttachReceived(jsonObject: JsonObject) {
/* val params = jsonObject.getAsJsonObject("params")
val callId = UUID.fromString(params.get("callID").asString)*/

val attachCall = call!!.copy(
context = context,
Expand Down Expand Up @@ -1572,14 +1585,16 @@ class TelnyxClient(
// Set global callID
callId = offerCallId


peerConnection = Peer(
context, client, providedTurn, providedStun, offerCallId.toString(),
object : PeerConnectionObserver() {
override fun onIceCandidate(p0: IceCandidate?) {
super.onIceCandidate(p0)
peerConnection?.addIceCandidate(p0)
Timber.d("On IceCandidate Added")
Timber.d("Event-IceCandidate Generated")
if (calls[callId]?.getCallState()?.value != CallState.ACTIVE){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a space between parentheses and braces ){.

And check if peerConnection Exist before printing the log

peerConnection?.addIceCandidate(p0)
Timber.d("Event-IceCandidate Added")
}
}

override fun onRenegotiationNeeded() {
Expand Down Expand Up @@ -1608,7 +1623,7 @@ class TelnyxClient(
Call.ICE_CANDIDATE_DELAY
)
}

attachCall.callStateLiveData.value = CallState.ACTIVE
calls[attachCall.callId] = attachCall
}

Expand Down
Loading