From ff3d7dff12f0e7b1fd06906620f4daf6aa5a02f6 Mon Sep 17 00:00:00 2001 From: Shraddha Basantwani Date: Wed, 6 Jul 2022 12:46:52 +0530 Subject: [PATCH 1/2] Fix VP8 Reader Update VP8 header to check if the S bit is set. Variable fragmentedSampleSizeBytes is initialised with -1, and reader is directly adding fragmentSize to this variable. Updated it to check if the size is unset. Bug: 238153477 Test: manual Change-Id: I9d5735422a4a0eeb2967af93809b879b434e3c57 --- .../media3/exoplayer/rtsp/reader/RtpVp8Reader.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8Reader.java b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8Reader.java index 72b739edd2..dda04bf8c6 100644 --- a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8Reader.java +++ b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8Reader.java @@ -113,7 +113,11 @@ public void consume( int fragmentSize = data.bytesLeft(); trackOutput.sampleData(data, fragmentSize); - fragmentedSampleSizeBytes += fragmentSize; + if (fragmentedSampleSizeBytes == C.LENGTH_UNSET) { + fragmentedSampleSizeBytes = fragmentSize; + } else { + fragmentedSampleSizeBytes += fragmentSize; + } if (rtpMarker) { if (firstReceivedTimestamp == C.TIME_UNSET) { @@ -150,7 +154,7 @@ private boolean validateVp8Descriptor(ParsableByteArray payload, int packetSeque if (!gotFirstPacketOfVp8Frame) { // TODO(b/198620566) Consider using ParsableBitArray. // For start of VP8 partition S=1 and PID=0 as per RFC7741 Section 4.2. - if ((header & 0x10) != 0x1 || (header & 0x07) != 0) { + if ((header & 0x10) != 0x10 || (header & 0x07) != 0) { Log.w(TAG, "RTP packet is not the start of a new VP8 partition, skipping."); return false; } From 1de4ee3af519f73e686f0b301eb2cfb117fc55e4 Mon Sep 17 00:00:00 2001 From: Shraddha Basantwani Date: Mon, 25 Apr 2022 15:05:36 +0530 Subject: [PATCH 2/2] Add RTP VP8 Reader Test Update VP8 Reader to handle missing frames/fragments. Change-Id: I9eede8f1e3a20fb0ff2e7add0dfc60f0780ec769 --- .../exoplayer/rtsp/reader/RtpVp8Reader.java | 60 ++++-- .../rtsp/reader/RtpVp8ReaderTest.java | 180 ++++++++++++++++++ 2 files changed, 219 insertions(+), 21 deletions(-) create mode 100644 libraries/exoplayer_rtsp/src/test/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8ReaderTest.java diff --git a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8Reader.java b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8Reader.java index dda04bf8c6..eb5e589901 100644 --- a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8Reader.java +++ b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8Reader.java @@ -15,6 +15,7 @@ */ package androidx.media3.exoplayer.rtsp.reader; +import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkStateNotNull; import androidx.media3.common.C; @@ -50,6 +51,7 @@ private int previousSequenceNumber; /** The combined size of a sample that is fragmented into multiple RTP packets. */ private int fragmentedSampleSizeBytes; + private long sampleTimeUsOfFragmentedSample; private long startTimeOffsetUs; /** @@ -67,6 +69,7 @@ public RtpVp8Reader(RtpPayloadFormat payloadFormat) { firstReceivedTimestamp = C.TIME_UNSET; previousSequenceNumber = C.INDEX_UNSET; fragmentedSampleSizeBytes = C.LENGTH_UNSET; + sampleTimeUsOfFragmentedSample = C.TIME_UNSET; // The start time offset must be 0 until the first seek. startTimeOffsetUs = 0; gotFirstPacketOfVp8Frame = false; @@ -81,7 +84,10 @@ public void createTracks(ExtractorOutput extractorOutput, int trackId) { } @Override - public void onReceivingFirstPacket(long timestamp, int sequenceNumber) {} + public void onReceivingFirstPacket(long timestamp, int sequenceNumber) { + checkState(firstReceivedTimestamp == C.TIME_UNSET); + firstReceivedTimestamp = timestamp; + } @Override public void consume( @@ -119,19 +125,11 @@ public void consume( fragmentedSampleSizeBytes += fragmentSize; } + sampleTimeUsOfFragmentedSample = + toSampleUs(startTimeOffsetUs, timestamp, firstReceivedTimestamp); + if (rtpMarker) { - if (firstReceivedTimestamp == C.TIME_UNSET) { - firstReceivedTimestamp = timestamp; - } - long timeUs = toSampleUs(startTimeOffsetUs, timestamp, firstReceivedTimestamp); - trackOutput.sampleMetadata( - timeUs, - isKeyFrame ? C.BUFFER_FLAG_KEY_FRAME : 0, - fragmentedSampleSizeBytes, - /* offset= */ 0, - /* cryptoData= */ null); - fragmentedSampleSizeBytes = C.LENGTH_UNSET; - gotFirstPacketOfVp8Frame = false; + outputSampleMetadataForFragmentedPackets(); } previousSequenceNumber = sequenceNumber; } @@ -151,18 +149,18 @@ public void seek(long nextRtpTimestamp, long timeUs) { private boolean validateVp8Descriptor(ParsableByteArray payload, int packetSequenceNumber) { // VP8 Payload Descriptor is defined in RFC7741 Section 4.2. int header = payload.readUnsignedByte(); - if (!gotFirstPacketOfVp8Frame) { - // TODO(b/198620566) Consider using ParsableBitArray. - // For start of VP8 partition S=1 and PID=0 as per RFC7741 Section 4.2. - if ((header & 0x10) != 0x10 || (header & 0x07) != 0) { - Log.w(TAG, "RTP packet is not the start of a new VP8 partition, skipping."); - return false; + // TODO(b/198620566) Consider using ParsableBitArray. + // For start of VP8 partition S=1 and PID=0 as per RFC7741 Section 4.2. + if ((header & 0x10) == 0x10 && (header & 0x07) == 0) { + if (gotFirstPacketOfVp8Frame && fragmentedSampleSizeBytes > 0) { + // Received new VP8 fragment, output data of previous fragment to decoder. + outputSampleMetadataForFragmentedPackets(); } gotFirstPacketOfVp8Frame = true; - } else { + } else if (gotFirstPacketOfVp8Frame) { // Check that this packet is in the sequence of the previous packet. int expectedSequenceNumber = RtpPacket.getNextSequenceNumber(previousSequenceNumber); - if (packetSequenceNumber != expectedSequenceNumber) { + if (packetSequenceNumber < expectedSequenceNumber) { Log.w( TAG, Util.formatInvariant( @@ -171,6 +169,9 @@ private boolean validateVp8Descriptor(ParsableByteArray payload, int packetSeque expectedSequenceNumber, packetSequenceNumber)); return false; } + } else { + Log.w(TAG, "RTP packet is not the start of a new VP8 partition, skipping."); + return false; } // Check if optional X header is present. @@ -199,6 +200,23 @@ private boolean validateVp8Descriptor(ParsableByteArray payload, int packetSeque return true; } + /** + * Outputs sample metadata. + * + *

Call this method only when receiving a end of VP8 partition + */ + private void outputSampleMetadataForFragmentedPackets() { + trackOutput.sampleMetadata( + sampleTimeUsOfFragmentedSample, + isKeyFrame ? C.BUFFER_FLAG_KEY_FRAME : 0, + fragmentedSampleSizeBytes, + /* offset= */ 0, + /* cryptoData= */ null); + fragmentedSampleSizeBytes = 0; + sampleTimeUsOfFragmentedSample = C.TIME_UNSET; + gotFirstPacketOfVp8Frame = false; + } + private static long toSampleUs( long startTimeOffsetUs, long rtpTimestamp, long firstReceivedRtpTimestamp) { return startTimeOffsetUs diff --git a/libraries/exoplayer_rtsp/src/test/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8ReaderTest.java b/libraries/exoplayer_rtsp/src/test/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8ReaderTest.java new file mode 100644 index 0000000000..8bd2e347b4 --- /dev/null +++ b/libraries/exoplayer_rtsp/src/test/java/androidx/media3/exoplayer/rtsp/reader/RtpVp8ReaderTest.java @@ -0,0 +1,180 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package androidx.media3.exoplayer.rtsp.reader; + +import static androidx.media3.common.util.Util.getBytesFromHexString; +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.when; + +import androidx.media3.common.Format; +import androidx.media3.common.MimeTypes; +import androidx.media3.common.util.ParsableByteArray; +import androidx.media3.exoplayer.rtsp.RtpPacket; +import androidx.media3.exoplayer.rtsp.RtpPayloadFormat; +import androidx.media3.extractor.ExtractorOutput; +import androidx.media3.test.utils.FakeTrackOutput; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.common.collect.ImmutableMap; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +/** + * Unit test for {@link RtpVp8Reader}. + */ +@RunWith(AndroidJUnit4.class) +public final class RtpVp8ReaderTest { + + private final RtpPacket frame1fragment1 = + createRtpPacket( + /* timestamp= */ 2599168056L, + /* sequenceNumber= */ 40289, + /* marker= */ false, + /* payloadData= */ getBytesFromHexString("10000102030405060708090A")); + private final RtpPacket frame1fragment2 = + createRtpPacket( + /* timestamp= */ 2599168056L, + /* sequenceNumber= */ 40290, + /* marker= */ true, + /* payloadData= */ getBytesFromHexString("000B0C0D0E")); + private final byte[] frame1Data = getBytesFromHexString("000102030405060708090A0B0C0D0E"); + private final RtpPacket frame2fragment1 = + createRtpPacket( + /* timestamp= */ 2599168344L, + /* sequenceNumber= */ 40291, + /* marker= */ false, + /* payloadData= */ getBytesFromHexString("100D0C0B0A090807060504")); + // Add optional headers + private final RtpPacket frame2fragment2 = + createRtpPacket( + /* timestamp= */ 2599168344L, + /* sequenceNumber= */ 40292, + /* marker= */ true, + /* payloadData= */ getBytesFromHexString("80D6AA95396103020100")); + private final byte[] frame2Data = getBytesFromHexString("0D0C0B0A09080706050403020100"); + + private static final RtpPayloadFormat VP8_FORMAT = + new RtpPayloadFormat( + new Format.Builder() + .setSampleMimeType(MimeTypes.VIDEO_VP8) + .setSampleRate(500000) + .build(), + /* rtpPayloadType= */ 97, + /* clockRate= */ 48_000, + /* fmtpParameters= */ ImmutableMap.of()); + + @Rule + public final MockitoRule mockito = MockitoJUnit.rule(); + + private ParsableByteArray packetData; + + private RtpVp8Reader vp8Reader; + private FakeTrackOutput trackOutput; + @Mock + private ExtractorOutput extractorOutput; + + @Before + public void setUp() { + packetData = new ParsableByteArray(); + trackOutput = new FakeTrackOutput(/* deduplicateConsecutiveFormats= */ true); + when(extractorOutput.track(anyInt(), anyInt())).thenReturn(trackOutput); + vp8Reader = new RtpVp8Reader(VP8_FORMAT); + vp8Reader.createTracks(extractorOutput, /* trackId= */ 0); + } + + @Test + public void consume_validPackets() { + vp8Reader.onReceivingFirstPacket(frame1fragment1.timestamp, frame1fragment1.sequenceNumber); + consume(frame1fragment1); + consume(frame1fragment2); + consume(frame2fragment1); + consume(frame2fragment2); + + assertThat(trackOutput.getSampleCount()).isEqualTo(2); + assertThat(trackOutput.getSampleData(0)).isEqualTo(frame1Data); + assertThat(trackOutput.getSampleTimeUs(0)).isEqualTo(0); + assertThat(trackOutput.getSampleData(1)).isEqualTo(frame2Data); + assertThat(trackOutput.getSampleTimeUs(1)).isEqualTo(3200); + } + + @Test + public void consume_fragmentedFrameMissingFirstFragment() { + // First packet timing information is transmitted over RTSP, not RTP. + vp8Reader.onReceivingFirstPacket(frame1fragment1.timestamp, frame1fragment1.sequenceNumber); + consume(frame1fragment2); + consume(frame2fragment1); + consume(frame2fragment2); + + assertThat(trackOutput.getSampleCount()).isEqualTo(1); + assertThat(trackOutput.getSampleData(0)).isEqualTo(frame2Data); + assertThat(trackOutput.getSampleTimeUs(0)).isEqualTo(3200); + } + + @Test + public void consume_fragmentedFrameMissingBoundaryFragment() { + vp8Reader.onReceivingFirstPacket(frame1fragment1.timestamp, frame1fragment1.sequenceNumber); + consume(frame1fragment1); + consume(frame2fragment1); + consume(frame2fragment2); + + assertThat(trackOutput.getSampleCount()).isEqualTo(2); + assertThat(trackOutput.getSampleData(0)) + .isEqualTo(getBytesFromHexString("000102030405060708090A")); + assertThat(trackOutput.getSampleTimeUs(0)).isEqualTo(0); + assertThat(trackOutput.getSampleData(1)).isEqualTo(frame2Data); + assertThat(trackOutput.getSampleTimeUs(1)).isEqualTo(3200); + } + + @Test + public void consume_outOfOrderFragmentedFrame() { + vp8Reader.onReceivingFirstPacket(frame1fragment1.timestamp, frame1fragment1.sequenceNumber); + consume(frame1fragment1); + consume(frame2fragment1); + consume(frame1fragment2); + consume(frame2fragment2); + + assertThat(trackOutput.getSampleCount()).isEqualTo(2); + assertThat(trackOutput.getSampleData(0)) + .isEqualTo(getBytesFromHexString("000102030405060708090A")); + assertThat(trackOutput.getSampleTimeUs(0)).isEqualTo(0); + assertThat(trackOutput.getSampleData(1)).isEqualTo(frame2Data); + assertThat(trackOutput.getSampleTimeUs(1)).isEqualTo(3200); + } + + private static RtpPacket createRtpPacket( + long timestamp, int sequenceNumber, boolean marker, byte[] payloadData) { + return new RtpPacket.Builder() + .setTimestamp((int) timestamp) + .setSequenceNumber(sequenceNumber) + .setMarker(marker) + .setPayloadData(payloadData) + .build(); + } + + private void consume(RtpPacket rtpPacket) { + packetData.reset(rtpPacket.payloadData); + vp8Reader.consume( + packetData, + rtpPacket.timestamp, + rtpPacket.sequenceNumber, + /* isFrameBoundary= */ rtpPacket.marker); + } +}