webapp uplink packets lost can not be recovered better than downlink directory

mediasoup libraries
1

For example, peer A send video, peer B receive video.
If peer A send packets lost(Peer A -> mediasoup lost), the peer B will request mediasoup resend through NACK. Mediasoup also send NACK to peer A.When mediasoup receive NACK from peer B, the losted packets are still not ready on mediasoup.

In downlink directory, there is no this problem(as you can think the source is mediasoup).

2

Yes, this is a common scenario. mediasoup cannot do magic. Peer A will eventually retransmit those packets and they will be received by Peer B. In addition, Peer B keeps sending NACK for those packets for a while.

No issue here.

3

Do you think it’s better to buffer the packets before mediasoup forward packets? The Nack will be sent under two conditions at receive side: 1) when receiving; or 2) check in a timer.
If mediasoup buffered a while, the Nack will be fullfilled under condition 1.

4

No. That would generate artificial delay in packets transmission.

Really, there is no issue here. Already explained above. Nothing needs to be fixed.

5

I think this is the reason(perform bad when uplink packets lost ), some RTX packets are dropped when forward.

I modify the code and works well now:

mediasoup\worker\src\RTC\NackGenerator.cpp
bool NackGenerator::ReceivePacket(RTC::RtpPacket* packet, bool isRecovered)
// Out of order packet or already handled NACKed packet.
if (!isRecovered)
{
MS_WARN_DEV(
“ignoring older packet not present in the NACK list [ssrc:%” PRIu32 “, seq:%” PRIu16 “]”,
packet->GetSsrc(),
packet->GetSequenceNumber());
return false; // <=here
}
return true; // <=here

And:

if (isRecovered)
{
this->recoveredList.insert(seq);

		// Remove old ones so we don't accumulate recovered packets.
		auto it = this->recoveredList.lower_bound(seq - MaxPacketAge);

		if (it != this->recoveredList.begin())
			this->recoveredList.erase(this->recoveredList.begin(), it);

		// Do not let a packet pass if it's newer than last seen seq and came via
		// RTX.
		return true; // <=here
	}
6

Please make a proper Pull Request in GitHub explaining the rationale of this change.