Skip to content

Commit

Permalink
Extend TransferListener with onTransferInitializing and additional pa…
Browse files Browse the repository at this point in the history 
…rameters.

This allows more fine-grained analysis of transfers.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=203950327
  • Loading branch information
@tonihei @ojw28
tonihei authored and ojw28 committed Jul 11, 2018
1 parent 2b1434d commit c3df64f
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 39 deletions.
1 change: 1 addition & 0 deletions RELEASENOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
directly reading data should implement `BaseDataSource` to handle the
registration correctly. Custom `DataSource`'s forwarding to other sources
should forward all calls to `addTransferListener`.
* Extend `TransferListener` with additional callback parameters.
* Error handling:
* Allow configuration of the Loader retry delay
([#3370](https://github.com/google/ExoPlayer/issues/3370)).
Expand Down
62 changes: 41 additions & 21 deletions ...s/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ public void testRequestHeadersSet() throws HttpDataSourceException {
public void testRequestOpen() throws HttpDataSourceException {
mockResponseStartSuccess();
assertThat(dataSourceUnderTest.open(testDataSpec)).isEqualTo(TEST_CONTENT_LENGTH);
verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testDataSpec);
verify(mockTransferListener)
.onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
}

@Test
Expand All @@ -225,7 +226,8 @@ public void testRequestOpenGzippedCompressedReturnsDataSpecLength()
mockResponseStartSuccess();

assertThat(dataSourceUnderTest.open(testDataSpec)).isEqualTo(5000 /* contentLength */);
verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testDataSpec);
verify(mockTransferListener)
.onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
}

@Test
Expand All @@ -239,7 +241,8 @@ public void testRequestOpenFail() {
// Check for connection not automatically closed.
assertThat(e.getCause() instanceof UnknownHostException).isFalse();
verify(mockUrlRequest, never()).cancel();
verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
verify(mockTransferListener, never())
.onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
}
}

Expand All @@ -256,7 +259,8 @@ public void testRequestOpenFailDueToDnsFailure() {
// Check for connection not automatically closed.
assertThat(e.getCause() instanceof UnknownHostException).isTrue();
verify(mockUrlRequest, never()).cancel();
verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
verify(mockTransferListener, never())
.onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
}
}

Expand All @@ -272,7 +276,8 @@ public void testRequestOpenValidatesStatusCode() {
assertThat(e instanceof HttpDataSource.InvalidResponseCodeException).isTrue();
// Check for connection not automatically closed.
verify(mockUrlRequest, never()).cancel();
verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
verify(mockTransferListener, never())
.onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
}
}

Expand All @@ -298,7 +303,8 @@ public void testPostRequestOpen() throws HttpDataSourceException {

dataSourceUnderTest.setRequestProperty("Content-Type", TEST_CONTENT_TYPE);
assertThat(dataSourceUnderTest.open(testPostDataSpec)).isEqualTo(TEST_CONTENT_LENGTH);
verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testPostDataSpec);
verify(mockTransferListener)
.onTransferStart(dataSourceUnderTest, testPostDataSpec, /* isNetwork= */ true);
}

@Test
Expand Down Expand Up @@ -346,7 +352,8 @@ public void testRequestReadTwice() throws HttpDataSourceException {

// Should have only called read on cronet once.
verify(mockUrlRequest, times(1)).read(any(ByteBuffer.class));
verify(mockTransferListener, times(2)).onBytesTransferred(dataSourceUnderTest, 8);
verify(mockTransferListener, times(2))
.onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 8);
}

@Test
Expand Down Expand Up @@ -386,7 +393,8 @@ public void testReadWithOffset() throws HttpDataSourceException {
int bytesRead = dataSourceUnderTest.read(returnedBuffer, 8, 8);
assertThat(bytesRead).isEqualTo(8);
assertThat(returnedBuffer).isEqualTo(prefixZeros(buildTestDataArray(0, 8), 16));
verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 8);
verify(mockTransferListener)
.onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 8);
}

