[core] fix m_GroupOf->updateReadState() in message mode

[gou4shi1]

Fix #2186 and part of #2046.
I noticed that the return value of ackDataUpTo() is only used to update group readiness, so it should use the last readable seq.
More PRs will be extracted from #2046, base on the new rcv buffer.

[maxsharabayko]

@gou4shi1 Thanks for the PR!

Looks like the return value of the CUDT::ackDataUpTo(..) in the case of the old receiver buffer would be different, It returns the sequence number of the first packet available for reading.
While with the new receiver buffer the function returns the sequence number of the latest packet available for reading. This is the correct behavior, as the group needs the value to update its read-ready state.

P.S. Added description comments to related functions:
comments.patch.zip

[gou4shi1]

Looks like the return value of the CUDT::ackDataUpTo(..) in the case of the old receiver buffer would be different, It returns the sequence number of the first packet available for reading.

Yes, the behavior with old recv buffer is not correct, when will the old recv buffer be deleted?

Added description comments to related functions:

Thanks, I have applied your patch in this PR :)

[maxsharabayko]

when will the old recv buffer be deleted?

It might stay in the next release to have a fallback option in case of unexpected issues. And can be removed afterwards.

[maxsharabayko]

While the old receiver buffer is being supported, better to have it also functioning correctly.
Here is a proposed fix of the CUDT::ackDataUpTo(..) function with respect to the old receiver buffer. The function should now behave almost similarly both with the new, and with the old receiver buffer. At least return the latest readable seqno, not the first one.

int32_t srt::CUDT::ackDataUpTo(int32_t ack)
{
    // ...
#else
    if (acksize > 0)
    {
        m_pRcvBuffer->ackData(acksize);
    }

    return ack;
#endif
}

[gou4shi1]

Here is a proposed fix

It's similar as #2186, applied :)

At least return the latest readable seqno, not the first one.

Actually "maybe readable" for messages across multiple packets (with old rcv buffer), right?

[maxsharabayko]

Here is a proposed fix

It's similar as #2186, applied :)

When three independent solutions come to a single (similar) one, it confirms the solution must be correct 🙂

At least return the latest readable seqno, not the first one.

Actually "maybe readable" for messages across multiple packets (with old rcv buffer), right?

Hopefully readable 🙂

[gou4shi1]

When three independent solutions come to a single (similar) one, it confirms the solution must be correct slightly_smiling_face

LOL