Skip to content

Commit

Permalink
Refactor ExtractorInput javadoc about allowEndOfInput
Browse files Browse the repository at this point in the history 
This parameter is a little confusing, especially as the behaviour
can be surprising if the intended use-case isn't clear. This change
moves the description of the parameter into the class javadoc,
adds context/justification and slims down each method's
javadoc to refer to the class-level.

Related to investigating/fixing issue:#6700

PiperOrigin-RevId: 283724826
  • Loading branch information
@icbaker
icbaker committed Dec 5, 2019
1 parent 8494c3a commit b045b62
Showing 1 changed file with 63 additions and 31 deletions.
94 changes: 63 additions & 31 deletions library/core/src/main/java/com/google/android/exoplayer2/extractor/ExtractorInput.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,50 @@
import com.google.android.exoplayer2.C;
import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;

/**
* Provides data to be consumed by an {@link Extractor}.
*
* <p>This interface provides two modes of accessing the underlying input. See the subheadings below
* for more info about each mode.
*
* <ul>
* <li>The {@code read()} and {@code skip()} methods provide {@link InputStream}-like byte-level
* access operations.
* <li>The {@code read/skip/peekFully()} and {@code advancePeekPosition()} methods assume the user
* wants to read an entire block/frame/header of known length.
* </ul>
*
* <h4>{@link InputStream}-like methods</h4>
*
* <p>The {@code read()} and {@code skip()} methods provide {@link InputStream}-like byte-level
* access operations. The {@code length} parameter is a maximum, and each method returns the number
* of bytes actually processed. This may be less than {@code length} because the end of the input
* was reached, or the method was interrupted, or the operation was aborted early for another
* reason.
*
* <h4>Block-based methods</h4>
*
* <p>The {@code read/skip/peekFully()} and {@code advancePeekPosition()} methods assume the user
* wants to read an entire block/frame/header of known length.
*
* <p>These methods all have a variant that takes a boolean {@code allowEndOfInput} parameter. This
* parameter is intended to be set to true when the caller believes the input might be fully
* exhausted before the call is made (i.e. they've previously read/skipped/peeked the final
* block/frame/header). It's <b>not</b> intended to allow a partial read (i.e. greater than 0 bytes,
* but less than {@code length}) to succeed - this will always throw an {@link EOFException} from
* these methods (a partial read is assumed to indicate a malformed block/frame/header - and
* therefore a malformed file).
*
* <p>The expected behaviour of the block-based methods is therefore:
*
* <ul>
* <li>Already at end-of-input and {@code allowEndOfInput=false}: Throw {@link EOFException}.
* <li>Already at end-of-input and {@code allowEndOfInput=true}: Return {@code false}.
* <li>Encounter end-of-input during read/skip/peek/advance: Throw {@link EOFException}
* (regardless of {@code allowEndOfInput}).
* </ul>
*/
public interface ExtractorInput {

Expand All @@ -41,22 +82,16 @@ public interface ExtractorInput {

/**
* Like {@link #read(byte[], int, int)}, but reads the requested {@code length} in full.
* <p>
* If the end of the input is found having read no data, then behavior is dependent on
* {@code allowEndOfInput}. If {@code allowEndOfInput == true} then {@code false} is returned.
* Otherwise an {@link EOFException} is thrown.
* <p>
* Encountering the end of input having partially satisfied the read is always considered an
* error, and will result in an {@link EOFException} being thrown.
*
* @param target A target array into which data should be written.
* @param offset The offset into the target array at which to write.
* @param length The number of bytes to read from the input.
* @param allowEndOfInput True if encountering the end of the input having read no data is
* allowed, and should result in {@code false} being returned. False if it should be
* considered an error, causing an {@link EOFException} to be thrown.
* @return True if the read was successful. False if the end of the input was encountered having
* read no data.
* considered an error, causing an {@link EOFException} to be thrown. See note in class
* Javadoc.
* @return True if the read was successful. False if {@code allowEndOfInput=true} and the end of
* the input was encountered having read no data.
* @throws EOFException If the end of input was encountered having partially satisfied the read
* (i.e. having read at least one byte, but fewer than {@code length}), or if no bytes were
* read and {@code allowEndOfInput} is false.
Expand Down Expand Up @@ -94,9 +129,10 @@ boolean readFully(byte[] target, int offset, int length, boolean allowEndOfInput
* @param length The number of bytes to skip from the input.
* @param allowEndOfInput True if encountering the end of the input having skipped no data is
* allowed, and should result in {@code false} being returned. False if it should be
* considered an error, causing an {@link EOFException} to be thrown.
* @return True if the skip was successful. False if the end of the input was encountered having
* skipped no data.
* considered an error, causing an {@link EOFException} to be thrown. See note in class
* Javadoc.
* @return True if the skip was successful. False if {@code allowEndOfInput=true} and the end of
* the input was encountered having skipped no data.
* @throws EOFException If the end of input was encountered having partially satisfied the skip
* (i.e. having skipped at least one byte, but fewer than {@code length}), or if no bytes were
* skipped and {@code allowEndOfInput} is false.
Expand All @@ -121,12 +157,8 @@ boolean readFully(byte[] target, int offset, int length, boolean allowEndOfInput
/**
* Peeks {@code length} bytes from the peek position, writing them into {@code target} at index
* {@code offset}. The current read position is left unchanged.
* <p>
* If the end of the input is found having peeked no data, then behavior is dependent on
* {@code allowEndOfInput}. If {@code allowEndOfInput == true} then {@code false} is returned.
* Otherwise an {@link EOFException} is thrown.
* <p>
* Calling {@link #resetPeekPosition()} resets the peek position to equal the current read
*
* <p>Calling {@link #resetPeekPosition()} resets the peek position to equal the current read
* position, so the caller can peek the same data again. Reading or skipping also resets the peek
* position.
*
Expand All @@ -135,9 +167,10 @@ boolean readFully(byte[] target, int offset, int length, boolean allowEndOfInput
* @param length The number of bytes to peek from the input.
* @param allowEndOfInput True if encountering the end of the input having peeked no data is
* allowed, and should result in {@code false} being returned. False if it should be
* considered an error, causing an {@link EOFException} to be thrown.
* @return True if the peek was successful. False if the end of the input was encountered having
* peeked no data.
* considered an error, causing an {@link EOFException} to be thrown. See note in class
* Javadoc.
* @return True if the peek was successful. False if {@code allowEndOfInput=true} and the end of
* the input was encountered having peeked no data.
* @throws EOFException If the end of input was encountered having partially satisfied the peek
* (i.e. having peeked at least one byte, but fewer than {@code length}), or if no bytes were
* peeked and {@code allowEndOfInput} is false.
Expand Down Expand Up @@ -165,18 +198,16 @@ boolean peekFully(byte[] target, int offset, int length, boolean allowEndOfInput
void peekFully(byte[] target, int offset, int length) throws IOException, InterruptedException;

/**
* Advances the peek position by {@code length} bytes.
* <p>
* If the end of the input is encountered before advancing the peek position, then behavior is
* dependent on {@code allowEndOfInput}. If {@code allowEndOfInput == true} then {@code false} is
* returned. Otherwise an {@link EOFException} is thrown.
* Advances the peek position by {@code length} bytes. Like {@link #peekFully(byte[], int, int,
* boolean)} except the data is skipped instead of read.
*
* @param length The number of bytes by which to advance the peek position.
* @param allowEndOfInput True if encountering the end of the input before advancing is allowed,
* and should result in {@code false} being returned. False if it should be considered an
* error, causing an {@link EOFException} to be thrown.
* @return True if advancing the peek position was successful. False if the end of the input was
* encountered before the peek position could be advanced.
* error, causing an {@link EOFException} to be thrown. See note in class Javadoc.
* @return True if advancing the peek position was successful. False if {@code
* allowEndOfInput=true} and the end of the input was encountered before advancing over any
* data.
* @throws EOFException If the end of input was encountered having partially advanced (i.e. having
* advanced by at least one byte, but fewer than {@code length}), or if the end of input was
* encountered before advancing and {@code allowEndOfInput} is false.
Expand All @@ -187,7 +218,8 @@ boolean advancePeekPosition(int length, boolean allowEndOfInput)
throws IOException, InterruptedException;

/**
* Advances the peek position by {@code length} bytes.
* Advances the peek position by {@code length} bytes. Like {@link #peekFully(byte[], int, int,)}
* except the data is skipped instead of read.
*
* @param length The number of bytes to peek from the input.
* @throws EOFException If the end of input was encountered.
Expand Down

0 comments on commit b045b62

Please sign in to comment.