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

Added connection callback, called when the async connection succeeded or failed #1487

Merged
merged 18 commits into from
Sep 11, 2020

Conversation

ethouris
Copy link
Collaborator

@ethouris ethouris commented Aug 18, 2020

Fixes #1464

This adds the connection callback, which is called in case when the connection that is to be completed in the background is finished, that is, succeeded or failed.

The statement as to whether this could be called in synchronous mode should be left undefined. The only cases when it is intended to be used is:

  1. When you use non-blocking mode for a single socket (SRTO_RCVSYN=false) - as an alternative for epoll.
  2. When you use group connections, in which case single socket connections are done always in the background. In this case it's the only way how you can get error information for every single socket and associate it with the link in your application's link table.

By registerring a callback by srt_connect_callback, you install a function that will be called at the moment, when the connection process about the single socket is done. The callback gets:

  • the socket in question
  • the error code (including SRT_SUCCESS if the connection succeeded)
  • the target address by sockaddr*

The additional reject reason can be obtained by srt_getrejectreason on the socket.

@ethouris ethouris changed the title Dev add connection callback Added connection callback, called when the async connection succeeded or failed Aug 19, 2020
@ethouris ethouris requested a review from maxsharabayko August 19, 2020 07:23
@ethouris ethouris self-assigned this Aug 19, 2020
@ethouris ethouris added [core] Area: Changes in SRT library core Impact: Significant Priority: High Type: Enhancement Indicates new feature requests labels Aug 19, 2020
@ethouris ethouris added this to the v1.5.0 - Sprint 21 milestone Aug 19, 2020
@ethouris ethouris marked this pull request as ready for review August 19, 2020 07:24
@maxsharabayko maxsharabayko modified the milestones: v1.5.0 - Sprint 21, v1.5.0 - Sprint 22 Aug 25, 2020
@maxsharabayko
Copy link
Collaborator

maxsharabayko commented Sep 2, 2020

@maxsharabayko
Copy link
Collaborator

maxsharabayko commented Sep 4, 2020

  • Fix API.md for listener callback (listeneing - > listening)

This call installs a callback hook, which will be executed on a socket that is automatically created to handle the incoming connection on the listeneing socket

@maxsharabayko
Copy link
Collaborator

We have listener callback, that is called on every incoming connection request (conclusion stage) before accepting it.

Now we are going to have connection callback that is called only if the connection has failed, and not called when the connection succeeds.
Maybe it should rather be named "connection failure callback". 🤔

@jeandube
Copy link
Collaborator

jeandube commented Sep 4, 2020

Connection callback upon success at the link level is of less interest than failure. connection success is of interest to start sending and this is more for the group socket than the individual links. Connection failure/timeout is of interest at the link level to reissue a link reconnect.

@ethouris
Copy link
Collaborator Author

ethouris commented Sep 4, 2020

Yes, but it should be there for consistency. You still can distinguish these situations by checking the error code. Also it's not exactly for knowing if to "start sending" - that you know from the group readiness. When the group is connected, it doesn't report any new links connected - you still you the group for sending, no matter how many links it is currently utilizing.

@maxsharabayko
Copy link
Collaborator

In the case of listener callback, there is a possibility to reject the connection.
The connection callback, as I understand, does not provide this possibility, right?
Maybe it should be rather a "connection failure callback"? Especially given that it is intended only for group connections.

Note. Don't forget to update the docs with the new function.

@maxsharabayko maxsharabayko modified the milestones: v1.5.0 - Sprint 22, v1.5.0 - Sprint 23 Sep 8, 2020
@ethouris
Copy link
Collaborator Author

ethouris commented Sep 8, 2020

It is intended mainly for the failed group member connections, but:

  1. If someone already uses it, they can simplify the application architecture by relying on the callback report in every case.
  2. The error status check is enough to know if the callback was called in case of a failure.
  3. The success callback is also an opportunity to update quickly the application's connection array, or use it as an alternative to epoll-write check in case of a single-socket connections. I've seen already that users generally don't like it and check the socket status periodically instead, which has a disadvantage of having no wait-trigger ability.

Copy link
Collaborator

@stevomatthews stevomatthews left a comment

Choose a reason for hiding this comment

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

Minor edits.

Co-authored-by: stevomatthews <smatthews@haivision.com>
@maxsharabayko maxsharabayko merged commit e756477 into Haivision:master Sep 11, 2020
@mbakholdina mbakholdina modified the milestones: v1.5.0 - Sprint 23, v1.4.2 Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Priority: High Type: Enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Add a callback function for connection resolution readiness
5 participants