Pull request review fixes

library/core/proguard-rules.txt

@@ -5,3 +5,6 @@
 -keepclassmembers class com.google.android.exoplayer2.text.cea.Cea708Decoder {
     public <init>(int);
 }
+-keepclassmembers class com.google.android.exoplayer2.text.dvb.DvbDecoder {
+    public <init>(java.util.List);
+}
\ No newline at end of file

library/core/src/main/java/com/google/android/exoplayer2/extractor/mkv/MatroskaExtractor.java

@@ -1235,7 +1235,8 @@ public final class MatroskaExtractor implements Extractor {
         || CODEC_ID_SUBRIP.equals(codecId)
         || CODEC_ID_VOBSUB.equals(codecId)
         || CODEC_ID_PGS.equals(codecId)
-        || CODEC_ID_DVBSUB.equals(codecId);  }
+        || CODEC_ID_DVBSUB.equals(codecId);
+  }
 
   /**
    * Returns an array that can store (at least) {@code length} elements, which will be either a new
@@ -1464,11 +1465,8 @@ public final class MatroskaExtractor implements Extractor {
           break;
         case CODEC_ID_DVBSUB:
           mimeType = MimeTypes.APPLICATION_DVBSUBS;
-          initializationData = new ArrayList<>(4);
-          initializationData.add(null);
-          initializationData.add(new byte[] {codecPrivate[0], codecPrivate[1]});
-          initializationData.add(new byte[] {codecPrivate[2], codecPrivate[3]});
-          initializationData.add("mkv".getBytes());
+          initializationData = Collections.singletonList(new byte[] {
+              (byte) 0x01, codecPrivate[0], codecPrivate[1], codecPrivate[2], codecPrivate[3]});
           break;
         default:
           throw new ParserException("Unrecognized codec identifier.");

library/core/src/main/java/com/google/android/exoplayer2/extractor/ts/DvbSubtitlesReader.java

@@ -24,66 +24,68 @@ import com.google.android.exoplayer2.extractor.ts.TsPayloadReader.TrackIdGenerat
 import com.google.android.exoplayer2.util.MimeTypes;
 import com.google.android.exoplayer2.util.ParsableByteArray;
 
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 
-public class DvbSubtitlesReader implements ElementaryStreamReader {
+/**
+ * Output PES packets to a {@link TrackOutput}.
+ */
+public final class DvbSubtitlesReader implements ElementaryStreamReader {
 
-    private static final String TAG= "DVBSubsReader";
-    private final String language;
-    private List<byte[]> initializationData = new ArrayList<>();
+  private final String language;
+  private List<byte[]> initializationData;
 
-    private long sampleTimeUs;
-    private int totalBytesWritten;
-    private boolean writingSample;
+  private long sampleTimeUs;
+  private int sampleBytesWritten;
+  private boolean writingSample;
 
-    private TrackOutput output;
+  private TrackOutput output;
 
-    public DvbSubtitlesReader(TsPayloadReader.EsInfo esInfo) {
-        // we only support one subtitle service per PID
-        this.language = esInfo.language;
-        this.initializationData.add(new byte[] {esInfo.descriptorBytes[5]}); // subtitle subtype
-        this.initializationData.add(new byte[] {esInfo.descriptorBytes[6], esInfo.descriptorBytes[7]}); // subtitle compose page
-        this.initializationData.add(new byte[] {esInfo.descriptorBytes[8], esInfo.descriptorBytes[9]}); // subtitle ancillary page
-        this.initializationData.add("mp2t".getBytes());
-    }
+  public DvbSubtitlesReader(TsPayloadReader.EsInfo esInfo) {
+    this.language = esInfo.language;
+    initializationData = Collections.singletonList(new byte[] {(byte) 0x00,
+        esInfo.descriptorBytes[6], esInfo.descriptorBytes[7],
+        esInfo.descriptorBytes[8], esInfo.descriptorBytes[9]});
+  }
 
 
-    @Override
-    public void seek() {
-        writingSample = false;
-    }
+  @Override
+  public void seek() {
+    writingSample = false;
+  }
 
-    @Override
-    public void createTracks(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) {
-        idGenerator.generateNewId();
-        this.output = extractorOutput.track(idGenerator.getTrackId(), C.TRACK_TYPE_TEXT);
-        output.format(Format.createImageSampleFormat(idGenerator.getFormatId(), MimeTypes.APPLICATION_DVBSUBS, null, Format.NO_VALUE, initializationData, language, null));
-    }
+  @Override
+  public void createTracks(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) {
+    idGenerator.generateNewId();
+    this.output = extractorOutput.track(idGenerator.getTrackId(), C.TRACK_TYPE_TEXT);
+    output.format(Format.createImageSampleFormat(idGenerator.getFormatId(),
+        MimeTypes.APPLICATION_DVBSUBS, null, Format.NO_VALUE, initializationData, language, null));
+  }
 
 
-    @Override
-    public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) {
-        if (!dataAlignmentIndicator) {
-            return;
-        }
-        writingSample = true;
-        sampleTimeUs = pesTimeUs;
-        totalBytesWritten = 0;
+  @Override
+  public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) {
+    if (!dataAlignmentIndicator) {
+      return;
     }
-
-    @Override
-    public void packetFinished() {
-        output.sampleMetadata(sampleTimeUs, C.BUFFER_FLAG_KEY_FRAME, totalBytesWritten, 0, null);
-        writingSample = false;
-    }
-
-    @Override
-    public void consume(ParsableByteArray data) {
-        if (writingSample) {
-            totalBytesWritten += data.bytesLeft();
-            output.sampleData(data, data.bytesLeft());
-        }
+    writingSample = true;
+    sampleTimeUs = pesTimeUs;
+    sampleBytesWritten = 0;
+  }
+
+  @Override
+  public void packetFinished() {
+    output.sampleMetadata(sampleTimeUs, C.BUFFER_FLAG_KEY_FRAME, sampleBytesWritten, 0, null);
+    writingSample = false;
+  }
+
+  @Override
+  public void consume(ParsableByteArray data) {
+    if (writingSample) {
+      int bytesAvailable = data.bytesLeft();
+      output.sampleData(data, bytesAvailable);
+      sampleBytesWritten += bytesAvailable;
     }
+  }
 }
