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] Update group read state using ack instead of first unread seqno #2186

Closed
wants to merge 1 commit into from

Conversation

xiaozhihong
Copy link
Contributor

@xiaozhihong xiaozhihong commented Nov 18, 2021

I used srt backup mode in 2 links, one 4G, another Wifi, and push live stream on it.
My option is live mode, but tspbd: off, tlpktdrop: off.

When I disconnect the Wifi(main) connection, the group socket will never be readable.
And I check the code, I found the problem.

For example, the buffer state below
image

After the Wifi disconnected, when recv packet from 4G, the function below will be called.

updateReadState(socket_id_of_4g, 4/*first seq unread*/)

but It always false, because 4 is less than 5(CUDTGroup::m_RcvBaseSeqNo), and the group socket can not read anymore.

This PR show we should check the group readable using ack seqno instead of first unread seqno in recv buffer.
And it works well in my case.

@gou4shi1
Copy link
Contributor

you can try #2046

@xiaozhihong
Copy link
Contributor Author

you can try #2046

I have seen it, maybe your PR is too complex

@gou4shi1
Copy link
Contributor

gou4shi1 commented Nov 18, 2021

With tspbd and tlpktdrop off, SRT behaves like message mode (except congestion control), and the current group connect is only designed for tsbpd on.
For in-order message, using ack may result in useless epolls.
For out-of-order message, using ack may miss useful epolls.
I have not yet check the code of new recv buffer, maybe the new recv buffer could help to make #2046 simpler.

@codecov-commenter

This comment has been minimized.

@xiaozhihong
Copy link
Contributor Author

With tspbd and tlpktdrop off, SRT behaves like message mode (except congestion control), and the current group connect is only designed for tsbpd on. For in-order message, using ack may result in useless epolls. For out-of-order message, using ack may miss useful epolls. I have not yet check the code of new recv buffer, maybe the new recv buffer could help to make #2046 simpler.

@gou4shi1 Hi, I sent an email to you , can you receive it ?

@maxsharabayko maxsharabayko added this to the v1.4.5 milestone Nov 24, 2021
@maxsharabayko maxsharabayko added [core] Area: Changes in SRT library core Type: Bug Indicates an unexpected problem or unintended behavior labels Nov 24, 2021
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.

4 participants