Some extractor fixes

- Fix Ogg extractor to work without sniffing.
- Fix extractors to handle seek() before init().
- Add tests for both issues.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=163992343
This commit is contained in:
olly 2017-08-02 09:26:04 -07:00 committed by Oliver Woodman
parent cad25e5a4d
commit a3df29a246
20 changed files with 132 additions and 95 deletions

View File

@ -26,7 +26,7 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public class FlacExtractorTest extends InstrumentationTestCase {
public void testSample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new FlacExtractor();

View File

@ -159,13 +159,17 @@ public final class FlacExtractor implements Extractor {
if (position == 0) {
metadataParsed = false;
}
decoderJni.reset(position);
if (decoderJni != null) {
decoderJni.reset(position);
}
}
@Override
public void release() {
decoderJni.release();
decoderJni = null;
if (decoderJni != null) {
decoderJni.release();
decoderJni = null;
}
}
}

View File

@ -26,7 +26,7 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public final class FlvExtractorTest extends InstrumentationTestCase {
public void testSample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new FlvExtractor();

View File

@ -26,7 +26,7 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public final class MatroskaExtractorTest extends InstrumentationTestCase {
public void testMkvSample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new MatroskaExtractor();
@ -35,7 +35,7 @@ public final class MatroskaExtractorTest extends InstrumentationTestCase {
}
public void testWebmSubsampleEncryption() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new MatroskaExtractor();
@ -44,7 +44,7 @@ public final class MatroskaExtractorTest extends InstrumentationTestCase {
}
public void testWebmSubsampleEncryptionWithAltrefFrames() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new MatroskaExtractor();

View File

@ -26,7 +26,7 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public final class Mp3ExtractorTest extends InstrumentationTestCase {
public void testMp3Sample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new Mp3Extractor();
@ -35,7 +35,7 @@ public final class Mp3ExtractorTest extends InstrumentationTestCase {
}
public void testTrimmedMp3Sample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new Mp3Extractor();

View File

@ -27,13 +27,13 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public final class FragmentedMp4ExtractorTest extends InstrumentationTestCase {
public void testSample() throws Exception {
ExtractorAsserts
.assertOutput(getExtractorFactory(), "mp4/sample_fragmented.mp4", getInstrumentation());
ExtractorAsserts.assertBehavior(getExtractorFactory(), "mp4/sample_fragmented.mp4",
getInstrumentation());
}
public void testSampleWithSeiPayloadParsing() throws Exception {
// Enabling the CEA-608 track enables SEI payload parsing.
ExtractorAsserts.assertOutput(
ExtractorAsserts.assertBehavior(
getExtractorFactory(FragmentedMp4Extractor.FLAG_ENABLE_CEA608_TRACK),
"mp4/sample_fragmented_sei.mp4", getInstrumentation());
}

View File

@ -28,7 +28,7 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public final class Mp4ExtractorTest extends InstrumentationTestCase {
public void testMp4Sample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new Mp4Extractor();

View File

@ -36,20 +36,21 @@ public final class OggExtractorTest extends InstrumentationTestCase {
};
public void testOpus() throws Exception {
ExtractorAsserts.assertOutput(OGG_EXTRACTOR_FACTORY, "ogg/bear.opus", getInstrumentation());
ExtractorAsserts.assertBehavior(OGG_EXTRACTOR_FACTORY, "ogg/bear.opus", getInstrumentation());
}
public void testFlac() throws Exception {
ExtractorAsserts.assertOutput(OGG_EXTRACTOR_FACTORY, "ogg/bear_flac.ogg", getInstrumentation());
ExtractorAsserts.assertBehavior(OGG_EXTRACTOR_FACTORY, "ogg/bear_flac.ogg",
getInstrumentation());
}
public void testFlacNoSeektable() throws Exception {
ExtractorAsserts.assertOutput(OGG_EXTRACTOR_FACTORY, "ogg/bear_flac_noseektable.ogg",
ExtractorAsserts.assertBehavior(OGG_EXTRACTOR_FACTORY, "ogg/bear_flac_noseektable.ogg",
getInstrumentation());
}
public void testVorbis() throws Exception {
ExtractorAsserts.assertOutput(OGG_EXTRACTOR_FACTORY, "ogg/bear_vorbis.ogg",
ExtractorAsserts.assertBehavior(OGG_EXTRACTOR_FACTORY, "ogg/bear_vorbis.ogg",
getInstrumentation());
}

View File

@ -30,7 +30,7 @@ import com.google.android.exoplayer2.util.MimeTypes;
public final class RawCcExtractorTest extends InstrumentationTestCase {
public void testRawCcSample() throws Exception {
ExtractorAsserts.assertOutput(
ExtractorAsserts.assertBehavior(
new ExtractorFactory() {
@Override
public Extractor create() {

View File

@ -26,7 +26,7 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public final class Ac3ExtractorTest extends InstrumentationTestCase {
public void testSample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new Ac3Extractor();

View File

@ -26,7 +26,7 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public final class AdtsExtractorTest extends InstrumentationTestCase {
public void testSample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new AdtsExtractor();

View File

@ -26,7 +26,7 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public final class PsExtractorTest extends InstrumentationTestCase {
public void testSample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new PsExtractor();

View File

@ -45,7 +45,7 @@ public final class TsExtractorTest extends InstrumentationTestCase {
private static final int TS_SYNC_BYTE = 0x47; // First byte of each TS packet.
public void testSample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new TsExtractor();

View File

@ -26,7 +26,7 @@ import com.google.android.exoplayer2.testutil.ExtractorAsserts.ExtractorFactory;
public final class WavExtractorTest extends InstrumentationTestCase {
public void testSample() throws Exception {
ExtractorAsserts.assertOutput(new ExtractorFactory() {
ExtractorAsserts.assertBehavior(new ExtractorFactory() {
@Override
public Extractor create() {
return new WavExtractor();

View File

@ -63,7 +63,8 @@ public interface Extractor {
void init(ExtractorOutput output);
/**
* Extracts data read from a provided {@link ExtractorInput}.
* Extracts data read from a provided {@link ExtractorInput}. Must not be called before
* {@link #init(ExtractorOutput)}.
* <p>
* A single call to this method will block until some progress has been made, but will not block
* for longer than this. Hence each call will consume only a small amount of input data.

View File

@ -45,30 +45,14 @@ public class OggExtractor implements Extractor {
private static final int MAX_VERIFICATION_BYTES = 8;
private ExtractorOutput output;
private StreamReader streamReader;
private boolean streamReaderInitialized;
@Override
public boolean sniff(ExtractorInput input) throws IOException, InterruptedException {
try {
OggPageHeader header = new OggPageHeader();
if (!header.populate(input, true) || (header.type & 0x02) != 0x02) {
return false;
}
int length = Math.min(header.bodySize, MAX_VERIFICATION_BYTES);
ParsableByteArray scratch = new ParsableByteArray(length);
input.peekFully(scratch.data, 0, length);
if (FlacReader.verifyBitstreamType(resetPosition(scratch))) {
streamReader = new FlacReader();
} else if (VorbisReader.verifyBitstreamType(resetPosition(scratch))) {
streamReader = new VorbisReader();
} else if (OpusReader.verifyBitstreamType(resetPosition(scratch))) {
streamReader = new OpusReader();
} else {
return false;
}
return true;
return sniffInternal(input);
} catch (ParserException e) {
return false;
}
@ -76,15 +60,14 @@ public class OggExtractor implements Extractor {
@Override
public void init(ExtractorOutput output) {
TrackOutput trackOutput = output.track(0, C.TRACK_TYPE_AUDIO);
output.endTracks();
// TODO: fix the case if sniff() isn't called
streamReader.init(output, trackOutput);
this.output = output;
}
@Override
public void seek(long position, long timeUs) {
streamReader.seek(position, timeUs);
if (streamReader != null) {
streamReader.seek(position, timeUs);
}
}
@Override
@ -95,12 +78,41 @@ public class OggExtractor implements Extractor {
@Override
public int read(ExtractorInput input, PositionHolder seekPosition)
throws IOException, InterruptedException {
if (streamReader == null) {
if (!sniffInternal(input)) {
throw new ParserException("Failed to determine bitstream type");
}
input.resetPeekPosition();
}
if (!streamReaderInitialized) {
TrackOutput trackOutput = output.track(0, C.TRACK_TYPE_AUDIO);
output.endTracks();
streamReader.init(output, trackOutput);
streamReaderInitialized = true;
}
return streamReader.read(input, seekPosition);
}
//@VisibleForTesting
/* package */ StreamReader getStreamReader() {
return streamReader;
private boolean sniffInternal(ExtractorInput input) throws IOException, InterruptedException {
OggPageHeader header = new OggPageHeader();
if (!header.populate(input, true) || (header.type & 0x02) != 0x02) {
return false;
}
int length = Math.min(header.bodySize, MAX_VERIFICATION_BYTES);
ParsableByteArray scratch = new ParsableByteArray(length);
input.peekFully(scratch.data, 0, length);
if (FlacReader.verifyBitstreamType(resetPosition(scratch))) {
streamReader = new FlacReader();
} else if (VorbisReader.verifyBitstreamType(resetPosition(scratch))) {
streamReader = new VorbisReader();
} else if (OpusReader.verifyBitstreamType(resetPosition(scratch))) {
streamReader = new OpusReader();
} else {
return false;
}
return true;
}
private static ParsableByteArray resetPosition(ParsableByteArray scratch) {

View File

@ -41,7 +41,8 @@ import java.io.IOException;
OggSeeker oggSeeker;
}
private OggPacket oggPacket;
private final OggPacket oggPacket;
private TrackOutput trackOutput;
private ExtractorOutput extractorOutput;
private OggSeeker oggSeeker;
@ -55,11 +56,13 @@ import java.io.IOException;
private boolean seekMapSet;
private boolean formatSet;
public StreamReader() {
oggPacket = new OggPacket();
}
void init(ExtractorOutput output, TrackOutput trackOutput) {
this.extractorOutput = output;
this.trackOutput = trackOutput;
this.oggPacket = new OggPacket();
reset(true);
}

View File

@ -56,9 +56,9 @@ public final class Ac3Extractor implements Extractor {
private static final int ID3_TAG = Util.getIntegerCodeForString("ID3");
private final long firstSampleTimestampUs;
private final Ac3Reader reader;
private final ParsableByteArray sampleData;
private Ac3Reader reader;
private boolean startedPacket;
public Ac3Extractor() {
@ -67,6 +67,7 @@ public final class Ac3Extractor implements Extractor {
public Ac3Extractor(long firstSampleTimestampUs) {
this.firstSampleTimestampUs = firstSampleTimestampUs;
reader = new Ac3Reader();
sampleData = new ParsableByteArray(MAX_SYNC_FRAME_SIZE);
}
@ -117,7 +118,6 @@ public final class Ac3Extractor implements Extractor {
@Override
public void init(ExtractorOutput output) {
reader = new Ac3Reader(); // TODO: Add support for embedded ID3.
reader.createTracks(output, new TrackIdGenerator(0, 1));
output.endTracks();
output.seekMap(new SeekMap.Unseekable(C.TIME_UNSET));

View File

@ -55,10 +55,9 @@ public final class AdtsExtractor implements Extractor {
private static final int MAX_SNIFF_BYTES = 8 * 1024;
private final long firstSampleTimestampUs;
private final AdtsReader reader;
private final ParsableByteArray packetBuffer;
// Accessed only by the loading thread.
private AdtsReader reader;
private boolean startedPacket;
public AdtsExtractor() {
@ -67,6 +66,7 @@ public final class AdtsExtractor implements Extractor {
public AdtsExtractor(long firstSampleTimestampUs) {
this.firstSampleTimestampUs = firstSampleTimestampUs;
reader = new AdtsReader(true);
packetBuffer = new ParsableByteArray(MAX_PACKET_SIZE);
}
@ -127,7 +127,6 @@ public final class AdtsExtractor implements Extractor {
@Override
public void init(ExtractorOutput output) {
reader = new AdtsReader(true);
reader.createTracks(output, new TrackIdGenerator(0, 1));
output.endTracks();
output.seekMap(new SeekMap.Unseekable(C.TIME_UNSET));

View File

@ -18,6 +18,8 @@ package com.google.android.exoplayer2.testutil;
import android.app.Instrumentation;
import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.extractor.Extractor;
import com.google.android.exoplayer2.extractor.ExtractorInput;
import com.google.android.exoplayer2.extractor.ExtractorOutput;
import com.google.android.exoplayer2.extractor.PositionHolder;
import com.google.android.exoplayer2.extractor.SeekMap;
import com.google.android.exoplayer2.testutil.FakeExtractorInput.SimulatedIOException;
@ -42,46 +44,57 @@ public final class ExtractorAsserts {
private static final String UNKNOWN_LENGTH_EXTENSION = ".unklen" + DUMP_EXTENSION;
/**
* Calls {@link #assertOutput(Extractor, String, byte[], Instrumentation, boolean, boolean,
* boolean)} with all possible combinations of "simulate" parameters.
* Asserts that an extractor behaves correctly given valid input data:
* <ul>
* <li>Calls {@link Extractor#seek(long, long)} and {@link Extractor#release()} without calling
* {@link Extractor#init(ExtractorOutput)} to check these calls do not fail.</li>
* <li>Calls {@link #assertOutput(Extractor, String, byte[], Instrumentation, boolean, boolean,
* boolean, boolean)} with all possible combinations of "simulate" parameters.</li>
* </ul>
*
* @param factory An {@link ExtractorFactory} which creates instances of the {@link Extractor}
* class which is to be tested.
* @param sampleFile The path to the input sample.
* @param file The path to the input sample.
* @param instrumentation To be used to load the sample file.
* @throws IOException If reading from the input fails.
* @throws InterruptedException If interrupted while reading from the input.
* @see #assertOutput(Extractor, String, byte[], Instrumentation, boolean, boolean, boolean)
*/
public static void assertOutput(ExtractorFactory factory, String sampleFile,
public static void assertBehavior(ExtractorFactory factory, String file,
Instrumentation instrumentation) throws IOException, InterruptedException {
byte[] fileData = TestUtil.getByteArray(instrumentation, sampleFile);
assertOutput(factory, sampleFile, fileData, instrumentation);
// Check behavior prior to initialization.
Extractor extractor = factory.create();
extractor.seek(0, 0);
extractor.release();
// Assert output.
byte[] fileData = TestUtil.getByteArray(instrumentation, file);
assertOutput(factory, file, fileData, instrumentation);
}
/**
* Calls {@link #assertOutput(Extractor, String, byte[], Instrumentation, boolean, boolean,
* boolean)} with all possible combinations of "simulate" parameters.
* boolean, boolean)} with all possible combinations of "simulate" parameters with
* {@code sniffFirst} set to true, and makes one additional call with the "simulate" and
* {@code sniffFirst} parameters all set to false.
*
* @param factory An {@link ExtractorFactory} which creates instances of the {@link Extractor}
* class which is to be tested.
* @param sampleFile The path to the input sample.
* @param fileData Content of the input file.
* @param file The path to the input sample.
* @param data Content of the input file.
* @param instrumentation To be used to load the sample file.
* @throws IOException If reading from the input fails.
* @throws InterruptedException If interrupted while reading from the input.
* @see #assertOutput(Extractor, String, byte[], Instrumentation, boolean, boolean, boolean)
*/
public static void assertOutput(ExtractorFactory factory, String sampleFile, byte[] fileData,
public static void assertOutput(ExtractorFactory factory, String file, byte[] data,
Instrumentation instrumentation) throws IOException, InterruptedException {
assertOutput(factory.create(), sampleFile, fileData, instrumentation, false, false, false);
assertOutput(factory.create(), sampleFile, fileData, instrumentation, true, false, false);
assertOutput(factory.create(), sampleFile, fileData, instrumentation, false, true, false);
assertOutput(factory.create(), sampleFile, fileData, instrumentation, true, true, false);
assertOutput(factory.create(), sampleFile, fileData, instrumentation, false, false, true);
assertOutput(factory.create(), sampleFile, fileData, instrumentation, true, false, true);
assertOutput(factory.create(), sampleFile, fileData, instrumentation, false, true, true);
assertOutput(factory.create(), sampleFile, fileData, instrumentation, true, true, true);
assertOutput(factory.create(), file, data, instrumentation, true, false, false, false);
assertOutput(factory.create(), file, data, instrumentation, true, false, false, true);
assertOutput(factory.create(), file, data, instrumentation, true, false, true, false);
assertOutput(factory.create(), file, data, instrumentation, true, false, true, true);
assertOutput(factory.create(), file, data, instrumentation, true, true, false, false);
assertOutput(factory.create(), file, data, instrumentation, true, true, false, true);
assertOutput(factory.create(), file, data, instrumentation, true, true, true, false);
assertOutput(factory.create(), file, data, instrumentation, true, true, true, true);
assertOutput(factory.create(), file, data, instrumentation, false, false, false, false);
}
/**
@ -91,34 +104,38 @@ public final class ExtractorAsserts {
* #UNKNOWN_LENGTH_EXTENSION}" exists, it's preferred.
*
* @param extractor The {@link Extractor} to be tested.
* @param sampleFile The path to the input sample.
* @param fileData Content of the input file.
* @param file The path to the input sample.
* @param data Content of the input file.
* @param instrumentation To be used to load the sample file.
* @param simulateIOErrors If true simulates IOErrors.
* @param simulateUnknownLength If true simulates unknown input length.
* @param simulatePartialReads If true simulates partial reads.
* @param sniffFirst Whether to sniff the data by calling {@link Extractor#sniff(ExtractorInput)}
* prior to consuming it.
* @param simulateIOErrors Whether to simulate IO errors.
* @param simulateUnknownLength Whether to simulate unknown input length.
* @param simulatePartialReads Whether to simulate partial reads.
* @return The {@link FakeExtractorOutput} used in the test.
* @throws IOException If reading from the input fails.
* @throws InterruptedException If interrupted while reading from the input.
*/
public static FakeExtractorOutput assertOutput(Extractor extractor, String sampleFile,
byte[] fileData, Instrumentation instrumentation, boolean simulateIOErrors,
public static FakeExtractorOutput assertOutput(Extractor extractor, String file, byte[] data,
Instrumentation instrumentation, boolean sniffFirst, boolean simulateIOErrors,
boolean simulateUnknownLength, boolean simulatePartialReads) throws IOException,
InterruptedException {
FakeExtractorInput input = new FakeExtractorInput.Builder().setData(fileData)
FakeExtractorInput input = new FakeExtractorInput.Builder().setData(data)
.setSimulateIOErrors(simulateIOErrors)
.setSimulateUnknownLength(simulateUnknownLength)
.setSimulatePartialReads(simulatePartialReads).build();
Assert.assertTrue(TestUtil.sniffTestData(extractor, input));
input.resetPeekPosition();
FakeExtractorOutput extractorOutput = consumeTestData(extractor, input, 0, true);
if (sniffFirst) {
Assert.assertTrue(TestUtil.sniffTestData(extractor, input));
input.resetPeekPosition();
}
FakeExtractorOutput extractorOutput = consumeTestData(extractor, input, 0, true);
if (simulateUnknownLength
&& assetExists(instrumentation, sampleFile + UNKNOWN_LENGTH_EXTENSION)) {
extractorOutput.assertOutput(instrumentation, sampleFile + UNKNOWN_LENGTH_EXTENSION);
&& assetExists(instrumentation, file + UNKNOWN_LENGTH_EXTENSION)) {
extractorOutput.assertOutput(instrumentation, file + UNKNOWN_LENGTH_EXTENSION);
} else {
extractorOutput.assertOutput(instrumentation, sampleFile + ".0" + DUMP_EXTENSION);
extractorOutput.assertOutput(instrumentation, file + ".0" + DUMP_EXTENSION);
}
SeekMap seekMap = extractorOutput.seekMap;
@ -133,7 +150,7 @@ public final class ExtractorAsserts {
}
consumeTestData(extractor, input, timeUs, extractorOutput, false);
extractorOutput.assertOutput(instrumentation, sampleFile + '.' + j + DUMP_EXTENSION);
extractorOutput.assertOutput(instrumentation, file + '.' + j + DUMP_EXTENSION);
}
}