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

do not treat WSAECONNRESET as a fatal error (#468) #469

Merged
merged 2 commits into from
Oct 11, 2018
Merged

do not treat WSAECONNRESET as a fatal error (#468) #469

merged 2 commits into from
Oct 11, 2018

Conversation

maxtomilov
Copy link
Contributor

@maxtomilov maxtomilov commented Sep 27, 2018

treating WSAECONNRESET as a fatal error for UDP socket causes IPE when e.g. trying to connect to not responding SRT peer.

@ethouris
Copy link
Collaborator

@rndi , this is ok to merge.

@alexpokotilo
Copy link
Contributor

Hi @rndi, do you have any questions or concerns about this commit ?

@rndi
Copy link
Collaborator

rndi commented Oct 8, 2018

Hi @alexpokotilo , yes, ideally I would like to better understand the reasons for the differences observed between Windows and Linux in this scenario before introducing this change, as per discussions in #468. Will probably need to write a bit of test code to validate current assumptions.

@alexpokotilo
Copy link
Contributor

alexpokotilo commented Oct 9, 2018

Do you expect anything from our team during this investigation?
Now Windows async SRT version doesn't work so you really need to add some tests to the project and probably confirm|change some assumptions about Windows|Linux differences. E.g this issue was found by UT in our project. We can send you our SRT unit tests if you want. We use google unit test so you can change our tests to fit your needs.
I just want to confirm that the ball is on your side and you don't wait for anything from us.

@rndi
Copy link
Collaborator

rndi commented Oct 9, 2018

Hello @alexpokotilo , the ball is definitely on our side, no worries :) I've put together a small test and by the looks of it, I am able to replicate your results on Linux which is what I needed. I will get back to you on this.

If you think you can open a separate PR with some of your google unit tests included (and, preferably, enabled with CMake) that will probably be the best way to get that added to the project.

@rndi rndi merged commit c4c53b6 into Haivision:master Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants