Remove Unwanted IPS from SDP #358
Conversation
Outbound Inbound Attach
| callStateLiveData.postValue(CallState.ACTIVE) | ||
| callStateLiveData.value = CallState.ACTIVE |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| @@ -1274,6 +1279,7 @@ class TelnyxClient( | |||
| peerConnection?.onRemoteSessionReceived(sdp) | |||
|
|
|||
| callStateLiveData.postValue(CallState.ACTIVE) | |||
| callStateLiveData.value = CallState.ACTIVE | |||
There was a problem hiding this comment.
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.
| peerConnection?.addIceCandidate(p0) | ||
| Timber.d("Event-IceCandidate Generated") | ||
| if (call.getCallState().value != CallState.ACTIVE){ | ||
| peerConnection?.addIceCandidate(p0) |
There was a problem hiding this comment.
we should check if peerConnection exist before printing the log that the ICE candidate was added
|
|
||
| // 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){ |
There was a problem hiding this comment.
A little change here. Please add a space between parentheses and braces ){
| @@ -1309,6 +1315,7 @@ class TelnyxClient( | |||
| ) | |||
| ) | |||
| callStateLiveData.postValue(CallState.ACTIVE) | |||
| callStateLiveData.value = CallState.ACTIVE | |||
There was a problem hiding this comment.
same comment as before
| Timber.d("Event-IceCandidate Generated") | ||
| Timber.d("Event-IceCandidate ${calls[callId]?.getCallState()?.value}") | ||
| if (calls[callId]?.getCallState()?.value != CallState.ACTIVE){ | ||
| peerConnection?.addIceCandidate(p0) |
There was a problem hiding this comment.
check if peerConnection exist before printing the log
There was a problem hiding this comment.
Peer connection should always exit on line 1425 peerConnection = Peer(
| peerConnection?.addIceCandidate(p0) | ||
| Timber.d("On IceCandidate Added") | ||
| Timber.d("Event-IceCandidate Generated") | ||
| if (calls[callId]?.getCallState()?.value != CallState.ACTIVE){ |
There was a problem hiding this comment.
Please add a space between parentheses and braces ){.
And check if peerConnection Exist before printing the log
WebRTC-2355 - Ticket Description.