awslabs / amazon-kinesis-video-streams-webrtc-sdk-c

Amazon Kinesis Video Streams Webrtc SDK is for developers to install and customize realtime communication between devices and enable secure streaming of video, audio to Kinesis Video Streams.
https://awslabs.github.io/amazon-kinesis-video-streams-webrtc-sdk-c/group__PublicMemberFunctions.html
Apache License 2.0
997 stars 299 forks source link

[BUG] resendPacketOnNack() potentially putting RTP Packet back into wrong location in Rolling Buffer when RTP sequence overflows #1453

Open suggestedfixes opened 2 years ago

suggestedfixes commented 2 years ago

Describe the bug Observation, when nack count from chrome://webrtc-internals exceeds around 3.5k, chrome is no longer able to decode video. When turning on logging lvl 1, the observation is that when resendPacketOnNack is called, RTP sequence number starts to roll back from 65535 (UINT16) to 0, and rolling buffer will complain about STATUS_ROLLING_BUFFER_NOT_IN_RANGE, At first glance, it might seem to be the rolling buffer not handling the edge case where the index rolls back. However, after a closer look. It might seem that the Rtp packet putting back into the rollingbuffer isn't the same as the one extracted.

Source: https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/blob/master/src/source/PeerConnection/Retransmitter.c

Item retrieved: retStatus = rollingBufferExtractData(pSenderTranceiver->sender.packetBuffer->pRollingBuffer, pRetransmitter->validIndexList[index], &item);

Item put back: retStatus = rollingBufferInsertData(pSenderTranceiver->sender.packetBuffer->pRollingBuffer, pRetransmitter->sequenceNumberList[index], item);

Expected to be put back retStatus = rollingBufferInsertData(pSenderTranceiver->sender.packetBuffer->pRollingBuffer, pRetransmitter->validIndexList[index], item);

To Reproduce Steps to reproduce the behavior:

  1. Increase the bitrate, or decrease the network bandwidth such that we observe a linear increase of Nacks on the browser side.
  2. On the master side, turn on all log levels.
  3. Observe "Resent packet ssrc %lu seq %lu succeeded", where seq reaches beyond 65535 and rolls back to 0.
  4. Also observe that chrome will stop decoding video correctly

Expected behavior reendPacketOnNack() added back the correct RtpPacket back into the rolling buffer.

Screenshots image

Additional context Add any other context about the problem here.

github-actions[bot] commented 1 year ago

This is a very old issue. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

github-actions[bot] commented 1 year ago

This is a very old issue. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

github-actions[bot] commented 8 months ago

This is a very old issue. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

github-actions[bot] commented 1 month ago

This is a very old issue. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

wei-zhang-simplisafe commented 1 month ago

The ticket was not fixed. Please see new ticket https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/issues/1991