From b23940dc611d47cc1a7a2021716f3463c37e0556 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 3 Feb 2020 16:02:10 +0000 Subject: [PATCH] Clean up Vorbis header The functional change is to set the bitrate fields to Format.NO_VALUE in the case that they're non-positive in the header data. Else it's very easy to to take the fields and copy them directly into Format as incorrect values. Issue #2863 PiperOrigin-RevId: 292920141 --- .../exoplayer2/extractor/VorbisUtil.java | 85 ++++++++++++++----- .../extractor/ogg/VorbisReader.java | 4 +- .../exoplayer2/extractor/VorbisUtilTest.java | 5 +- .../extractor/ogg/VorbisReaderTest.java | 4 +- 4 files changed, 69 insertions(+), 29 deletions(-) diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/VorbisUtil.java b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/VorbisUtil.java index 5066c3a7bd..67d469b759 100644 --- a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/VorbisUtil.java +++ b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/VorbisUtil.java @@ -15,6 +15,7 @@ */ package com.google.android.exoplayer2.extractor; +import com.google.android.exoplayer2.Format; import com.google.android.exoplayer2.ParserException; import com.google.android.exoplayer2.util.Log; import com.google.android.exoplayer2.util.ParsableByteArray; @@ -37,27 +38,54 @@ public final class VorbisUtil { } } - /** Vorbis identification header. */ + /** + * Vorbis identification header. + * + * @see Vorbis + * spec/Identification header + */ public static final class VorbisIdHeader { - public final long version; + /** The {@code vorbis_version} field. */ + public final int version; + /** The {@code audio_channels} field. */ public final int channels; - public final long sampleRate; - public final int bitrateMax; + /** The {@code audio_sample_rate} field. */ + public final int sampleRate; + /** The {@code bitrate_maximum} field, or {@link Format#NO_VALUE} if not greater than zero. */ + public final int bitrateMaximum; + /** The {@code bitrate_nominal} field, or {@link Format#NO_VALUE} if not greater than zero. */ public final int bitrateNominal; - public final int bitrateMin; + /** The {@code bitrate_minimum} field, or {@link Format#NO_VALUE} if not greater than zero. */ + public final int bitrateMinimum; + /** The {@code blocksize_0} field. */ public final int blockSize0; + /** The {@code blocksize_1} field. */ public final int blockSize1; + /** The {@code framing_flag} field. */ public final boolean framingFlag; + /** The raw header data. */ public final byte[] data; + /** + * @param version See {@link #version}. + * @param channels See {@link #channels}. + * @param sampleRate See {@link #sampleRate}. + * @param bitrateMaximum See {@link #bitrateMaximum}. + * @param bitrateNominal See {@link #bitrateNominal}. + * @param bitrateMinimum See {@link #bitrateMinimum}. + * @param blockSize0 See {@link #version}. + * @param blockSize1 See {@link #blockSize1}. + * @param framingFlag See {@link #framingFlag}. + * @param data See {@link #data}. + */ public VorbisIdHeader( - long version, + int version, int channels, - long sampleRate, - int bitrateMax, + int sampleRate, + int bitrateMaximum, int bitrateNominal, - int bitrateMin, + int bitrateMinimum, int blockSize0, int blockSize1, boolean framingFlag, @@ -65,18 +93,14 @@ public final class VorbisUtil { this.version = version; this.channels = channels; this.sampleRate = sampleRate; - this.bitrateMax = bitrateMax; + this.bitrateMaximum = bitrateMaximum; this.bitrateNominal = bitrateNominal; - this.bitrateMin = bitrateMin; + this.bitrateMinimum = bitrateMinimum; this.blockSize0 = blockSize0; this.blockSize1 = blockSize1; this.framingFlag = framingFlag; this.data = data; } - - public int getApproximateBitrate() { - return bitrateNominal == 0 ? (bitrateMin + bitrateMax) / 2 : bitrateNominal; - } } /** Vorbis setup header modes. */ @@ -128,13 +152,21 @@ public final class VorbisUtil { verifyVorbisHeaderCapturePattern(0x01, headerData, false); - long version = headerData.readLittleEndianUnsignedInt(); + int version = headerData.readLittleEndianUnsignedIntToInt(); int channels = headerData.readUnsignedByte(); - long sampleRate = headerData.readLittleEndianUnsignedInt(); - int bitrateMax = headerData.readLittleEndianInt(); + int sampleRate = headerData.readLittleEndianUnsignedIntToInt(); + int bitrateMaximum = headerData.readLittleEndianInt(); + if (bitrateMaximum <= 0) { + bitrateMaximum = Format.NO_VALUE; + } int bitrateNominal = headerData.readLittleEndianInt(); - int bitrateMin = headerData.readLittleEndianInt(); - + if (bitrateNominal <= 0) { + bitrateNominal = Format.NO_VALUE; + } + int bitrateMinimum = headerData.readLittleEndianInt(); + if (bitrateMinimum <= 0) { + bitrateMinimum = Format.NO_VALUE; + } int blockSize = headerData.readUnsignedByte(); int blockSize0 = (int) Math.pow(2, blockSize & 0x0F); int blockSize1 = (int) Math.pow(2, (blockSize & 0xF0) >> 4); @@ -143,8 +175,17 @@ public final class VorbisUtil { // raw data of Vorbis setup header has to be passed to decoder as CSD buffer #1 byte[] data = Arrays.copyOf(headerData.data, headerData.limit()); - return new VorbisIdHeader(version, channels, sampleRate, bitrateMax, bitrateNominal, bitrateMin, - blockSize0, blockSize1, framingFlag, data); + return new VorbisIdHeader( + version, + channels, + sampleRate, + bitrateMaximum, + bitrateNominal, + bitrateMinimum, + blockSize0, + blockSize1, + framingFlag, + data); } /** diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/VorbisReader.java b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/VorbisReader.java index 5c573e2ec0..87de7db68f 100644 --- a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/VorbisReader.java +++ b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/VorbisReader.java @@ -108,9 +108,9 @@ import java.util.ArrayList; null, MimeTypes.AUDIO_VORBIS, /* codecs= */ null, - this.vorbisSetup.idHeader.bitrateNominal, + vorbisSetup.idHeader.bitrateNominal, Format.NO_VALUE, - this.vorbisSetup.idHeader.channels, + vorbisSetup.idHeader.channels, (int) this.vorbisSetup.idHeader.sampleRate, codecInitialisationData, null, diff --git a/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/VorbisUtilTest.java b/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/VorbisUtilTest.java index 15add339bd..6d840c7d2f 100644 --- a/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/VorbisUtilTest.java +++ b/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/VorbisUtilTest.java @@ -61,10 +61,9 @@ public final class VorbisUtilTest { assertThat(vorbisIdHeader.channels).isEqualTo(2); assertThat(vorbisIdHeader.blockSize0).isEqualTo(512); assertThat(vorbisIdHeader.blockSize1).isEqualTo(1024); - assertThat(vorbisIdHeader.bitrateMax).isEqualTo(-1); - assertThat(vorbisIdHeader.bitrateMin).isEqualTo(-1); + assertThat(vorbisIdHeader.bitrateMaximum).isEqualTo(-1); + assertThat(vorbisIdHeader.bitrateMinimum).isEqualTo(-1); assertThat(vorbisIdHeader.bitrateNominal).isEqualTo(66666); - assertThat(vorbisIdHeader.getApproximateBitrate()).isEqualTo(66666); } @Test diff --git a/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/ogg/VorbisReaderTest.java b/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/ogg/VorbisReaderTest.java index 5895116e7d..eec1088e4f 100644 --- a/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/ogg/VorbisReaderTest.java +++ b/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/ogg/VorbisReaderTest.java @@ -77,8 +77,8 @@ public final class VorbisReaderTest { assertThat(vorbisSetup.idHeader.data).hasLength(30); assertThat(vorbisSetup.setupHeaderData).hasLength(3597); - assertThat(vorbisSetup.idHeader.bitrateMax).isEqualTo(-1); - assertThat(vorbisSetup.idHeader.bitrateMin).isEqualTo(-1); + assertThat(vorbisSetup.idHeader.bitrateMaximum).isEqualTo(-1); + assertThat(vorbisSetup.idHeader.bitrateMinimum).isEqualTo(-1); assertThat(vorbisSetup.idHeader.bitrateNominal).isEqualTo(66666); assertThat(vorbisSetup.idHeader.blockSize0).isEqualTo(512); assertThat(vorbisSetup.idHeader.blockSize1).isEqualTo(1024);