Clean up ID3 frame implementations

library/src/main/java/com/google/android/exoplayer2/metadata/id3/ApicFrame.java

@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.metadata.id3;
 
 import android.os.Parcel;
 import android.os.Parcelable;
+import com.google.android.exoplayer2.util.Util;
 import java.util.Arrays;
 
 /**
@@ -39,8 +40,8 @@ public final class ApicFrame extends Id3Frame {
     this.pictureData = pictureData;
   }
 
-  public ApicFrame(Parcel in) {
-    super(in);
+  /* package */ ApicFrame(Parcel in) {
+    super(ID);
     mimeType = in.readString();
     description = in.readString();
     pictureType = in.readInt();
@@ -48,53 +49,49 @@ public final class ApicFrame extends Id3Frame {
   }
 
   @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-
-    ApicFrame that = (ApicFrame) o;
-
-    if (id != null ? !id.equals(that.id) : that.id != null) return false;
-    if (pictureType != that.pictureType) return false;
-    if (mimeType != null ? !mimeType.equals(that.mimeType) : that.mimeType != null)
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+    if (obj == null || getClass() != obj.getClass()) {
       return false;
-    if (description != null ? !description.equals(that.description) : that.description != null)
-      return false;
-    return Arrays.equals(pictureData, that.pictureData);
+    }
+    ApicFrame other = (ApicFrame) obj;
+    return pictureType == other.pictureType && Util.areEqual(mimeType, other.mimeType)
+        && Util.areEqual(description, other.description)
+        && Arrays.equals(pictureData, other.pictureData);
   }
 
   @Override
   public int hashCode() {
-    int result = id != null ? id.hashCode() : 0;
+    int result = 17;
+    result = 31 * result + pictureType;
     result = 31 * result + (mimeType != null ? mimeType.hashCode() : 0);
     result = 31 * result + (description != null ? description.hashCode() : 0);
-    result = 31 * result + pictureType;
     result = 31 * result + Arrays.hashCode(pictureData);
     return result;
   }
 
   @Override
   public void writeToParcel(Parcel dest, int flags) {
-    dest.writeString(id);
     dest.writeString(mimeType);
     dest.writeString(description);
     dest.writeInt(pictureType);
     dest.writeByteArray(pictureData);
   }
 
-  public static final Parcelable.Creator<ApicFrame> CREATOR =
-      new Parcelable.Creator<ApicFrame>() {
+  public static final Parcelable.Creator<ApicFrame> CREATOR = new Parcelable.Creator<ApicFrame>() {
 
-        @Override
-        public ApicFrame createFromParcel(Parcel in) {
-          return new ApicFrame(in);
-        }
+    @Override
+    public ApicFrame createFromParcel(Parcel in) {
+      return new ApicFrame(in);
+    }
 
-        @Override
-        public ApicFrame[] newArray(int size) {
-          return new ApicFrame[size];
-        }
+    @Override
+    public ApicFrame[] newArray(int size) {
+      return new ApicFrame[size];
+    }
 
-      };
+  };
 
 }

library/src/main/java/com/google/android/exoplayer2/metadata/id3/BinaryFrame.java