\ No newline at end of file

library/core/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java

@@ -502,7 +502,6 @@ public final class TsExtractor implements Extractor {
           // Audio type is ignored.
         } else if (descriptorTag == TS_PMT_DESC_DVBSUBS) {
           streamType = TS_STREAM_TYPE_DVBSUBS;
-          // we only support one subtitle service per PID
           language = new String(data.data, data.getPosition(), 3).trim();
         }
         // Skip unused bytes of current descriptor.

library/core/src/main/java/com/google/android/exoplayer2/text/Cue.java

@@ -168,6 +168,12 @@ public class Cue {
   public final float size;
 
   /**
+   * The bitmap height as a fraction of the of the viewport size, or -1 if the bitmap should be
+   * displayed at its natural height given for its specified {@link #size}.
+   */
+  public final float bitmapHeight;
+
+  /**
    * Specifies whether or not the {@link #windowColor} property is set.
    */
   public final boolean windowColorSet;
@@ -178,30 +184,6 @@ public class Cue {
   public final int windowColor;
 
   /**
-   * The Storage Aspect Ratio of the Cue
-   */
-  public final float sar;
-
-  /**   * Creates an image cue.
-   *
-   * @param bitmap See {@link #bitmap}.
-   * @param horizontalPosition The position of the horizontal anchor within the viewport, expressed
-   *     as a fraction of the viewport width.
-   * @param horizontalPositionAnchor The horizontal anchor. One of {@link #ANCHOR_TYPE_START},
-   *     {@link #ANCHOR_TYPE_MIDDLE}, {@link #ANCHOR_TYPE_END} and {@link #TYPE_UNSET}.
-   * @param verticalPosition The position of the vertical anchor within the viewport, expressed as a
-   *     fraction of the viewport height.
-   * @param verticalPositionAnchor The vertical anchor. One of {@link #ANCHOR_TYPE_START},
-   *     {@link #ANCHOR_TYPE_MIDDLE}, {@link #ANCHOR_TYPE_END} and {@link #TYPE_UNSET}.
-   * @param width The width of the cue, expressed as a fraction of the viewport width.
-   */
-  public Cue(Bitmap bitmap, float horizontalPosition, @AnchorType int horizontalPositionAnchor,
-      float verticalPosition, @AnchorType int verticalPositionAnchor, float width) {
-    this(null, null, bitmap, verticalPosition, LINE_TYPE_FRACTION, verticalPositionAnchor,
-        horizontalPosition, horizontalPositionAnchor, width, false, Color.BLACK, (float) 1.7777);
-  }
-
-  /**
    * Creates an image cue.
    *
    * @param bitmap See {@link #bitmap}.
@@ -214,12 +196,13 @@ public class Cue {
    * @param verticalPositionAnchor The vertical anchor. One of {@link #ANCHOR_TYPE_START},
    *     {@link #ANCHOR_TYPE_MIDDLE}, {@link #ANCHOR_TYPE_END} and {@link #TYPE_UNSET}.
    * @param width The width of the cue, expressed as a fraction of the viewport width.
-   * @param sar The Storage Aspect Ratio of the cue, defaults to FHD SAR unless otherwise specified.
+   * @param height The width of the cue, expressed as a fraction of the viewport width.
    */
   public Cue(Bitmap bitmap, float horizontalPosition, @AnchorType int horizontalPositionAnchor,
-             float verticalPosition, @AnchorType int verticalPositionAnchor, float width, float sar) {
+             float verticalPosition, @AnchorType int verticalPositionAnchor, float width, float
+                 height) {
     this(null, null, bitmap, verticalPosition, LINE_TYPE_FRACTION, verticalPositionAnchor,
-           horizontalPosition, horizontalPositionAnchor, width, false, Color.BLACK, sar);
+           horizontalPosition, horizontalPositionAnchor, width, height, false, Color.BLACK);
   }
 
   /**
@@ -265,15 +248,16 @@ public class Cue {
    * @param windowColor See {@link #windowColor}.
    */
   public Cue(CharSequence text, Alignment textAlignment, float line, @LineType int lineType,
-      @AnchorType int lineAnchor, float position, @AnchorType int positionAnchor, float size,
-      boolean windowColorSet, int windowColor) {
+             @AnchorType int lineAnchor, float position, @AnchorType int positionAnchor, float size,
+             boolean windowColorSet, int windowColor) {
     this(text, textAlignment, null, line, lineType, lineAnchor, position, positionAnchor, size,
-        windowColorSet, windowColor, 1);
+        -1, windowColorSet, windowColor);
   }
 
   private Cue(CharSequence text, Alignment textAlignment, Bitmap bitmap, float line,
       @LineType int lineType, @AnchorType int lineAnchor, float position,
-      @AnchorType int positionAnchor, float size, boolean windowColorSet, int windowColor, float sar) {
+      @AnchorType int positionAnchor, float size, float bitmapHeight, boolean windowColorSet,
+      int windowColor) {
     this.text = text;
     this.textAlignment = textAlignment;
     this.bitmap = bitmap;
@@ -283,9 +267,9 @@ public class Cue {
     this.position = position;
     this.positionAnchor = positionAnchor;
     this.size = size;
+    this.bitmapHeight = bitmapHeight;
     this.windowColorSet = windowColorSet;
     this.windowColor = windowColor;
-    this.sar = sar;
   }
 
 }

