[core] fix group recv in message mode

[gou4shi1]

Fix #2004 and #1504

• update group read state with ready message
• change the base unit of Group::recv() from packet to message if tsbpd is disabled
• clear group read state before throw if nready is 0

[gou4shi1]

Question: Is it necessary to use ready message to update group read state, or just use the largest received message number (may be incomplete)?

[lgtm-com]

This pull request introduces 1 alert when merging 98ca44f into e932e8f - view on LGTM.com

new alerts:

• 1 for Comparison result is always the same

[gou4shi1]

Transmission.FileUpload was failed since receiveBuffer() didn't lock m_RcvBufferLock

[gou4shi1]

emmm，I found that recv_WaitForReadReady() is still buggy in message mode, which caused many extra useless re-polling:
receiveMessage() report not new message -> re-polling -> recv_WaitForReadReady() report ready -> receiveMessage() report not new message -> re-polling -> ...

So I should also make recv_WaitForReadReady() accurate in message mode with the help of my newly added maps?

[gou4shi1]

Or just skip re-polling in async mode?

[gou4shi1]

// We predict to have only one packet ahead, others are pending to be reported by tsbpd.
// This will be "re-enabled" if the later check puts any new packet into ahead.
m_pGlobal->m_EPoll.update_events(id(), m_sPollID, SRT_EPOLL_IN, false);

Why predict that atmost only one packet ahead, even in tsbpd mode, we may have several ready packets distrusted in different sockets.
Why not also checkPacketAhead() here?

[ethouris]

The current implementation that uses reading from single socket buffers is limited by definition for live mode only and was never intended to work in any other mode.

Early plans were to make a common buffer for a group, but due to problems with implementation we had to put these plans aside.

[gou4shi1]

was never intended to work in any other mode.

But with minor change (recv_GetNo() in this pr), it works in most case.

The another half of this pr is trying to find a better way to define read ready in message mode

[gou4shi1]

Even not in group, the previous read-readiness in message mode is also not accurate, right?

[ethouris]

Message mode hasn't changed in functionality since the original UDT. Do you have any test case that could demonstrate the problem?

[gou4shi1]

For example, message A has 10 packets, the sendCtrlAck() only acked first 5 packets, but it will call m_EPoll.update_events(m_SocketID, m_sPollID, SRT_EPOLL_IN, true) to mark it readable,
then upstream user will be waked up to call receiveMessage() which will fail since the message is still incomplete, right?

[gou4shi1]

• mark socket read-readiness only if the message is really ready
• change the base unit of Group::recv() from pktseq to msgno if tsbpd is disabled
• clear group read state before throw if nready is 0

I can separate this pr into 3 pr

[gou4shi1]

Any comments after several weeks?

[maxsharabayko]

Hi @gou4shi1
First of all, thanks for the contribution!

Certain improvements to the receiver buffer are happening in #1964.
I have a plan to take a look at your PR in the process.
Then we'll also have to schedule a retest of the group receiver, to ensure that things still work properly.

[gou4shi1]

Thanks, I will also take a loop at your great improvement #1964 to see if I can learn sth to improve this PR, this PR is not very good, so I'm eager for any suggestion :)

[gou4shi1]

Found a shortcoming of this PR: if one message consists of multiple packets, all these packets must come from same socket (different messages can come from different sockets), while in tsbpd mode, different packets of same frame can come from different sockets.

[ethouris]

At least one thing is important here: in Live mode you get the packet when it's ready to play (by its TSBPD time) - that is, there's no "framing" on the SRT level - while in file/message mode you get the packet when all packets from the message have been received. This is how the read-readiness is defined.

[gou4shi1]

Found another issue of this PR: m_iNextMsgNo is not synchronized in synchronizeWithGroup()