From ff80a41f908cfe4b0d7203f466b1b4e4aa6c27d7 Mon Sep 17 00:00:00 2001 From: Rakesh Kumar Date: Thu, 5 May 2022 14:01:06 +0530 Subject: [PATCH] Fix review comment in RTP H263 Reader Change-Id: I987baf379ecf3ba3f387cb38f22646023739addb --- .../media3/exoplayer/rtsp/RtspMediaTrack.java | 20 +++++++++++-- .../exoplayer/rtsp/reader/RtpH263Reader.java | 28 +++++++++++++------ 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaTrack.java b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaTrack.java index 69f23b9e34..0d2ff5912a 100644 --- a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaTrack.java +++ b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaTrack.java @@ -101,8 +101,23 @@ import com.google.common.collect.ImmutableMap; */ private static final int DEFAULT_VP8_HEIGHT = 240; - /** Default width and height for H263. */ + /** + * Default height for H263. + * + *

RFC4629 does not mandate codec specific data (like width and height) in the fmtp attribute. + * These values are taken from Android's software H263 decoder. + */ private static final int DEFAULT_H263_WIDTH = 352; + /** + * Default height for H263. + * + *

RFC4629 does not mandate codec specific data (like width and height) in the fmtp attribute. + * These values are taken from Android's software H263 decoder. + */ private static final int DEFAULT_H263_HEIGHT = 288; /** The track's associated {@link RtpPayloadFormat}. */ @@ -189,7 +204,8 @@ import com.google.common.collect.ImmutableMap; processMPEG4FmtpAttribute(formatBuilder, fmtpParameters); break; case MimeTypes.VIDEO_H263: - // H263 does not require a FMTP attribute. So Setting default width and height. + // H263 never uses fmtp width and height attributes (RFC4629 Section 8.2), setting default + // width and height. formatBuilder.setWidth(DEFAULT_H263_WIDTH).setHeight(DEFAULT_H263_HEIGHT); break; case MimeTypes.VIDEO_H264: diff --git a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpH263Reader.java b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpH263Reader.java index 1c12075f89..f4f96cd1e9 100644 --- a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpH263Reader.java +++ b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpH263Reader.java @@ -30,28 +30,31 @@ import androidx.media3.extractor.TrackOutput; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** - * Parses a H263 byte stream carried on RTP packets, and extracts H263 individual video frames as - * defined in RFC4629. + * Parses a H263 byte stream carried on RTP packets, and extracts H263 frames as defined in RFC4629. */ /* package */ final class RtpH263Reader implements RtpPayloadReader { private static final String TAG = "RtpH263Reader"; private static final long MEDIA_CLOCK_FREQUENCY = 90_000; - /** VOP unit type. */ + /** I-frame VOP unit type. */ private static final int I_VOP = 0; private final RtpPayloadFormat payloadFormat; private @MonotonicNonNull TrackOutput trackOutput; + /** + * First received RTP timestamp. All RTP timestamps are dimension-less, the time base is defined + * by {@link #MEDIA_CLOCK_FREQUENCY}. + */ private long firstReceivedTimestamp; private int previousSequenceNumber; /** The combined size of a sample that is fragmented into multiple RTP packets. */ private int fragmentedSampleSizeBytes; - private static int width; - private static int height; - private static boolean isKeyFrame; + private int width; + private int height; + private boolean isKeyFrame; private boolean isOutputFormatSet; private long startTimeOffsetUs; @@ -144,7 +147,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; isKeyFrame ? C.BUFFER_FLAG_KEY_FRAME : 0, fragmentedSampleSizeBytes, /* offset= */ 0, - /* encryptionData= */ null); + /* cryptoData= */ null); fragmentedSampleSizeBytes = 0; isKeyFrame = false; } @@ -163,12 +166,21 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; * Parses VOP Coding type and resolution. */ private void getBufferFlagsAndResolutionFromVop(ParsableByteArray data, boolean gotResolution) { + // Picture Segment Packets (RFC4629 Section 6.1). // Search for SHORT_VIDEO_START_MARKER (0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0). int currPosition = data.getPosition(); - if (data.readUnsignedInt() >> 10 == 0x20) { + long shortHeader = data.readUnsignedInt(); + if ((shortHeader & 0xffff) >> 10 == 0x20) { int header = data.peekUnsignedByte(); int vopType = ((header >> 1) & 0x01); if (!gotResolution && vopType == I_VOP) { + /** + * Parsing resolution from source format. + * + *

These values are taken from Android's software H263 decoder. + */ int sourceFormat = ((header >> 2) & 0x07); if (sourceFormat == 1) { width = 128;