@@ -26,31 +26,32 @@ public final class BinaryFrame extends Id3Frame {
 
   public final byte[] data;
 
-  public BinaryFrame(String type, byte[] data) {
-    super(type);
+  public BinaryFrame(String id, byte[] data) {
+    super(id);
     this.data = data;
   }
 
-  public BinaryFrame(Parcel in) {
-    super(in);
+  /* package */ BinaryFrame(Parcel in) {
+    super(in.readString());
     data = in.createByteArray();
   }
 
   @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-
-    BinaryFrame that = (BinaryFrame) o;
-
-    if (id != null ? !id.equals(that.id) : that.id != null)
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+    if (obj == null || getClass() != obj.getClass()) {
       return false;
-    return Arrays.equals(data, that.data);
+    }
+    BinaryFrame other = (BinaryFrame) obj;
+    return id.equals(other.id) && Arrays.equals(data, other.data);
   }
 
   @Override
   public int hashCode() {
-    int result = id != null ? id.hashCode() : 0;
+    int result = 17;
+    result = 31 * result + id.hashCode();
     result = 31 * result + Arrays.hashCode(data);
     return result;
   }

library/src/main/java/com/google/android/exoplayer2/metadata/id3/CommentFrame.java

@@ -17,43 +17,51 @@ package com.google.android.exoplayer2.metadata.id3;
 
 import android.os.Parcel;
 import android.os.Parcelable;
+import com.google.android.exoplayer2.util.Util;
 
 /**
  * Comment ID3 frame.
  */
 public final class CommentFrame extends Id3Frame {
 
+  public static final String ID = "COMM";
+
   public final String language;
+  public final String description;
   public final String text;
 
   public CommentFrame(String language, String description, String text) {
-    super(description);
+    super(ID);
     this.language = language;
+    this.description = description;
     this.text = text;
   }
 
-  public CommentFrame(Parcel in) {
-    super(in);
+  /* package */ CommentFrame(Parcel in) {
+    super(ID);
     language = in.readString();
+    description = in.readString();
     text = in.readString();
   }
 
   @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-
-    CommentFrame that = (CommentFrame) o;
-
-    if (id != null ? !id.equals(that.id) : that.id != null) return false;
-    if (language != null ? !language.equals(that.language) : that.language != null) return false;
-    return text != null ? text.equals(that.text) : that.text == null;
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+    if (obj == null || getClass() != obj.getClass()) {
+      return false;
+    }
+    CommentFrame other = (CommentFrame) obj;
+    return Util.areEqual(description, other.description) && Util.areEqual(language, other.language)
+        && Util.areEqual(text, other.text);
   }
 
   @Override
   public int hashCode() {
-    int result = id != null ? id.hashCode() : 0;
+    int result = 17;
     result = 31 * result + (language != null ? language.hashCode() : 0);
+    result = 31 * result + (description != null ? description.hashCode() : 0);
     result = 31 * result + (text != null ? text.hashCode() : 0);
     return result;
   }

library/src/main/java/com/google/android/exoplayer2/metadata/id3/GeobFrame.java

@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.metadata.id3;
 
 import android.os.Parcel;
 import android.os.Parcelable;
+import com.google.android.exoplayer2.util.Util;
 import java.util.Arrays;
 
 /**
@@ -39,8 +40,8 @@ public final class GeobFrame extends Id3Frame {
     this.data = data;
   }
 
-  public GeobFrame(Parcel in) {
-    super(in);
+  /* package */ GeobFrame(Parcel in) {
+    super(ID);
     mimeType = in.readString();
     filename = in.readString();
     description = in.readString();
@@ -48,25 +49,21 @@ public final class GeobFrame extends Id3Frame {
   }
 
   @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-
-    GeobFrame that = (GeobFrame) o;
-
-    if (id != null ? !id.equals(that.id) : that.id != null) return false;
-    if (mimeType != null ? !mimeType.equals(that.mimeType) : that.mimeType != null)
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+    if (obj == null || getClass() != obj.getClass()) {
       return false;
-    if (filename != null ? !filename.equals(that.filename) : that.filename != null)
-      return false;
-    if (description != null ? !description.equals(that.description) : that.description != null)
-      return false;
-    return Arrays.equals(data, that.data);
+    }
+    GeobFrame other = (GeobFrame) obj;
+    return Util.areEqual(mimeType, other.mimeType) && Util.areEqual(filename, other.filename)
+        && Util.areEqual(description, other.description) && Arrays.equals(data, other.data);
   }
 
   @Override
   public int hashCode() {
-    int result = id != null ? id.hashCode() : 0;
+    int result = 17;
     result = 31 * result + (mimeType != null ? mimeType.hashCode() : 0);
     result = 31 * result + (filename != null ? filename.hashCode() : 0);
     result = 31 * result + (description != null ? description.hashCode() : 0);
@@ -76,26 +73,24 @@ public final class GeobFrame extends Id3Frame {
 
   @Override
   public void writeToParcel(Parcel dest, int flags) {
-    dest.writeString(id);
     dest.writeString(mimeType);
     dest.writeString(filename);
     dest.writeString(description);
     dest.writeByteArray(data);
   }
 
-  public static final Parcelable.Creator<GeobFrame> CREATOR =
-      new Parcelable.Creator<GeobFrame>() {
+  public static final Parcelable.Creator<GeobFrame> CREATOR = new Parcelable.Creator<GeobFrame>() {
 
-        @Override
-        public GeobFrame createFromParcel(Parcel in) {
-          return new GeobFrame(in);
-        }
+    @Override
+    public GeobFrame createFromParcel(Parcel in) {
+      return new GeobFrame(in);
+    }
 
-        @Override
-        public GeobFrame[] newArray(int size) {
-          return new GeobFrame[size];
-        }
+    @Override
+    public GeobFrame[] newArray(int size) {
+      return new GeobFrame[size];
+    }
 
-      };
+  };
 
 }

library/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java

@@ -345,8 +345,8 @@ public final class Id3Decoder implements MetadataDecoder<Metadata> {
     return new TextInformationFrame(id, description);
   }
 
-  private static CommentFrame decodeCommentFrame(ParsableByteArray id3Data,
-      int frameSize) throws UnsupportedEncodingException {
+  private static CommentFrame decodeCommentFrame(ParsableByteArray id3Data, int frameSize)
+      throws UnsupportedEncodingException {
     int encoding = id3Data.readUnsignedByte();
     String charset = getCharsetName(encoding);
 
@@ -360,11 +360,11 @@ public final class Id3Decoder implements MetadataDecoder<Metadata> {
     int descriptionEndIndex = indexOfEos(data, 0, encoding);
     String description = new String(data, 0, descriptionEndIndex, charset);
 
-    int valueStartIndex = descriptionEndIndex + delimiterLength(encoding);
-    int valueEndIndex = indexOfEos(data, valueStartIndex, encoding);
-    String value = new String(data, valueStartIndex, valueEndIndex - valueStartIndex, charset);
+    int textStartIndex = descriptionEndIndex + delimiterLength(encoding);
+    int textEndIndex = indexOfEos(data, textStartIndex, encoding);
+    String text = new String(data, textStartIndex, textEndIndex - textStartIndex, charset);
 
-    return new CommentFrame(language, description, value);
+    return new CommentFrame(language, description, text);
   }
 
   private static BinaryFrame decodeBinaryFrame(ParsableByteArray id3Data, int frameSize,

library/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Frame.java

@@ -15,8 +15,8 @@
  */
 package com.google.android.exoplayer2.metadata.id3;
 
-import android.os.Parcel;
 import android.os.Parcelable;
+import com.google.android.exoplayer2.util.Assertions;
 
 /**
  * Base class for ID3 frames.
@@ -29,11 +29,7 @@ public abstract class Id3Frame implements Parcelable {
   public final String id;
 
   public Id3Frame(String id) {
-    this.id = id;
-  }
-
-  protected Id3Frame(Parcel in) {
-    id = in.readString();
+    this.id = Assertions.checkNotNull(id);
   }
 
   @Override

library/src/main/java/com/google/android/exoplayer2/metadata/id3/PrivFrame.java

@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.metadata.id3;
 
 import android.os.Parcel;
 import android.os.Parcelable;
+import com.google.android.exoplayer2.util.Util;
 import java.util.Arrays;
 
 /**
@@ -35,27 +36,27 @@ public final class PrivFrame extends Id3Frame {
     this.privateData = privateData;
   }
 
-  public PrivFrame(Parcel in) {
-    super(in);
+  /* package */ PrivFrame(Parcel in) {
+    super(ID);
     owner = in.readString();
     privateData = in.createByteArray();
   }
 
   @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-
-    PrivFrame that = (PrivFrame) o;
-
-    if (id != null ? !id.equals(that.id) : that.id != null) return false;
-    if (owner != null ? !owner.equals(that.owner) : that.owner != null) return false;
-    return Arrays.equals(privateData, that.privateData);
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+    if (obj == null || getClass() != obj.getClass()) {
+      return false;
+    }
+    PrivFrame other = (PrivFrame) obj;
+    return Util.areEqual(owner, other.owner) && Arrays.equals(privateData, other.privateData);
   }
 
   @Override
   public int hashCode() {
-    int result = id != null ? id.hashCode() : 0;
+    int result = 17;
     result = 31 * result + (owner != null ? owner.hashCode() : 0);
     result = 31 * result + Arrays.hashCode(privateData);
     return result;
@@ -63,24 +64,22 @@ public final class PrivFrame extends Id3Frame {
 
   @Override
   public void writeToParcel(Parcel dest, int flags) {
-    dest.writeString(id);
     dest.writeString(owner);
     dest.writeByteArray(privateData);
   }
 
-  public static final Parcelable.Creator<PrivFrame> CREATOR =
-      new Parcelable.Creator<PrivFrame>() {
+  public static final Parcelable.Creator<PrivFrame> CREATOR = new Parcelable.Creator<PrivFrame>() {
 
-        @Override
-        public PrivFrame createFromParcel(Parcel in) {
-          return new PrivFrame(in);
-        }
+    @Override
+    public PrivFrame createFromParcel(Parcel in) {
+      return new PrivFrame(in);
+    }
 
-        @Override
-        public PrivFrame[] newArray(int size) {
-          return new PrivFrame[size];
-        }
+    @Override
+    public PrivFrame[] newArray(int size) {
+      return new PrivFrame[size];
+    }
 
-      };
+  };
 
 }

library/src/main/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrame.java

@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.metadata.id3;
 
 import android.os.Parcel;
 import android.os.Parcelable;
+import com.google.android.exoplayer2.util.Util;
 
 /**
  * Text information ("T000" - "TZZZ", excluding "TXXX") ID3 frame.
@@ -30,25 +31,27 @@ public final class TextInformationFrame extends Id3Frame {
     this.description = description;
   }
 
-  public TextInformationFrame(Parcel in) {
-    super(in);
+  /* package */ TextInformationFrame(Parcel in) {
+    super(in.readString());
     description = in.readString();
   }
 
   @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-
-    TextInformationFrame that = (TextInformationFrame) o;
-
-    if (id != null ? !id.equals(that.id) : that.id != null) return false;
-    return description != null ? description.equals(that.description) : that.description == null;
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+    if (obj == null || getClass() != obj.getClass()) {
+      return false;
+    }
+    TextInformationFrame other = (TextInformationFrame) obj;
+    return id.equals(other.id) && Util.areEqual(description, other.description);
   }
 
   @Override
   public int hashCode() {
-    int result = id != null ? id.hashCode() : 0;
+    int result = 17;
+    result = 31 * result + id.hashCode();
     result = 31 * result + (description != null ? description.hashCode() : 0);
     return result;
   }

library/src/main/java/com/google/android/exoplayer2/metadata/id3/TxxxFrame.java

@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.metadata.id3;
 
 import android.os.Parcel;
 import android.os.Parcelable;
+import com.google.android.exoplayer2.util.Util;
 
 /**
  * TXXX (User defined text information) ID3 frame.
@@ -34,28 +35,27 @@ public final class TxxxFrame extends Id3Frame {
     this.value = value;
   }
 
-  public TxxxFrame(Parcel in) {
-    super(in);
+  /* package */ TxxxFrame(Parcel in) {
+    super(ID);
     description = in.readString();
     value = in.readString();
   }
 
   @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-
-    TxxxFrame that = (TxxxFrame) o;
-
-    if (id != null ? !id.equals(that.id) : that.id != null) return false;
-    if (description != null ? !description.equals(that.description) : that.description != null)
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+    if (obj == null || getClass() != obj.getClass()) {
       return false;
-    return value != null ? value.equals(that.value) : that.value == null;
+    }
+    TxxxFrame other = (TxxxFrame) obj;
+    return Util.areEqual(description, other.description) && Util.areEqual(value, other.value);
   }
 
   @Override
   public int hashCode() {
-    int result = id != null ? id.hashCode() : 0;
+    int result = 17;
     result = 31 * result + (description != null ? description.hashCode() : 0);
     result = 31 * result + (value != null ? value.hashCode() : 0);
     return result;
@@ -63,24 +63,22 @@ public final class TxxxFrame extends Id3Frame {
 
   @Override
   public void writeToParcel(Parcel dest, int flags) {
-    dest.writeString(id);
     dest.writeString(description);
     dest.writeString(value);
   }
 
-  public static final Parcelable.Creator<TxxxFrame> CREATOR =
-      new Parcelable.Creator<TxxxFrame>() {
+  public static final Parcelable.Creator<TxxxFrame> CREATOR = new Parcelable.Creator<TxxxFrame>() {
 
-        @Override
-        public TxxxFrame createFromParcel(Parcel in) {
-          return new TxxxFrame(in);
-        }
+    @Override
+    public TxxxFrame createFromParcel(Parcel in) {
+      return new TxxxFrame(in);
+    }
 
-        @Override
-        public TxxxFrame[] newArray(int size) {
-          return new TxxxFrame[size];
-        }
+    @Override
+    public TxxxFrame[] newArray(int size) {
+      return new TxxxFrame[size];
+    }
 
-      };
+  };
 
 }