Fix sequence number calculation logic.

Issue: google/ExoPlayer#9697

Before, the `MAX_SEQUENCE_NUMBER` is 65535, such that the logic to get the next
sequence number:

`previousSeqNumber + 1 % MAX_SEQUENCE_NUMBER`

yields 0 when `previousSeqNumber` is 65534. However, the next sequence number
should be 65535.

PiperOrigin-RevId: 410530098
This commit is contained in:
claincly 2021-11-17 16:06:16 +00:00 committed by Ian Baker
parent 26153add56
commit f527fb729c
4 changed files with 41 additions and 18 deletions

View File

@ -24,6 +24,7 @@ import androidx.media3.common.C;
import androidx.media3.common.util.ParsableByteArray;
import androidx.media3.common.util.UnstableApi;
import androidx.media3.common.util.Util;
import com.google.common.math.IntMath;
import java.nio.ByteBuffer;
/**
@ -134,6 +135,16 @@ public final class RtpPacket {
public static final int MAX_SEQUENCE_NUMBER = 0xFFFF;
public static final int CSRC_SIZE = 4;
/** Returns the next sequence number of the {@code sequenceNumber}. */
public static int getNextSequenceNumber(int sequenceNumber) {
return IntMath.mod(sequenceNumber + 1, MAX_SEQUENCE_NUMBER + 1);
}
/** Returns the previous sequence number from the {@code sequenceNumber}. */
public static int getPreviousSequenceNumber(int sequenceNumber) {
return IntMath.mod(sequenceNumber - 1, MAX_SEQUENCE_NUMBER + 1);
}
private static final byte[] EMPTY = new byte[0];
/** The RTP version field (Word 0, bits 0-1), should always be 2. */

View File

@ -34,8 +34,6 @@ import java.util.TreeSet;
/** The maximum sequence number discontinuity allowed without resetting the re-ordering buffer. */
@VisibleForTesting /* package */ static final int MAX_SEQUENCE_LEAP_ALLOWED = 1000;
private static final int MAX_SEQUENCE_NUMBER = RtpPacket.MAX_SEQUENCE_NUMBER;
/** Queue size threshold for resetting the queue. 5000 packets equate about 7MB in buffer size. */
private static final int QUEUE_SIZE_THRESHOLD_FOR_RESET = 5000;
@ -96,13 +94,13 @@ import java.util.TreeSet;
int packetSequenceNumber = packet.sequenceNumber;
if (!started) {
reset();
lastDequeuedSequenceNumber = prevSequenceNumber(packetSequenceNumber);
lastDequeuedSequenceNumber = RtpPacket.getPreviousSequenceNumber(packetSequenceNumber);
started = true;
addToQueue(new RtpPacketContainer(packet, receivedTimestampMs));
return true;
}
int expectedSequenceNumber = nextSequenceNumber(lastReceivedSequenceNumber);
int expectedSequenceNumber = RtpPacket.getNextSequenceNumber(lastReceivedSequenceNumber);
// A positive shift means the packet succeeds the last received packet.
int sequenceNumberShift =
calculateSequenceNumberShift(packetSequenceNumber, expectedSequenceNumber);
@ -114,7 +112,7 @@ import java.util.TreeSet;
}
} else {
// Discard all previous received packets and start subsequent receiving from here.
lastDequeuedSequenceNumber = prevSequenceNumber(packetSequenceNumber);
lastDequeuedSequenceNumber = RtpPacket.getPreviousSequenceNumber(packetSequenceNumber);
packetQueue.clear();
addToQueue(new RtpPacketContainer(packet, receivedTimestampMs));
return true;
@ -140,7 +138,7 @@ import java.util.TreeSet;
RtpPacketContainer packetContainer = packetQueue.first();
int packetSequenceNumber = packetContainer.packet.sequenceNumber;
if (packetSequenceNumber == nextSequenceNumber(lastDequeuedSequenceNumber)
if (packetSequenceNumber == RtpPacket.getNextSequenceNumber(lastDequeuedSequenceNumber)
|| cutoffTimestampMs >= packetContainer.receivedTimestampMs) {
packetQueue.pollFirst();
lastDequeuedSequenceNumber = packetSequenceNumber;
@ -168,16 +166,6 @@ import java.util.TreeSet;
}
}
private static int nextSequenceNumber(int sequenceNumber) {
return (sequenceNumber + 1) % MAX_SEQUENCE_NUMBER;
}
private static int prevSequenceNumber(int sequenceNumber) {
return sequenceNumber == 0
? MAX_SEQUENCE_NUMBER - 1
: (sequenceNumber - 1) % MAX_SEQUENCE_NUMBER;
}
/**
* Calculates the sequence number shift, accounting for wrapping around.
*
@ -193,7 +181,7 @@ import java.util.TreeSet;
int shift =
min(sequenceNumber, previousSequenceNumber)
- max(sequenceNumber, previousSequenceNumber)
+ MAX_SEQUENCE_NUMBER;
+ RtpPacket.MAX_SEQUENCE_NUMBER;
// Check whether this is actually an wrap-over. For example, it is a wrap around if receiving
// 65500 (prevSequenceNumber) after 1 (sequenceNumber); but it is not when prevSequenceNumber
// is 30000.

View File

@ -259,7 +259,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
fuScratchBuffer.setPosition(1);
} else {
// Check that this packet is in the sequence of the previous packet.
int expectedSequenceNumber = (previousSequenceNumber + 1) % RtpPacket.MAX_SEQUENCE_NUMBER;
int expectedSequenceNumber = RtpPacket.getNextSequenceNumber(previousSequenceNumber);
if (packetSequenceNumber != expectedSequenceNumber) {
Log.w(
TAG,

View File

@ -18,6 +18,8 @@ package androidx.media3.exoplayer.rtsp;
import static androidx.media3.common.util.Assertions.checkNotNull;
import static androidx.media3.common.util.Util.getBytesFromHexString;
import static androidx.media3.exoplayer.rtsp.RtpPacket.getNextSequenceNumber;
import static androidx.media3.exoplayer.rtsp.RtpPacket.getPreviousSequenceNumber;
import static com.google.common.truth.Truth.assertThat;
import androidx.media3.common.C;
@ -184,4 +186,26 @@ public final class RtpPacketTest {
builtPacket.writeToBuffer(builtPacketBytes, /* offset= */ 0, packetSize);
assertThat(builtPacketBytes).isEqualTo(rtpDataWithLargeTimestamp);
}
@Test
public void getNextSequenceNumber_invokingAtWrapOver() {
assertThat(getNextSequenceNumber(65534)).isEqualTo(65535);
assertThat(getNextSequenceNumber(65535)).isEqualTo(0);
assertThat(getNextSequenceNumber(0)).isEqualTo(1);
}
@Test
public void getPreviousSequenceNumber_invokingAtWrapOver() {
assertThat(getPreviousSequenceNumber(1)).isEqualTo(0);
assertThat(getPreviousSequenceNumber(0)).isEqualTo(65535);
assertThat(getPreviousSequenceNumber(65535)).isEqualTo(65534);
}
@Test
public void getSequenceNumber_isSymmetric() {
for (int i = 0; i < RtpPacket.MAX_SEQUENCE_NUMBER; i++) {
assertThat(getPreviousSequenceNumber(getNextSequenceNumber(i))).isEqualTo(i);
assertThat(getNextSequenceNumber(getPreviousSequenceNumber(i))).isEqualTo(i);
}
}
}