library/core/src/main/java/com/google/android/exoplayer2/text/SubtitleDecoderFactory.java

@@ -85,7 +85,7 @@ public interface SubtitleDecoderFactory {
         } else if (format.sampleMimeType.equals(MimeTypes.APPLICATION_CEA708)) {
           return clazz.asSubclass(SubtitleDecoder.class).getConstructor(Integer.TYPE)
               .newInstance(format.accessibilityChannel);
-        } else if (format.sampleMimeType.equals(MimeTypes.APPLICATION_DVBSUBS) && format.initializationData != null) {
+        } else if (format.sampleMimeType.equals(MimeTypes.APPLICATION_DVBSUBS)) {
           return clazz.asSubclass(SubtitleDecoder.class).getConstructor(List.class).newInstance(format.initializationData);
         } else {
           return clazz.asSubclass(SubtitleDecoder.class).getConstructor().newInstance();
@@ -117,7 +117,7 @@ public interface SubtitleDecoderFactory {
           case MimeTypes.APPLICATION_CEA708:
             return Class.forName("com.google.android.exoplayer2.text.cea.Cea708Decoder");
           case MimeTypes.APPLICATION_DVBSUBS:
-            return Class.forName("com.google.android.exoplayer2.text.dvbsubs.DvbSubsDecoder");
+            return Class.forName("com.google.android.exoplayer2.text.dvb.DvbDecoder");
           default:
             return null;
         }

library/core/src/main/java/com/google/android/exoplayer2/text/dvb/DvbDecoder.java

+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.android.exoplayer2.text.dvb;
+
+import com.google.android.exoplayer2.text.SimpleSubtitleDecoder;
+
+import java.util.List;
+
+/**
+ * A {@link SimpleSubtitleDecoder} for DVB Subtitles.
+ */
+public final class DvbDecoder extends SimpleSubtitleDecoder {
+
+  private final DvbParser parser;
+
+  public DvbDecoder(List<byte[]> initializationData) {
+    super("DvbDecoder");
+
+    int subtitleCompositionPage = 1;
+    int subtitleAncillaryPage = 1;
+    int flags = 0;
+    byte[] tempByteArray;
+
+    if ((tempByteArray = initializationData.get(0)) != null && tempByteArray.length == 5) {
+      if (tempByteArray[0] == 0x01) {
+        flags |= DvbParser.FLAG_PES_STRIPPED_DVBSUB;
+      }
+      subtitleCompositionPage = ((tempByteArray[1] & 0xFF) << 8) | (tempByteArray[2] & 0xFF);
+      subtitleAncillaryPage = ((tempByteArray[3] & 0xFF) << 8) | (tempByteArray[4] & 0xFF);
+    }
+
+    parser = new DvbParser(subtitleCompositionPage, subtitleAncillaryPage, flags);
+  }
+
+  @Override
+  protected DvbSubtitle decode(byte[] data, int length) {
+    return new DvbSubtitle(parser.dvbSubsDecode(data, length));
+  }
+}
\ No newline at end of file

library/core/src/main/java/com/google/android/exoplayer2/text/dvbsubs/DvbSubsSubtitle.java → library/core/src/main/java/com/google/android/exoplayer2/text/dvb/DvbSubtitle.java

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package com.google.android.exoplayer2.text.dvbsubs;
+package com.google.android.exoplayer2.text.dvb;
 
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.text.Cue;
@@ -22,10 +22,13 @@ import com.google.android.exoplayer2.text.Subtitle;
 import java.util.Collections;
 import java.util.List;
 
-final class DvbSubsSubtitle implements Subtitle {
+/**
+ * A representation of a DVB subtitle.
+ */
+/* package */ final class DvbSubtitle implements Subtitle {
     private final List<Cue> cues;
 
-    public DvbSubsSubtitle(List<Cue> cues) {
+    public DvbSubtitle(List<Cue> cues) {
         if (cues == null) {
             this.cues = Collections.emptyList();
         } else {

library/core/src/main/java/com/google/android/exoplayer2/text/dvbsubs/DvbSubsDecoder.java

-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.google.android.exoplayer2.text.dvbsubs;
-
-import com.google.android.exoplayer2.text.SimpleSubtitleDecoder;
-
-import java.util.List;
-
-public final class DvbSubsDecoder extends SimpleSubtitleDecoder {
-    private final String TAG = "DVBSubs Decoder";
-
-    private int subtitilingType;
-    private int subtitleCompositionPage;
-    private int subtitleAncillaryPage;
-    private String subtitleContainer;
-
-    private int flags = 0;
-
-    DvbSubtitlesParser parser;
-
-    public DvbSubsDecoder() {
-        super("dvbsubs");
-        parser = new DvbSubtitlesParser();
-    }
-
-    public DvbSubsDecoder(List<byte[]> initializationData) {
-        super("dvbsubs");
-
-        byte[] tempByteArray;
-
-        tempByteArray = initializationData.get(0);
-        subtitilingType = tempByteArray != null ? tempByteArray[0] & 0xFF: -1;
-
-        tempByteArray = initializationData.get(3);
-        if (tempByteArray != null ) {
-            subtitleContainer = new String(tempByteArray);
-            if (subtitleContainer.equals("mkv")) {
-                flags |= DvbSubtitlesParser.FLAG_PES_STRIPPED_DVBSUB;
-            }
-        }
-
-        if ((tempByteArray = initializationData.get(1)) != null) {
-            this.subtitleCompositionPage = ((tempByteArray[0] & 0xFF) << 8) | (tempByteArray[1] & 0xFF);
-            if ((tempByteArray = initializationData.get(2)) != null) {
-                this.subtitleAncillaryPage = ((tempByteArray[0] & 0xFF) << 8) | (tempByteArray[1] & 0xFF);
-                parser = new DvbSubtitlesParser(this.subtitleCompositionPage, this.subtitleAncillaryPage, flags);
-            }
-        } else {
-            parser = new DvbSubtitlesParser();
-        }
-
-    }
-
-    @Override
-    protected DvbSubsSubtitle decode(byte[] data, int length) {
-        return new DvbSubsSubtitle(parser.dvbSubsDecode(data, length));
-    }
-}
\ No newline at end of file

library/ui/src/main/java/com/google/android/exoplayer2/ui/SubtitlePainter.java

@@ -77,7 +77,7 @@ import com.google.android.exoplayer2.util.Util;
   @Cue.AnchorType
   private int cuePositionAnchor;
   private float cueSize;
-  private float cueSar;
+  private float cueBitmapHeight;
   private boolean applyEmbeddedStyles;
   private int foregroundColor;
   private int backgroundColor;
@@ -174,7 +174,7 @@ import com.google.android.exoplayer2.util.Util;
         && this.cuePosition == cue.position
         && Util.areEqual(this.cuePositionAnchor, cue.positionAnchor)
         && this.cueSize == cue.size
-        && this.cueSar == cue.sar
+        && this.cueBitmapHeight == cue.bitmapHeight
         && this.applyEmbeddedStyles == applyEmbeddedStyles
         && this.foregroundColor == style.foregroundColor
         && this.backgroundColor == style.backgroundColor
@@ -202,7 +202,7 @@ import com.google.android.exoplayer2.util.Util;
     this.cuePosition = cue.position;
     this.cuePositionAnchor = cue.positionAnchor;
     this.cueSize = cue.size;
-    this.cueSar = cue.sar;
+    this.cueBitmapHeight = cue.bitmapHeight;
     this.applyEmbeddedStyles = applyEmbeddedStyles;
     this.foregroundColor = style.foregroundColor;
     this.backgroundColor = style.backgroundColor;
@@ -315,7 +315,8 @@ import com.google.android.exoplayer2.util.Util;
     float anchorX = parentLeft + (parentWidth * cuePosition);
     float anchorY = parentTop + (parentHeight * cueLine);
     int width = Math.round(parentWidth * cueSize);
-    int height = Math.round(width * ((float) cueBitmap.getHeight() / cueBitmap.getWidth()) / (((float) parentWidth / parentHeight) / cueSar));
+    int height = cueBitmapHeight != -1 ? Math.round(parentHeight * cueBitmapHeight)
+        : Math.round(width * ((float) cueBitmap.getHeight() / cueBitmap.getWidth()));
     int x = Math.round(cueLineAnchor == Cue.ANCHOR_TYPE_END ? (anchorX - width)
         : cueLineAnchor == Cue.ANCHOR_TYPE_MIDDLE ? (anchorX - (width / 2)) : anchorX);
     int y = Math.round(cuePositionAnchor == Cue.ANCHOR_TYPE_END ? (anchorY - height)