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

peerconnection and newRTCSession events order? #521

Closed
edvinv opened this issue Jun 4, 2018 · 1 comment
Closed

peerconnection and newRTCSession events order? #521

edvinv opened this issue Jun 4, 2018 · 1 comment

Comments

@edvinv
Copy link

edvinv commented Jun 4, 2018

Hi,
I looking at event order on caller and callee side and I see that peerconnection event on caller is triggered before newRTCSession session event and IMHO this is not correct. On callee side there are correctly triggered.

Here is a trace:

CALLER:
UA: connecting
sip-user-agent.ts:176 UA: connected
sip-user-agent.ts:182 UA: registered
vendor.js:21756 JsSIP:RTCSession emit "peerconnection" +3ms
sip-user-agent.ts:191 UA: newRTCSession
vendor.js:21756 JsSIP:RTCSession emit "connecting" +0ms
vendor.js:21756 JsSIP:RTCSession emit "sdp" +104ms
vendor.js:21756 JsSIP:RTCSession emit "sending" [
vendor.js:21756 JsSIP:RTCSession emit "sdp" +1ms
vendor.js:21756 JsSIP:RTCSession emit "accepted" +2ms
vendor.js:21756 JsSIP:RTCSession emit "confirmed" +1ms
CALLEE:
UA: connecting
sip-user-agent.ts:176 UA: connected
sip-user-agent.ts:182 UA: registered
sip-user-agent.ts:191 UA: newRTCSession
vendor.js:21756 JsSIP:RTCSession emit "peerconnection" +2ms
vendor.js:21756 JsSIP:RTCSession emit "progress" +0ms
vendor.js:21756 JsSIP:RTCSession emit "sdp" +1s
vendor.js:21756 JsSIP:RTCSession emit "connecting" +0ms
vendor.js:21756 JsSIP:RTCSession emit "sdp" +103ms
vendor.js:21756 JsSIP:RTCSession emit "accepted" +0ms
vendor.js:21756 JsSIP:RTCSession emit "confirmed" +1ms

Why I think this is important:

  • it is nice to have uniform behavior on both side
  • it simplifies code, because in newRTCSession you can attach all JsSIP.RTCSession events for both incoming and outgoing calls, and you don't need to use eventHandlers of call() options. If you do this now "peerconnection" event will be lost, because it is triggered before newRTCSession (for caller)
  • JsSIP.RTCSession is "kind of wrapper" for RTCPeerConnection so first event must be wrapper creation event.
  • in new newRTCSession you can also create own session wrapper for JsSIP.RTCSession

Is this design planed?
Anyway thanks for great lib and have a nice day

edvin

@jmillan
Copy link
Member

jmillan commented Jun 5, 2018

Agreed we should not trigger any RTCSession related event before firing UA::newRTCSession() itself.

Thanks. Will fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants