Further adjustments to PR #1015

- Added back parsing of scheme data for version 1 as it's technically
  allowed by the spec.
- Made constructor of PsshAtom private to only publish the data class
  and not the constructor.
- Formatting and Javadoc adjustments
- Additional tests

libraries/extractor/src/main/java/androidx/media3/extractor/mp4/PsshAtomUtil.java

@@ -163,19 +163,15 @@ public final class PsshAtomUtil {
     atomData.setPosition(0);
     int bufferLength = atomData.bytesLeft();
     int atomSize = atomData.readInt();
-
     if (atomSize != bufferLength) {
       Log.w(
           TAG,
-          "Advertised atom size ("
+          "Advertised atom size (" + atomSize + ") does not match buffer size: " + bufferLength);
-              + atomSize
-              + ") does not match current buffer size: "
-              + bufferLength);
       return null;
     }
     int atomType = atomData.readInt();
     if (atomType != Atom.TYPE_pssh) {
-      Log.w(TAG, "Atom Type is not pssh: " + atomType);
+      Log.w(TAG, "Atom type is not pssh: " + atomType);
       return null;
     }
     int atomVersion = Atom.parseFullAtomVersion(atomData.readInt());
@@ -185,42 +181,41 @@ public final class PsshAtomUtil {
     }
     UUID uuid = new UUID(atomData.readLong(), atomData.readLong());
     UUID[] keyIds = null;
-    int dataSize = 0;
     if (atomVersion == 1) {
       int keyIdCount = atomData.readUnsignedIntToInt();
       keyIds = new UUID[keyIdCount];
       for (int i = 0; i < keyIdCount; ++i) {
         keyIds[i] = new UUID(atomData.readLong(), atomData.readLong());
       }
-    } else if (atomVersion == 0) {
+    }
-      dataSize = atomData.readUnsignedIntToInt();
+    int dataSize = atomData.readUnsignedIntToInt();
     bufferLength = atomData.bytesLeft();
     if (dataSize != bufferLength) {
       Log.w(
-            TAG,
+          TAG, "Atom data size (" + dataSize + ") does not match the bytes left: " + bufferLength);
-            "Atom data size (" + dataSize + ") does not match the bytes left: " + bufferLength);
       return null;
     }
-    }
     byte[] data = new byte[dataSize];
     atomData.readBytes(data, 0, dataSize);
     return new PsshAtom(uuid, atomVersion, data, keyIds);
   }
 
-  /**
+  /** A class representing the mp4 PSSH Atom as specified in ISO/IEC 23001-7. */
-   * A class representing the mp4 PSSH Atom as specified in the CENC standard - systemId the UUID of
+  public static final class PsshAtom {
-   * the encryption system as specified in ISO/IEC 23009-1 section 5.8.4.1 - version the version of
-   * the PSSH atom, should be 0 or 1 - schemaData the binary data in the atom - keyIds the optional
-   * set of keyIds associated with the
-   */
-  public static class PsshAtom {
 
+    /** The UUID of the encryption system as specified in ISO/IEC 23009-1 section 5.8.4.1. */
     public final UUID uuid;
+
+    /** The version of the PSSH atom, either 0 or 1. */
     public final int version;
+
+    /** Binary scheme data. */
     public final byte[] schemeData;
+
+    /** Array of key IDs. Always null for version 0 and non-null for version 1. */
     @Nullable public final UUID[] keyIds;
 
-    public PsshAtom(UUID uuid, int version, byte[] schemeData, @Nullable UUID[] keyIds) {
+    /* package */ PsshAtom(UUID uuid, int version, byte[] schemeData, @Nullable UUID[] keyIds) {
       this.uuid = uuid;
       this.version = version;
       this.schemeData = schemeData;

libraries/extractor/src/test/java/androidx/media3/extractor/mp4/PsshAtomUtilTest.java

@@ -33,10 +33,11 @@ import org.junit.runner.RunWith;
 public final class PsshAtomUtilTest {
 
   @Test
-  public void buildPsshAtom() {
+  public void buildPsshAtom_version0_returnsCorrectBytes() {
     byte[] schemeData = new byte[] {0, 1, 2, 3, 4, 5};
+
     byte[] psshAtom = PsshAtomUtil.buildPsshAtom(C.WIDEVINE_UUID, schemeData);
-    // Read the PSSH atom back and assert its content is as expected.
+
     ParsableByteArray parsablePsshAtom = new ParsableByteArray(psshAtom);
     assertThat(parsablePsshAtom.readUnsignedIntToInt()).isEqualTo(psshAtom.length); // length
     assertThat(parsablePsshAtom.readInt()).isEqualTo(TYPE_pssh); // type
@@ -52,12 +53,14 @@ public final class PsshAtomUtilTest {
   }
 
   @Test
-  public void buildVersion1Pssh() {
+  public void buildPsshAtom_version1_returnsCorrectBytes() {
     byte[] schemeData = new byte[] {0, 1, 2, 3, 4, 5};
     UUID[] keyIds = new UUID[2];
     keyIds[0] = UUID.fromString("edef8ba9-79d6-4ace-a3c8-27dcd51d21ed");
     keyIds[1] = UUID.fromString("dc03d7f3-334d-b858-f114-9ab759a925fb");
+
     byte[] psshAtom = PsshAtomUtil.buildPsshAtom(C.WIDEVINE_UUID, keyIds, schemeData);
+
     ParsableByteArray parsablePsshAtom = new ParsableByteArray(psshAtom);
     assertThat(parsablePsshAtom.readUnsignedIntToInt()).isEqualTo(psshAtom.length);
     assertThat(parsablePsshAtom.readInt()).isEqualTo(TYPE_pssh); // type
@@ -78,25 +81,178 @@ public final class PsshAtomUtilTest {
   }
 
   @Test
-  public void parseV1Atom() {
+  public void parsePsshAtom_version0_parsesCorrectData() {
     byte[] psshBuffer = {
-        0x00, 0x00, 0x00, 0x44, 0x70, 0x73, 0x73, 0x68, // BMFF box header (68 bytes, 'pssh')
+      // BMFF box header (36 bytes, 'pssh')
-        0x01, 0x00, 0x00, 0x00, // Full box header (version = 1, flags = 0)
+      0x00,
-        0x10, 0x77, -0x11, -0x14, -0x40, -0x4e, 0x4d, 0x02, // SystemID
+      0x00,
-        -0x54, -0x1d, 0x3c, 0x1e, 0x52, -0x1e, -0x05, 0x4b, 0x00, 0x00, 0x00, 0x02, // KID_count (2)
+      0x00,
-        0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, // First KID ("0123456789012345")
+      0x24,
-        0x38, 0x39, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47,
+      0x70,
-        0x48, // Second KID ("ABCDEFGHIJKLMNOP")
+      0x73,
-        0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, 0x50, 0x00, 0x00, 0x00, 0x00, // Size of Data (0)
+      0x73,
+      0x68,
+      // Full box header (version = 0, flags = 0)
+      0x00,
+      0x00,
+      0x00,
+      0x00,
+      // SystemID
+      0x10,
+      0x77,
+      -0x11,
+      -0x14,
+      -0x40,
+      -0x4e,
+      0x4d,
+      0x02,
+      -0x54,
+      -0x1d,
+      0x3c,
+      0x1e,
+      0x52,
+      -0x1e,
+      -0x05,
+      0x4b,
+      // Size of Data (4)
+      0x00,
+      0x00,
+      0x00,
+      0x04,
+      // Data bytes
+      0x1a,
+      0x1b,
+      0x1c,
+      0x1d
     };
+
     PsshAtomUtil.PsshAtom psshAtom = PsshAtomUtil.parsePsshAtom(psshBuffer);
+
+    assertThat(psshAtom).isNotNull();
+    assertThat(psshAtom.version).isEqualTo(0);
+    assertThat(psshAtom.uuid).isEqualTo(UUID.fromString("1077efec-c0b2-4d02-ace3-3c1e52e2fb4b"));
+    assertThat(psshAtom.keyIds).isNull();
+    assertThat(psshAtom.schemeData).isEqualTo(new byte[] {0x1a, 0x1b, 0x1c, 0x1d});
+  }
+
+  @Test
+  public void parsePsshAtom_version1_parsesCorrectData() {
+    byte[] psshBuffer = {
+      // BMFF box header (68 bytes, 'pssh')
+      0x00,
+      0x00,
+      0x00,
+      0x44,
+      0x70,
+      0x73,
+      0x73,
+      0x68,
+      // Full box header (version = 1, flags = 0)
+      0x01,
+      0x00,
+      0x00,
+      0x00,
+      // SystemID
+      0x10,
+      0x77,
+      -0x11,
+      -0x14,
+      -0x40,
+      -0x4e,
+      0x4d,
+      0x02,
+      -0x54,
+      -0x1d,
+      0x3c,
+      0x1e,
+      0x52,
+      -0x1e,
+      -0x05,
+      0x4b,
+      // KID_count (2)
+      0x00,
+      0x00,
+      0x00,
+      0x02,
+      // First KID ("0123456789012345")
+      0x30,
+      0x31,
+      0x32,
+      0x33,
+      0x34,
+      0x35,
+      0x36,
+      0x37,
+      0x38,
+      0x39,
+      0x30,
+      0x31,
+      0x32,
+      0x33,
+      0x34,
+      0x35,
+      // Second KID ("ABCDEFGHIJKLMNOP")
+      0x41,
+      0x42,
+      0x43,
+      0x44,
+      0x45,
+      0x46,
+      0x47,
+      0x48,
+      0x49,
+      0x4a,
+      0x4b,
+      0x4c,
+      0x4d,
+      0x4e,
+      0x4f,
+      0x50,
+      // Size of Data (0)
+      0x00,
+      0x00,
+      0x00,
+      0x00,
+    };
+
+    PsshAtomUtil.PsshAtom psshAtom = PsshAtomUtil.parsePsshAtom(psshBuffer);
+
     assertThat(psshAtom).isNotNull();
-    System.out.println("psshAtom: " + psshAtom);
     assertThat(psshAtom.version).isEqualTo(1);
-    assertThat(psshAtom.uuid)
+    assertThat(psshAtom.uuid).isEqualTo(UUID.fromString("1077efec-c0b2-4d02-ace3-3c1e52e2fb4b"));
-        .isEqualTo(UUID.fromString("1077efec-c0b2-4d02-ace3-3c1e52e2fb4b"));
     assertThat(psshAtom.keyIds).isNotNull();
-    assertThat(psshAtom.keyIds.length).isEqualTo(2);
+    assertThat(psshAtom.keyIds).hasLength(2);
     assertThat(psshAtom.schemeData).isEmpty();
   }
+
+  @Test
+  public void parsePsshAtom_version0FromBuildPsshAtom_returnsEqualData() {
+    byte[] schemeData = new byte[] {0, 1, 2, 3, 4, 5};
+
+    PsshAtomUtil.PsshAtom parsedAtom =
+        PsshAtomUtil.parsePsshAtom(PsshAtomUtil.buildPsshAtom(C.WIDEVINE_UUID, schemeData));
+
+    assertThat(parsedAtom).isNotNull();
+    assertThat(parsedAtom.version).isEqualTo(0);
+    assertThat(parsedAtom.keyIds).isNull();
+    assertThat(parsedAtom.uuid).isEqualTo(C.WIDEVINE_UUID);
+    assertThat(parsedAtom.schemeData).isEqualTo(new byte[] {0, 1, 2, 3, 4, 5});
+  }
+
+  @Test
+  public void parsePsshAtom_version1FromBuildPsshAtom_returnsEqualData() {
+    byte[] schemeData = new byte[] {0, 1, 2, 3, 4, 5};
+    UUID[] keyIds = new UUID[2];
+    keyIds[0] = UUID.fromString("edef8ba9-79d6-4ace-a3c8-27dcd51d21ed");
+    keyIds[1] = UUID.fromString("dc03d7f3-334d-b858-f114-9ab759a925fb");
+
+    PsshAtomUtil.PsshAtom parsedAtom =
+        PsshAtomUtil.parsePsshAtom(PsshAtomUtil.buildPsshAtom(C.WIDEVINE_UUID, keyIds, schemeData));
+
+    assertThat(parsedAtom).isNotNull();
+    assertThat(parsedAtom.version).isEqualTo(1);
+    assertThat(parsedAtom.keyIds).isEqualTo(keyIds);
+    assertThat(parsedAtom.uuid).isEqualTo(C.WIDEVINE_UUID);
+    assertThat(parsedAtom.schemeData).isEqualTo(new byte[] {0, 1, 2, 3, 4, 5});
+  }
 }