@Test
Expand All @@ -402,7 +410,8 @@ public void testRangeRequestWith206Response() throws HttpDataSourceException {
int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 16);
assertThat(bytesRead).isEqualTo(16);
assertThat(returnedBuffer).isEqualTo(buildTestDataArray(1000, 16));
verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 16);
verify(mockTransferListener)
.onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 16);
}

@Test
Expand All @@ -418,7 +427,8 @@ public void testRangeRequestWith200Response() throws HttpDataSourceException {
int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 16);
assertThat(bytesRead).isEqualTo(16);
assertThat(returnedBuffer).isEqualTo(buildTestDataArray(1000, 16));
verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 16);
verify(mockTransferListener)
.onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 16);
}

@Test
Expand All @@ -433,7 +443,8 @@ public void testReadWithUnsetLength() throws HttpDataSourceException {
int bytesRead = dataSourceUnderTest.read(returnedBuffer, 8, 8);
assertThat(returnedBuffer).isEqualTo(prefixZeros(buildTestDataArray(0, 8), 16));
assertThat(bytesRead).isEqualTo(8);
verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 8);
verify(mockTransferListener)
.onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 8);
}

@Test
Expand All @@ -447,7 +458,8 @@ public void testReadReturnsWhatItCan() throws HttpDataSourceException {
int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 24);
assertThat(returnedBuffer).isEqualTo(suffixZeros(buildTestDataArray(0, 16), 24));
assertThat(bytesRead).isEqualTo(16);
verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 16);
verify(mockTransferListener)
.onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 16);
}

@Test
Expand All @@ -464,7 +476,8 @@ public void testClosedMeansClosed() throws HttpDataSourceException {
assertThat(bytesRead).isEqualTo(8);

dataSourceUnderTest.close();
verify(mockTransferListener).onTransferEnd(dataSourceUnderTest);
verify(mockTransferListener)
.onTransferEnd(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);

try {
bytesRead += dataSourceUnderTest.read(returnedBuffer, 0, 8);
Expand Down Expand Up @@ -505,9 +518,12 @@ public void testOverread() throws HttpDataSourceException {

// Should have only called read on cronet once.
verify(mockUrlRequest, times(1)).read(any(ByteBuffer.class));
verify(mockTransferListener, times(1)).onBytesTransferred(dataSourceUnderTest, 8);
verify(mockTransferListener, times(1)).onBytesTransferred(dataSourceUnderTest, 6);
verify(mockTransferListener, times(1)).onBytesTransferred(dataSourceUnderTest, 2);
verify(mockTransferListener, times(1))
.onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 8);
verify(mockTransferListener, times(1))
.onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 6);
verify(mockTransferListener, times(1))
.onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 2);

