Relate NACK to RTT

[algesten]

Close #744

[lherman-cs]

I've been testing this new patch locally with a simulated network that is close to the lte network here, #744.

        sudo tc qdisc add dev lo root netem \
                rate 3mbit \
                delay 45ms 35ms distribution pareto \
                loss 0.15%; \

The test setup:

WHIP Publisher -> SFU -> WHEP Subscriber

• WHIP Publisher publishes 1 video without simulcast, and uses a pre-recorded video. https://codepen.io/lherman-cs/pen/xbVYqrJ
• WHEP Subscriber: https://codepen.io/lherman-cs/pen/pvgVZar
• SFU: RTP Mode as a dumb pipe, no buffering.

Result:

fwiw, the result is indefinitive, maybe due to too many non-deterministic parts in the tests. I can't really say one is better than the other. But, I noticed that the SFU now reports less NACKs to the sender.

The main metrics I looked at were: Sender's NACK & PacketLost, and Receiver's PacketLost.

Before:

• Sender's NACK: ~38 NACKs
• Sender's PacketLost: ~2
• Receiver's PacketLost: ~10

After:

• Sender's NACK: ~20NACKs
• Sender's PacketLost: 0
• Receiver's PacketLost: ~11

Note: I tried to stop the stream at a similar duration. But, this could've been done better with a shorter video..

Before

sender:

receiver:

After

sender:

receiver:

[lherman-cs]

@algesten just ran another run with real network this time, and statistic interval set to 200ms. I noticed that the packet lost is higher with this PR as observed in remote-inbound-rtp stats. But, it looks like it recovers the packets in the next receiver reports. The original NACK reporter seems to be more responsive as there are less packet loss reported in the receiver reports.

The CPU usage seems higher? I sometimes observed CPU spikes in some occasions. Looking at the flamegraph, "do_poll_output" is a bit more expensive, and it seems to create more timer wheel churns.

My rtc loop looks more or less like this:

let rtc_timer = tokio::time::sleep_until(current_deadline);
tokio::pin!(rtc_timer);

loop {
    let Some(new_deadline) = self.core.poll_rtc() else {
        break;
    };
    if new_deadline != current_deadline {
        rtc_timer.as_mut().reset(new_deadline);
        current_deadline = new_deadline;
    }
    
    tokio::select! {...}
}

original nack reporter

this PR nack reporter

[algesten]

@lherman-cs thanks for testing!

There's a bug in this PR that would have caused a constant incorrect NACK reporting. Would you mind testing again?

But, it looks like it recovers the packets in the next receiver reports.

I might be looking at the wrong thing, but to me these graphs look like an improvement. Instead of hovering around a constant 4 packets lost, we are going to 0-1?

Before

After

The CPU usage seems higher? I sometimes observed CPU spikes in some occasions. Looking at the flamegraph, "do_poll_output" is a bit more expensive, and it seems to create more timer wheel churns.

Yeah, it's doing a lot more now, so CPU would be higher. If we can get the functionality correct, we can look at how to optimize it.

[algesten]

@lherman-cs i also found a bug that meant nack reports were sent at times they shouldn't bee (too soon).

I also fixed the RTT being read from TWCC (like @k0nserv mentioned).

[lherman-cs]

@lherman-cs thanks for testing!

There's a bug in this PR that would have caused a constant incorrect NACK reporting. Would you mind testing again?

But, it looks like it recovers the packets in the next receiver reports.

I might be looking at the wrong thing, but to me these graphs look like an improvement. Instead of hovering around a constant 4 packets lost, we are going to 0-1?

Before

After

The CPU usage seems higher? I sometimes observed CPU spikes in some occasions. Looking at the flamegraph, "do_poll_output" is a bit more expensive, and it seems to create more timer wheel churns.

Yeah, it's doing a lot more now, so CPU would be higher. If we can get the functionality correct, we can look at how to optimize it.

Ah, this is my bad. I forgot to mention that the stats from the original probably got messed up at ~1:03:30PM, since I started a profiler at this moment.

From my perspective, there are 2 ideal behaviors that would lead to improved end-to-end UX (from publisher to subscriber):

1. Only generate NACK reports when necessary: This will reduce network pressure for the publisher, allowing more bandwidth used for the actual streams, aka higher goodput. This can be observed through a proxy metric from candidate-pair's availableOutgoingBandwidth.
2. No reported packet lost in remote-inbound-rtp/rtcp receiver report from sfu: I typically treat sustained packet lost and temporary packet lost as equal. Either case, the packet recovery is probably too slow if the packet lost gets reported since each receiver report takes ~1 second interval for video, https://github.com/PulseBeamDev/str0m/blob/9b28349eb828410a40b038888efc7b40e00fcd10//src/streams/mod.rs#L35-L36.

I considered the original to perform better because there are fewer packet lost reported overall, which implies a higher success rate in recovering the stream.

[lherman-cs]

@algesten I still see similar packet lost reports from the latest change here unfortunately. The RTT graph is very noisy. This makes me think that this is related to the rtt calculation. Maybe we need a windowed histogram, and take P50 or even P30?

original

rtt