Changes based on internal review

androidx · Nov 30, 2023 · 5a446c2 · 5a446c2
1 parent 2c3eac4
commit 5a446c2
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 28 deletions.
diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/ts/AdtsReader.java b/libraries/extractor/src/main/java/androidx/media3/extractor/ts/AdtsReader.java
@@ -574,12 +574,11 @@ private void parseAdtsHeader() throws ParserException {
   }
 
   @RequiresNonNull("currentOutput")
-  void readAacProgramConfigElement() {
-    Format pendingOutputFormat = checkNotNull(this.pendingOutputFormat);
+  private void readAacProgramConfigElement() throws ParserException {
     ParsableBitArray pceBuffer = checkNotNull(this.pceBuffer);
 
     // See ISO 13818-7 Advanced Audio Coding (2006) Table 36 for PCE tag encoding.
-    if (pceBuffer.readBits(3) == 5 /* PCE tag */) {
+    if (pceBuffer.readBits(3) != 5 /* PCE tag */) {
       // See ISO 13818-7 Advanced Audio Coding (2006) Table 25 for syntax of a PCE.
       pceBuffer.skipBits(10); // element_instance_tag(4), profile(2), element_instance_tag(4)
 
@@ -614,37 +613,40 @@ void readAacProgramConfigElement() {
       int commentSizeBits = 8; // comment_field_bytes
 
       // Beyond this point, pceBuffer may be empty, so check before consuming.
-      if (pceBuffer.bitsLeft() >= channelBits + numAlignmentBits + commentSizeBits) {
-        pceBuffer.skipBits(channelBits);
+      if (pceBuffer.bitsLeft() < channelBits + numAlignmentBits + commentSizeBits) {
+        throw ParserException.createForMalformedContainer(/* message= */ null, /* cause= */ null);
+      }
 
-        // Store PCE size excluding initial PCE tag, alignment bits and comment for later.
-        int numPceBits = pceBuffer.getPosition() - 3 /* PCE tag */;
+      pceBuffer.skipBits(channelBits);
 
-        pceBuffer.skipBits(numAlignmentBits);
-        int commentSize = pceBuffer.readBits(commentSizeBits);
+      // Store PCE size excluding initial PCE tag, alignment bits and comment for later.
+      int numPceBits = pceBuffer.getPosition() - 3 /* PCE tag */;
+      pceBuffer.skipBits(numAlignmentBits);
+      int commentSize = pceBuffer.readBits(commentSizeBits);
 
-        if (sampleSize >= pceBuffer.getBytePosition() + commentSize) {
-          // Append PCE to format's audio specific config.
-          byte[] oldConfig = pendingOutputFormat.initializationData.get(0);
+      if (sampleSize < pceBuffer.getBytePosition() + commentSize) {
+        throw ParserException.createForMalformedContainer(/* message= */ null, /* cause= */ null);
+      }
 
-          int configSize = oldConfig.length;
-          configSize += (numPceBits + 7) / 8 + 1; // Byte align and add a zero length comment.
-          byte[] newConfig = Arrays.copyOf(oldConfig, configSize);
+      Format pendingOutputFormat = checkNotNull(this.pendingOutputFormat);
+      // Append PCE to format's audio specific config.
+      byte[] oldConfig = pendingOutputFormat.initializationData.get(0);
+      int configSize = oldConfig.length;
+      configSize += (numPceBits + 7) / 8 + 1; // Byte align and add a zero length comment.
+      byte[] newConfig = Arrays.copyOf(oldConfig, configSize);
 
-          pceBuffer.setPosition(3 /* PCE tag */);
-          pceBuffer.readBits(newConfig, oldConfig.length, numPceBits);
+      pceBuffer.setPosition(3 /* PCE tag */);
+      pceBuffer.readBits(newConfig, oldConfig.length, numPceBits);
 
-          pendingOutputFormat =
-              pendingOutputFormat
-                  .buildUpon()
-                  .setInitializationData(ImmutableList.of(newConfig))
-                  .build();
+      pendingOutputFormat =
+          pendingOutputFormat
+              .buildUpon()
+              .setInitializationData(ImmutableList.of(newConfig))
+              .build();
 
-          // Submit PCE-appended output format.
-          currentOutput.format(pendingOutputFormat);
-          hasOutputFormat = true;
-        }
-      }
+      // Submit PCE-appended output format.
+      this.currentOutput.format(pendingOutputFormat);
+      this.hasOutputFormat = true;
     }
 
     // Pass through all accumulated data as sample data.

diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/ts/AdtsReaderTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/ts/AdtsReaderTest.java
@@ -209,6 +209,22 @@ public void aacPceData() throws ParserException {
     adtsOutput.assertSample(0, AAC_PCE_ADTS_CONTENT, 0, C.BUFFER_FLAG_KEY_FRAME, null);
   }
 
+  @Test
+  public void aacPceDataSplit() throws ParserException {
+    byte[] first = Arrays.copyOf(AAC_PCE_TEST_DATA, AAC_PCE_ADTS_HEADER.length + 1);
+    byte[] second =
+        Arrays.copyOfRange(
+            AAC_PCE_TEST_DATA, AAC_PCE_ADTS_HEADER.length + 1, AAC_PCE_TEST_DATA.length);
+
+    data = new ParsableByteArray(first);
+    feed();
+    data = new ParsableByteArray(second);
+    feed();
+
+    assertSampleCounts(0, 1);
+    adtsOutput.assertSample(0, AAC_PCE_ADTS_CONTENT, 0, C.BUFFER_FLAG_KEY_FRAME, null);
+  }
+
   @Test
   public void aacPceDataFail() throws ParserException {
     data = new ParsableByteArray(Arrays.copyOf(AAC_PCE_TEST_DATA, AAC_PCE_TEST_DATA.length));
@@ -219,7 +235,7 @@ public void aacPceDataFail() throws ParserException {
     bytes[AAC_PCE_ADTS_HEADER.length] |= 0x20;
 
     // Should throw as FakeTrackOutput expects a format before sampleMetadata.
-    assertThrows(IllegalStateException.class, this::feed);
+    assertThrows(ParserException.class, this::feed);
   }
 
   private void feedLimited(int limit) throws ParserException {