// Now we already returned the 16 bytes initially asked.
// Try to read again even though all requested 16 bytes are already returned.
Expand All @@ -518,7 +534,8 @@ public void testOverread() throws HttpDataSourceException {
assertThat(returnedBuffer).isEqualTo(new byte[16]);
// C.RESULT_END_OF_INPUT should not be reported though the TransferListener.
verify(mockTransferListener, never())
.onBytesTransferred(dataSourceUnderTest, C.RESULT_END_OF_INPUT);
.onBytesTransferred(
dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, C.RESULT_END_OF_INPUT);
// There should still be only one call to read on cronet.
verify(mockUrlRequest, times(1)).read(any(ByteBuffer.class));
// Check for connection not automatically closed.
Expand Down Expand Up @@ -559,7 +576,8 @@ public void run() {
ShadowSystemClock.setCurrentTimeMillis(startTimeMs + TEST_CONNECT_TIMEOUT_MS + 10);
timedOutLatch.await();

verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
verify(mockTransferListener, never())
.onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
}

@Test
Expand Down Expand Up @@ -597,7 +615,8 @@ public void run() {
thread.interrupt();
timedOutLatch.await();

verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
verify(mockTransferListener, never())
.onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
}

@Test
Expand Down Expand Up @@ -678,7 +697,8 @@ public void run() {
ShadowSystemClock.setCurrentTimeMillis(startTimeMs + newTimeoutMs + 10);
timedOutLatch.await();

verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
verify(mockTransferListener, never())
.onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
assertThat(openExceptions.get()).isEqualTo(1);
}

Expand Down Expand Up @@ -797,7 +817,7 @@ public void testExceptionFromTransferListener() throws HttpDataSourceException {
// the subsequent open() call succeeds.
doThrow(new NullPointerException())
.when(mockTransferListener)
.onTransferEnd(dataSourceUnderTest);
.onTransferEnd(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
dataSourceUnderTest.open(testDataSpec);
try {
dataSourceUnderTest.close();
Expand Down
20 changes: 16 additions & 4 deletions library/core/src/main/java/com/google/android/exoplayer2/upstream/BaseDataSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package com.google.android.exoplayer2.upstream;

import android.support.annotation.Nullable;
import com.google.android.exoplayer2.util.Assertions;
import java.util.ArrayList;

/**
Expand All @@ -29,6 +31,8 @@ public abstract class BaseDataSource implements DataSource {
private final boolean isNetwork;
private final ArrayList<TransferListener<? super DataSource>> listeners;

private @Nullable DataSpec dataSpec;

/**
* Creates base data source.
*
Expand All @@ -50,7 +54,9 @@ public final void addTransferListener(TransferListener<? super DataSource> trans
* @param dataSpec {@link DataSpec} describing the data for initializing transfer.
*/
protected final void transferInitializing(DataSpec dataSpec) {
// TODO: notify listeners.
for (int i = 0; i < listeners.size(); i++) {
listeners.get(i).onTransferInitializing(/* source= */ this, dataSpec, isNetwork);
}
}

/**
Expand All @@ -59,8 +65,9 @@ protected final void transferInitializing(DataSpec dataSpec) {
* @param dataSpec {@link DataSpec} describing the data being transferred.
*/
protected final void transferStarted(DataSpec dataSpec) {
this.dataSpec = dataSpec;
for (int i = 0; i < listeners.size(); i++) {
listeners.get(i).onTransferStart(/* source= */ this, dataSpec);
listeners.get(i).onTransferStart(/* source= */ this, dataSpec, isNetwork);
}
}

Expand All @@ -71,15 +78,20 @@ protected final void transferStarted(DataSpec dataSpec) {
* (or if the first call, since the transfer was started).
*/
protected final void bytesTransferred(int bytesTransferred) {
DataSpec dataSpec = Assertions.checkNotNull(this.dataSpec);
for (int i = 0; i < listeners.size(); i++) {
listeners.get(i).onBytesTransferred(/* source= */ this, bytesTransferred);
listeners
.get(i)
.onBytesTransferred(/* source= */ this, dataSpec, isNetwork, bytesTransferred);
}
}

/** Notifies listeners that a transfer ended. */
protected final void transferEnded() {
DataSpec dataSpec = Assertions.checkNotNull(this.dataSpec);
for (int i = 0; i < listeners.size(); i++) {
listeners.get(i).onTransferEnd(/* source= */ this);
listeners.get(i).onTransferEnd(/* source= */ this, dataSpec, isNetwork);
}
this.dataSpec = null;
}
}
25 changes: 20 additions & 5 deletions library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
import com.google.android.exoplayer2.util.SlidingPercentile;

/**
* Estimates bandwidth by listening to data transfers. The bandwidth estimate is calculated using
* a {@link SlidingPercentile} and is updated each time a transfer ends.
* Estimates bandwidth by listening to data transfers. The bandwidth estimate is calculated using a
* {@link SlidingPercentile} and is updated each time a transfer ends.
*/
public final class DefaultBandwidthMeter implements BandwidthMeter, TransferListener<Object> {

Expand Down Expand Up @@ -177,20 +177,35 @@ public synchronized long getBitrateEstimate() {
}

@Override
public synchronized void onTransferStart(Object source, DataSpec dataSpec) {
public void onTransferInitializing(Object source, DataSpec dataSpec, boolean isNetwork) {
// Do nothing.
}

@Override
public synchronized void onTransferStart(Object source, DataSpec dataSpec, boolean isNetwork) {
if (!isNetwork) {
return;
}
if (streamCount == 0) {
sampleStartTimeMs = clock.elapsedRealtime();
}
streamCount++;
}

@Override
public synchronized void onBytesTransferred(Object source, int bytes) {
public synchronized void onBytesTransferred(
Object source, DataSpec dataSpec, boolean isNetwork, int bytes) {
if (!isNetwork) {
return;
}
sampleBytesTransferred += bytes;
}

@Override
public synchronized void onTransferEnd(Object source) {
public synchronized void onTransferEnd(Object source, DataSpec dataSpec, boolean isNetwork) {
if (!isNetwork) {
return;
}
Assertions.checkState(streamCount > 0);
long nowMs = clock.elapsedRealtime();
int sampleElapsedTimeMs = (int) (nowMs - sampleStartTimeMs);
Expand Down
41 changes: 35 additions & 6 deletions library/core/src/main/java/com/google/android/exoplayer2/upstream/TransferListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,60 @@

/**
* A listener of data transfer events.
*
* <p>A transfer usually progresses through multiple steps:
*
* <ol>
* <li>Initializing the underlying resource (e.g. opening a HTTP connection). {@link
* #onTransferInitializing(Object, DataSpec, boolean)} is called before the initialization
* starts.
* <li>Starting the transfer after successfully initializing the resource. {@link
* #onTransferStart(Object, DataSpec, boolean)} is called. Note that this only happens if the
* initialization was successful.
* <li>Transferring data. {@link #onBytesTransferred(Object, DataSpec, boolean, int)} is called
* frequently during the transfer to indicate progress.
* <li>Closing the transfer and the underlying resource. {@link #onTransferEnd(Object, DataSpec,
* boolean)} is called. Note that each {@link #onTransferStart(Object, DataSpec, boolean)}
* will have exactly one corresponding call to {@link #onTransferEnd(Object, DataSpec,
* boolean)}.
* </ol>
*/
public interface TransferListener<S> {

/**
* Called when a transfer is being initialized.
*
* @param source The source performing the transfer.
* @param dataSpec Describes the data for which the transfer is initialized.
* @param isNetwork Whether the data is transferred through a network.
*/
void onTransferInitializing(S source, DataSpec dataSpec, boolean isNetwork);

/**
* Called when a transfer starts.
*
* @param source The source performing the transfer.
* @param dataSpec Describes the data being transferred.
* @param isNetwork Whether the data is transferred through a network.
*/
void onTransferStart(S source, DataSpec dataSpec);
void onTransferStart(S source, DataSpec dataSpec, boolean isNetwork);

/**
* Called incrementally during a transfer.
*
* @param source The source performing the transfer.
* @param bytesTransferred The number of bytes transferred since the previous call to this
* method (or if the first call, since the transfer was started).
* @param dataSpec Describes the data being transferred.
* @param isNetwork Whether the data is transferred through a network.
* @param bytesTransferred The number of bytes transferred since the previous call to this method
*/
void onBytesTransferred(S source, int bytesTransferred);
void onBytesTransferred(S source, DataSpec dataSpec, boolean isNetwork, int bytesTransferred);

/**
* Called when a transfer ends.
*
* @param source The source performing the transfer.
* @param dataSpec Describes the data being transferred.
* @param isNetwork Whether the data is transferred through a network.
*/
void onTransferEnd(S source);

void onTransferEnd(S source, DataSpec dataSpec, boolean isNetwork);
}
Loading

0 comments on commit c3df64f

Please sign in to comment.