Fixed review comments in RtpOpusReader

3a87039b · Manisha Jajoo · be108b9e · 3a87039b · 3a87039b · 3a87039b
Commit 3a87039b authored Mar 24, 2022 by Manisha Jajoo
Showing with 25 additions and 32 deletions
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtpPayloadFormat.java
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaTrack.java
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpOpusReader.java
--- 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;

--- 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 != C.INDEX_UNSET);
-        checkArgument(channelCount == 2, "Invalid channel count");
+        // RFC7587 Section 6.1.
-        // 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());

--- 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();
+      checkForOpusIdHeader(data);
-      checkArgument(isOpusIDHeader(data), "ID Header missing");
-      data.setPosition(currPosition);
      List<byte[]> 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);
+          toSampleTimeUs(startTimeOffsetUs, timestamp, firstReceivedTimestamp);
-      trackOutput.sampleMetadata(timeUs, C.BUFFER_FLAG_KEY_FRAME, size, 0, null);
+      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
+    // Identification header RFC7845 Section 5.1.
-    if (sampleSize < 19 || !header.equals("OpusHead")) {
+    checkArgument(sampleSize > 18 && header.equals("OpusHead"), "ID Header missing");
-      Log.e(
-          TAG,
-          Util.formatInvariant(
-              "first data octet of the RTP packet is not the beginning of a OpusHeader "
-                  + "Dropping current packet"));
-      return false;
-    }
    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);
  }
 }