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

[core] To fix missing new link update when no transmission on existing links #1891

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ethouris
Copy link
Collaborator

First draft, adds a Unit Test that shows where the problem is and it is now failing.

@ethouris
Copy link
Collaborator Author

An imaginable possible solution:

  1. Remove the m_listener field from the group as it is completely useless anyway.
  2. Make THE GROUP the event sink for the new link update. The m_RcvEID should be then subscribed for SRT_EPOLL_UPDATE on "itself" (that is, the group ID).
  3. The SRT_EPOLL_UPDATE event is issued on the group UPON ADDING A NEW ACCEPTED SOCKET to the group. This should make the waiting on reading interrupt and RECHECK ALL MEMBER SOCKETS to see that they are all added to m_RcvEID.

@ethouris
Copy link
Collaborator Author

The test has been adopted for #2444 and it passes there. That PR is then considered a replacement for this one.

ethouris pushed a commit to ethouris/srt that referenced this pull request Sep 10, 2024
@maxsharabayko maxsharabayko added this to the v1.6.0 milestone Sep 10, 2024
@maxsharabayko maxsharabayko added Type: Bug Indicates an unexpected problem or unintended behavior [core] Area: Changes in SRT library core labels Sep 10, 2024
@ethouris
Copy link
Collaborator Author

Ok, there are still some mysterious fixes here that need not be cleared also with #2444. Need to investigate.

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 Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants