Replace DataSpec.absoluteStreamPosition with uriPositionOffset

This is a preliminary step toward adding a DataSpec.Builder,
which is needed for sanity when adding DataSpec.customData.

The existing absoluteStreamPosition field is very error prone,
because anyone using a Builder to adjust the request position
will need to remember to adjust two values:

dataSpec
    .buildUpon()
    .setAbsoluteStreamPosition(x)
    .setPosition(x)
    .build();

Furthermore, the difference between position and
absoluteStreamPosition is irrelevant in nearly all cases. In
the core library, the difference is only relevant when initializing
AES encryption/decryption to write/read cache files.

Replacing absoluteStreamPosition with uriPositionOffset will
simplify the code block above to:

dataSpec
    .buildUpon()
    .setPosition(x)
    .build();

PiperOrigin-RevId: 294485644
This commit is contained in:
olly 2020-02-11 19:43:03 +00:00 committed by Oliver Woodman
parent 58bc460ba3
commit 829282fa12
11 changed files with 109 additions and 69 deletions

View file

@ -111,6 +111,22 @@ public final class DataSpec {
/** The {@link Uri} from which data should be read. */
public final Uri uri;
/**
* The offset of the data located at {@link #uri} within an original resource.
*
* <p>Equal to 0 unless {@link #uri} provides access to a subset of an original resource. As an
* example, consider a resource that can be requested over the network and is 1000 bytes long. If
* {@link #uri} points to a local file that contains just bytes [200-300], then this field will be
* set to {@code 200}.
*
* <p>This field can be ignored except for in specific circumstances where the absolute position
* in the original resource is required in a {@link DataSource} chain. One example is when a
* {@link DataSource} needs to decrypt the content as it's read. In this case the absolute
* position in the original resource is typically needed to correctly initialize the decryption
* algorithm.
*/
public final long uriPositionOffset;
/**
* The HTTP method to use when requesting the data. This value will be ignored by non-HTTP {@link
* DataSource} implementations.
@ -126,15 +142,16 @@ public final class DataSpec {
/** Immutable map containing the headers to use in HTTP requests. */
public final Map<String, String> httpRequestHeaders;
/** The absolute position of the data in the full stream. */
public final long absoluteStreamPosition;
/**
* The position of the data when read from {@link #uri}.
* <p>
* Always equal to {@link #absoluteStreamPosition} unless the {@link #uri} defines the location
* of a subset of the underlying data.
* The absolute position of the data in the full stream.
*
* @deprecated Use {@link #position} except for specific use cases where the absolute position
* within the original resource is required within a {@link DataSource} chain. Where the
* absolute position is required, use {@code uriPositionOffset + position}.
*/
@Deprecated public final long absoluteStreamPosition;
/** The position of the data when read from {@link #uri}. */
public final long position;
/**
@ -152,7 +169,7 @@ public final class DataSpec {
public final @Flags int flags;
/**
* Construct a data spec for the given uri and with {@link #key} set to null.
* Constructs an instance.
*
* @param uri {@link #uri}.
*/
@ -161,7 +178,7 @@ public final class DataSpec {
}
/**
* Construct a data spec for the given uri and with {@link #key} set to null.
* Constructs an instance.
*
* @param uri {@link #uri}.
* @param flags {@link #flags}.
@ -171,10 +188,10 @@ public final class DataSpec {
}
/**
* Construct a data spec where {@link #position} equals {@link #absoluteStreamPosition}.
* Constructs an instance.
*
* @param uri {@link #uri}.
* @param position {@link #position}, equal to {@link #absoluteStreamPosition}.
* @param position {@link #position}.
* @param length {@link #length}.
* @param key {@link #key}.
*/
@ -183,10 +200,10 @@ public final class DataSpec {
}
/**
* Construct a data spec where {@link #position} equals {@link #absoluteStreamPosition}.
* Constructs an instance.
*
* @param uri {@link #uri}.
* @param position {@link #position}, equal to {@link #absoluteStreamPosition}.
* @param position {@link #position}.
* @param length {@link #length}.
* @param key {@link #key}.
* @param flags {@link #flags}.
@ -196,11 +213,10 @@ public final class DataSpec {
}
/**
* Construct a data spec where {@link #position} equals {@link #absoluteStreamPosition} and has
* request headers.
* Constructs an instance.
*
* @param uri {@link #uri}.
* @param position {@link #position}, equal to {@link #absoluteStreamPosition}.
* @param position {@link #position}, equal to {@link #position}.
* @param length {@link #length}.
* @param key {@link #key}.
* @param flags {@link #flags}.
@ -226,10 +242,10 @@ public final class DataSpec {
}
/**
* Construct a data spec where {@link #position} may differ from {@link #absoluteStreamPosition}.
* Constructs an instance where {@link #uriPositionOffset} may be non-zero.
*
* @param uri {@link #uri}.
* @param absoluteStreamPosition {@link #absoluteStreamPosition}.
* @param absoluteStreamPosition The sum of {@link #uriPositionOffset} and {@link #position}.
* @param position {@link #position}.
* @param length {@link #length}.
* @param key {@link #key}.
@ -246,14 +262,15 @@ public final class DataSpec {
}
/**
* Construct a data spec by inferring the {@link #httpMethod} based on the {@code postBody}
* parameter. If postBody is non-null, then httpMethod is set to {@link #HTTP_METHOD_POST}. If
* postBody is null, then httpMethod is set to {@link #HTTP_METHOD_GET}.
* Construct a instance where {@link #uriPositionOffset} may be non-zero. The {@link #httpMethod}
* is inferred from {@code postBody}. If {@code postBody} is non-null then {@link #httpMethod} is
* set to {@link #HTTP_METHOD_POST}. If {@code postBody} is null then {@link #httpMethod} is set
* to {@link #HTTP_METHOD_GET}.
*
* @param uri {@link #uri}.
* @param postBody {@link #httpBody} The body of the HTTP request, which is also used to infer the
* {@link #httpMethod}.
* @param absoluteStreamPosition {@link #absoluteStreamPosition}.
* @param absoluteStreamPosition The sum of {@link #uriPositionOffset} and {@link #position}.
* @param position {@link #position}.
* @param length {@link #length}.
* @param key {@link #key}.
@ -279,12 +296,12 @@ public final class DataSpec {
}
/**
* Construct a data spec where {@link #position} may differ from {@link #absoluteStreamPosition}.
* Construct a instance where {@link #uriPositionOffset} may be non-zero.
*
* @param uri {@link #uri}.
* @param httpMethod {@link #httpMethod}.
* @param httpBody {@link #httpBody}.
* @param absoluteStreamPosition {@link #absoluteStreamPosition}.
* @param absoluteStreamPosition The sum of {@link #uriPositionOffset} and {@link #position}.
* @param position {@link #position}.
* @param length {@link #length}.
* @param key {@link #key}.
@ -312,12 +329,12 @@ public final class DataSpec {
}
/**
* Construct a data spec with request parameters to be used as HTTP headers inside HTTP requests.
* Construct a instance where {@link #uriPositionOffset} may be non-zero.
*
* @param uri {@link #uri}.
* @param httpMethod {@link #httpMethod}.
* @param httpBody {@link #httpBody}.
* @param absoluteStreamPosition {@link #absoluteStreamPosition}.
* @param absoluteStreamPosition The sum of {@link #uriPositionOffset} and {@link #position}.
* @param position {@link #position}.
* @param length {@link #length}.
* @param key {@link #key}.
@ -334,18 +351,44 @@ public final class DataSpec {
@Nullable String key,
@Flags int flags,
Map<String, String> httpRequestHeaders) {
Assertions.checkArgument(absoluteStreamPosition >= 0);
this(
uri,
/* uriPositionOffset= */ absoluteStreamPosition - position,
httpMethod,
httpBody,
httpRequestHeaders,
position,
length,
key,
flags);
}
@SuppressWarnings("deprecation")
private DataSpec(
Uri uri,
long uriPositionOffset,
@HttpMethod int httpMethod,
@Nullable byte[] httpBody,
Map<String, String> httpRequestHeaders,
long position,
long length,
@Nullable String key,
@Flags int flags) {
// TODO: Replace this assertion with a stricter one checking "uriPositionOffset >= 0", after
// validating there are no violations in ExoPlayer and 1P apps.
Assertions.checkArgument(uriPositionOffset + position >= 0);
Assertions.checkArgument(position >= 0);
Assertions.checkArgument(length > 0 || length == C.LENGTH_UNSET);
this.uri = uri;
this.uriPositionOffset = uriPositionOffset;
this.httpMethod = httpMethod;
this.httpBody = (httpBody != null && httpBody.length != 0) ? httpBody : null;
this.absoluteStreamPosition = absoluteStreamPosition;
this.httpBody = httpBody != null && httpBody.length != 0 ? httpBody : null;
this.httpRequestHeaders = Collections.unmodifiableMap(new HashMap<>(httpRequestHeaders));
this.position = position;
this.absoluteStreamPosition = uriPositionOffset + position;
this.length = length;
this.key = key;
this.flags = flags;
this.httpRequestHeaders = Collections.unmodifiableMap(new HashMap<>(httpRequestHeaders));
}
/**
@ -389,14 +432,14 @@ public final class DataSpec {
} else {
return new DataSpec(
uri,
uriPositionOffset,
httpMethod,
httpBody,
absoluteStreamPosition + offset,
httpRequestHeaders,
position + offset,
length,
key,
flags,
httpRequestHeaders);
flags);
}
}
@ -409,14 +452,14 @@ public final class DataSpec {
public DataSpec withUri(Uri uri) {
return new DataSpec(
uri,
uriPositionOffset,
httpMethod,
httpBody,
absoluteStreamPosition,
httpRequestHeaders,
position,
length,
key,
flags,
httpRequestHeaders);
flags);
}
/**
@ -429,14 +472,14 @@ public final class DataSpec {
public DataSpec withRequestHeaders(Map<String, String> httpRequestHeaders) {
return new DataSpec(
uri,
uriPositionOffset,
httpMethod,
httpBody,
absoluteStreamPosition,
httpRequestHeaders,
position,
length,
key,
flags,
httpRequestHeaders);
flags);
}
/**
@ -452,14 +495,14 @@ public final class DataSpec {
httpRequestHeaders.putAll(additionalHttpRequestHeaders);
return new DataSpec(
uri,
uriPositionOffset,
httpMethod,
httpBody,
absoluteStreamPosition,
httpRequestHeaders,
position,
length,
key,
flags,
httpRequestHeaders);
flags);
}
@Override

View file

@ -267,7 +267,7 @@ public abstract class SegmentDownloader<M extends FilterableManifest<M>> impleme
private static boolean canMergeSegments(DataSpec dataSpec1, DataSpec dataSpec2) {
return dataSpec1.uri.equals(dataSpec2.uri)
&& dataSpec1.length != C.LENGTH_UNSET
&& (dataSpec1.absoluteStreamPosition + dataSpec1.length == dataSpec2.absoluteStreamPosition)
&& (dataSpec1.position + dataSpec1.length == dataSpec2.position)
&& Util.areEqual(dataSpec1.key, dataSpec2.key)
&& dataSpec1.flags == dataSpec2.flags
&& dataSpec1.httpMethod == dataSpec2.httpMethod

View file

@ -126,7 +126,7 @@ public class ContainerMediaChunk extends BaseMediaChunk {
DataSpec loadDataSpec = dataSpec.subrange(nextLoadPosition);
ExtractorInput input =
new DefaultExtractorInput(
dataSource, loadDataSpec.absoluteStreamPosition, dataSource.open(loadDataSpec));
dataSource, loadDataSpec.position, dataSource.open(loadDataSpec));
// Load and decode the sample data.
try {
Extractor extractor = extractorWrapper.extractor;
@ -136,7 +136,7 @@ public class ContainerMediaChunk extends BaseMediaChunk {
}
Assertions.checkState(result != Extractor.RESULT_SEEK);
} finally {
nextLoadPosition = input.getPosition() - dataSpec.absoluteStreamPosition;
nextLoadPosition = input.getPosition() - dataSpec.position;
}
} finally {
Util.closeQuietly(dataSource);

View file

@ -93,7 +93,7 @@ public final class InitializationChunk extends Chunk {
DataSpec loadDataSpec = dataSpec.subrange(nextLoadPosition);
ExtractorInput input =
new DefaultExtractorInput(
dataSource, loadDataSpec.absoluteStreamPosition, dataSource.open(loadDataSpec));
dataSource, loadDataSpec.position, dataSource.open(loadDataSpec));
// Load and decode the initialization data.
try {
Extractor extractor = extractorWrapper.extractor;
@ -103,7 +103,7 @@ public final class InitializationChunk extends Chunk {
}
Assertions.checkState(result != Extractor.RESULT_SEEK);
} finally {
nextLoadPosition = input.getPosition() - dataSpec.absoluteStreamPosition;
nextLoadPosition = input.getPosition() - dataSpec.position;
}
} finally {
Util.closeQuietly(dataSource);

View file

@ -169,9 +169,7 @@ public final class CacheDataSink implements DataSink {
dataSpec.length == C.LENGTH_UNSET
? C.LENGTH_UNSET
: Math.min(dataSpec.length - dataSpecBytesWritten, dataSpecFragmentSize);
file =
cache.startFile(
dataSpec.key, dataSpec.absoluteStreamPosition + dataSpecBytesWritten, length);
file = cache.startFile(dataSpec.key, dataSpec.position + dataSpecBytesWritten, length);
FileOutputStream underlyingFileOutputStream = new FileOutputStream(file);
if (bufferSize > 0) {
if (bufferedOutputStream == null) {

View file

@ -79,7 +79,7 @@ public final class CacheUtil {
public static Pair<Long, Long> getCached(
DataSpec dataSpec, Cache cache, @Nullable CacheKeyFactory cacheKeyFactory) {
String key = buildCacheKey(dataSpec, cacheKeyFactory);
long position = dataSpec.absoluteStreamPosition;
long position = dataSpec.position;
long requestLength = getRequestLength(dataSpec, cache, key);
long bytesAlreadyCached = 0;
long bytesLeft = requestLength;
@ -193,7 +193,7 @@ public final class CacheUtil {
bytesLeft = getRequestLength(dataSpec, cache, key);
}
long position = dataSpec.absoluteStreamPosition;
long position = dataSpec.position;
boolean lengthUnset = bytesLeft == C.LENGTH_UNSET;
while (bytesLeft != 0) {
throwExceptionIfInterruptedOrCancelled(isCanceled);
@ -238,18 +238,16 @@ public final class CacheUtil {
return dataSpec.length;
} else {
long contentLength = ContentMetadata.getContentLength(cache.getContentMetadata(key));
return contentLength == C.LENGTH_UNSET
? C.LENGTH_UNSET
: contentLength - dataSpec.absoluteStreamPosition;
return contentLength == C.LENGTH_UNSET ? C.LENGTH_UNSET : contentLength - dataSpec.position;
}
}
/**
* Reads and discards all data specified by the {@code dataSpec}.
*
* @param dataSpec Defines the data to be read. {@code absoluteStreamPosition} and {@code length}
* fields are overwritten by the following parameters.
* @param absoluteStreamPosition The absolute position of the data to be read.
* @param dataSpec Defines the data to be read. The {@code position} and {@code length} fields are
* overwritten by the following parameters.
* @param position The position of the data to be read.
* @param length Length of the data to be read, or {@link C#LENGTH_UNSET} if it is unknown.
* @param dataSource The {@link DataSource} to read the data from.
* @param buffer The buffer to be used while downloading.
@ -264,7 +262,7 @@ public final class CacheUtil {
*/
private static long readAndDiscard(
DataSpec dataSpec,
long absoluteStreamPosition,
long position,
long length,
DataSource dataSource,
byte[] buffer,
@ -274,7 +272,7 @@ public final class CacheUtil {
boolean isLastBlock,
@Nullable AtomicBoolean isCanceled)
throws IOException, InterruptedException {
long positionOffset = absoluteStreamPosition - dataSpec.absoluteStreamPosition;
long positionOffset = position - dataSpec.position;
long initialPositionOffset = positionOffset;
long endOffset = length != C.LENGTH_UNSET ? positionOffset + length : C.POSITION_UNSET;
while (true) {

View file

@ -68,8 +68,9 @@ public final class AesCipherDataSink implements DataSink {
public void open(DataSpec dataSpec) throws IOException {
wrappedDataSink.open(dataSpec);
long nonce = CryptoUtil.getFNV64Hash(dataSpec.key);
cipher = new AesFlushingCipher(Cipher.ENCRYPT_MODE, secretKey, nonce,
dataSpec.absoluteStreamPosition);
cipher =
new AesFlushingCipher(
Cipher.ENCRYPT_MODE, secretKey, nonce, dataSpec.uriPositionOffset + dataSpec.position);
}
@Override

View file

@ -52,8 +52,9 @@ public final class AesCipherDataSource implements DataSource {
public long open(DataSpec dataSpec) throws IOException {
long dataLength = upstream.open(dataSpec);
long nonce = CryptoUtil.getFNV64Hash(dataSpec.key);
cipher = new AesFlushingCipher(Cipher.DECRYPT_MODE, secretKey, nonce,
dataSpec.absoluteStreamPosition);
cipher =
new AesFlushingCipher(
Cipher.DECRYPT_MODE, secretKey, nonce, dataSpec.uriPositionOffset + dataSpec.position);
return dataLength;
}

View file

@ -546,7 +546,7 @@ public final class CacheDataSourceTest {
private void assertReadData(
CacheDataSource cacheDataSource, DataSpec dataSpec, boolean unknownLength)
throws IOException {
int position = (int) dataSpec.absoluteStreamPosition;
int position = (int) dataSpec.position;
int requestLength = (int) dataSpec.length;
int readLength = TEST_DATA.length - position;
if (requestLength != C.LENGTH_UNSET) {

View file

@ -112,7 +112,7 @@ public final class CacheDataSourceTest2 {
byte[] scratch = new byte[4096];
Random random = new Random(0);
source.open(dataSpec);
int position = (int) dataSpec.absoluteStreamPosition;
int position = (int) dataSpec.position;
int bytesRead = 0;
while (bytesRead != C.RESULT_END_OF_INPUT) {
int maxBytesToRead = random.nextInt(scratch.length) + 1;
@ -134,7 +134,6 @@ public final class CacheDataSourceTest2 {
DataSpec[] openedDataSpecs = upstreamSource.getAndClearOpenedDataSpecs();
assertThat(openedDataSpecs).hasLength(1);
assertThat(openedDataSpecs[0].position).isEqualTo(start);
assertThat(openedDataSpecs[0].absoluteStreamPosition).isEqualTo(start);
assertThat(openedDataSpecs[0].length).isEqualTo(end - start);
}

View file

@ -376,7 +376,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
result = extractor.read(input, DUMMY_POSITION_HOLDER);
}
} finally {
nextLoadPosition = (int) (input.getPosition() - dataSpec.absoluteStreamPosition);
nextLoadPosition = (int) (input.getPosition() - dataSpec.position);
}
} finally {
Util.closeQuietly(dataSource);
@ -389,7 +389,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
throws IOException, InterruptedException {
long bytesToRead = dataSource.open(dataSpec);
DefaultExtractorInput extractorInput =
new DefaultExtractorInput(dataSource, dataSpec.absoluteStreamPosition, bytesToRead);
new DefaultExtractorInput(dataSource, dataSpec.position, bytesToRead);
if (extractor == null) {
long id3Timestamp = peekId3PrivTimestamp(extractorInput);