diff --git a/extensions/cronet/src/androidTest/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java b/extensions/cronet/src/androidTest/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java index dcc5bc9b97..b0de0784de 100644 --- a/extensions/cronet/src/androidTest/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java +++ b/extensions/cronet/src/androidTest/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java @@ -46,7 +46,6 @@ import com.google.android.exoplayer2.upstream.HttpDataSource.HttpDataSourceExcep import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.util.Clock; import com.google.android.exoplayer2.util.Predicate; - import java.io.IOException; import java.net.SocketTimeoutException; import java.net.UnknownHostException; @@ -82,7 +81,6 @@ public final class CronetDataSourceTest { private static final String TEST_CONTENT_TYPE = "test/test"; private static final byte[] TEST_POST_BODY = "test post body".getBytes(); private static final long TEST_CONTENT_LENGTH = 16000L; - private static final int TEST_BUFFER_SIZE = 16; private static final int TEST_CONNECTION_STATUS = 5; private DataSpec testDataSpec; @@ -231,10 +229,8 @@ public final class CronetDataSourceTest { @Test public void testRequestHeadersSet() throws HttpDataSourceException { - mockResponseStartSuccess(); - testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); - testResponseHeader.put("Content-Length", Long.toString(5000L)); + mockResponseStartSuccess(); dataSourceUnderTest.setRequestProperty("firstHeader", "firstValue"); dataSourceUnderTest.setRequestProperty("secondHeader", "secondValue"); @@ -257,13 +253,11 @@ public final class CronetDataSourceTest { @Test public void testRequestOpenGzippedCompressedReturnsDataSpecLength() throws HttpDataSourceException { + testDataSpec = new DataSpec(Uri.parse(TEST_URL), 0, 5000, null); testResponseHeader.put("Content-Encoding", "gzip"); - testUrlResponseInfo = createUrlResponseInfo(200); // statusCode + testResponseHeader.put("Content-Length", Long.toString(50L)); mockResponseStartSuccess(); - // Data spec's requested length, 5000. Test response's length, 16,000. - testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); - assertEquals(5000 /* contentLength */, dataSourceUnderTest.open(testDataSpec)); verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testDataSpec); } @@ -370,7 +364,7 @@ public final class CronetDataSourceTest { @Test public void testRequestReadTwice() throws HttpDataSourceException { mockResponseStartSuccess(); - mockReadSuccess(); + mockReadSuccess(0, 16); dataSourceUnderTest.open(testDataSpec); @@ -392,28 +386,23 @@ public final class CronetDataSourceTest { @Test public void testSecondRequestNoContentLength() throws HttpDataSourceException { mockResponseStartSuccess(); - mockReadSuccess(); - - byte[] returnedBuffer = new byte[8]; + testResponseHeader.put("Content-Length", Long.toString(1L)); + mockReadSuccess(0, 16); // First request. - testResponseHeader.put("Content-Length", Long.toString(1L)); - testUrlResponseInfo = createUrlResponseInfo(200); // statusCode dataSourceUnderTest.open(testDataSpec); + byte[] returnedBuffer = new byte[8]; dataSourceUnderTest.read(returnedBuffer, 0, 1); dataSourceUnderTest.close(); - // Second request. There's no Content-Length response header. testResponseHeader.remove("Content-Length"); - testUrlResponseInfo = createUrlResponseInfo(200); // statusCode + mockReadSuccess(0, 16); + + // Second request. dataSourceUnderTest.open(testDataSpec); returnedBuffer = new byte[16]; int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 10); assertEquals(10, bytesRead); - - mockResponseFinished(); - - // Should read whats left in the buffer first. bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 10); assertEquals(6, bytesRead); bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 10); @@ -423,23 +412,54 @@ public final class CronetDataSourceTest { @Test public void testReadWithOffset() throws HttpDataSourceException { mockResponseStartSuccess(); - mockReadSuccess(); + mockReadSuccess(0, 16); dataSourceUnderTest.open(testDataSpec); byte[] returnedBuffer = new byte[16]; int bytesRead = dataSourceUnderTest.read(returnedBuffer, 8, 8); - assertArrayEquals(prefixZeros(buildTestDataArray(0, 8), 16), returnedBuffer); assertEquals(8, bytesRead); + assertArrayEquals(prefixZeros(buildTestDataArray(0, 8), 16), returnedBuffer); verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 8); } + @Test + public void testRangeRequestWith206Response() throws HttpDataSourceException { + mockResponseStartSuccess(); + mockReadSuccess(1000, 5000); + testUrlResponseInfo = createUrlResponseInfo(206); // Server supports range requests. + testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); + + dataSourceUnderTest.open(testDataSpec); + + byte[] returnedBuffer = new byte[16]; + int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 16); + assertEquals(16, bytesRead); + assertArrayEquals(buildTestDataArray(1000, 16), returnedBuffer); + verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 16); + } + + @Test + public void testRangeRequestWith200Response() throws HttpDataSourceException { + mockResponseStartSuccess(); + mockReadSuccess(0, 7000); + testUrlResponseInfo = createUrlResponseInfo(200); // Server does not support range requests. + testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); + + dataSourceUnderTest.open(testDataSpec); + + byte[] returnedBuffer = new byte[16]; + int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 16); + assertEquals(16, bytesRead); + assertArrayEquals(buildTestDataArray(1000, 16), returnedBuffer); + verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 16); + } + @Test public void testReadWithUnsetLength() throws HttpDataSourceException { testResponseHeader.remove("Content-Length"); - testUrlResponseInfo = createUrlResponseInfo(200); // statusCode mockResponseStartSuccess(); - mockReadSuccess(); + mockReadSuccess(0, 16); dataSourceUnderTest.open(testDataSpec); @@ -453,7 +473,7 @@ public final class CronetDataSourceTest { @Test public void testReadReturnsWhatItCan() throws HttpDataSourceException { mockResponseStartSuccess(); - mockReadSuccess(); + mockReadSuccess(0, 16); dataSourceUnderTest.open(testDataSpec); @@ -467,7 +487,7 @@ public final class CronetDataSourceTest { @Test public void testClosedMeansClosed() throws HttpDataSourceException { mockResponseStartSuccess(); - mockReadSuccess(); + mockReadSuccess(0, 16); int bytesRead = 0; dataSourceUnderTest.open(testDataSpec); @@ -493,32 +513,29 @@ public final class CronetDataSourceTest { @Test public void testOverread() throws HttpDataSourceException { - mockResponseStartSuccess(); - mockReadSuccess(); - - // Ask for 16 bytes - testDataSpec = new DataSpec(Uri.parse(TEST_URL), 10000, 16, null); - // Let the response promise to give 16 bytes back. + testDataSpec = new DataSpec(Uri.parse(TEST_URL), 0, 16, null); testResponseHeader.put("Content-Length", Long.toString(16L)); + mockResponseStartSuccess(); + mockReadSuccess(0, 16); dataSourceUnderTest.open(testDataSpec); byte[] returnedBuffer = new byte[8]; int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 8); - assertArrayEquals(buildTestDataArray(0, 8), returnedBuffer); assertEquals(8, bytesRead); + assertArrayEquals(buildTestDataArray(0, 8), returnedBuffer); // The current buffer is kept if not completely consumed by DataSource reader. returnedBuffer = new byte[8]; bytesRead += dataSourceUnderTest.read(returnedBuffer, 0, 6); - assertArrayEquals(suffixZeros(buildTestDataArray(8, 6), 8), returnedBuffer); assertEquals(14, bytesRead); + assertArrayEquals(suffixZeros(buildTestDataArray(8, 6), 8), returnedBuffer); // 2 bytes left at this point. returnedBuffer = new byte[8]; bytesRead += dataSourceUnderTest.read(returnedBuffer, 0, 8); - assertArrayEquals(suffixZeros(buildTestDataArray(14, 2), 8), returnedBuffer); assertEquals(16, bytesRead); + assertArrayEquals(suffixZeros(buildTestDataArray(14, 2), 8), returnedBuffer); // Should have only called read on cronet once. verify(mockUrlRequest, times(1)).read(any(ByteBuffer.class)); @@ -752,16 +769,24 @@ public final class CronetDataSourceTest { }).when(mockUrlRequest).start(); } - private void mockReadSuccess() { + private void mockReadSuccess(int position, int length) { + final int[] positionAndRemaining = new int[] {position, length}; doAnswer(new Answer() { @Override public Void answer(InvocationOnMock invocation) throws Throwable { - ByteBuffer inputBuffer = (ByteBuffer) invocation.getArguments()[0]; - inputBuffer.put(buildTestDataBuffer()); - dataSourceUnderTest.onReadCompleted( - mockUrlRequest, - testUrlResponseInfo, - inputBuffer); + if (positionAndRemaining[1] == 0) { + dataSourceUnderTest.onSucceeded(mockUrlRequest, testUrlResponseInfo); + } else { + ByteBuffer inputBuffer = (ByteBuffer) invocation.getArguments()[0]; + int readLength = Math.min(positionAndRemaining[1], inputBuffer.remaining()); + inputBuffer.put(buildTestDataBuffer(positionAndRemaining[0], readLength)); + positionAndRemaining[0] += readLength; + positionAndRemaining[1] -= readLength; + dataSourceUnderTest.onReadCompleted( + mockUrlRequest, + testUrlResponseInfo, + inputBuffer); + } return null; } }).when(mockUrlRequest).read(any(ByteBuffer.class)); @@ -780,16 +805,6 @@ public final class CronetDataSourceTest { }).when(mockUrlRequest).read(any(ByteBuffer.class)); } - private void mockResponseFinished() { - doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) throws Throwable { - dataSourceUnderTest.onSucceeded(mockUrlRequest, testUrlResponseInfo); - return null; - } - }).when(mockUrlRequest).read(any(ByteBuffer.class)); - } - private ConditionVariable buildUrlRequestStartedCondition() { final ConditionVariable startedCondition = new ConditionVariable(); doAnswer(new Answer() { @@ -802,8 +817,8 @@ public final class CronetDataSourceTest { return startedCondition; } - private static byte[] buildTestDataArray(int start, int length) { - return Arrays.copyOfRange(buildTestDataBuffer().array(), start, start + length); + private static byte[] buildTestDataArray(int position, int length) { + return buildTestDataBuffer(position, length).array(); } public static byte[] prefixZeros(byte[] data, int requiredLength) { @@ -816,10 +831,10 @@ public final class CronetDataSourceTest { return Arrays.copyOf(data, requiredLength); } - private static ByteBuffer buildTestDataBuffer() { - ByteBuffer testBuffer = ByteBuffer.allocate(TEST_BUFFER_SIZE); - for (byte i = 1; i <= TEST_BUFFER_SIZE; i++) { - testBuffer.put(i); + private static ByteBuffer buildTestDataBuffer(int position, int length) { + ByteBuffer testBuffer = ByteBuffer.allocate(length); + for (int i = 0; i < length; i++) { + testBuffer.put((byte) (position + i)); } testBuffer.flip(); return testBuffer; diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java index a758f71f45..15ffe5f141 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java @@ -103,6 +103,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou // Accessed by the calling thread only. private boolean opened; + private long bytesToSkip; private long bytesRemaining; // Written from the calling thread only. currentUrlRequest.start() calls ensure writes are visible @@ -242,9 +243,10 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou } } - // TODO: Handle the case where we requested a range starting from a non-zero position and - // received a 200 rather than a 206. This occurs if the server does not support partial - // requests, and requires that the source skips to the requested position. + // If we requested a range starting from a non-zero position and received a 200 rather than a + // 206, then the server does not support partial requests. We'll need to manually skip to the + // requested position. + bytesToSkip = responseCode == 200 && dataSpec.position != 0 ? dataSpec.position : 0; // Calculate the content length. if (!getIsCompressed(responseInfo)) { @@ -281,7 +283,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou readBuffer = ByteBuffer.allocateDirect(READ_BUFFER_SIZE_BYTES); readBuffer.limit(0); } - if (!readBuffer.hasRemaining()) { + while (!readBuffer.hasRemaining()) { // Fill readBuffer with more data from Cronet. operation.close(); readBuffer.clear(); @@ -301,6 +303,12 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou } else { // The operation didn't time out, fail or finish, and therefore data must have been read. readBuffer.flip(); + Assertions.checkState(readBuffer.hasRemaining()); + if (bytesToSkip > 0) { + int bytesSkipped = (int) Math.min(readBuffer.remaining(), bytesToSkip); + readBuffer.position(readBuffer.position() + bytesSkipped); + bytesToSkip -= bytesSkipped; + } } }