Tricky case with Depacketizer and fec::Reader

[gavv]

It seems that in current implementation there is a case when we're unnecessary dropping received packet(s).

Assume sender generates three packets:

+----+----+----+
| P1 | P2 | P3 |
+----+----+----+

Then they're reordered by interleaver or network:

+----+----+----+
| P3 | P1 | P2 |
+----+----+----+

and P1 and P2 are delayed a bit:

+----+-------+----+----+
| P3 | delay | P1 | P2 |
+----+-------+----+----+

When Depacketizer wants to read P1 from fec::Reader, only P3 is already received:

+----+-------+
| P3 | delay |
+----+-------+

Thus, fec::Reader considers that P1 and P2 are lost and returns P3. Depacketizer notes that P1 and P2 are lost. Depacketizer renders zeros instead of P1.

Then, P1 and P2 are received:

+----+----+
| P1 | P2 |
+----+----+

After this, Depacketizer is going to render samples for P2, previously marked as lost.

Here is the problem:

• P2 is received and it's time to extract samples from P2
• however, Depacketizer renders zeros instead of P2, because it already got P3 from fec::Reader and thinks that P2 is lost
• anyway, fec::Reader drops P2 because it never returns late packets

However, technically nothing prevents us from rendering P2 in this situation.

Note that with fec::Reader, the situation when queue is empty or almost empty is not rare in practice, because network latency jitter value may be close to aim_queue_size - fec_block_size (7 packets currently).

Is this a problem?

[baranovmv]

Looks like Depacketizer have to read() a packet only if it is time to render samples from this particular packet. I'm sorry but it seems like we need IAudioPacketReader::head().

[baranovmv]

• Add IAudioPacketReader::head()
• Edit Depacketizer in order to check if right packet is available every time Streamer::read() is called.

[gavv]

IPacketReader::head(), you mean (or probably peek() is a better name).

I agree, it seems we need it now.

[gavv]

BTW, I've removed IAudioPacketReader interface; we now have only IPacketReader.

[gavv]

If we'll add IPacketReader::head(), we could also remove annoying packet queues from fec::Decoder.

[baranovmv]

It's not recommended in #56 to interleave source packets. In #6 I've suggested to provide an ability to interleave only FEC packets. This approach gives Roc lower latency and is more reliable.
So let's just close this issue.

[gavv]

Yes, but packets may be reordered by the network.

Imagine that we're sending packets over Internet, and P1 + P2 were routed to a slow path, while P3 happened to be routed to a fast path.

But I agree that this issues is not critical for 0.1.

[gavv]

Fixed in #731.