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

RTCSession: Consider emitting "started" once ACK is received #253

Closed
ibc opened this issue Oct 10, 2014 · 10 comments
Closed

RTCSession: Consider emitting "started" once ACK is received #253

ibc opened this issue Oct 10, 2014 · 10 comments
Assignees

Comments

@ibc
Copy link
Member

ibc commented Oct 10, 2014

For incoming calls it may be reasonable to wait for the ACK before "started" event is fired. If for example we decide to accept body-less incoming INVITE (so the remote ACK must contain the SDP answer) this makes lot of sense.

@jmillan @saghul thoughts?

@ibc ibc self-assigned this Oct 10, 2014
@saghul
Copy link
Contributor

saghul commented Oct 10, 2014

Agreed, I think it's a good idea. You may also consider emitting some
"starting" event in the 200.
On Oct 10, 2014 12:57 PM, "Iñaki Baz Castillo" notifications@github.com
wrote:

For incoming calls it may be reasonable to wait for the ACK before
"started" event is fired. If for example we decide to accept body-less
incoming INVITE (so the remote ACK must contain the SDP answer) this makes
lot of sense.

@jmillan https://github.com/jmillan @saghul https://github.com/saghul
thoughts?


Reply to this email directly or view it on GitHub
#253.

@ibc
Copy link
Member Author

ibc commented Oct 10, 2014

Is such a "starting" event really useful? for what? Consider please also the outgoing calls in which we immediately send the ACK.

@jmillan
Copy link
Member

jmillan commented Oct 10, 2014

Hey,

From my point of view, regardless of the SDP answer being present in the "200 OK" or in the "ACK", I would say that:

1- "200 OK" reception/emission should fire "accepted" event, meaning that the user/peer accepted the call
2- "ACK" reception/emission should not fire any event. The absence of "ACK" already terminates the call.
3- RTCPeerConnection ICE connection should fire "connected" event, meaning that the media is being exchanged between peers.

@ibc
Copy link
Member Author

ibc commented Oct 10, 2014

This is not related to media but just signaling. There are some complex use cases in which the app needs to know whether the ACK has been received or not before it does something.

  • May we fire another event upon receipt of the ACK?
  • May we fire "started" twice by adding Boolean hasACK to the Event data?

@ibc
Copy link
Member Author

ibc commented Oct 10, 2014

Guys, what about a new confirmed event that it fires once the session receives the ACK or sends the ACK (for a 2XX I mean)? It would not hurt.

@jmillan
Copy link
Member

jmillan commented Oct 10, 2014

I like the confirmed one on ACK reception/emission. I also think we should rename the started event by accepted. It makes sense to me the transition from acceptedto confirmed

@ibc
Copy link
Member Author

ibc commented Oct 10, 2014

ok

@ibc ibc closed this as completed in f0cc4c1 Oct 10, 2014
@sickpig
Copy link

sickpig commented Oct 10, 2014

On Fri, Oct 10, 2014 at 2:52 PM, Iñaki Baz Castillo <
notifications@github.com> wrote:

Closed #253 #253 via f0cc4c1
f0cc4c1
.

Does this apply (make sense) also to the master branch?

@ibc
Copy link
Member Author

ibc commented Oct 10, 2014

master branch will be frozen until new-design branch can be merged into it. There are too many changes in new-design so picking up specific commits in master is unfeasible.

@sickpig
Copy link

sickpig commented Oct 10, 2014

thanks for the clarification.

On Fri, Oct 10, 2014 at 4:43 PM, Iñaki Baz Castillo <
notifications@github.com> wrote:

master branch will be frozen until new-design branch can be merged into
it. There are too many changes in new-design so picking up specific commits
in master is unfeasible.


Reply to this email directly or view it on GitHub
#253 (comment).

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

No branches or pull requests

4 participants