Add a configurable delay before sending NACK (II)

[ibc]

Same as #609 but with some additions:

• Make NACK sending delay be set via constructor argument in NackGenerator given by RtpStreamRecv which receives it from Producer.
• In Producer.cpp we use a constant SendNackDelay{ 10u } (ms).
• In tests we use delay 0 since otherwise it's terribly hard to make them work.

[ibc]

@penguinol your thoughts about this?

[penguinol]

It's ok

[ggarber]

Thx for looking at this @ibc

But given that the nack timer is 40ms can't it happen that after a lost packet it can take up to 50 ms to request a retransmission?

I don't know well this code so maybe i'm wrong but if that's the case i think it wouldn't be ideal.

[ibc]

Actually I didn't write the code of this PR but I don't understand your concern. Which timer do you mean?

[ibc]

Ok, I see the timer of 40ms and I understand the question. @penguinol @jmillan thoughts? Problem here seems to be that, indeed, if a packet is lost (we receive SEQ 3, then SEQ 5 so SEQ 4 is lost) we don't send NACK for SEQ 4 immediately but instead wait at least 10ms, but the truth is that there is a 40ms timer meaning that such a NACK could be sent after 40ms or 50ms.

However I have my doubts. If you see the NackGenerator::ReceivePacket() code, when we receive SEQ 5 and didn't receive SEQ 4 before, will call:

// Check if there are any nacks that are waiting for this seq number.
std::vector<uint16_t> nackBatch = GetNackBatch(NackFilter::SEQ);

if (!nackBatch.empty())
	this->listener->OnNackGeneratorNackRequired(nackBatch);

and GetNackBatch() will NOT generate the NACK for SEQ 4 since it didn't elapse 10ms yet, but it will send such a NACK for SEQ 4 when SEQ 6 or SEQ 7 or whatever packet arrives (assuming 10ms have elapsed). Otherwise the timer will do it eventually.

[penguinol]

@ibc yes, you're right.
The code will try to send nack packet when any new packet arrives. So the nack packet should be sent in 10~50ms.

[ibc]

Clear. Thanks.