From 98714235ad93caee62f586c6fa66f3fe3b9b572a Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 4 Jul 2019 11:23:52 +0100 Subject: [PATCH] Simplify FlacExtractor (step toward enabling nullness checking) - Inline some unnecessarily split out helper methods - Clear ExtractorInput from FlacDecoderJni data after usage - Clean up exception handling for StreamInfo decode failures PiperOrigin-RevId: 256524955 --- .../ext/flac/FlacBinarySearchSeekerTest.java | 4 +- .../ext/flac/FlacExtractorTest.java | 2 +- .../exoplayer2/ext/flac/FlacDecoder.java | 8 +- .../exoplayer2/ext/flac/FlacDecoderJni.java | 36 ++-- .../exoplayer2/ext/flac/FlacExtractor.java | 192 ++++++++---------- 5 files changed, 119 insertions(+), 123 deletions(-) diff --git a/extensions/flac/src/androidTest/java/com/google/android/exoplayer2/ext/flac/FlacBinarySearchSeekerTest.java b/extensions/flac/src/androidTest/java/com/google/android/exoplayer2/ext/flac/FlacBinarySearchSeekerTest.java index 435279fc45..934d7cf106 100644 --- a/extensions/flac/src/androidTest/java/com/google/android/exoplayer2/ext/flac/FlacBinarySearchSeekerTest.java +++ b/extensions/flac/src/androidTest/java/com/google/android/exoplayer2/ext/flac/FlacBinarySearchSeekerTest.java @@ -52,7 +52,7 @@ public final class FlacBinarySearchSeekerTest { FlacBinarySearchSeeker seeker = new FlacBinarySearchSeeker( - decoderJni.decodeMetadata(), /* firstFramePosition= */ 0, data.length, decoderJni); + decoderJni.decodeStreamInfo(), /* firstFramePosition= */ 0, data.length, decoderJni); SeekMap seekMap = seeker.getSeekMap(); assertThat(seekMap).isNotNull(); @@ -70,7 +70,7 @@ public final class FlacBinarySearchSeekerTest { decoderJni.setData(input); FlacBinarySearchSeeker seeker = new FlacBinarySearchSeeker( - decoderJni.decodeMetadata(), /* firstFramePosition= */ 0, data.length, decoderJni); + decoderJni.decodeStreamInfo(), /* firstFramePosition= */ 0, data.length, decoderJni); seeker.setSeekTargetUs(/* timeUs= */ 1000); assertThat(seeker.isSeeking()).isTrue(); diff --git a/extensions/flac/src/androidTest/java/com/google/android/exoplayer2/ext/flac/FlacExtractorTest.java b/extensions/flac/src/androidTest/java/com/google/android/exoplayer2/ext/flac/FlacExtractorTest.java index d9cbac6ad5..97f152cea4 100644 --- a/extensions/flac/src/androidTest/java/com/google/android/exoplayer2/ext/flac/FlacExtractorTest.java +++ b/extensions/flac/src/androidTest/java/com/google/android/exoplayer2/ext/flac/FlacExtractorTest.java @@ -28,7 +28,7 @@ import org.junit.runner.RunWith; public class FlacExtractorTest { @Before - public void setUp() throws Exception { + public void setUp() { if (!FlacLibrary.isAvailable()) { fail("Flac library not available."); } diff --git a/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacDecoder.java b/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacDecoder.java index 9b15aff846..d20c18e957 100644 --- a/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacDecoder.java +++ b/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacDecoder.java @@ -17,6 +17,7 @@ package com.google.android.exoplayer2.ext.flac; import androidx.annotation.Nullable; import com.google.android.exoplayer2.Format; +import com.google.android.exoplayer2.ParserException; import com.google.android.exoplayer2.decoder.DecoderInputBuffer; import com.google.android.exoplayer2.decoder.SimpleDecoder; import com.google.android.exoplayer2.decoder.SimpleOutputBuffer; @@ -59,14 +60,13 @@ import java.util.List; decoderJni.setData(ByteBuffer.wrap(initializationData.get(0))); FlacStreamInfo streamInfo; try { - streamInfo = decoderJni.decodeMetadata(); + streamInfo = decoderJni.decodeStreamInfo(); + } catch (ParserException e) { + throw new FlacDecoderException("Failed to decode StreamInfo", e); } catch (IOException | InterruptedException e) { // Never happens. throw new IllegalStateException(e); } - if (streamInfo == null) { - throw new FlacDecoderException("Metadata decoding failed"); - } int initialInputBufferSize = maxInputBufferSize != Format.NO_VALUE ? maxInputBufferSize : streamInfo.maxFrameSize; diff --git a/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacDecoderJni.java b/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacDecoderJni.java index a97d99fa54..32ef22dab0 100644 --- a/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacDecoderJni.java +++ b/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacDecoderJni.java @@ -17,6 +17,7 @@ package com.google.android.exoplayer2.ext.flac; import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; +import com.google.android.exoplayer2.ParserException; import com.google.android.exoplayer2.extractor.ExtractorInput; import com.google.android.exoplayer2.util.FlacStreamInfo; import com.google.android.exoplayer2.util.Util; @@ -48,7 +49,6 @@ import java.nio.ByteBuffer; @Nullable private byte[] tempBuffer; private boolean endOfExtractorInput; - @SuppressWarnings("nullness:method.invocation.invalid") public FlacDecoderJni() throws FlacDecoderException { if (!FlacLibrary.isAvailable()) { throw new FlacDecoderException("Failed to load decoder native libraries."); @@ -60,37 +60,46 @@ import java.nio.ByteBuffer; } /** - * Sets data to be parsed by libflac. + * Sets the data to be parsed. * * @param byteBufferData Source {@link ByteBuffer}. */ public void setData(ByteBuffer byteBufferData) { this.byteBufferData = byteBufferData; this.extractorInput = null; - this.tempBuffer = null; } /** - * Sets data to be parsed by libflac. + * Sets the data to be parsed. * * @param extractorInput Source {@link ExtractorInput}. */ public void setData(ExtractorInput extractorInput) { this.byteBufferData = null; this.extractorInput = extractorInput; - if (tempBuffer == null) { - this.tempBuffer = new byte[TEMP_BUFFER_SIZE]; - } endOfExtractorInput = false; + if (tempBuffer == null) { + tempBuffer = new byte[TEMP_BUFFER_SIZE]; + } } + /** + * Returns whether the end of the data to be parsed has been reached, or true if no data was set. + */ public boolean isEndOfData() { if (byteBufferData != null) { return byteBufferData.remaining() == 0; } else if (extractorInput != null) { return endOfExtractorInput; + } else { + return true; } - return true; + } + + /** Clears the data to be parsed. */ + public void clearData() { + byteBufferData = null; + extractorInput = null; } /** @@ -99,12 +108,11 @@ import java.nio.ByteBuffer; *

This method blocks until at least one byte of data can be read, the end of the input is * detected or an exception is thrown. * - *

This method is called from the native code. - * * @param target A target {@link ByteBuffer} into which data should be written. * @return Returns the number of bytes read, or -1 on failure. If all of the data has already been * read from the source, then 0 is returned. */ + @SuppressWarnings("unused") // Called from native code. public int read(ByteBuffer target) throws IOException, InterruptedException { int byteCount = target.remaining(); if (byteBufferData != null) { @@ -135,8 +143,12 @@ import java.nio.ByteBuffer; } /** Decodes and consumes the StreamInfo section from the FLAC stream. */ - public FlacStreamInfo decodeMetadata() throws IOException, InterruptedException { - return flacDecodeMetadata(nativeDecoderContext); + public FlacStreamInfo decodeStreamInfo() throws IOException, InterruptedException { + FlacStreamInfo streamInfo = flacDecodeMetadata(nativeDecoderContext); + if (streamInfo == null) { + throw new ParserException("Failed to decode StreamInfo"); + } + return streamInfo; } /** diff --git a/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacExtractor.java b/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacExtractor.java index bb72e114fe..491b962129 100644 --- a/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacExtractor.java +++ b/extensions/flac/src/main/java/com/google/android/exoplayer2/ext/flac/FlacExtractor.java @@ -21,7 +21,7 @@ import androidx.annotation.IntDef; import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; -import com.google.android.exoplayer2.extractor.BinarySearchSeeker; +import com.google.android.exoplayer2.extractor.BinarySearchSeeker.OutputFrameHolder; import com.google.android.exoplayer2.extractor.Extractor; import com.google.android.exoplayer2.extractor.ExtractorInput; import com.google.android.exoplayer2.extractor.ExtractorOutput; @@ -75,22 +75,19 @@ public final class FlacExtractor implements Extractor { private static final byte[] FLAC_SIGNATURE = {'f', 'L', 'a', 'C', 0, 0, 0, 0x22}; private final Id3Peeker id3Peeker; - private final boolean isId3MetadataDisabled; + private final boolean id3MetadataDisabled; - private FlacDecoderJni decoderJni; + @Nullable private FlacDecoderJni decoderJni; + @Nullable private ExtractorOutput extractorOutput; + @Nullable private TrackOutput trackOutput; - private ExtractorOutput extractorOutput; - private TrackOutput trackOutput; + private boolean streamInfoDecoded; + @Nullable private FlacStreamInfo streamInfo; + @Nullable private ParsableByteArray outputBuffer; + @Nullable private OutputFrameHolder outputFrameHolder; - private ParsableByteArray outputBuffer; - private ByteBuffer outputByteBuffer; - private BinarySearchSeeker.OutputFrameHolder outputFrameHolder; - private FlacStreamInfo streamInfo; - - private Metadata id3Metadata; - private @Nullable FlacBinarySearchSeeker flacBinarySearchSeeker; - - private boolean readPastStreamInfo; + @Nullable private Metadata id3Metadata; + @Nullable private FlacBinarySearchSeeker binarySearchSeeker; /** Constructs an instance with flags = 0. */ public FlacExtractor() { @@ -104,7 +101,7 @@ public final class FlacExtractor implements Extractor { */ public FlacExtractor(int flags) { id3Peeker = new Id3Peeker(); - isId3MetadataDisabled = (flags & FLAG_DISABLE_ID3_METADATA) != 0; + id3MetadataDisabled = (flags & FLAG_DISABLE_ID3_METADATA) != 0; } @Override @@ -130,48 +127,53 @@ public final class FlacExtractor implements Extractor { @Override public int read(final ExtractorInput input, PositionHolder seekPosition) throws IOException, InterruptedException { - if (input.getPosition() == 0 && !isId3MetadataDisabled && id3Metadata == null) { + if (input.getPosition() == 0 && !id3MetadataDisabled && id3Metadata == null) { id3Metadata = peekId3Data(input); } decoderJni.setData(input); - readPastStreamInfo(input); - - if (flacBinarySearchSeeker != null && flacBinarySearchSeeker.isSeeking()) { - return handlePendingSeek(input, seekPosition); - } - - long lastDecodePosition = decoderJni.getDecodePosition(); try { - decoderJni.decodeSampleWithBacktrackPosition(outputByteBuffer, lastDecodePosition); - } catch (FlacDecoderJni.FlacFrameDecodeException e) { - throw new IOException("Cannot read frame at position " + lastDecodePosition, e); - } - int outputSize = outputByteBuffer.limit(); - if (outputSize == 0) { - return RESULT_END_OF_INPUT; - } + decodeStreamInfo(input); - writeLastSampleToOutput(outputSize, decoderJni.getLastFrameTimestamp()); - return decoderJni.isEndOfData() ? RESULT_END_OF_INPUT : RESULT_CONTINUE; + if (binarySearchSeeker != null && binarySearchSeeker.isSeeking()) { + return handlePendingSeek(input, seekPosition); + } + + ByteBuffer outputByteBuffer = outputFrameHolder.byteBuffer; + long lastDecodePosition = decoderJni.getDecodePosition(); + try { + decoderJni.decodeSampleWithBacktrackPosition(outputByteBuffer, lastDecodePosition); + } catch (FlacDecoderJni.FlacFrameDecodeException e) { + throw new IOException("Cannot read frame at position " + lastDecodePosition, e); + } + int outputSize = outputByteBuffer.limit(); + if (outputSize == 0) { + return RESULT_END_OF_INPUT; + } + + outputSample(outputBuffer, outputSize, decoderJni.getLastFrameTimestamp()); + return decoderJni.isEndOfData() ? RESULT_END_OF_INPUT : RESULT_CONTINUE; + } finally { + decoderJni.clearData(); + } } @Override public void seek(long position, long timeUs) { if (position == 0) { - readPastStreamInfo = false; + streamInfoDecoded = false; } if (decoderJni != null) { decoderJni.reset(position); } - if (flacBinarySearchSeeker != null) { - flacBinarySearchSeeker.setSeekTargetUs(timeUs); + if (binarySearchSeeker != null) { + binarySearchSeeker.setSeekTargetUs(timeUs); } } @Override public void release() { - flacBinarySearchSeeker = null; + binarySearchSeeker = null; if (decoderJni != null) { decoderJni.release(); decoderJni = null; @@ -179,16 +181,15 @@ public final class FlacExtractor implements Extractor { } /** - * Peeks ID3 tag data (if present) at the beginning of the input. + * Peeks ID3 tag data at the beginning of the input. * - * @return The first ID3 tag decoded into a {@link Metadata} object. May be null if ID3 tag is not - * present in the input. + * @return The first ID3 tag {@link Metadata}, or null if an ID3 tag is not present in the input. */ @Nullable private Metadata peekId3Data(ExtractorInput input) throws IOException, InterruptedException { input.resetPeekPosition(); Id3Decoder.FramePredicate id3FramePredicate = - isId3MetadataDisabled ? Id3Decoder.NO_FRAMES_PREDICATE : null; + id3MetadataDisabled ? Id3Decoder.NO_FRAMES_PREDICATE : null; return id3Peeker.peekId3Data(input, id3FramePredicate); } @@ -199,68 +200,61 @@ public final class FlacExtractor implements Extractor { */ private boolean peekFlacSignature(ExtractorInput input) throws IOException, InterruptedException { byte[] header = new byte[FLAC_SIGNATURE.length]; - input.peekFully(header, 0, FLAC_SIGNATURE.length); + input.peekFully(header, /* offset= */ 0, FLAC_SIGNATURE.length); return Arrays.equals(header, FLAC_SIGNATURE); } - private void readPastStreamInfo(ExtractorInput input) throws InterruptedException, IOException { - if (readPastStreamInfo) { + private void decodeStreamInfo(ExtractorInput input) throws InterruptedException, IOException { + if (streamInfoDecoded) { return; } - FlacStreamInfo streamInfo = decodeStreamInfo(input); - readPastStreamInfo = true; - if (this.streamInfo == null) { - updateFlacStreamInfo(input, streamInfo); - } - } - - private void updateFlacStreamInfo(ExtractorInput input, FlacStreamInfo streamInfo) { - this.streamInfo = streamInfo; - outputSeekMap(input, streamInfo); - outputFormat(streamInfo); - outputBuffer = new ParsableByteArray(streamInfo.maxDecodedFrameSize()); - outputByteBuffer = ByteBuffer.wrap(outputBuffer.data); - outputFrameHolder = new BinarySearchSeeker.OutputFrameHolder(outputByteBuffer); - } - - private FlacStreamInfo decodeStreamInfo(ExtractorInput input) - throws InterruptedException, IOException { + FlacStreamInfo streamInfo; try { - FlacStreamInfo streamInfo = decoderJni.decodeMetadata(); - if (streamInfo == null) { - throw new IOException("Metadata decoding failed"); - } - return streamInfo; + streamInfo = decoderJni.decodeStreamInfo(); } catch (IOException e) { - decoderJni.reset(0); - input.setRetryPosition(0, e); + decoderJni.reset(/* newPosition= */ 0); + input.setRetryPosition(/* position= */ 0, e); throw e; } + + streamInfoDecoded = true; + if (this.streamInfo == null) { + this.streamInfo = streamInfo; + outputSeekMap(streamInfo, input.getLength()); + outputFormat(streamInfo, id3MetadataDisabled ? null : id3Metadata); + outputBuffer = new ParsableByteArray(streamInfo.maxDecodedFrameSize()); + outputFrameHolder = new OutputFrameHolder(ByteBuffer.wrap(outputBuffer.data)); + } } - private void outputSeekMap(ExtractorInput input, FlacStreamInfo streamInfo) { - boolean hasSeekTable = decoderJni.getSeekPosition(0) != -1; - SeekMap seekMap = - hasSeekTable - ? new FlacSeekMap(streamInfo.durationUs(), decoderJni) - : getSeekMapForNonSeekTableFlac(input, streamInfo); + private int handlePendingSeek(ExtractorInput input, PositionHolder seekPosition) + throws InterruptedException, IOException { + int seekResult = binarySearchSeeker.handlePendingSeek(input, seekPosition, outputFrameHolder); + ByteBuffer outputByteBuffer = outputFrameHolder.byteBuffer; + if (seekResult == RESULT_CONTINUE && outputByteBuffer.limit() > 0) { + outputSample(outputBuffer, outputByteBuffer.limit(), outputFrameHolder.timeUs); + } + return seekResult; + } + + private void outputSeekMap(FlacStreamInfo streamInfo, long inputLength) { + boolean hasSeekTable = decoderJni.getSeekPosition(/* timeUs= */ 0) != -1; + SeekMap seekMap; + if (hasSeekTable) { + seekMap = new FlacSeekMap(streamInfo.durationUs(), decoderJni); + } else if (inputLength != C.LENGTH_UNSET) { + long firstFramePosition = decoderJni.getDecodePosition(); + binarySearchSeeker = + new FlacBinarySearchSeeker(streamInfo, firstFramePosition, inputLength, decoderJni); + seekMap = binarySearchSeeker.getSeekMap(); + } else { + seekMap = new SeekMap.Unseekable(streamInfo.durationUs()); + } extractorOutput.seekMap(seekMap); } - private SeekMap getSeekMapForNonSeekTableFlac(ExtractorInput input, FlacStreamInfo streamInfo) { - long inputLength = input.getLength(); - if (inputLength != C.LENGTH_UNSET) { - long firstFramePosition = decoderJni.getDecodePosition(); - flacBinarySearchSeeker = - new FlacBinarySearchSeeker(streamInfo, firstFramePosition, inputLength, decoderJni); - return flacBinarySearchSeeker.getSeekMap(); - } else { // can't seek at all, because there's no SeekTable and the input length is unknown. - return new SeekMap.Unseekable(streamInfo.durationUs()); - } - } - - private void outputFormat(FlacStreamInfo streamInfo) { + private void outputFormat(FlacStreamInfo streamInfo, Metadata metadata) { Format mediaFormat = Format.createAudioSampleFormat( /* id= */ null, @@ -277,25 +271,15 @@ public final class FlacExtractor implements Extractor { /* drmInitData= */ null, /* selectionFlags= */ 0, /* language= */ null, - isId3MetadataDisabled ? null : id3Metadata); + metadata); trackOutput.format(mediaFormat); } - private int handlePendingSeek(ExtractorInput input, PositionHolder seekPosition) - throws InterruptedException, IOException { - int seekResult = - flacBinarySearchSeeker.handlePendingSeek(input, seekPosition, outputFrameHolder); - ByteBuffer outputByteBuffer = outputFrameHolder.byteBuffer; - if (seekResult == RESULT_CONTINUE && outputByteBuffer.limit() > 0) { - writeLastSampleToOutput(outputByteBuffer.limit(), outputFrameHolder.timeUs); - } - return seekResult; - } - - private void writeLastSampleToOutput(int size, long lastSampleTimestamp) { - outputBuffer.setPosition(0); - trackOutput.sampleData(outputBuffer, size); - trackOutput.sampleMetadata(lastSampleTimestamp, C.BUFFER_FLAG_KEY_FRAME, size, 0, null); + private void outputSample(ParsableByteArray sampleData, int size, long timeUs) { + sampleData.setPosition(0); + trackOutput.sampleData(sampleData, size); + trackOutput.sampleMetadata( + timeUs, C.BUFFER_FLAG_KEY_FRAME, size, /* offset= */ 0, /* encryptionData= */ null); } /** A {@link SeekMap} implementation using a SeekTable within the Flac stream. */