Update oldestRtpBaseSequenceNumber when advance the queue state #72
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I met a strange bug related to the RtpAudioQueue recently, when I was using moonlight-qt in a network with very low quality. When some FEC blocks were lost, we decided to give up waiting them, and advanced the queue state in
handleMissingPackets
. Here, we only updatedqueue->nextRtpSequenceNumber
, but notqueue->oldestRtpBaseSequenceNumber
. Then, some of the lost blocks (packets) came accidentally. This time,getFecBlockForRtpPacket
allocated a new block for the packet, and placed it at the head of the queue, since its sequence number was before those of existed blocks. After that, the behavior offreeFecBlockHead
became unexpected. It freed the oldest block (the head of the queue) but not the complete block, so that the later one was still in the queue. As itsnextDataPacketIndex
had already been up toRTPA_DATA_SHARDS
, it would cause crash when being accessed inRtpaGetQueuedPacket
.For an example, the
sequenceNumber
order of the received packets is:When
1060
came,handleMissingPackets
advanced the queue state without updatingqueue->oldestRtpBaseSequenceNumber
. When1004
came,getFecBlockForRtpPacket
allocated a new block (base 1004) for it at the head of the queue. When1063
came, the block base 1060 was complete.freeFecBlockHead
freed the block of base 1004 but not the block of base 1060. The later one was still in the queue, and caused crash when being accessed later.The way to fix it is simple, that is, updating
queue->oldestRtpBaseSequenceNumber
when advance the queue state, preventinggetFecBlockForRtpPacket
from allocating new blocks for the outdated packets.