From 3a87039ba19a72dc6671573036a1d11a873476fc Mon Sep 17 00:00:00 2001 From: Manisha Jajoo Date: Thu, 24 Mar 2022 01:39:10 +0530 Subject: [PATCH] Fixed review comments in RtpOpusReader --- .../exoplayer/rtsp/RtpPayloadFormat.java | 6 +-- .../media3/exoplayer/rtsp/RtspMediaTrack.java | 10 +++-- .../exoplayer/rtsp/reader/RtpOpusReader.java | 41 ++++++++----------- 3 files changed, 25 insertions(+), 32 deletions(-) diff --git a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtpPayloadFormat.java b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtpPayloadFormat.java index d1c56b3176..2e97fca3d5 100644 --- a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtpPayloadFormat.java +++ b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtpPayloadFormat.java @@ -37,19 +37,19 @@ import java.util.Map; public final class RtpPayloadFormat { private static final String RTP_MEDIA_AC3 = "AC3"; - private static final String RTP_MEDIA_OPUS = "OPUS"; - private static final String RTP_MEDIA_MPEG4_GENERIC = "MPEG4-GENERIC"; private static final String RTP_MEDIA_H264 = "H264"; private static final String RTP_MEDIA_H265 = "H265"; + private static final String RTP_MEDIA_MPEG4_GENERIC = "MPEG4-GENERIC"; + private static final String RTP_MEDIA_OPUS = "OPUS"; /** Returns whether the format of a {@link MediaDescription} is supported. */ public static boolean isFormatSupported(MediaDescription mediaDescription) { switch (Ascii.toUpperCase(mediaDescription.rtpMapAttribute.mediaEncoding)) { case RTP_MEDIA_AC3: - case RTP_MEDIA_OPUS: case RTP_MEDIA_H264: case RTP_MEDIA_H265: case RTP_MEDIA_MPEG4_GENERIC: + case RTP_MEDIA_OPUS: return true; default: return false; 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 b3e033c365..87c8e79893 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 @@ -56,6 +56,9 @@ import com.google.common.collect.ImmutableMap; private static final String GENERIC_CONTROL_ATTR = "*"; + /** RFC7587 Section 6.1 Sampling rate for OPUS is fixed at 48KHz. */ + private static final int OPUS_SAMPLING_RATE = 48000; + /** The track's associated {@link RtpPayloadFormat}. */ public final RtpPayloadFormat payloadFormat; /** The track's URI. */ @@ -122,12 +125,11 @@ import com.google.common.collect.ImmutableMap; processAacFmtpAttribute(formatBuilder, fmtpParameters, channelCount, clockRate); break; case MimeTypes.AUDIO_OPUS: - // RFC7587 Section 7 - checkArgument(channelCount == 2, "Invalid channel count"); - // RFC7587 Section 6.1 + checkArgument(channelCount != C.INDEX_UNSET); + // RFC7587 Section 6.1. // the RTP timestamp is incremented with a 48000 Hz clock rate // for all modes of Opus and all sampling rates. - checkArgument(clockRate == 48000, "Invalid sampling rate"); + checkArgument(clockRate == OPUS_SAMPLING_RATE, "Invalid sampling rate"); break; case MimeTypes.VIDEO_H264: checkArgument(!fmtpParameters.isEmpty()); diff --git a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpOpusReader.java b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpOpusReader.java index de1c8af21b..460f074a8d 100644 --- a/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpOpusReader.java +++ b/libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpOpusReader.java @@ -39,11 +39,11 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; private static final String TAG = "RtpOpusReader"; private final RtpPayloadFormat payloadFormat; + private static final long MEDIA_CLOCK_FREQUENCY = 48_000; + private @MonotonicNonNull TrackOutput trackOutput; private long firstReceivedTimestamp; private long startTimeOffsetUs; - - private final int sampleRate; private int previousSequenceNumber; private boolean foundOpusIDHeader; private boolean foundOpusCommentHeader; @@ -51,7 +51,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; public RtpOpusReader(RtpPayloadFormat payloadFormat) { this.payloadFormat = payloadFormat; this.firstReceivedTimestamp = C.INDEX_UNSET; - this.sampleRate = this.payloadFormat.clockRate; this.previousSequenceNumber = C.INDEX_UNSET; this.foundOpusIDHeader = false; this.foundOpusCommentHeader = false; @@ -75,23 +74,20 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ParsableByteArray data, long timestamp, int sequenceNumber, boolean rtpMarker) { checkStateNotNull(trackOutput); - /* RFC7845 Section 3 + /* RFC7845 Section 3. * +---------+ +----------------+ +--------------------+ +----- * |ID Header| | Comment Header | |Audio Data Packet 1 | | ... * +---------+ +----------------+ +--------------------+ +----- */ if (!foundOpusIDHeader) { - int currPosition = data.getPosition(); - checkArgument(isOpusIDHeader(data), "ID Header missing"); - - data.setPosition(currPosition); + checkForOpusIdHeader(data); List initializationData = OpusUtil.buildInitializationData(data.getData()); Format.Builder formatBuilder = payloadFormat.format.buildUpon(); formatBuilder.setInitializationData(initializationData); trackOutput.format(formatBuilder.build()); foundOpusIDHeader = true; } else if (!foundOpusCommentHeader) { - // Comment Header RFC7845 Section 5.2 + // Comment Header RFC7845 Section 5.2. String header = data.readString(8); checkArgument(header.equals("OpusTags"), "Comment Header should follow ID Header"); foundOpusCommentHeader = true; @@ -106,12 +102,13 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; expectedSequenceNumber, sequenceNumber)); } - // sending opus data + // sending opus data. int size = data.bytesLeft(); trackOutput.sampleData(data, size); long timeUs = - toSampleTimeUs(startTimeOffsetUs, timestamp, firstReceivedTimestamp, sampleRate); - trackOutput.sampleMetadata(timeUs, C.BUFFER_FLAG_KEY_FRAME, size, 0, null); + toSampleTimeUs(startTimeOffsetUs, timestamp, firstReceivedTimestamp); + trackOutput.sampleMetadata( + timeUs, C.BUFFER_FLAG_KEY_FRAME, size, /* offset*/ 0, /* cryptoData*/ null); } previousSequenceNumber = sequenceNumber; } @@ -124,29 +121,23 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; // Internal methods. - private static boolean isOpusIDHeader(ParsableByteArray data) { + private static void checkForOpusIdHeader(ParsableByteArray data) { + int currPosition = data.getPosition(); int sampleSize = data.limit(); String header = data.readString(8); - // Identification header RFC7845 Section 5.1 - if (sampleSize < 19 || !header.equals("OpusHead")) { - Log.e( - TAG, - Util.formatInvariant( - "first data octet of the RTP packet is not the beginning of a OpusHeader " - + "Dropping current packet")); - return false; - } + // Identification header RFC7845 Section 5.1. + checkArgument(sampleSize > 18 && header.equals("OpusHead"), "ID Header missing"); checkArgument(data.readUnsignedByte() == 1, "version number must always be 1"); - return true; + data.setPosition(currPosition); } /** Returns the correct sample time from RTP timestamp, accounting for the OPUS sampling rate. */ private static long toSampleTimeUs( - long startTimeOffsetUs, long rtpTimestamp, long firstReceivedRtpTimestamp, int sampleRate) { + long startTimeOffsetUs, long rtpTimestamp, long firstReceivedRtpTimestamp) { return startTimeOffsetUs + Util.scaleLargeTimestamp( rtpTimestamp - firstReceivedRtpTimestamp, /* multiplier= */ C.MICROS_PER_SECOND, - /* divisor= */ sampleRate); + /* divisor= */ MEDIA_CLOCK_FREQUENCY); } }