From 880bdc181adf9805ec984b7d77687499d73c598f Mon Sep 17 00:00:00 2001 From: Satoshi Matsumoto Date: Fri, 7 Oct 2016 21:00:27 +0900 Subject: [PATCH 01/24] Use Call.Factory instead of OkHttpClient This allows using alternate implementation of an HTTP client. We can use OkHttpClient as before as it implements Call.Factory. --- .../ext/okhttp/OkHttpDataSource.java | 26 +++++++++---------- .../ext/okhttp/OkHttpDataSourceFactory.java | 14 +++++----- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java index e4e3d7ba00..7a8f3bca3f 100644 --- a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java +++ b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java @@ -32,21 +32,21 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import okhttp3.CacheControl; +import okhttp3.Call; import okhttp3.HttpUrl; import okhttp3.MediaType; -import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.RequestBody; import okhttp3.Response; /** - * An {@link HttpDataSource} that delegates to Square's {@link OkHttpClient}. + * An {@link HttpDataSource} that delegates to Square's {@link Call.Factory}. */ public class OkHttpDataSource implements HttpDataSource { private static final AtomicReference skipBufferReference = new AtomicReference<>(); - private final OkHttpClient okHttpClient; + private final Call.Factory callFactory; private final String userAgent; private final Predicate contentTypePredicate; private final TransferListener listener; @@ -65,31 +65,31 @@ public class OkHttpDataSource implements HttpDataSource { private long bytesRead; /** - * @param client An {@link OkHttpClient} for use by the source. + * @param callFactory An {@link Call.Factory} for use by the source. * @param userAgent The User-Agent string that should be used. * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the * predicate then a InvalidContentTypeException} is thrown from {@link #open(DataSpec)}. */ - public OkHttpDataSource(OkHttpClient client, String userAgent, + public OkHttpDataSource(Call.Factory callFactory, String userAgent, Predicate contentTypePredicate) { - this(client, userAgent, contentTypePredicate, null); + this(callFactory, userAgent, contentTypePredicate, null); } /** - * @param client An {@link OkHttpClient} for use by the source. + * @param callFactory An {@link Call.Factory} for use by the source. * @param userAgent The User-Agent string that should be used. * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the * predicate then a {@link InvalidContentTypeException} is thrown from * {@link #open(DataSpec)}. * @param listener An optional listener. */ - public OkHttpDataSource(OkHttpClient client, String userAgent, + public OkHttpDataSource(Call.Factory callFactory, String userAgent, Predicate contentTypePredicate, TransferListener listener) { - this(client, userAgent, contentTypePredicate, listener, null); + this(callFactory, userAgent, contentTypePredicate, listener, null); } /** - * @param client An {@link OkHttpClient} for use by the source. + * @param callFactory An {@link Call.Factory} for use by the source. * @param userAgent The User-Agent string that should be used. * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the * predicate then a {@link InvalidContentTypeException} is thrown from @@ -98,10 +98,10 @@ public class OkHttpDataSource implements HttpDataSource { * @param cacheControl An optional {@link CacheControl} which sets all requests' Cache-Control * header. For example, you could force the network response for all requests. */ - public OkHttpDataSource(OkHttpClient client, String userAgent, + public OkHttpDataSource(Call.Factory callFactory, String userAgent, Predicate contentTypePredicate, TransferListener listener, CacheControl cacheControl) { - this.okHttpClient = Assertions.checkNotNull(client); + this.callFactory = Assertions.checkNotNull(callFactory); this.userAgent = Assertions.checkNotEmpty(userAgent); this.contentTypePredicate = contentTypePredicate; this.listener = listener; @@ -150,7 +150,7 @@ public class OkHttpDataSource implements HttpDataSource { this.bytesSkipped = 0; Request request = makeRequest(dataSpec); try { - response = okHttpClient.newCall(request).execute(); + response = callFactory.newCall(request).execute(); responseByteStream = response.body().byteStream(); } catch (IOException e) { throw new HttpDataSourceException("Unable to connect to " + dataSpec.uri.toString(), e, diff --git a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceFactory.java b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceFactory.java index 1cfb7219d6..a4dd10a8d3 100644 --- a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceFactory.java +++ b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceFactory.java @@ -19,26 +19,26 @@ import com.google.android.exoplayer2.upstream.DataSource; import com.google.android.exoplayer2.upstream.HttpDataSource.Factory; import com.google.android.exoplayer2.upstream.TransferListener; import okhttp3.CacheControl; -import okhttp3.OkHttpClient; +import okhttp3.Call; /** * A {@link Factory} that produces {@link OkHttpDataSource}. */ public final class OkHttpDataSourceFactory implements Factory { - private final OkHttpClient client; + private final Call.Factory callFactory; private final String userAgent; private final TransferListener transferListener; private final CacheControl cacheControl; - public OkHttpDataSourceFactory(OkHttpClient client, String userAgent, + public OkHttpDataSourceFactory(Call.Factory callFactory, String userAgent, TransferListener transferListener) { - this(client, userAgent, transferListener, null); + this(callFactory, userAgent, transferListener, null); } - public OkHttpDataSourceFactory(OkHttpClient client, String userAgent, + public OkHttpDataSourceFactory(Call.Factory callFactory, String userAgent, TransferListener transferListener, CacheControl cacheControl) { - this.client = client; + this.callFactory = callFactory; this.userAgent = userAgent; this.transferListener = transferListener; this.cacheControl = cacheControl; @@ -46,7 +46,7 @@ public final class OkHttpDataSourceFactory implements Factory { @Override public OkHttpDataSource createDataSource() { - return new OkHttpDataSource(client, userAgent, null, transferListener, cacheControl); + return new OkHttpDataSource(callFactory, userAgent, null, transferListener, cacheControl); } } From 5803b2538d4f6a8ef45e95ec73b6c42cc974dcd9 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 6 Oct 2016 10:13:27 -0700 Subject: [PATCH 02/24] No-op reorder of CronetDataSource methods This change is a no-op reodering, as a precursor to further cleanup. The public methods are grouped by the class/interface they implement. The private methods are ordered with things that will become static in a subsequent change at the bottom. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135372629 --- .../ext/cronet/CronetDataSource.java | 427 +++++++++--------- 1 file changed, 213 insertions(+), 214 deletions(-) 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 cae28731ea..04b468291d 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 @@ -169,6 +169,8 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou connectionState = IDLE_CONNECTION; } + // HttpDataSource implementation. + @Override public void setRequestProperty(String name, String value) { synchronized (requestProperties) { @@ -195,6 +197,11 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou return responseInfo == null ? null : responseInfo.getAllHeaders(); } + @Override + public Uri getUri() { + return Uri.parse(currentUrl); + } + @Override public long open(DataSpec dataSpec) throws HttpDataSourceException { Assertions.checkNotNull(dataSpec); @@ -224,186 +231,6 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou return contentLength; } - private void startRequest(DataSpec dataSpec) throws HttpDataSourceException { - currentUrl = dataSpec.uri.toString(); - currentDataSpec = dataSpec; - UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(currentUrl, this, executor, - cronetEngine); - fillCurrentRequestHeader(urlRequestBuilder); - fillCurrentRequestPostBody(urlRequestBuilder, dataSpec); - currentUrlRequest = urlRequestBuilder.build(); - currentUrlRequest.start(); - } - - private void fillCurrentRequestHeader(UrlRequest.Builder urlRequestBuilder) { - synchronized (requestProperties) { - for (Entry headerEntry : requestProperties.entrySet()) { - urlRequestBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue()); - } - } - if (currentDataSpec.position == 0 && currentDataSpec.length == C.LENGTH_UNSET) { - // Not required. - return; - } - StringBuilder rangeValue = new StringBuilder(); - rangeValue.append("bytes="); - rangeValue.append(currentDataSpec.position); - rangeValue.append("-"); - if (currentDataSpec.length != C.LENGTH_UNSET) { - rangeValue.append(currentDataSpec.position + currentDataSpec.length - 1); - } - urlRequestBuilder.addHeader("Range", rangeValue.toString()); - } - - private void fillCurrentRequestPostBody(UrlRequest.Builder urlRequestBuilder, DataSpec dataSpec) - throws HttpDataSourceException { - if (dataSpec.postBody != null) { - if (!requestProperties.containsKey("Content-Type")) { - throw new OpenException("POST requests must set a Content-Type header", dataSpec, - getCurrentRequestStatus()); - } - urlRequestBuilder.setUploadDataProvider( - new ByteArrayUploadDataProvider(dataSpec.postBody), executor); - } - } - - @Override - public synchronized void onFailed(UrlRequest request, UrlResponseInfo info, - UrlRequestException error) { - if (request != currentUrlRequest) { - return; - } - if (connectionState == OPENING_CONNECTION) { - IOException cause = error.getErrorCode() == UrlRequestException.ERROR_HOSTNAME_NOT_RESOLVED - ? new UnknownHostException() : error; - exception = new OpenException(cause, currentDataSpec, getCurrentRequestStatus()); - } else if (connectionState == OPEN_CONNECTION) { - exception = new HttpDataSourceException(error, currentDataSpec, - HttpDataSourceException.TYPE_READ); - } - operation.open(); - } - - @Override - public synchronized void onResponseStarted(UrlRequest request, UrlResponseInfo info) { - if (request != currentUrlRequest) { - return; - } - try { - validateResponse(info); - responseInfo = info; - - if (isCompressed(info)) { - contentLength = currentDataSpec.length; - } else { - // Check content length. - contentLength = getContentLength(info.getAllHeaders()); - // If a specific length is requested and a specific length is returned but the 2 don't match - // it's an error. - if (currentDataSpec.length != C.LENGTH_UNSET - && contentLength != C.LENGTH_UNSET - && currentDataSpec.length != contentLength) { - throw new OpenException("Content length did not match requested length", currentDataSpec, - getCurrentRequestStatus()); - } - } - - if (contentLength > 0) { - expectedBytesRemainingToRead = new AtomicLong(contentLength); - } - - // Keep track of redirects. - currentUrl = responseInfo.getUrl(); - connectionState = CONNECTED_CONNECTION; - } catch (HttpDataSourceException e) { - exception = e; - } finally { - operation.open(); - } - } - - /** - * Returns {@code true} iff the content is compressed. - * - *

If {@code true}, clients cannot use the value of content length from the request headers to - * read the data, since Cronet returns the uncompressed data and this content length reflects the - * compressed content length. - */ - private boolean isCompressed(UrlResponseInfo info) { - for (Map.Entry entry : info.getAllHeadersAsList()) { - if (entry.getKey().equalsIgnoreCase("Content-Encoding")) { - return !entry.getValue().equalsIgnoreCase("identity"); - } - } - - return false; - } - - private void validateResponse(UrlResponseInfo info) throws HttpDataSourceException { - // Check for a valid response code. - int responseCode = info.getHttpStatusCode(); - if (responseCode < 200 || responseCode > 299) { - InvalidResponseCodeException exception = new InvalidResponseCodeException( - responseCode, info.getAllHeaders(), currentDataSpec); - if (responseCode == 416) { - exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); - } - throw exception; - } - // Check for a valid content type. - try { - String contentType = info.getAllHeaders().get("Content-Type").get(0); - if (contentTypePredicate != null && !contentTypePredicate.evaluate(contentType)) { - throw new InvalidContentTypeException(contentType, currentDataSpec); - } - } catch (IndexOutOfBoundsException e) { - throw new InvalidContentTypeException(null, currentDataSpec); - } - } - - private long getContentLength(Map> headers) { - // Logic copied from {@code DefaultHttpDataSource} - long contentLength = C.LENGTH_UNSET; - List contentLengthHeader = headers.get("Content-Length"); - if (contentLengthHeader != null - && !contentLengthHeader.isEmpty() - && !TextUtils.isEmpty(contentLengthHeader.get(0))) { - try { - contentLength = Long.parseLong(contentLengthHeader.get(0)); - } catch (NumberFormatException e) { - log(Log.ERROR, "Unexpected Content-Length [" + contentLengthHeader + "]"); - } - } - List contentRangeHeader = headers.get("Content-Range"); - if (contentRangeHeader != null - && !contentRangeHeader.isEmpty() - && !TextUtils.isEmpty(contentRangeHeader.get(0))) { - Matcher matcher = CONTENT_RANGE_HEADER_PATTERN.matcher(contentRangeHeader.get(0)); - if (matcher.find()) { - try { - long contentLengthFromRange = - Long.parseLong(matcher.group(2)) - Long.parseLong(matcher.group(1)) + 1; - if (contentLength < 0) { - // Some proxy servers strip the Content-Length header. Fall back to the length - // calculated here in this case. - contentLength = contentLengthFromRange; - } else if (contentLength != contentLengthFromRange) { - // If there is a discrepancy between the Content-Length and Content-Range headers, - // assume the one with the larger value is correct. We have seen cases where carrier - // change one of them to reduce the size of a request, but it is unlikely anybody - // would increase it. - log(Log.WARN, "Inconsistent headers [" + contentLengthHeader + "] [" - + contentRangeHeader + "]"); - contentLength = Math.max(contentLength, contentLengthFromRange); - } - } catch (NumberFormatException e) { - log(Log.ERROR, "Unexpected Content-Range [" + contentRangeHeader + "]"); - } - } - } - return contentLength; - } - @Override public int read(byte[] buffer, int offset, int readLength) throws HttpDataSourceException { synchronized (this) { @@ -451,6 +278,31 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou return bytesRead; } + @Override + public synchronized void close() { + if (currentUrlRequest != null) { + currentUrlRequest.cancel(); + currentUrlRequest = null; + } + currentDataSpec = null; + currentUrl = null; + exception = null; + contentLength = 0; + hasData = false; + responseInfo = null; + expectedBytesRemainingToRead = null; + responseFinished = false; + try { + if (listener != null && connectionState == OPEN_CONNECTION) { + listener.onTransferEnd(this); + } + } finally { + connectionState = IDLE_CONNECTION; + } + } + + // UrlRequest.Callback implementation + @Override public void onRedirectReceived(UrlRequest request, UrlResponseInfo info, String newLocationUrl) { if (request != currentUrlRequest) { @@ -474,6 +326,44 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou request.followRedirect(); } + @Override + public synchronized void onResponseStarted(UrlRequest request, UrlResponseInfo info) { + if (request != currentUrlRequest) { + return; + } + try { + validateResponse(info); + responseInfo = info; + + if (isCompressed(info)) { + contentLength = currentDataSpec.length; + } else { + // Check content length. + contentLength = getContentLength(info.getAllHeaders()); + // If a specific length is requested and a specific length is returned but the 2 don't match + // it's an error. + if (currentDataSpec.length != C.LENGTH_UNSET + && contentLength != C.LENGTH_UNSET + && currentDataSpec.length != contentLength) { + throw new OpenException("Content length did not match requested length", currentDataSpec, + getCurrentRequestStatus()); + } + } + + if (contentLength > 0) { + expectedBytesRemainingToRead = new AtomicLong(contentLength); + } + + // Keep track of redirects. + currentUrl = responseInfo.getUrl(); + connectionState = CONNECTED_CONNECTION; + } catch (HttpDataSourceException e) { + exception = e; + } finally { + operation.open(); + } + } + @Override public synchronized void onReadCompleted(UrlRequest request, UrlResponseInfo info, ByteBuffer buffer) { @@ -495,37 +385,154 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou } @Override - public synchronized void close() { - if (currentUrlRequest != null) { - currentUrlRequest.cancel(); - currentUrlRequest = null; + public synchronized void onFailed(UrlRequest request, UrlResponseInfo info, + UrlRequestException error) { + if (request != currentUrlRequest) { + return; } - currentDataSpec = null; - currentUrl = null; - exception = null; - contentLength = 0; - hasData = false; - responseInfo = null; - expectedBytesRemainingToRead = null; - responseFinished = false; - try { - if (listener != null && connectionState == OPEN_CONNECTION) { - listener.onTransferEnd(this); + if (connectionState == OPENING_CONNECTION) { + IOException cause = error.getErrorCode() == UrlRequestException.ERROR_HOSTNAME_NOT_RESOLVED + ? new UnknownHostException() : error; + exception = new OpenException(cause, currentDataSpec, getCurrentRequestStatus()); + } else if (connectionState == OPEN_CONNECTION) { + exception = new HttpDataSourceException(error, currentDataSpec, + HttpDataSourceException.TYPE_READ); + } + operation.open(); + } + + // Internal methods. + + private void fillCurrentRequestHeader(UrlRequest.Builder urlRequestBuilder) { + synchronized (requestProperties) { + for (Entry headerEntry : requestProperties.entrySet()) { + urlRequestBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue()); } - } finally { - connectionState = IDLE_CONNECTION; + } + if (currentDataSpec.position == 0 && currentDataSpec.length == C.LENGTH_UNSET) { + // Not required. + return; + } + StringBuilder rangeValue = new StringBuilder(); + rangeValue.append("bytes="); + rangeValue.append(currentDataSpec.position); + rangeValue.append("-"); + if (currentDataSpec.length != C.LENGTH_UNSET) { + rangeValue.append(currentDataSpec.position + currentDataSpec.length - 1); + } + urlRequestBuilder.addHeader("Range", rangeValue.toString()); + } + + private void fillCurrentRequestPostBody(UrlRequest.Builder urlRequestBuilder, DataSpec dataSpec) + throws HttpDataSourceException { + if (dataSpec.postBody != null) { + if (!requestProperties.containsKey("Content-Type")) { + throw new OpenException("POST requests must set a Content-Type header", dataSpec, + getCurrentRequestStatus()); + } + urlRequestBuilder.setUploadDataProvider( + new ByteArrayUploadDataProvider(dataSpec.postBody), executor); } } - @Override - public Uri getUri() { - return Uri.parse(currentUrl); + private void startRequest(DataSpec dataSpec) throws HttpDataSourceException { + currentUrl = dataSpec.uri.toString(); + currentDataSpec = dataSpec; + UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(currentUrl, this, executor, + cronetEngine); + fillCurrentRequestHeader(urlRequestBuilder); + fillCurrentRequestPostBody(urlRequestBuilder, dataSpec); + currentUrlRequest = urlRequestBuilder.build(); + currentUrlRequest.start(); } - private void log(int priority, String message) { - if (Log.isLoggable(TAG, priority)) { - Log.println(priority, TAG, message); + private void validateResponse(UrlResponseInfo info) throws HttpDataSourceException { + // Check for a valid response code. + int responseCode = info.getHttpStatusCode(); + if (responseCode < 200 || responseCode > 299) { + InvalidResponseCodeException exception = new InvalidResponseCodeException( + responseCode, info.getAllHeaders(), currentDataSpec); + if (responseCode == 416) { + exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); + } + throw exception; } + // Check for a valid content type. + try { + String contentType = info.getAllHeaders().get("Content-Type").get(0); + if (contentTypePredicate != null && !contentTypePredicate.evaluate(contentType)) { + throw new InvalidContentTypeException(contentType, currentDataSpec); + } + } catch (IndexOutOfBoundsException e) { + throw new InvalidContentTypeException(null, currentDataSpec); + } + } + + private boolean blockUntilConnectTimeout() { + long now = clock.elapsedRealtime(); + boolean opened = false; + while (!opened && now < currentConnectTimeoutMs) { + opened = operation.block(currentConnectTimeoutMs - now + 5 /* fudge factor */); + now = clock.elapsedRealtime(); + } + return opened; + } + + private void resetConnectTimeout() { + currentConnectTimeoutMs = clock.elapsedRealtime() + connectTimeoutMs; + } + + private boolean isCompressed(UrlResponseInfo info) { + for (Map.Entry entry : info.getAllHeadersAsList()) { + if (entry.getKey().equalsIgnoreCase("Content-Encoding")) { + return !entry.getValue().equalsIgnoreCase("identity"); + } + } + + return false; + } + + private long getContentLength(Map> headers) { + // Logic copied from {@code DefaultHttpDataSource} + long contentLength = C.LENGTH_UNSET; + List contentLengthHeader = headers.get("Content-Length"); + if (contentLengthHeader != null + && !contentLengthHeader.isEmpty() + && !TextUtils.isEmpty(contentLengthHeader.get(0))) { + try { + contentLength = Long.parseLong(contentLengthHeader.get(0)); + } catch (NumberFormatException e) { + log(Log.ERROR, "Unexpected Content-Length [" + contentLengthHeader + "]"); + } + } + List contentRangeHeader = headers.get("Content-Range"); + if (contentRangeHeader != null + && !contentRangeHeader.isEmpty() + && !TextUtils.isEmpty(contentRangeHeader.get(0))) { + Matcher matcher = CONTENT_RANGE_HEADER_PATTERN.matcher(contentRangeHeader.get(0)); + if (matcher.find()) { + try { + long contentLengthFromRange = + Long.parseLong(matcher.group(2)) - Long.parseLong(matcher.group(1)) + 1; + if (contentLength < 0) { + // Some proxy servers strip the Content-Length header. Fall back to the length + // calculated here in this case. + contentLength = contentLengthFromRange; + } else if (contentLength != contentLengthFromRange) { + // If there is a discrepancy between the Content-Length and Content-Range headers, + // assume the one with the larger value is correct. We have seen cases where carrier + // change one of them to reduce the size of a request, but it is unlikely anybody + // would increase it. + log(Log.WARN, "Inconsistent headers [" + contentLengthHeader + "] [" + + contentRangeHeader + "]"); + contentLength = Math.max(contentLength, contentLengthFromRange); + } + } catch (NumberFormatException e) { + log(Log.ERROR, "Unexpected Content-Range [" + contentRangeHeader + "]"); + } + } + } + return contentLength; } private int getCurrentRequestStatus() { @@ -544,18 +551,10 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou return result.get(); } - private boolean blockUntilConnectTimeout() { - long now = clock.elapsedRealtime(); - boolean opened = false; - while (!opened && now < currentConnectTimeoutMs) { - opened = operation.block(currentConnectTimeoutMs - now + 5 /* fudge factor */); - now = clock.elapsedRealtime(); + private void log(int priority, String message) { + if (Log.isLoggable(TAG, priority)) { + Log.println(priority, TAG, message); } - return opened; - } - - private void resetConnectTimeout() { - currentConnectTimeoutMs = clock.elapsedRealtime() + connectTimeoutMs; } } From ba56f9165cd7feae9a6039779de0bc311d8736d7 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 6 Oct 2016 10:20:55 -0700 Subject: [PATCH 03/24] Partial cleanup of CronetDataSource - Fix bug in getCurrentRequestStatus where we weren't blocking on the condition variable. - Make methods static where possible. - Clean up getUri implementation. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135373586 --- .../ext/cronet/CronetDataSource.java | 76 +++++++------------ 1 file changed, 29 insertions(+), 47 deletions(-) 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 04b468291d..266feca6d6 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 @@ -37,12 +37,12 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.Executor; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.chromium.net.CronetEngine; import org.chromium.net.UrlRequest; +import org.chromium.net.UrlRequest.Status; import org.chromium.net.UrlRequestException; import org.chromium.net.UrlResponseInfo; @@ -112,7 +112,6 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou private UrlResponseInfo responseInfo; /* package */ volatile int connectionState; - private volatile String currentUrl; private volatile long currentConnectTimeoutMs; private volatile HttpDataSourceException exception; private volatile long contentLength; @@ -199,7 +198,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou @Override public Uri getUri() { - return Uri.parse(currentUrl); + return responseInfo == null ? null : Uri.parse(responseInfo.getUrl()); } @Override @@ -212,7 +211,14 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou operation.close(); resetConnectTimeout(); - startRequest(dataSpec); + + currentDataSpec = dataSpec; + UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(dataSpec.uri.toString(), this, + executor, cronetEngine); + fillCurrentRequestHeader(urlRequestBuilder); + fillCurrentRequestPostBody(urlRequestBuilder, dataSpec); + currentUrlRequest = urlRequestBuilder.build(); + currentUrlRequest.start(); boolean requestStarted = blockUntilConnectTimeout(); if (exception != null) { @@ -220,7 +226,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou throw exception; } else if (!requestStarted) { // The timeout was reached before the connection was opened. - throw new OpenException(new SocketTimeoutException(), dataSpec, getCurrentRequestStatus()); + throw new OpenException(new SocketTimeoutException(), dataSpec, getStatus(currentUrlRequest)); } // Connection was opened. @@ -285,7 +291,6 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou currentUrlRequest = null; } currentDataSpec = null; - currentUrl = null; exception = null; contentLength = 0; hasData = false; @@ -315,7 +320,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou // redirect is followed. if (responseCode == 307 || responseCode == 308) { exception = new OpenException("POST request redirected with 307 or 308 response code", - currentDataSpec, getCurrentRequestStatus()); + currentDataSpec, getStatus(request)); operation.open(); return; } @@ -333,9 +338,9 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou } try { validateResponse(info); - responseInfo = info; - if (isCompressed(info)) { + responseInfo = info; + if (getIsCompressed(info)) { contentLength = currentDataSpec.length; } else { // Check content length. @@ -346,16 +351,13 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou && contentLength != C.LENGTH_UNSET && currentDataSpec.length != contentLength) { throw new OpenException("Content length did not match requested length", currentDataSpec, - getCurrentRequestStatus()); + getStatus(request)); } } - if (contentLength > 0) { expectedBytesRemainingToRead = new AtomicLong(contentLength); } - // Keep track of redirects. - currentUrl = responseInfo.getUrl(); connectionState = CONNECTED_CONNECTION; } catch (HttpDataSourceException e) { exception = e; @@ -393,7 +395,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou if (connectionState == OPENING_CONNECTION) { IOException cause = error.getErrorCode() == UrlRequestException.ERROR_HOSTNAME_NOT_RESOLVED ? new UnknownHostException() : error; - exception = new OpenException(cause, currentDataSpec, getCurrentRequestStatus()); + exception = new OpenException(cause, currentDataSpec, getStatus(request)); } else if (connectionState == OPEN_CONNECTION) { exception = new HttpDataSourceException(error, currentDataSpec, HttpDataSourceException.TYPE_READ); @@ -428,24 +430,13 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou if (dataSpec.postBody != null) { if (!requestProperties.containsKey("Content-Type")) { throw new OpenException("POST requests must set a Content-Type header", dataSpec, - getCurrentRequestStatus()); + Status.IDLE); } urlRequestBuilder.setUploadDataProvider( new ByteArrayUploadDataProvider(dataSpec.postBody), executor); } } - private void startRequest(DataSpec dataSpec) throws HttpDataSourceException { - currentUrl = dataSpec.uri.toString(); - currentDataSpec = dataSpec; - UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(currentUrl, this, executor, - cronetEngine); - fillCurrentRequestHeader(urlRequestBuilder); - fillCurrentRequestPostBody(urlRequestBuilder, dataSpec); - currentUrlRequest = urlRequestBuilder.build(); - currentUrlRequest.start(); - } - private void validateResponse(UrlResponseInfo info) throws HttpDataSourceException { // Check for a valid response code. int responseCode = info.getHttpStatusCode(); @@ -482,17 +473,16 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou currentConnectTimeoutMs = clock.elapsedRealtime() + connectTimeoutMs; } - private boolean isCompressed(UrlResponseInfo info) { + private static boolean getIsCompressed(UrlResponseInfo info) { for (Map.Entry entry : info.getAllHeadersAsList()) { if (entry.getKey().equalsIgnoreCase("Content-Encoding")) { return !entry.getValue().equalsIgnoreCase("identity"); } } - return false; } - private long getContentLength(Map> headers) { + private static long getContentLength(Map> headers) { // Logic copied from {@code DefaultHttpDataSource} long contentLength = C.LENGTH_UNSET; List contentLengthHeader = headers.get("Content-Length"); @@ -502,7 +492,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou try { contentLength = Long.parseLong(contentLengthHeader.get(0)); } catch (NumberFormatException e) { - log(Log.ERROR, "Unexpected Content-Length [" + contentLengthHeader + "]"); + Log.e(TAG, "Unexpected Content-Length [" + contentLengthHeader + "]"); } } List contentRangeHeader = headers.get("Content-Range"); @@ -523,38 +513,30 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou // assume the one with the larger value is correct. We have seen cases where carrier // change one of them to reduce the size of a request, but it is unlikely anybody // would increase it. - log(Log.WARN, "Inconsistent headers [" + contentLengthHeader + "] [" - + contentRangeHeader + "]"); + Log.w(TAG, "Inconsistent headers [" + contentLengthHeader + "] [" + contentRangeHeader + + "]"); contentLength = Math.max(contentLength, contentLengthFromRange); } } catch (NumberFormatException e) { - log(Log.ERROR, "Unexpected Content-Range [" + contentRangeHeader + "]"); + Log.e(TAG, "Unexpected Content-Range [" + contentRangeHeader + "]"); } } } return contentLength; } - private int getCurrentRequestStatus() { - if (currentUrlRequest == null) { - return UrlRequest.Status.IDLE; - } + private static int getStatus(UrlRequest request) { final ConditionVariable conditionVariable = new ConditionVariable(); - final AtomicInteger result = new AtomicInteger(); - currentUrlRequest.getStatus(new UrlRequest.StatusListener() { + final int[] statusHolder = new int[1]; + request.getStatus(new UrlRequest.StatusListener() { @Override public void onStatus(int status) { - result.set(status); + statusHolder[0] = status; conditionVariable.open(); } }); - return result.get(); - } - - private void log(int priority, String message) { - if (Log.isLoggable(TAG, priority)) { - Log.println(priority, TAG, message); - } + conditionVariable.block(); + return statusHolder[0]; } } From 661b14020e02210f1bff436bc145c2c38663f115 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 6 Oct 2016 10:32:54 -0700 Subject: [PATCH 04/24] Partial cleanup of CronetDataSource II - Allow null Content-Type in response headers. - Inline validateResponse, just because it makes it clearer what thread it's being executed on when inlined. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135375063 --- .../ext/cronet/CronetDataSource.java | 59 +++++++++---------- 1 file changed, 27 insertions(+), 32 deletions(-) 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 266feca6d6..b563b70419 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 @@ -85,6 +85,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou public static final int DEFAULT_READ_TIMEOUT_MILLIS = 8 * 1000; private static final String TAG = "CronetDataSource"; + private static final String CONTENT_TYPE = "Content-Type"; private static final Pattern CONTENT_RANGE_HEADER_PATTERN = Pattern.compile("^bytes (\\d+)-(\\d+)/(\\d+)$"); // The size of read buffer passed to cronet UrlRequest.read(). @@ -337,7 +338,25 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou return; } try { - validateResponse(info); + // Check for a valid response code. + int responseCode = info.getHttpStatusCode(); + if (responseCode < 200 || responseCode > 299) { + InvalidResponseCodeException exception = new InvalidResponseCodeException( + responseCode, info.getAllHeaders(), currentDataSpec); + if (responseCode == 416) { + exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); + } + throw exception; + } + // Check for a valid content type. + if (contentTypePredicate != null) { + List contentTypeHeaders = info.getAllHeaders().get(CONTENT_TYPE); + String contentType = contentTypeHeaders == null || contentTypeHeaders.isEmpty() ? null + : contentTypeHeaders.get(0); + if (!contentTypePredicate.evaluate(contentType)) { + throw new InvalidContentTypeException(contentType, currentDataSpec); + } + } responseInfo = info; if (getIsCompressed(info)) { @@ -347,8 +366,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou contentLength = getContentLength(info.getAllHeaders()); // If a specific length is requested and a specific length is returned but the 2 don't match // it's an error. - if (currentDataSpec.length != C.LENGTH_UNSET - && contentLength != C.LENGTH_UNSET + if (currentDataSpec.length != C.LENGTH_UNSET && contentLength != C.LENGTH_UNSET && currentDataSpec.length != contentLength) { throw new OpenException("Content length did not match requested length", currentDataSpec, getStatus(request)); @@ -427,36 +445,14 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou private void fillCurrentRequestPostBody(UrlRequest.Builder urlRequestBuilder, DataSpec dataSpec) throws HttpDataSourceException { - if (dataSpec.postBody != null) { - if (!requestProperties.containsKey("Content-Type")) { - throw new OpenException("POST requests must set a Content-Type header", dataSpec, - Status.IDLE); - } - urlRequestBuilder.setUploadDataProvider( - new ByteArrayUploadDataProvider(dataSpec.postBody), executor); + if (dataSpec.postBody == null) { + return; } - } - - private void validateResponse(UrlResponseInfo info) throws HttpDataSourceException { - // Check for a valid response code. - int responseCode = info.getHttpStatusCode(); - if (responseCode < 200 || responseCode > 299) { - InvalidResponseCodeException exception = new InvalidResponseCodeException( - responseCode, info.getAllHeaders(), currentDataSpec); - if (responseCode == 416) { - exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); - } - throw exception; - } - // Check for a valid content type. - try { - String contentType = info.getAllHeaders().get("Content-Type").get(0); - if (contentTypePredicate != null && !contentTypePredicate.evaluate(contentType)) { - throw new InvalidContentTypeException(contentType, currentDataSpec); - } - } catch (IndexOutOfBoundsException e) { - throw new InvalidContentTypeException(null, currentDataSpec); + if (!requestProperties.containsKey(CONTENT_TYPE)) { + throw new OpenException("POST request must set Content-Type", dataSpec, Status.IDLE); } + urlRequestBuilder.setUploadDataProvider(new ByteArrayUploadDataProvider(dataSpec.postBody), + executor); } private boolean blockUntilConnectTimeout() { @@ -483,7 +479,6 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou } private static long getContentLength(Map> headers) { - // Logic copied from {@code DefaultHttpDataSource} long contentLength = C.LENGTH_UNSET; List contentLengthHeader = headers.get("Content-Length"); if (contentLengthHeader != null From d95baa3ee95121b227497d0fe63a0481b7b8b0e8 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 6 Oct 2016 10:46:38 -0700 Subject: [PATCH 05/24] CronetDataSource: Fix thread safety issue with requestProperties The access in fillCurrentRequestPostBody wasn't protected with synchronization. Furthermore, just synchronizing it wouldn't be sufficient, since what we really need to check is whether the Content-Type header has been added to the UrlRequest.Builder. The contents of requestProperties may have changed between the headers being added to UrlRequest.Builder and the call to fillCurrentRequestPostBody. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135376904 --- .../ext/cronet/CronetDataSource.java | 54 +++++++++---------- 1 file changed, 24 insertions(+), 30 deletions(-) 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 b563b70419..594ea8cb17 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 @@ -212,13 +212,8 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou operation.close(); resetConnectTimeout(); - currentDataSpec = dataSpec; - UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(dataSpec.uri.toString(), this, - executor, cronetEngine); - fillCurrentRequestHeader(urlRequestBuilder); - fillCurrentRequestPostBody(urlRequestBuilder, dataSpec); - currentUrlRequest = urlRequestBuilder.build(); + currentUrlRequest = buildRequest(dataSpec); currentUrlRequest.start(); boolean requestStarted = blockUntilConnectTimeout(); @@ -423,36 +418,35 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou // Internal methods. - private void fillCurrentRequestHeader(UrlRequest.Builder urlRequestBuilder) { + private UrlRequest buildRequest(DataSpec dataSpec) throws OpenException { + UrlRequest.Builder requestBuilder = new UrlRequest.Builder(dataSpec.uri.toString(), this, + executor, cronetEngine); + // Set the headers. synchronized (requestProperties) { + if (dataSpec.postBody != null && !requestProperties.containsKey(CONTENT_TYPE)) { + throw new OpenException("POST request must set Content-Type", dataSpec, Status.IDLE); + } for (Entry headerEntry : requestProperties.entrySet()) { - urlRequestBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue()); + requestBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue()); } } - if (currentDataSpec.position == 0 && currentDataSpec.length == C.LENGTH_UNSET) { - // Not required. - return; + // Set the Range header. + if (currentDataSpec.position != 0 || currentDataSpec.length != C.LENGTH_UNSET) { + StringBuilder rangeValue = new StringBuilder(); + rangeValue.append("bytes="); + rangeValue.append(currentDataSpec.position); + rangeValue.append("-"); + if (currentDataSpec.length != C.LENGTH_UNSET) { + rangeValue.append(currentDataSpec.position + currentDataSpec.length - 1); + } + requestBuilder.addHeader("Range", rangeValue.toString()); } - StringBuilder rangeValue = new StringBuilder(); - rangeValue.append("bytes="); - rangeValue.append(currentDataSpec.position); - rangeValue.append("-"); - if (currentDataSpec.length != C.LENGTH_UNSET) { - rangeValue.append(currentDataSpec.position + currentDataSpec.length - 1); + // Set the body. + if (dataSpec.postBody != null) { + requestBuilder.setUploadDataProvider(new ByteArrayUploadDataProvider(dataSpec.postBody), + executor); } - urlRequestBuilder.addHeader("Range", rangeValue.toString()); - } - - private void fillCurrentRequestPostBody(UrlRequest.Builder urlRequestBuilder, DataSpec dataSpec) - throws HttpDataSourceException { - if (dataSpec.postBody == null) { - return; - } - if (!requestProperties.containsKey(CONTENT_TYPE)) { - throw new OpenException("POST request must set Content-Type", dataSpec, Status.IDLE); - } - urlRequestBuilder.setUploadDataProvider(new ByteArrayUploadDataProvider(dataSpec.postBody), - executor); + return requestBuilder.build(); } private boolean blockUntilConnectTimeout() { From 97020e0bab87d8f7321c3f29ba4041904e227426 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 6 Oct 2016 10:55:09 -0700 Subject: [PATCH 06/24] CronetDataSource: Fix getContentLength logging This is a minor cleanup. The main thing it fixes is that the "Inconsistent headers" and "Unexpected Content-Range" log messages were printing List objects, rather than the actual headers they are supposed to print. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135378074 --- .../ext/cronet/CronetDataSource.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) 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 594ea8cb17..8e53e32331 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 @@ -358,7 +358,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou contentLength = currentDataSpec.length; } else { // Check content length. - contentLength = getContentLength(info.getAllHeaders()); + contentLength = getContentLength(info); // If a specific length is requested and a specific length is returned but the 2 don't match // it's an error. if (currentDataSpec.length != C.LENGTH_UNSET && contentLength != C.LENGTH_UNSET @@ -472,23 +472,25 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou return false; } - private static long getContentLength(Map> headers) { + private static long getContentLength(UrlResponseInfo info) { long contentLength = C.LENGTH_UNSET; - List contentLengthHeader = headers.get("Content-Length"); - if (contentLengthHeader != null - && !contentLengthHeader.isEmpty() - && !TextUtils.isEmpty(contentLengthHeader.get(0))) { - try { - contentLength = Long.parseLong(contentLengthHeader.get(0)); - } catch (NumberFormatException e) { - Log.e(TAG, "Unexpected Content-Length [" + contentLengthHeader + "]"); + Map> headers = info.getAllHeaders(); + List contentLengthHeaders = headers.get("Content-Length"); + String contentLengthHeader = null; + if (contentLengthHeaders != null && !contentLengthHeaders.isEmpty()) { + contentLengthHeader = contentLengthHeaders.get(0); + if (!TextUtils.isEmpty(contentLengthHeader)) { + try { + contentLength = Long.parseLong(contentLengthHeader); + } catch (NumberFormatException e) { + Log.e(TAG, "Unexpected Content-Length [" + contentLengthHeader + "]"); + } } } - List contentRangeHeader = headers.get("Content-Range"); - if (contentRangeHeader != null - && !contentRangeHeader.isEmpty() - && !TextUtils.isEmpty(contentRangeHeader.get(0))) { - Matcher matcher = CONTENT_RANGE_HEADER_PATTERN.matcher(contentRangeHeader.get(0)); + List contentRangeHeaders = headers.get("Content-Range"); + if (contentRangeHeaders != null && !contentRangeHeaders.isEmpty()) { + String contentRangeHeader = contentRangeHeaders.get(0); + Matcher matcher = CONTENT_RANGE_HEADER_PATTERN.matcher(contentRangeHeader); if (matcher.find()) { try { long contentLengthFromRange = From 91f8328a5f1675a25d6546d4c79bcfdc175a535c Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 28 Jul 2016 08:48:37 +0100 Subject: [PATCH 07/24] UI component improvements - Make sure no events are posted on PlaybackControlView if it's not attached to a window. This can cause leaks. The target hide time is recorded if necessary and processed when the view is re-attached. - Deduplicated PlaybackControlView.VisibilityListener invocations. - Fixed timeouts to be more intuitive (I think). - Fixed initial visibility of PlaybackControlView when used as part of SimpleExoPlayerView. - Made some more attributes configurable from layout xml. Issue: #1908 Issue: #1919 Issue: #1923 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135679988 --- .../exoplayer2/ui/PlaybackControlView.java | 144 +++++++++++++----- .../exoplayer2/ui/SimpleExoPlayerView.java | 119 ++++++++++----- library/src/main/res/values/attrs.xml | 12 ++ 3 files changed, 199 insertions(+), 76 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/ui/PlaybackControlView.java b/library/src/main/java/com/google/android/exoplayer2/ui/PlaybackControlView.java index 13c5d14df0..3823f1760e 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ui/PlaybackControlView.java +++ b/library/src/main/java/com/google/android/exoplayer2/ui/PlaybackControlView.java @@ -16,6 +16,8 @@ package com.google.android.exoplayer2.ui; import android.content.Context; +import android.content.res.TypedArray; +import android.os.SystemClock; import android.util.AttributeSet; import android.view.KeyEvent; import android.view.LayoutInflater; @@ -52,7 +54,7 @@ public class PlaybackControlView extends FrameLayout { public static final int DEFAULT_FAST_FORWARD_MS = 15000; public static final int DEFAULT_REWIND_MS = 5000; - public static final int DEFAULT_SHOW_DURATION_MS = 5000; + public static final int DEFAULT_SHOW_TIMEOUT_MS = 5000; private static final int PROGRESS_BAR_MAX = 1000; private static final long MAX_POSITION_FOR_SEEK_TO_PREVIOUS = 3000; @@ -74,9 +76,10 @@ public class PlaybackControlView extends FrameLayout { private VisibilityListener visibilityListener; private boolean dragging; - private int rewindMs = DEFAULT_REWIND_MS; - private int fastForwardMs = DEFAULT_FAST_FORWARD_MS; - private int showDurationMs = DEFAULT_SHOW_DURATION_MS; + private int rewindMs; + private int fastForwardMs; + private int showTimeoutMs; + private long hideAtMs; private final Runnable updateProgressAction = new Runnable() { @Override @@ -103,6 +106,22 @@ public class PlaybackControlView extends FrameLayout { public PlaybackControlView(Context context, AttributeSet attrs, int defStyleAttr) { super(context, attrs, defStyleAttr); + rewindMs = DEFAULT_REWIND_MS; + fastForwardMs = DEFAULT_FAST_FORWARD_MS; + showTimeoutMs = DEFAULT_SHOW_TIMEOUT_MS; + if (attrs != null) { + TypedArray a = context.getTheme().obtainStyledAttributes(attrs, + R.styleable.PlaybackControlView, 0, 0); + try { + rewindMs = a.getInt(R.styleable.PlaybackControlView_rewind_increment, rewindMs); + fastForwardMs = a.getInt(R.styleable.PlaybackControlView_fastforward_increment, + fastForwardMs); + showTimeoutMs = a.getInt(R.styleable.PlaybackControlView_show_timeout, showTimeoutMs); + } finally { + a.recycle(); + } + } + currentWindow = new Timeline.Window(); formatBuilder = new StringBuilder(); formatter = new Formatter(formatBuilder, Locale.getDefault()); @@ -124,7 +143,6 @@ public class PlaybackControlView extends FrameLayout { rewindButton.setOnClickListener(componentListener); fastForwardButton = findViewById(R.id.ffwd); fastForwardButton.setOnClickListener(componentListener); - updateAll(); } /** @@ -169,6 +187,7 @@ public class PlaybackControlView extends FrameLayout { */ public void setRewindIncrementMs(int rewindMs) { this.rewindMs = rewindMs; + updateNavigation(); } /** @@ -178,51 +197,60 @@ public class PlaybackControlView extends FrameLayout { */ public void setFastForwardIncrementMs(int fastForwardMs) { this.fastForwardMs = fastForwardMs; + updateNavigation(); } /** - * Sets the duration to show the playback control in milliseconds. + * Returns the playback controls timeout. The playback controls are automatically hidden after + * this duration of time has elapsed without user input. * - * @param showDurationMs The duration in milliseconds. + * @return The duration in milliseconds. A non-positive value indicates that the controls will + * remain visible indefinitely. */ - public void setShowDurationMs(int showDurationMs) { - this.showDurationMs = showDurationMs; + public int getShowTimeoutMs() { + return showTimeoutMs; } /** - * Shows the controller for the duration last passed to {@link #setShowDurationMs(int)}, or for - * {@link #DEFAULT_SHOW_DURATION_MS} if {@link #setShowDurationMs(int)} has not been called. + * Sets the playback controls timeout. The playback controls are automatically hidden after this + * duration of time has elapsed without user input. + * + * @param showTimeoutMs The duration in milliseconds. A non-positive value will cause the controls + * to remain visible indefinitely. + */ + public void setShowTimeoutMs(int showTimeoutMs) { + this.showTimeoutMs = showTimeoutMs; + } + + /** + * Shows the playback controls. If {@link #getShowTimeoutMs()} is positive then the controls will + * be automatically hidden after this duration of time has elapsed without user input. */ public void show() { - show(showDurationMs); - } - - /** - * Shows the controller for the {@code durationMs}. If {@code durationMs} is 0 the controller is - * shown until {@link #hide()} is called. - * - * @param durationMs The duration in milliseconds. - */ - public void show(int durationMs) { - setVisibility(VISIBLE); - if (visibilityListener != null) { - visibilityListener.onVisibilityChange(getVisibility()); + if (!isVisible()) { + setVisibility(VISIBLE); + if (visibilityListener != null) { + visibilityListener.onVisibilityChange(getVisibility()); + } + updateAll(); } - updateAll(); - showDurationMs = durationMs; - hideDeferred(); + // Call hideAfterTimeout even if already visible to reset the timeout. + hideAfterTimeout(); } /** * Hides the controller. */ public void hide() { - setVisibility(GONE); - if (visibilityListener != null) { - visibilityListener.onVisibilityChange(getVisibility()); + if (isVisible()) { + setVisibility(GONE); + if (visibilityListener != null) { + visibilityListener.onVisibilityChange(getVisibility()); + } + removeCallbacks(updateProgressAction); + removeCallbacks(hideAction); + hideAtMs = C.TIME_UNSET; } - removeCallbacks(updateProgressAction); - removeCallbacks(hideAction); } /** @@ -232,10 +260,15 @@ public class PlaybackControlView extends FrameLayout { return getVisibility() == VISIBLE; } - private void hideDeferred() { + private void hideAfterTimeout() { removeCallbacks(hideAction); - if (showDurationMs > 0) { - postDelayed(hideAction, showDurationMs); + if (showTimeoutMs > 0) { + hideAtMs = SystemClock.uptimeMillis() + showTimeoutMs; + if (isAttachedToWindow()) { + postDelayed(hideAction, showTimeoutMs); + } + } else { + hideAtMs = C.TIME_UNSET; } } @@ -246,7 +279,7 @@ public class PlaybackControlView extends FrameLayout { } private void updatePlayPauseButton() { - if (!isVisible()) { + if (!isVisible() || !isAttachedToWindow()) { return; } boolean playing = player != null && player.getPlayWhenReady(); @@ -258,7 +291,7 @@ public class PlaybackControlView extends FrameLayout { } private void updateNavigation() { - if (!isVisible()) { + if (!isVisible() || !isAttachedToWindow()) { return; } Timeline currentTimeline = player != null ? player.getCurrentTimeline() : null; @@ -276,13 +309,13 @@ public class PlaybackControlView extends FrameLayout { } setButtonEnabled(enablePrevious , previousButton); setButtonEnabled(enableNext, nextButton); - setButtonEnabled(isSeekable, fastForwardButton); - setButtonEnabled(isSeekable, rewindButton); + setButtonEnabled(fastForwardMs > 0 && isSeekable, fastForwardButton); + setButtonEnabled(rewindMs > 0 && isSeekable, rewindButton); progressBar.setEnabled(isSeekable); } private void updateProgress() { - if (!isVisible()) { + if (!isVisible() || !isAttachedToWindow()) { return; } long duration = player == null ? 0 : player.getDuration(); @@ -377,13 +410,40 @@ public class PlaybackControlView extends FrameLayout { } private void rewind() { + if (rewindMs <= 0) { + return; + } player.seekTo(Math.max(player.getCurrentPosition() - rewindMs, 0)); } private void fastForward() { + if (fastForwardMs <= 0) { + return; + } player.seekTo(Math.min(player.getCurrentPosition() + fastForwardMs, player.getDuration())); } + @Override + public void onAttachedToWindow() { + super.onAttachedToWindow(); + if (hideAtMs != C.TIME_UNSET) { + long delayMs = hideAtMs - SystemClock.uptimeMillis(); + if (delayMs <= 0) { + hide(); + } else { + postDelayed(hideAction, delayMs); + } + } + updateAll(); + } + + @Override + public void onDetachedFromWindow() { + super.onDetachedFromWindow(); + removeCallbacks(updateProgressAction); + removeCallbacks(hideAction); + } + @Override public boolean dispatchKeyEvent(KeyEvent event) { if (player == null || event.getAction() != KeyEvent.ACTION_DOWN) { @@ -440,7 +500,7 @@ public class PlaybackControlView extends FrameLayout { public void onStopTrackingTouch(SeekBar seekBar) { dragging = false; player.seekTo(positionValue(seekBar.getProgress())); - hideDeferred(); + hideAfterTimeout(); } @Override @@ -485,7 +545,7 @@ public class PlaybackControlView extends FrameLayout { } else if (playButton == view) { player.setPlayWhenReady(!player.getPlayWhenReady()); } - hideDeferred(); + hideAfterTimeout(); } } diff --git a/library/src/main/java/com/google/android/exoplayer2/ui/SimpleExoPlayerView.java b/library/src/main/java/com/google/android/exoplayer2/ui/SimpleExoPlayerView.java index cd0acb77fa..51955ccef3 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ui/SimpleExoPlayerView.java +++ b/library/src/main/java/com/google/android/exoplayer2/ui/SimpleExoPlayerView.java @@ -48,8 +48,10 @@ public final class SimpleExoPlayerView extends FrameLayout { private final AspectRatioFrameLayout layout; private final PlaybackControlView controller; private final ComponentListener componentListener; + private SimpleExoPlayer player; private boolean useController = true; + private int controllerShowTimeoutMs; public SimpleExoPlayerView(Context context) { this(context, null); @@ -64,6 +66,9 @@ public final class SimpleExoPlayerView extends FrameLayout { boolean useTextureView = false; int resizeMode = AspectRatioFrameLayout.RESIZE_MODE_FIT; + int rewindMs = PlaybackControlView.DEFAULT_REWIND_MS; + int fastForwardMs = PlaybackControlView.DEFAULT_FAST_FORWARD_MS; + int controllerShowTimeoutMs = PlaybackControlView.DEFAULT_SHOW_TIMEOUT_MS; if (attrs != null) { TypedArray a = context.getTheme().obtainStyledAttributes(attrs, R.styleable.SimpleExoPlayerView, 0, 0); @@ -73,6 +78,11 @@ public final class SimpleExoPlayerView extends FrameLayout { useTextureView); resizeMode = a.getInt(R.styleable.SimpleExoPlayerView_resize_mode, AspectRatioFrameLayout.RESIZE_MODE_FIT); + rewindMs = a.getInt(R.styleable.SimpleExoPlayerView_rewind_increment, rewindMs); + fastForwardMs = a.getInt(R.styleable.SimpleExoPlayerView_fastforward_increment, + fastForwardMs); + controllerShowTimeoutMs = a.getInt(R.styleable.SimpleExoPlayerView_show_timeout, + controllerShowTimeoutMs); } finally { a.recycle(); } @@ -82,12 +92,17 @@ public final class SimpleExoPlayerView extends FrameLayout { componentListener = new ComponentListener(); layout = (AspectRatioFrameLayout) findViewById(R.id.video_frame); layout.setResizeMode(resizeMode); - controller = (PlaybackControlView) findViewById(R.id.control); shutterView = findViewById(R.id.shutter); subtitleLayout = (SubtitleView) findViewById(R.id.subtitles); subtitleLayout.setUserDefaultStyle(); subtitleLayout.setUserDefaultTextSize(); + controller = (PlaybackControlView) findViewById(R.id.control); + controller.hide(); + controller.setRewindIncrementMs(rewindMs); + controller.setFastForwardIncrementMs(fastForwardMs); + this.controllerShowTimeoutMs = controllerShowTimeoutMs; + View view = useTextureView ? new TextureView(context) : new SurfaceView(context); ViewGroup.LayoutParams params = new ViewGroup.LayoutParams( ViewGroup.LayoutParams.MATCH_PARENT, @@ -122,6 +137,9 @@ public final class SimpleExoPlayerView extends FrameLayout { this.player.setVideoSurface(null); } this.player = player; + if (useController) { + controller.setPlayer(player); + } if (player != null) { if (surfaceView instanceof TextureView) { player.setVideoTextureView((TextureView) surfaceView); @@ -131,20 +149,36 @@ public final class SimpleExoPlayerView extends FrameLayout { player.setVideoListener(componentListener); player.addListener(componentListener); player.setTextOutput(componentListener); + maybeShowController(false); } else { shutterView.setVisibility(VISIBLE); - } - if (useController) { - controller.setPlayer(player); + controller.hide(); } } /** - * Set the {@code useController} flag which indicates whether the playback control view should - * be used or not. If set to {@code false} the controller is never visible and is disconnected - * from the player. + * Sets the resize mode which can be of value {@link AspectRatioFrameLayout#RESIZE_MODE_FIT}, + * {@link AspectRatioFrameLayout#RESIZE_MODE_FIXED_HEIGHT} or + * {@link AspectRatioFrameLayout#RESIZE_MODE_FIXED_WIDTH}. * - * @param useController If {@code false} the playback control is never used. + * @param resizeMode The resize mode. + */ + public void setResizeMode(int resizeMode) { + layout.setResizeMode(resizeMode); + } + + /** + * Returns whether the playback controls are enabled. + */ + public boolean getUseController() { + return useController; + } + + /** + * Sets whether playback controls are enabled. If set to {@code false} the playback controls are + * never visible and are disconnected from the player. + * + * @param useController Whether playback controls should be enabled. */ public void setUseController(boolean useController) { if (this.useController == useController) { @@ -160,14 +194,26 @@ public final class SimpleExoPlayerView extends FrameLayout { } /** - * Sets the resize mode which can be of value {@link AspectRatioFrameLayout#RESIZE_MODE_FIT}, - * {@link AspectRatioFrameLayout#RESIZE_MODE_FIXED_HEIGHT} or - * {@link AspectRatioFrameLayout#RESIZE_MODE_FIXED_WIDTH}. + * Returns the playback controls timeout. The playback controls are automatically hidden after + * this duration of time has elapsed without user input and with playback or buffering in + * progress. * - * @param resizeMode The resize mode. + * @return The timeout in milliseconds. A non-positive value will cause the controller to remain + * visible indefinitely. */ - public void setResizeMode(int resizeMode) { - layout.setResizeMode(resizeMode); + public int getControllerShowTimeoutMs() { + return controllerShowTimeoutMs; + } + + /** + * Sets the playback controls timeout. The playback controls are automatically hidden after this + * duration of time has elapsed without user input and with playback or buffering in progress. + * + * @param controllerShowTimeoutMs The timeout in milliseconds. A non-positive value will cause + * the controller to remain visible indefinitely. + */ + public void setControllerShowTimeoutMs(int controllerShowTimeoutMs) { + this.controllerShowTimeoutMs = controllerShowTimeoutMs; } /** @@ -197,15 +243,6 @@ public final class SimpleExoPlayerView extends FrameLayout { controller.setFastForwardIncrementMs(fastForwardMs); } - /** - * Sets the duration to show the playback control in milliseconds. - * - * @param showDurationMs The duration in milliseconds. - */ - public void setControlShowDurationMs(int showDurationMs) { - controller.setShowDurationMs(showDurationMs); - } - /** * Get the view onto which video is rendered. This is either a {@link SurfaceView} (default) * or a {@link TextureView} if the {@code use_texture_view} view attribute has been set to true. @@ -218,21 +255,23 @@ public final class SimpleExoPlayerView extends FrameLayout { @Override public boolean onTouchEvent(MotionEvent ev) { - if (useController && ev.getActionMasked() == MotionEvent.ACTION_DOWN) { - if (controller.isVisible()) { - controller.hide(); - } else { - controller.show(); - } + if (!useController || player == null || ev.getActionMasked() != MotionEvent.ACTION_DOWN) { + return false; + } + if (controller.isVisible()) { + controller.hide(); + } else { + maybeShowController(true); } return true; } + @Override public boolean onTrackballEvent(MotionEvent ev) { - if (!useController) { + if (!useController || player == null) { return false; } - controller.show(); + maybeShowController(true); return true; } @@ -241,6 +280,20 @@ public final class SimpleExoPlayerView extends FrameLayout { return useController ? controller.dispatchKeyEvent(event) : super.dispatchKeyEvent(event); } + private void maybeShowController(boolean isForced) { + if (!useController || player == null) { + return; + } + int playbackState = player.getPlaybackState(); + boolean showIndefinitely = playbackState == ExoPlayer.STATE_IDLE + || playbackState == ExoPlayer.STATE_ENDED || !player.getPlayWhenReady(); + boolean wasShowingIndefinitely = controller.isVisible() && controller.getShowTimeoutMs() <= 0; + controller.setShowTimeoutMs(showIndefinitely ? 0 : controllerShowTimeoutMs); + if (isForced || showIndefinitely || wasShowingIndefinitely) { + controller.show(); + } + } + private final class ComponentListener implements SimpleExoPlayer.VideoListener, TextRenderer.Output, ExoPlayer.EventListener { @@ -278,9 +331,7 @@ public final class SimpleExoPlayerView extends FrameLayout { @Override public void onPlayerStateChanged(boolean playWhenReady, int playbackState) { - if (useController && playbackState == ExoPlayer.STATE_ENDED) { - controller.show(0); - } + maybeShowController(false); } @Override diff --git a/library/src/main/res/values/attrs.xml b/library/src/main/res/values/attrs.xml index 5f39dcb6c4..d58882c0aa 100644 --- a/library/src/main/res/values/attrs.xml +++ b/library/src/main/res/values/attrs.xml @@ -20,10 +20,16 @@ + + + + + + @@ -31,4 +37,10 @@ + + + + + + From 1b81cfdb56b4ca1a56d8ca0427e6be6ed7068a62 Mon Sep 17 00:00:00 2001 From: eguven Date: Mon, 10 Oct 2016 09:09:47 -0700 Subject: [PATCH 08/24] Rename build flavors and remove pre hashing of DataSpec key values. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135682130 --- demo/build.gradle | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/demo/build.gradle b/demo/build.gradle index 593796bda5..bfbcd1aa4c 100644 --- a/demo/build.gradle +++ b/demo/build.gradle @@ -38,15 +38,15 @@ android { } productFlavors { - demo - demoExt + noExtensions + withExtensions } } dependencies { compile project(':library') - demoExtCompile project(path: ':extension-ffmpeg') - demoExtCompile project(path: ':extension-flac') - demoExtCompile project(path: ':extension-opus') - demoExtCompile project(path: ':extension-vp9') + withExtensionsCompile project(path: ':extension-ffmpeg') + withExtensionsCompile project(path: ':extension-flac') + withExtensionsCompile project(path: ':extension-opus') + withExtensionsCompile project(path: ':extension-vp9') } From d922a21160ebd8edd351e1d15e1b727e1768e0b2 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 10 Oct 2016 10:44:13 -0700 Subject: [PATCH 09/24] Select the cenc sinf atom explicitly ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135692709 --- .../exoplayer2/extractor/mp4/AtomParsers.java | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/mp4/AtomParsers.java b/library/src/main/java/com/google/android/exoplayer2/extractor/mp4/AtomParsers.java index 0ca0f216c6..0b2d5ec330 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/mp4/AtomParsers.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/mp4/AtomParsers.java @@ -47,6 +47,7 @@ import java.util.List; private static final int TYPE_sbtl = Util.getIntegerCodeForString("sbtl"); private static final int TYPE_subt = Util.getIntegerCodeForString("subt"); private static final int TYPE_clcp = Util.getIntegerCodeForString("clcp"); + private static final int TYPE_cenc = Util.getIntegerCodeForString("cenc"); /** * Parses a trak atom (defined in 14496-12). @@ -1004,7 +1005,7 @@ import java.util.List; /** * Parses encryption data from an audio/video sample entry, populating {@code out} and returning - * the unencrypted atom type, or 0 if no sinf atom was present. + * the unencrypted atom type, or 0 if no common encryption sinf atom was present. */ private static int parseSampleEntryEncryptionData(ParsableByteArray parent, int position, int size, StsdData out, int entryIndex) { @@ -1017,10 +1018,10 @@ import java.util.List; if (childAtomType == Atom.TYPE_sinf) { Pair result = parseSinfFromParent(parent, childPosition, childAtomSize); - Integer dataFormat = result.first; - Assertions.checkArgument(dataFormat != null, "frma atom is mandatory"); - out.trackEncryptionBoxes[entryIndex] = result.second; - return dataFormat; + if (result != null) { + out.trackEncryptionBoxes[entryIndex] = result.second; + return result.first; + } } childPosition += childAtomSize; } @@ -1032,6 +1033,7 @@ import java.util.List; int position, int size) { int childPosition = position + Atom.HEADER_SIZE; + boolean isCencScheme = false; TrackEncryptionBox trackEncryptionBox = null; Integer dataFormat = null; while (childPosition - position < size) { @@ -1042,15 +1044,20 @@ import java.util.List; dataFormat = parent.readInt(); } else if (childAtomType == Atom.TYPE_schm) { parent.skipBytes(4); - parent.readInt(); // schemeType. Expect cenc - parent.readInt(); // schemeVersion. Expect 0x00010000 + isCencScheme = parent.readInt() == TYPE_cenc; } else if (childAtomType == Atom.TYPE_schi) { trackEncryptionBox = parseSchiFromParent(parent, childPosition, childAtomSize); } childPosition += childAtomSize; } - return Pair.create(dataFormat, trackEncryptionBox); + if (isCencScheme) { + Assertions.checkArgument(dataFormat != null, "frma atom is mandatory"); + Assertions.checkArgument(trackEncryptionBox != null, "schi->tenc atom is mandatory"); + return Pair.create(dataFormat, trackEncryptionBox); + } else { + return null; + } } private static TrackEncryptionBox parseSchiFromParent(ParsableByteArray parent, int position, From 4fab402274c8bc9e8f495493868842cce05d0eff Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 10 Oct 2016 11:40:08 -0700 Subject: [PATCH 10/24] Fix missing generics ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135700280 --- .../android/exoplayer2/ExoPlayerFactory.java | 19 ++++++++++--------- .../android/exoplayer2/ExoPlayerImpl.java | 9 +++++---- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerFactory.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerFactory.java index d8bbd81dd8..91ab56805a 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerFactory.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerFactory.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2; import android.content.Context; import android.os.Looper; import com.google.android.exoplayer2.drm.DrmSessionManager; +import com.google.android.exoplayer2.drm.FrameworkMediaCrypto; import com.google.android.exoplayer2.trackselection.TrackSelector; /** @@ -41,7 +42,7 @@ public final class ExoPlayerFactory { * @param trackSelector The {@link TrackSelector} that will be used by the instance. * @param loadControl The {@link LoadControl} that will be used by the instance. */ - public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, + public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, LoadControl loadControl) { return newSimpleInstance(context, trackSelector, loadControl, null); } @@ -56,8 +57,8 @@ public final class ExoPlayerFactory { * @param drmSessionManager An optional {@link DrmSessionManager}. May be null if the instance * will not be used for DRM protected playbacks. */ - public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, - LoadControl loadControl, DrmSessionManager drmSessionManager) { + public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, + LoadControl loadControl, DrmSessionManager drmSessionManager) { return newSimpleInstance(context, trackSelector, loadControl, drmSessionManager, false); } @@ -74,8 +75,8 @@ public final class ExoPlayerFactory { * available extensions over those defined in the core library. Note that extensions must be * included in the application build for setting this flag to have any effect. */ - public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, - LoadControl loadControl, DrmSessionManager drmSessionManager, + public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, + LoadControl loadControl, DrmSessionManager drmSessionManager, boolean preferExtensionDecoders) { return newSimpleInstance(context, trackSelector, loadControl, drmSessionManager, preferExtensionDecoders, DEFAULT_ALLOWED_VIDEO_JOINING_TIME_MS); @@ -96,8 +97,8 @@ public final class ExoPlayerFactory { * @param allowedVideoJoiningTimeMs The maximum duration for which a video renderer can attempt to * seamlessly join an ongoing playback. */ - public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, - LoadControl loadControl, DrmSessionManager drmSessionManager, + public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, + LoadControl loadControl, DrmSessionManager drmSessionManager, boolean preferExtensionDecoders, long allowedVideoJoiningTimeMs) { return new SimpleExoPlayer(context, trackSelector, loadControl, drmSessionManager, preferExtensionDecoders, allowedVideoJoiningTimeMs); @@ -110,7 +111,7 @@ public final class ExoPlayerFactory { * @param renderers The {@link Renderer}s that will be used by the instance. * @param trackSelector The {@link TrackSelector} that will be used by the instance. */ - public static ExoPlayer newInstance(Renderer[] renderers, TrackSelector trackSelector) { + public static ExoPlayer newInstance(Renderer[] renderers, TrackSelector trackSelector) { return newInstance(renderers, trackSelector, new DefaultLoadControl()); } @@ -122,7 +123,7 @@ public final class ExoPlayerFactory { * @param trackSelector The {@link TrackSelector} that will be used by the instance. * @param loadControl The {@link LoadControl} that will be used by the instance. */ - public static ExoPlayer newInstance(Renderer[] renderers, TrackSelector trackSelector, + public static ExoPlayer newInstance(Renderer[] renderers, TrackSelector trackSelector, LoadControl loadControl) { return new ExoPlayerImpl(renderers, trackSelector, loadControl); } diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index d1f72ed059..a79ad1dba0 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -35,7 +35,7 @@ import java.util.concurrent.CopyOnWriteArraySet; private static final String TAG = "ExoPlayerImpl"; private final Handler eventHandler; - private final ExoPlayerImplInternal internalPlayer; + private final ExoPlayerImplInternal internalPlayer; private final CopyOnWriteArraySet listeners; private final Timeline.Window window; private final Timeline.Period period; @@ -63,7 +63,8 @@ import java.util.concurrent.CopyOnWriteArraySet; * @param loadControl The {@link LoadControl} that will be used by the instance. */ @SuppressLint("HandlerLeak") - public ExoPlayerImpl(Renderer[] renderers, TrackSelector trackSelector, LoadControl loadControl) { + public ExoPlayerImpl(Renderer[] renderers, TrackSelector trackSelector, + LoadControl loadControl) { Log.i(TAG, "Init " + ExoPlayerLibraryInfo.VERSION); Assertions.checkNotNull(renderers); Assertions.checkState(renderers.length > 0); @@ -79,8 +80,8 @@ import java.util.concurrent.CopyOnWriteArraySet; } }; playbackInfo = new ExoPlayerImplInternal.PlaybackInfo(0, 0); - internalPlayer = new ExoPlayerImplInternal(renderers, trackSelector, loadControl, playWhenReady, - eventHandler, playbackInfo); + internalPlayer = new ExoPlayerImplInternal<>(renderers, trackSelector, loadControl, + playWhenReady, eventHandler, playbackInfo); } @Override From 907b9bf4f65aef81ee12cb06840fd3fac555ed77 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 10 Oct 2016 11:55:09 -0700 Subject: [PATCH 11/24] Sanitize threading in CronetDataSource - Move nearly all logic onto the calling thread (i.e. the thread calling open/read/close), to make threading correctness more obvious. - Document which variables are read/written from which thread, and why the call sequences are safe. - Fix thread safety issue that I think could probably cause data corruption in the case of a read timeout followed by another request into the DataSource. Also: - Relaxed content length checking to be consistent with the other http DataSource implementations, and avoided parsing the headers where they're not used. - Fixed missing generics in CronetDataSourceFactory. - Added TODO to work with servers that don't support partial range requests. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135702217 --- .../ext/cronet/CronetDataSourceTest.java | 52 +---- .../ext/cronet/CronetDataSource.java | 219 +++++++++--------- .../ext/cronet/CronetDataSourceFactory.java | 2 +- .../ext/okhttp/OkHttpDataSource.java | 9 +- .../upstream/DefaultHttpDataSource.java | 11 +- 5 files changed, 120 insertions(+), 173 deletions(-) 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 ccd4dec191..dcc5bc9b97 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 @@ -174,10 +174,7 @@ public final class CronetDataSourceTest { @Test(expected = IllegalStateException.class) public void testOpeningTwiceThrows() throws HttpDataSourceException { mockResponseStartSuccess(); - - assertConnectionState(CronetDataSource.IDLE_CONNECTION); dataSourceUnderTest.open(testDataSpec); - assertConnectionState(CronetDataSource.OPEN_CONNECTION); dataSourceUnderTest.open(testDataSpec); } @@ -205,7 +202,7 @@ public final class CronetDataSourceTest { dataSourceUnderTest.onFailed( mockUrlRequest, testUrlResponseInfo, - null); + mockUrlRequestException); dataSourceUnderTest.onResponseStarted( mockUrlRequest2, testUrlResponseInfo); @@ -253,13 +250,10 @@ public final class CronetDataSourceTest { @Test public void testRequestOpen() throws HttpDataSourceException { mockResponseStartSuccess(); - assertEquals(TEST_CONTENT_LENGTH, dataSourceUnderTest.open(testDataSpec)); - assertConnectionState(CronetDataSource.OPEN_CONNECTION); verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testDataSpec); } - @Test public void testRequestOpenGzippedCompressedReturnsDataSpecLength() throws HttpDataSourceException { @@ -271,7 +265,6 @@ public final class CronetDataSourceTest { testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); assertEquals(5000 /* contentLength */, dataSourceUnderTest.open(testDataSpec)); - assertConnectionState(CronetDataSource.OPEN_CONNECTION); verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testDataSpec); } @@ -286,7 +279,6 @@ public final class CronetDataSourceTest { // Check for connection not automatically closed. assertFalse(e.getCause() instanceof UnknownHostException); verify(mockUrlRequest, never()).cancel(); - assertConnectionState(CronetDataSource.OPENING_CONNECTION); verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec); } } @@ -304,7 +296,6 @@ public final class CronetDataSourceTest { // Check for connection not automatically closed. assertTrue(e.getCause() instanceof UnknownHostException); verify(mockUrlRequest, never()).cancel(); - assertConnectionState(CronetDataSource.OPENING_CONNECTION); verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec); } } @@ -321,7 +312,6 @@ public final class CronetDataSourceTest { assertTrue(e instanceof HttpDataSource.InvalidResponseCodeException); // Check for connection not automatically closed. verify(mockUrlRequest, never()).cancel(); - assertConnectionState(CronetDataSource.OPENING_CONNECTION); verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec); } } @@ -338,37 +328,16 @@ public final class CronetDataSourceTest { assertTrue(e instanceof HttpDataSource.InvalidContentTypeException); // Check for connection not automatically closed. verify(mockUrlRequest, never()).cancel(); - assertConnectionState(CronetDataSource.OPENING_CONNECTION); verify(mockContentTypePredicate).evaluate(TEST_CONTENT_TYPE); } } - @Test - public void testRequestOpenValidatesContentLength() { - mockResponseStartSuccess(); - - // Data spec's requested length, 5000. Test response's length, 16,000. - testDataSpec = new DataSpec(Uri.parse(TEST_URL), 1000, 5000, null); - - try { - dataSourceUnderTest.open(testDataSpec); - fail("HttpDataSource.HttpDataSourceException expected"); - } catch (HttpDataSourceException e) { - verify(mockUrlRequest).addHeader("Range", "bytes=1000-5999"); - // Check for connection not automatically closed. - verify(mockUrlRequest, never()).cancel(); - assertConnectionState(CronetDataSource.OPENING_CONNECTION); - verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testPostDataSpec); - } - } - @Test public void testPostRequestOpen() throws HttpDataSourceException { mockResponseStartSuccess(); dataSourceUnderTest.setRequestProperty("Content-Type", TEST_CONTENT_TYPE); assertEquals(TEST_CONTENT_LENGTH, dataSourceUnderTest.open(testPostDataSpec)); - assertConnectionState(CronetDataSource.OPEN_CONNECTION); verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testPostDataSpec); } @@ -510,7 +479,6 @@ public final class CronetDataSourceTest { dataSourceUnderTest.close(); verify(mockTransferListener).onTransferEnd(dataSourceUnderTest); - assertConnectionState(CronetDataSource.IDLE_CONNECTION); try { bytesRead += dataSourceUnderTest.read(returnedBuffer, 0, 8); @@ -572,7 +540,6 @@ public final class CronetDataSourceTest { verify(mockUrlRequest, times(1)).read(any(ByteBuffer.class)); // Check for connection not automatically closed. verify(mockUrlRequest, never()).cancel(); - assertConnectionState(CronetDataSource.OPEN_CONNECTION); assertEquals(16, bytesRead); } @@ -603,15 +570,12 @@ public final class CronetDataSourceTest { // We should still be trying to open. assertFalse(timedOutCondition.block(50)); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); // We should still be trying to open as we approach the timeout. when(mockClock.elapsedRealtime()).thenReturn((long) TEST_CONNECT_TIMEOUT_MS - 1); assertFalse(timedOutCondition.block(50)); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); // Now we timeout. when(mockClock.elapsedRealtime()).thenReturn((long) TEST_CONNECT_TIMEOUT_MS); timedOutCondition.block(); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec); } @@ -637,15 +601,12 @@ public final class CronetDataSourceTest { // We should still be trying to open. assertFalse(openCondition.block(50)); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); // We should still be trying to open as we approach the timeout. when(mockClock.elapsedRealtime()).thenReturn((long) TEST_CONNECT_TIMEOUT_MS - 1); assertFalse(openCondition.block(50)); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); // The response arrives just in time. dataSourceUnderTest.onResponseStarted(mockUrlRequest, testUrlResponseInfo); openCondition.block(); - assertEquals(CronetDataSource.OPEN_CONNECTION, dataSourceUnderTest.connectionState); } @Test @@ -674,11 +635,9 @@ public final class CronetDataSourceTest { // We should still be trying to open. assertFalse(timedOutCondition.block(50)); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); // We should still be trying to open as we approach the timeout. when(mockClock.elapsedRealtime()).thenReturn((long) TEST_CONNECT_TIMEOUT_MS - 1); assertFalse(timedOutCondition.block(50)); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); // A redirect arrives just in time. dataSourceUnderTest.onRedirectReceived(mockUrlRequest, testUrlResponseInfo, "RandomRedirectedUrl1"); @@ -689,7 +648,6 @@ public final class CronetDataSourceTest { assertFalse(timedOutCondition.block(newTimeoutMs)); // We should still be trying to open as we approach the new timeout. assertFalse(timedOutCondition.block(50)); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); // A redirect arrives just in time. dataSourceUnderTest.onRedirectReceived(mockUrlRequest, testUrlResponseInfo, "RandomRedirectedUrl2"); @@ -700,11 +658,9 @@ public final class CronetDataSourceTest { assertFalse(timedOutCondition.block(newTimeoutMs)); // We should still be trying to open as we approach the new timeout. assertFalse(timedOutCondition.block(50)); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); // Now we timeout. when(mockClock.elapsedRealtime()).thenReturn(newTimeoutMs); timedOutCondition.block(); - assertEquals(CronetDataSource.OPENING_CONNECTION, dataSourceUnderTest.connectionState); verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec); assertEquals(1, openExceptions.get()); @@ -818,7 +774,7 @@ public final class CronetDataSourceTest { dataSourceUnderTest.onFailed( mockUrlRequest, createUrlResponseInfo(500), // statusCode - null); + mockUrlRequestException); return null; } }).when(mockUrlRequest).read(any(ByteBuffer.class)); @@ -869,8 +825,4 @@ public final class CronetDataSourceTest { return testBuffer; } - private void assertConnectionState(int state) { - assertEquals(state, dataSourceUnderTest.connectionState); - } - } 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 8e53e32331..a758f71f45 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 @@ -37,7 +37,6 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.Executor; -import java.util.concurrent.atomic.AtomicLong; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.chromium.net.CronetEngine; @@ -91,11 +90,6 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou // The size of read buffer passed to cronet UrlRequest.read(). private static final int READ_BUFFER_SIZE_BYTES = 32 * 1024; - /* package */ static final int IDLE_CONNECTION = 5; - /* package */ static final int OPENING_CONNECTION = 2; - /* package */ static final int CONNECTED_CONNECTION = 3; - /* package */ static final int OPEN_CONNECTION = 4; - private final CronetEngine cronetEngine; private final Executor executor; private final Predicate contentTypePredicate; @@ -105,20 +99,29 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou private final boolean resetTimeoutOnRedirects; private final Map requestProperties; private final ConditionVariable operation; - private final ByteBuffer readBuffer; private final Clock clock; + // Accessed by the calling thread only. + private boolean opened; + private long bytesRemaining; + + // Written from the calling thread only. currentUrlRequest.start() calls ensure writes are visible + // to reads made by the Cronet thread. private UrlRequest currentUrlRequest; private DataSpec currentDataSpec; - private UrlResponseInfo responseInfo; - /* package */ volatile int connectionState; + // Reference written and read by calling thread only. Passed to Cronet thread as a local variable. + // operation.open() calls ensure writes into the buffer are visible to reads made by the calling + // thread. + private ByteBuffer readBuffer; + + // Written from the Cronet thread only. operation.open() calls ensure writes are visible to reads + // made by the calling thread. + private UrlResponseInfo responseInfo; + private IOException exception; + private boolean finished; + private volatile long currentConnectTimeoutMs; - private volatile HttpDataSourceException exception; - private volatile long contentLength; - private volatile AtomicLong expectedBytesRemainingToRead; - private volatile boolean hasData; - private volatile boolean responseFinished; /** * @param cronetEngine A CronetEngine. @@ -163,10 +166,8 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou this.readTimeoutMs = readTimeoutMs; this.resetTimeoutOnRedirects = resetTimeoutOnRedirects; this.clock = Assertions.checkNotNull(clock); - readBuffer = ByteBuffer.allocateDirect(READ_BUFFER_SIZE_BYTES); requestProperties = new HashMap<>(); operation = new ConditionVariable(); - connectionState = IDLE_CONNECTION; } // HttpDataSource implementation. @@ -205,10 +206,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou @Override public long open(DataSpec dataSpec) throws HttpDataSourceException { Assertions.checkNotNull(dataSpec); - synchronized (this) { - Assertions.checkState(connectionState == IDLE_CONNECTION, "Connection already open"); - connectionState = OPENING_CONNECTION; - } + Assertions.checkState(!opened); operation.close(); resetConnectTimeout(); @@ -218,61 +216,99 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou boolean requestStarted = blockUntilConnectTimeout(); if (exception != null) { - // An error occurred opening the connection. - throw exception; + throw new OpenException(exception, currentDataSpec, getStatus(currentUrlRequest)); } else if (!requestStarted) { // The timeout was reached before the connection was opened. throw new OpenException(new SocketTimeoutException(), dataSpec, getStatus(currentUrlRequest)); } - // Connection was opened. + // Check for a valid response code. + int responseCode = responseInfo.getHttpStatusCode(); + if (responseCode < 200 || responseCode > 299) { + InvalidResponseCodeException exception = new InvalidResponseCodeException(responseCode, + responseInfo.getAllHeaders(), currentDataSpec); + if (responseCode == 416) { + exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); + } + throw exception; + } + + // Check for a valid content type. + if (contentTypePredicate != null) { + List contentTypeHeaders = responseInfo.getAllHeaders().get(CONTENT_TYPE); + String contentType = isEmpty(contentTypeHeaders) ? null : contentTypeHeaders.get(0); + if (!contentTypePredicate.evaluate(contentType)) { + throw new InvalidContentTypeException(contentType, currentDataSpec); + } + } + + // 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. + + // Calculate the content length. + if (!getIsCompressed(responseInfo)) { + if (dataSpec.length != C.LENGTH_UNSET) { + bytesRemaining = dataSpec.length; + } else { + bytesRemaining = getContentLength(responseInfo); + } + } else { + // If the response is compressed then the content length will be that of the compressed data + // which isn't what we want. Always use the dataSpec length in this case. + bytesRemaining = currentDataSpec.length; + } + + opened = true; if (listener != null) { listener.onTransferStart(this, dataSpec); } - connectionState = OPEN_CONNECTION; - return contentLength; + + return bytesRemaining; } @Override public int read(byte[] buffer, int offset, int readLength) throws HttpDataSourceException { - synchronized (this) { - Assertions.checkState(connectionState == OPEN_CONNECTION); - } + Assertions.checkState(opened); if (readLength == 0) { return 0; - } - if (expectedBytesRemainingToRead != null && expectedBytesRemainingToRead.get() == 0) { + } else if (bytesRemaining == 0) { return C.RESULT_END_OF_INPUT; } - if (!hasData) { - // Read more data from cronet. + if (readBuffer == null) { + readBuffer = ByteBuffer.allocateDirect(READ_BUFFER_SIZE_BYTES); + readBuffer.limit(0); + } + if (!readBuffer.hasRemaining()) { + // Fill readBuffer with more data from Cronet. operation.close(); readBuffer.clear(); currentUrlRequest.read(readBuffer); if (!operation.block(readTimeoutMs)) { + // We're timing out, but since the operation is still ongoing we'll need to replace + // readBuffer to avoid the possibility of it being written to by this operation during a + // subsequent request. + readBuffer = null; throw new HttpDataSourceException( new SocketTimeoutException(), currentDataSpec, HttpDataSourceException.TYPE_READ); - } - if (exception != null) { - throw exception; - } - // The expected response length is unknown, but cronet has indicated that the request - // already finished successfully. - if (responseFinished) { + } else if (exception != null) { + throw new HttpDataSourceException(exception, currentDataSpec, + HttpDataSourceException.TYPE_READ); + } else if (finished) { return C.RESULT_END_OF_INPUT; + } else { + // The operation didn't time out, fail or finish, and therefore data must have been read. + readBuffer.flip(); } } int bytesRead = Math.min(readBuffer.remaining(), readLength); readBuffer.get(buffer, offset, bytesRead); - if (!readBuffer.hasRemaining()) { - hasData = false; - } - if (expectedBytesRemainingToRead != null) { - expectedBytesRemainingToRead.addAndGet(-bytesRead); + if (bytesRemaining != C.LENGTH_UNSET) { + bytesRemaining -= bytesRead; } if (listener != null) { listener.onBytesTransferred(this, bytesRead); @@ -286,26 +322,26 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou currentUrlRequest.cancel(); currentUrlRequest = null; } + if (readBuffer != null) { + readBuffer.limit(0); + } currentDataSpec = null; - exception = null; - contentLength = 0; - hasData = false; responseInfo = null; - expectedBytesRemainingToRead = null; - responseFinished = false; - try { - if (listener != null && connectionState == OPEN_CONNECTION) { + exception = null; + finished = false; + if (opened) { + opened = false; + if (listener != null) { listener.onTransferEnd(this); } - } finally { - connectionState = IDLE_CONNECTION; } } // UrlRequest.Callback implementation @Override - public void onRedirectReceived(UrlRequest request, UrlResponseInfo info, String newLocationUrl) { + public synchronized void onRedirectReceived(UrlRequest request, UrlResponseInfo info, + String newLocationUrl) { if (request != currentUrlRequest) { return; } @@ -315,8 +351,8 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou // For other redirect response codes the POST request is converted to a GET request and the // redirect is followed. if (responseCode == 307 || responseCode == 308) { - exception = new OpenException("POST request redirected with 307 or 308 response code", - currentDataSpec, getStatus(request)); + exception = new InvalidResponseCodeException(responseCode, info.getAllHeaders(), + currentDataSpec); operation.open(); return; } @@ -332,51 +368,8 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou if (request != currentUrlRequest) { return; } - try { - // Check for a valid response code. - int responseCode = info.getHttpStatusCode(); - if (responseCode < 200 || responseCode > 299) { - InvalidResponseCodeException exception = new InvalidResponseCodeException( - responseCode, info.getAllHeaders(), currentDataSpec); - if (responseCode == 416) { - exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); - } - throw exception; - } - // Check for a valid content type. - if (contentTypePredicate != null) { - List contentTypeHeaders = info.getAllHeaders().get(CONTENT_TYPE); - String contentType = contentTypeHeaders == null || contentTypeHeaders.isEmpty() ? null - : contentTypeHeaders.get(0); - if (!contentTypePredicate.evaluate(contentType)) { - throw new InvalidContentTypeException(contentType, currentDataSpec); - } - } - - responseInfo = info; - if (getIsCompressed(info)) { - contentLength = currentDataSpec.length; - } else { - // Check content length. - contentLength = getContentLength(info); - // If a specific length is requested and a specific length is returned but the 2 don't match - // it's an error. - if (currentDataSpec.length != C.LENGTH_UNSET && contentLength != C.LENGTH_UNSET - && currentDataSpec.length != contentLength) { - throw new OpenException("Content length did not match requested length", currentDataSpec, - getStatus(request)); - } - } - if (contentLength > 0) { - expectedBytesRemainingToRead = new AtomicLong(contentLength); - } - - connectionState = CONNECTED_CONNECTION; - } catch (HttpDataSourceException e) { - exception = e; - } finally { - operation.open(); - } + responseInfo = info; + operation.open(); } @Override @@ -385,17 +378,15 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou if (request != currentUrlRequest) { return; } - readBuffer.flip(); - hasData = true; operation.open(); } @Override - public void onSucceeded(UrlRequest request, UrlResponseInfo info) { + public synchronized void onSucceeded(UrlRequest request, UrlResponseInfo info) { if (request != currentUrlRequest) { return; } - responseFinished = true; + finished = true; operation.open(); } @@ -405,14 +396,8 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou if (request != currentUrlRequest) { return; } - if (connectionState == OPENING_CONNECTION) { - IOException cause = error.getErrorCode() == UrlRequestException.ERROR_HOSTNAME_NOT_RESOLVED - ? new UnknownHostException() : error; - exception = new OpenException(cause, currentDataSpec, getStatus(request)); - } else if (connectionState == OPEN_CONNECTION) { - exception = new HttpDataSourceException(error, currentDataSpec, - HttpDataSourceException.TYPE_READ); - } + exception = error.getErrorCode() == UrlRequestException.ERROR_HOSTNAME_NOT_RESOLVED + ? new UnknownHostException() : error; operation.open(); } @@ -477,7 +462,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou Map> headers = info.getAllHeaders(); List contentLengthHeaders = headers.get("Content-Length"); String contentLengthHeader = null; - if (contentLengthHeaders != null && !contentLengthHeaders.isEmpty()) { + if (!isEmpty(contentLengthHeaders)) { contentLengthHeader = contentLengthHeaders.get(0); if (!TextUtils.isEmpty(contentLengthHeader)) { try { @@ -488,7 +473,7 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou } } List contentRangeHeaders = headers.get("Content-Range"); - if (contentRangeHeaders != null && !contentRangeHeaders.isEmpty()) { + if (!isEmpty(contentRangeHeaders)) { String contentRangeHeader = contentRangeHeaders.get(0); Matcher matcher = CONTENT_RANGE_HEADER_PATTERN.matcher(contentRangeHeader); if (matcher.find()) { @@ -530,4 +515,8 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou return statusHolder[0]; } + private static boolean isEmpty(List list) { + return list == null || list.isEmpty(); + } + } diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceFactory.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceFactory.java index 1cffee8188..0f94dad158 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceFactory.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceFactory.java @@ -41,7 +41,7 @@ public final class CronetDataSourceFactory implements Factory { private final CronetEngine cronetEngine; private final Executor executor; private final Predicate contentTypePredicate; - private final TransferListener transferListener; + private final TransferListener transferListener; private final int connectTimeoutMs; private final int readTimeoutMs; private final boolean resetTimeoutOnRedirects; diff --git a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java index 7a8f3bca3f..2b6eaa736d 100644 --- a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java +++ b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java @@ -185,9 +185,12 @@ public class OkHttpDataSource implements HttpDataSource { bytesToSkip = responseCode == 200 && dataSpec.position != 0 ? dataSpec.position : 0; // Determine the length of the data to be read, after skipping. - long contentLength = response.body().contentLength(); - bytesToRead = dataSpec.length != C.LENGTH_UNSET ? dataSpec.length - : (contentLength != -1 ? (contentLength - bytesToSkip) : C.LENGTH_UNSET); + if (dataSpec.length != C.LENGTH_UNSET) { + bytesToRead = dataSpec.length; + } else { + long contentLength = response.body().contentLength(); + bytesToRead = contentLength != -1 ? (contentLength - bytesToSkip) : C.LENGTH_UNSET; + } opened = true; if (listener != null) { diff --git a/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java b/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java index b193b446c2..34ec76b673 100644 --- a/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java +++ b/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java @@ -231,10 +231,13 @@ public class DefaultHttpDataSource implements HttpDataSource { // Determine the length of the data to be read, after skipping. if ((dataSpec.flags & DataSpec.FLAG_ALLOW_GZIP) == 0) { - long contentLength = getContentLength(connection); - bytesToRead = dataSpec.length != C.LENGTH_UNSET ? dataSpec.length - : contentLength != C.LENGTH_UNSET ? contentLength - bytesToSkip - : C.LENGTH_UNSET; + if (dataSpec.length != C.LENGTH_UNSET) { + bytesToRead = dataSpec.length; + } else { + long contentLength = getContentLength(connection); + bytesToRead = contentLength != C.LENGTH_UNSET ? (contentLength - bytesToSkip) + : C.LENGTH_UNSET; + } } else { // Gzip is enabled. If the server opts to use gzip then the content length in the response // will be that of the compressed data, which isn't what we want. Furthermore, there isn't a From 83107cc25d402a5b405d67cdf4b3687d472d4f53 Mon Sep 17 00:00:00 2001 From: olly Date: Tue, 11 Oct 2016 07:20:44 -0700 Subject: [PATCH 12/24] Fix missing release calls on loadingPeriodHolder playingPeriodHolder can be null in the case that the first period is still being prepared. We need to make sure we release the period that's being prepared in such cases, which is loadingPeriodHolder. Issue: #1914 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135793472 --- .../exoplayer2/ExoPlayerImplInternal.java | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index 85f365e51e..56b862073a 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -538,16 +538,23 @@ import java.io.IOException; periodIndex = C.INDEX_UNSET; } - // Clear the timeline, but keep the requested period if it is already prepared. - MediaPeriodHolder periodHolder = playingPeriodHolder; MediaPeriodHolder newPlayingPeriodHolder = null; - while (periodHolder != null) { - if (periodHolder.index == periodIndex && periodHolder.prepared) { - newPlayingPeriodHolder = periodHolder; - } else { - periodHolder.release(); + if (playingPeriodHolder == null) { + // We're still waiting for the first period to be prepared. + if (loadingPeriodHolder != null) { + loadingPeriodHolder.release(); + } + } else { + // Clear the timeline, but keep the requested period if it is already prepared. + MediaPeriodHolder periodHolder = playingPeriodHolder; + while (periodHolder != null) { + if (periodHolder.index == periodIndex && periodHolder.prepared) { + newPlayingPeriodHolder = periodHolder; + } else { + periodHolder.release(); + } + periodHolder = periodHolder.next; } - periodHolder = periodHolder.next; } // Disable all the renderers if the period is changing. @@ -892,7 +899,8 @@ import java.io.IOException; } // Release all loaded periods. - releasePeriodHoldersFrom(playingPeriodHolder); + releasePeriodHoldersFrom(playingPeriodHolder != null ? playingPeriodHolder + : loadingPeriodHolder); bufferAheadPeriodCount = 0; playingPeriodHolder = null; readingPeriodHolder = null; From 29f3eb5e5a0f720d55f73992c0817ac4f7c3b9b7 Mon Sep 17 00:00:00 2001 From: olly Date: Sat, 7 May 2016 08:13:17 +0100 Subject: [PATCH 13/24] Fixes for retries - Fix issue in ExoPlayerImpl where the timeline was null'd but onTimelineChanged was not fired. - Add the ability to not reset the timeline. This is useful for retries where you know the timeline will be the same as it was previously. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135797577 --- .../android/exoplayer2/demo/EventLogger.java | 3 +++ .../exoplayer2/demo/PlayerActivity.java | 27 +++++++++---------- .../google/android/exoplayer2/ExoPlayer.java | 11 +++++--- .../android/exoplayer2/ExoPlayerImpl.java | 12 ++++++--- .../android/exoplayer2/SimpleExoPlayer.java | 4 +-- 5 files changed, 34 insertions(+), 23 deletions(-) diff --git a/demo/src/main/java/com/google/android/exoplayer2/demo/EventLogger.java b/demo/src/main/java/com/google/android/exoplayer2/demo/EventLogger.java index 8c12290d35..d79de04657 100644 --- a/demo/src/main/java/com/google/android/exoplayer2/demo/EventLogger.java +++ b/demo/src/main/java/com/google/android/exoplayer2/demo/EventLogger.java @@ -97,6 +97,9 @@ import java.util.Locale; @Override public void onTimelineChanged(Timeline timeline, Object manifest) { + if (timeline == null) { + return; + } int periodCount = timeline.getPeriodCount(); int windowCount = timeline.getWindowCount(); Log.d(TAG, "sourceInfo [periodCount=" + periodCount + ", windowCount=" + windowCount); diff --git a/demo/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java b/demo/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java index 667ccd5bab..e9aa46f85f 100644 --- a/demo/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java +++ b/demo/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java @@ -101,6 +101,7 @@ public class PlayerActivity extends Activity implements OnClickListener, ExoPlay } private Handler mainHandler; + private Timeline.Window window; private EventLogger eventLogger; private SimpleExoPlayerView simpleExoPlayerView; private LinearLayout debugRootView; @@ -115,7 +116,7 @@ public class PlayerActivity extends Activity implements OnClickListener, ExoPlay private boolean playerNeedsSource; private boolean shouldAutoPlay; - private boolean shouldRestorePosition; + private boolean isTimelineStatic; private int playerWindow; private long playerPosition; @@ -127,6 +128,7 @@ public class PlayerActivity extends Activity implements OnClickListener, ExoPlay shouldAutoPlay = true; mediaDataSourceFactory = buildDataSourceFactory(true); mainHandler = new Handler(); + window = new Timeline.Window(); if (CookieHandler.getDefault() != DEFAULT_COOKIE_MANAGER) { CookieHandler.setDefault(DEFAULT_COOKIE_MANAGER); } @@ -147,7 +149,7 @@ public class PlayerActivity extends Activity implements OnClickListener, ExoPlay @Override public void onNewIntent(Intent intent) { releasePlayer(); - shouldRestorePosition = false; + isTimelineStatic = false; setIntent(intent); } @@ -262,7 +264,7 @@ public class PlayerActivity extends Activity implements OnClickListener, ExoPlay player.setVideoDebugListener(eventLogger); player.setId3Output(eventLogger); simpleExoPlayerView.setPlayer(player); - if (shouldRestorePosition) { + if (isTimelineStatic) { if (playerPosition == C.TIME_UNSET) { player.seekToDefaultPosition(playerWindow); } else { @@ -305,7 +307,7 @@ public class PlayerActivity extends Activity implements OnClickListener, ExoPlay } MediaSource mediaSource = mediaSources.length == 1 ? mediaSources[0] : new ConcatenatingMediaSource(mediaSources); - player.prepare(mediaSource, !shouldRestorePosition); + player.prepare(mediaSource, !isTimelineStatic, !isTimelineStatic); playerNeedsSource = false; updateButtonVisibilities(); } @@ -348,15 +350,11 @@ public class PlayerActivity extends Activity implements OnClickListener, ExoPlay debugViewHelper.stop(); debugViewHelper = null; shouldAutoPlay = player.getPlayWhenReady(); - shouldRestorePosition = false; + playerWindow = player.getCurrentWindowIndex(); + playerPosition = C.TIME_UNSET; Timeline timeline = player.getCurrentTimeline(); - if (timeline != null) { - playerWindow = player.getCurrentWindowIndex(); - Timeline.Window window = timeline.getWindow(playerWindow, new Timeline.Window()); - if (!window.isDynamic) { - shouldRestorePosition = true; - playerPosition = window.isSeekable ? player.getCurrentPosition() : C.TIME_UNSET; - } + if (timeline != null && timeline.getWindow(playerWindow, window).isSeekable) { + playerPosition = player.getCurrentPosition(); } player.release(); player = null; @@ -412,7 +410,8 @@ public class PlayerActivity extends Activity implements OnClickListener, ExoPlay @Override public void onTimelineChanged(Timeline timeline, Object manifest) { - // Do nothing. + isTimelineStatic = timeline != null && timeline.getWindowCount() > 0 + && !timeline.getWindow(timeline.getWindowCount() - 1, window).isDynamic; } @Override @@ -501,7 +500,7 @@ public class PlayerActivity extends Activity implements OnClickListener, ExoPlay button.setText(label); button.setTag(i); button.setOnClickListener(this); - debugRootView.addView(button); + debugRootView.addView(button, debugRootView.getChildCount() - 1); } } } diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayer.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayer.java index 4d53455151..e3c9b6e114 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayer.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayer.java @@ -130,8 +130,8 @@ public interface ExoPlayer { /** * Called when timeline and/or manifest has been refreshed. * - * @param timeline The latest timeline. - * @param manifest The latest manifest. + * @param timeline The latest timeline, or null if the timeline is being cleared. + * @param manifest The latest manifest, or null if the manifest is being cleared. */ void onTimelineChanged(Timeline timeline, Object manifest); @@ -247,7 +247,7 @@ public interface ExoPlayer { /** * Prepares the player to play the provided {@link MediaSource}. Equivalent to - * {@code prepare(mediaSource, true)}. + * {@code prepare(mediaSource, true, true)}. */ void prepare(MediaSource mediaSource); @@ -259,8 +259,11 @@ public interface ExoPlayer { * @param resetPosition Whether the playback position should be reset to the default position in * the first {@link Timeline.Window}. If false, playback will start from the position defined * by {@link #getCurrentWindowIndex()} and {@link #getCurrentPosition()}. + * @param resetTimeline Whether the timeline and manifest should be reset. Should be true unless + * the player is being prepared to play the same media as it was playing previously (e.g. if + * playback failed and is being retried). */ - void prepare(MediaSource mediaSource, boolean resetPosition); + void prepare(MediaSource mediaSource, boolean resetPosition, boolean resetTimeline); /** * Sets whether playback should proceed when {@link #getPlaybackState()} == {@link #STATE_READY}. diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index a79ad1dba0..3eb2ceb38b 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -101,12 +101,18 @@ import java.util.concurrent.CopyOnWriteArraySet; @Override public void prepare(MediaSource mediaSource) { - prepare(mediaSource, true); + prepare(mediaSource, true, true); } @Override - public void prepare(MediaSource mediaSource, boolean resetPosition) { - timeline = null; + public void prepare(MediaSource mediaSource, boolean resetPosition, boolean resetTimeline) { + if (resetTimeline && (timeline != null || manifest != null)) { + timeline = null; + manifest = null; + for (EventListener listener : listeners) { + listener.onTimelineChanged(null, null); + } + } internalPlayer.prepare(mediaSource, resetPosition); } diff --git a/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java b/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java index a8f04e5113..f2c26b9495 100644 --- a/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java +++ b/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java @@ -420,8 +420,8 @@ public final class SimpleExoPlayer implements ExoPlayer { } @Override - public void prepare(MediaSource mediaSource, boolean resetPosition) { - player.prepare(mediaSource, resetPosition); + public void prepare(MediaSource mediaSource, boolean resetPosition, boolean resetTimeline) { + player.prepare(mediaSource, resetPosition, resetTimeline); } @Override From 996fe47f8cbafd3a9e6a0b2025a036414bd70a04 Mon Sep 17 00:00:00 2001 From: olly Date: Tue, 11 Oct 2016 08:29:55 -0700 Subject: [PATCH 14/24] Fix NPE releasing HlsMediaPeriod Issue: #1907 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135798950 --- .../android/exoplayer2/source/hls/HlsMediaPeriod.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java index 7ed979c902..57925ed67a 100644 --- a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java +++ b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java @@ -103,8 +103,10 @@ import java.util.List; public void release() { continueLoadingHandler.removeCallbacksAndMessages(null); manifestFetcher.release(); - for (HlsSampleStreamWrapper sampleStreamWrapper : sampleStreamWrappers) { - sampleStreamWrapper.release(); + if (sampleStreamWrappers != null) { + for (HlsSampleStreamWrapper sampleStreamWrapper : sampleStreamWrappers) { + sampleStreamWrapper.release(); + } } } From 94c7ee72527d219a54e90281dab8c1d9cd994eb4 Mon Sep 17 00:00:00 2001 From: olly Date: Tue, 11 Oct 2016 11:27:27 -0700 Subject: [PATCH 15/24] Cronet - Skip if server doesn't support range requests ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135819142 --- .../ext/cronet/CronetDataSourceTest.java | 133 ++++++++++-------- .../ext/cronet/CronetDataSource.java | 16 ++- 2 files changed, 86 insertions(+), 63 deletions(-) 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; + } } } From f18373eeb23b67e46cbc84e1467beba79eebd67b Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Wed, 12 Oct 2016 06:18:06 -0700 Subject: [PATCH 16/24] Decouple TsExtractor's readers from TrackOutputs This allows the injectable reader factory to be a stateless factory, allows the seeking to be consistent and will allow multiple CC channel support later on. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135909712 --- .../extractor/ts/AdtsReaderTest.java | 11 ++- .../extractor/ts/TsExtractorTest.java | 28 ++++--- .../exoplayer2/extractor/ts/Ac3Extractor.java | 4 +- .../exoplayer2/extractor/ts/Ac3Reader.java | 18 +++-- .../extractor/ts/AdtsExtractor.java | 4 +- .../exoplayer2/extractor/ts/AdtsReader.java | 36 ++++++--- .../ts/DefaultStreamReaderFactory.java | 78 ++++++------------- .../exoplayer2/extractor/ts/DtsReader.java | 21 +++-- .../extractor/ts/ElementaryStreamReader.java | 41 ++++++---- .../exoplayer2/extractor/ts/H262Reader.java | 11 ++- .../exoplayer2/extractor/ts/H264Reader.java | 35 +++++---- .../exoplayer2/extractor/ts/H265Reader.java | 19 +++-- .../exoplayer2/extractor/ts/Id3Reader.java | 15 +++- .../extractor/ts/MpegAudioReader.java | 15 +++- .../exoplayer2/extractor/ts/PsExtractor.java | 10 ++- .../exoplayer2/extractor/ts/TsExtractor.java | 61 +++++++++++---- .../exoplayer2/source/hls/HlsChunkSource.java | 32 ++++---- 17 files changed, 259 insertions(+), 180 deletions(-) diff --git a/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/AdtsReaderTest.java b/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/AdtsReaderTest.java index 7faea926e0..d1b5ce600d 100644 --- a/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/AdtsReaderTest.java +++ b/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/AdtsReaderTest.java @@ -16,6 +16,8 @@ package com.google.android.exoplayer2.extractor.ts; import com.google.android.exoplayer2.C; +import com.google.android.exoplayer2.extractor.ts.ElementaryStreamReader.TrackIdGenerator; +import com.google.android.exoplayer2.testutil.FakeExtractorOutput; import com.google.android.exoplayer2.testutil.FakeTrackOutput; import com.google.android.exoplayer2.testutil.TestUtil; import com.google.android.exoplayer2.util.ParsableByteArray; @@ -66,9 +68,12 @@ public class AdtsReaderTest extends TestCase { @Override protected void setUp() throws Exception { - adtsOutput = new FakeTrackOutput(); - id3Output = new FakeTrackOutput(); - adtsReader = new AdtsReader(adtsOutput, id3Output); + FakeExtractorOutput fakeExtractorOutput = new FakeExtractorOutput(true); + adtsOutput = fakeExtractorOutput.track(0); + id3Output = fakeExtractorOutput.track(1); + adtsReader = new AdtsReader(true); + TrackIdGenerator idGenerator = new TrackIdGenerator(0, 1); + adtsReader.init(fakeExtractorOutput, idGenerator); data = new ParsableByteArray(TEST_DATA); firstFeed = true; } diff --git a/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/TsExtractorTest.java b/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/TsExtractorTest.java index dfbc9120c6..1f08507599 100644 --- a/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/TsExtractorTest.java +++ b/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/TsExtractorTest.java @@ -22,6 +22,7 @@ import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.PositionHolder; import com.google.android.exoplayer2.extractor.TimestampAdjuster; import com.google.android.exoplayer2.extractor.TrackOutput; +import com.google.android.exoplayer2.extractor.ts.ElementaryStreamReader.EsInfo; import com.google.android.exoplayer2.testutil.FakeExtractorInput; import com.google.android.exoplayer2.testutil.FakeExtractorOutput; import com.google.android.exoplayer2.testutil.FakeTrackOutput; @@ -72,7 +73,7 @@ public final class TsExtractorTest extends InstrumentationTestCase { public void testCustomPesReader() throws Exception { CustomEsReaderFactory factory = new CustomEsReaderFactory(); - TsExtractor tsExtractor = new TsExtractor(new TimestampAdjuster(0), factory); + TsExtractor tsExtractor = new TsExtractor(new TimestampAdjuster(0), factory, false); FakeExtractorInput input = new FakeExtractorInput.Builder() .setData(TestUtil.getByteArray(getInstrumentation(), "ts/sample.ts")) .setSimulateIOErrors(false) @@ -107,18 +108,25 @@ public final class TsExtractorTest extends InstrumentationTestCase { private static final class CustomEsReader extends ElementaryStreamReader { + private final String language; + private TrackOutput output; public int packetsRead = 0; - public CustomEsReader(TrackOutput output, String language) { - super(output); - output.format(Format.createTextSampleFormat("Overriding format", "mime", null, 0, 0, - language, null, 0)); + public CustomEsReader(String language) { + this.language = language; } @Override public void seek() { } + @Override + public void init(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) { + output = extractorOutput.track(idGenerator.getNextId()); + output.format(Format.createTextSampleFormat("Overriding format", "mime", null, 0, 0, + language, null, 0)); + } + @Override public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) { } @@ -148,16 +156,12 @@ public final class TsExtractorTest extends InstrumentationTestCase { } @Override - public ElementaryStreamReader onPmtEntry(int pid, int streamType, - ElementaryStreamReader.EsInfo esInfo, ExtractorOutput output) { + public ElementaryStreamReader createStreamReader(int streamType, EsInfo esInfo) { if (streamType == 3) { - // We need to manually avoid a duplicate custom reader creation. - if (reader == null) { - reader = new CustomEsReader(output.track(pid), esInfo.language); - } + reader = new CustomEsReader(esInfo.language); return reader; } else { - return defaultFactory.onPmtEntry(pid, streamType, esInfo, output); + return defaultFactory.createStreamReader(streamType, esInfo); } } diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Ac3Extractor.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Ac3Extractor.java index 979c7244a8..7fc8b429a8 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Ac3Extractor.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Ac3Extractor.java @@ -23,6 +23,7 @@ import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.ExtractorsFactory; import com.google.android.exoplayer2.extractor.PositionHolder; import com.google.android.exoplayer2.extractor.SeekMap; +import com.google.android.exoplayer2.extractor.ts.ElementaryStreamReader.TrackIdGenerator; import com.google.android.exoplayer2.util.ParsableByteArray; import com.google.android.exoplayer2.util.Util; @@ -117,7 +118,8 @@ public final class Ac3Extractor implements Extractor { @Override public void init(ExtractorOutput output) { - reader = new Ac3Reader(output.track(0)); // TODO: Add support for embedded ID3. + reader = new Ac3Reader(); // TODO: Add support for embedded ID3. + reader.init(output, new TrackIdGenerator(0, 1)); output.endTracks(); output.seekMap(new SeekMap.Unseekable(C.TIME_UNSET)); } diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Ac3Reader.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Ac3Reader.java index cbe6d2e9c8..a9d3319f87 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Ac3Reader.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Ac3Reader.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.extractor.ts; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; import com.google.android.exoplayer2.audio.Ac3Util; +import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.TrackOutput; import com.google.android.exoplayer2.util.ParsableBitArray; import com.google.android.exoplayer2.util.ParsableByteArray; @@ -37,6 +38,8 @@ import com.google.android.exoplayer2.util.ParsableByteArray; private final ParsableByteArray headerScratchBytes; private final String language; + private TrackOutput output; + private int state; private int bytesRead; @@ -54,21 +57,17 @@ import com.google.android.exoplayer2.util.ParsableByteArray; /** * Constructs a new reader for (E-)AC-3 elementary streams. - * - * @param output Track output for extracted samples. */ - public Ac3Reader(TrackOutput output) { - this(output, null); + public Ac3Reader() { + this(null); } /** * Constructs a new reader for (E-)AC-3 elementary streams. * - * @param output Track output for extracted samples. * @param language Track language. */ - public Ac3Reader(TrackOutput output, String language) { - super(output); + public Ac3Reader(String language) { headerScratchBits = new ParsableBitArray(new byte[HEADER_SIZE]); headerScratchBytes = new ParsableByteArray(headerScratchBits.data); state = STATE_FINDING_SYNC; @@ -82,6 +81,11 @@ import com.google.android.exoplayer2.util.ParsableByteArray; lastByteWas0B = false; } + @Override + public void init(ExtractorOutput extractorOutput, TrackIdGenerator generator) { + output = extractorOutput.track(generator.getNextId()); + } + @Override public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) { timeUs = pesTimeUs; diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/AdtsExtractor.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/AdtsExtractor.java index f131d8997b..7a9cbd4bb1 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/AdtsExtractor.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/AdtsExtractor.java @@ -22,6 +22,7 @@ import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.ExtractorsFactory; import com.google.android.exoplayer2.extractor.PositionHolder; import com.google.android.exoplayer2.extractor.SeekMap; +import com.google.android.exoplayer2.extractor.ts.ElementaryStreamReader.TrackIdGenerator; import com.google.android.exoplayer2.util.ParsableBitArray; import com.google.android.exoplayer2.util.ParsableByteArray; import com.google.android.exoplayer2.util.Util; @@ -126,7 +127,8 @@ public final class AdtsExtractor implements Extractor { @Override public void init(ExtractorOutput output) { - reader = new AdtsReader(output.track(0), output.track(1)); + reader = new AdtsReader(true); + reader.init(output, new TrackIdGenerator(0, 1)); output.endTracks(); output.seekMap(new SeekMap.Unseekable(C.TIME_UNSET)); } diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/AdtsReader.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/AdtsReader.java index ac493c7d32..d0474f7e44 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/AdtsReader.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/AdtsReader.java @@ -19,6 +19,8 @@ import android.util.Log; import android.util.Pair; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; +import com.google.android.exoplayer2.extractor.DummyTrackOutput; +import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.TrackOutput; import com.google.android.exoplayer2.util.CodecSpecificDataUtil; import com.google.android.exoplayer2.util.MimeTypes; @@ -53,11 +55,14 @@ import java.util.Collections; private static final int ID3_SIZE_OFFSET = 6; private static final byte[] ID3_IDENTIFIER = {'I', 'D', '3'}; + private final boolean exposeId3; private final ParsableBitArray adtsScratch; private final ParsableByteArray id3HeaderBuffer; - private final TrackOutput id3Output; private final String language; + private TrackOutput output; + private TrackOutput id3Output; + private int state; private int bytesRead; @@ -77,26 +82,21 @@ import java.util.Collections; private long currentSampleDuration; /** - * @param output A {@link TrackOutput} to which AAC samples should be written. - * @param id3Output A {@link TrackOutput} to which ID3 samples should be written. + * @param exposeId3 True if the reader should expose ID3 information. */ - public AdtsReader(TrackOutput output, TrackOutput id3Output) { - this(output, id3Output, null); + public AdtsReader(boolean exposeId3) { + this(exposeId3, null); } /** - * @param output A {@link TrackOutput} to which AAC samples should be written. - * @param id3Output A {@link TrackOutput} to which ID3 samples should be written. + * @param exposeId3 True if the reader should expose ID3 information. * @param language Track language. */ - public AdtsReader(TrackOutput output, TrackOutput id3Output, String language) { - super(output); - this.id3Output = id3Output; - id3Output.format(Format.createSampleFormat(null, MimeTypes.APPLICATION_ID3, null, - Format.NO_VALUE, null)); + public AdtsReader(boolean exposeId3, String language) { adtsScratch = new ParsableBitArray(new byte[HEADER_SIZE + CRC_SIZE]); id3HeaderBuffer = new ParsableByteArray(Arrays.copyOf(ID3_IDENTIFIER, ID3_HEADER_SIZE)); setFindingSampleState(); + this.exposeId3 = exposeId3; this.language = language; } @@ -105,6 +105,18 @@ import java.util.Collections; setFindingSampleState(); } + @Override + public void init(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) { + output = extractorOutput.track(idGenerator.getNextId()); + if (exposeId3) { + id3Output = extractorOutput.track(idGenerator.getNextId()); + id3Output.format(Format.createSampleFormat(null, MimeTypes.APPLICATION_ID3, null, + Format.NO_VALUE, null)); + } else { + id3Output = new DummyTrackOutput(); + } + } + @Override public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) { timeUs = pesTimeUs; diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/DefaultStreamReaderFactory.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/DefaultStreamReaderFactory.java index d5e3b78cfd..58a0e55f02 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/DefaultStreamReaderFactory.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/DefaultStreamReaderFactory.java @@ -16,9 +16,7 @@ package com.google.android.exoplayer2.extractor.ts; import android.support.annotation.IntDef; -import android.util.SparseBooleanArray; -import com.google.android.exoplayer2.extractor.DummyTrackOutput; -import com.google.android.exoplayer2.extractor.ExtractorOutput; +import com.google.android.exoplayer2.extractor.ts.ElementaryStreamReader.EsInfo; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -28,80 +26,54 @@ import java.lang.annotation.RetentionPolicy; public final class DefaultStreamReaderFactory implements ElementaryStreamReader.Factory { /** - * Flags controlling what workarounds are enabled for elementary stream readers. + * Flags controlling elementary stream readers behaviour. */ @Retention(RetentionPolicy.SOURCE) - @IntDef(flag = true, value = {WORKAROUND_ALLOW_NON_IDR_KEYFRAMES, WORKAROUND_IGNORE_AAC_STREAM, - WORKAROUND_IGNORE_H264_STREAM, WORKAROUND_DETECT_ACCESS_UNITS, WORKAROUND_MAP_BY_TYPE}) - public @interface WorkaroundFlags { + @IntDef(flag = true, value = {FLAG_ALLOW_NON_IDR_KEYFRAMES, FLAG_IGNORE_AAC_STREAM, + FLAG_IGNORE_H264_STREAM, FLAG_DETECT_ACCESS_UNITS}) + public @interface Flags { } - public static final int WORKAROUND_ALLOW_NON_IDR_KEYFRAMES = 1; - public static final int WORKAROUND_IGNORE_AAC_STREAM = 2; - public static final int WORKAROUND_IGNORE_H264_STREAM = 4; - public static final int WORKAROUND_DETECT_ACCESS_UNITS = 8; - public static final int WORKAROUND_MAP_BY_TYPE = 16; + public static final int FLAG_ALLOW_NON_IDR_KEYFRAMES = 1; + public static final int FLAG_IGNORE_AAC_STREAM = 2; + public static final int FLAG_IGNORE_H264_STREAM = 4; + public static final int FLAG_DETECT_ACCESS_UNITS = 8; - private static final int BASE_EMBEDDED_TRACK_ID = 0x2000; // 0xFF + 1. - - private final SparseBooleanArray trackIds; - @WorkaroundFlags - private final int workaroundFlags; - private Id3Reader id3Reader; - private int nextEmbeddedTrackId = BASE_EMBEDDED_TRACK_ID; + @Flags + private final int flags; public DefaultStreamReaderFactory() { this(0); } - public DefaultStreamReaderFactory(int workaroundFlags) { - trackIds = new SparseBooleanArray(); - this.workaroundFlags = workaroundFlags; + public DefaultStreamReaderFactory(@Flags int flags) { + this.flags = flags; } @Override - public ElementaryStreamReader onPmtEntry(int pid, int streamType, - ElementaryStreamReader.EsInfo esInfo, ExtractorOutput output) { - - if ((workaroundFlags & WORKAROUND_MAP_BY_TYPE) != 0 && id3Reader == null) { - // Setup an ID3 track regardless of whether there's a corresponding entry, in case one - // appears intermittently during playback. See b/20261500. - id3Reader = new Id3Reader(output.track(TsExtractor.TS_STREAM_TYPE_ID3)); - } - int trackId = (workaroundFlags & WORKAROUND_MAP_BY_TYPE) != 0 ? streamType : pid; - if (trackIds.get(trackId)) { - return null; - } - trackIds.put(trackId, true); + public ElementaryStreamReader createStreamReader(int streamType, EsInfo esInfo) { switch (streamType) { case TsExtractor.TS_STREAM_TYPE_MPA: case TsExtractor.TS_STREAM_TYPE_MPA_LSF: - return new MpegAudioReader(output.track(trackId), esInfo.language); + return new MpegAudioReader(esInfo.language); case TsExtractor.TS_STREAM_TYPE_AAC: - return (workaroundFlags & WORKAROUND_IGNORE_AAC_STREAM) != 0 ? null - : new AdtsReader(output.track(trackId), new DummyTrackOutput(), esInfo.language); + return (flags & FLAG_IGNORE_AAC_STREAM) != 0 ? null + : new AdtsReader(false, esInfo.language); case TsExtractor.TS_STREAM_TYPE_AC3: case TsExtractor.TS_STREAM_TYPE_E_AC3: - return new Ac3Reader(output.track(trackId), esInfo.language); + return new Ac3Reader(esInfo.language); case TsExtractor.TS_STREAM_TYPE_DTS: case TsExtractor.TS_STREAM_TYPE_HDMV_DTS: - return new DtsReader(output.track(trackId), esInfo.language); + return new DtsReader(esInfo.language); case TsExtractor.TS_STREAM_TYPE_H262: - return new H262Reader(output.track(trackId)); + return new H262Reader(); case TsExtractor.TS_STREAM_TYPE_H264: - return (workaroundFlags & WORKAROUND_IGNORE_H264_STREAM) != 0 - ? null : new H264Reader(output.track(trackId), - new SeiReader(output.track(nextEmbeddedTrackId++)), - (workaroundFlags & WORKAROUND_ALLOW_NON_IDR_KEYFRAMES) != 0, - (workaroundFlags & WORKAROUND_DETECT_ACCESS_UNITS) != 0); + return (flags & FLAG_IGNORE_H264_STREAM) != 0 ? null + : new H264Reader((flags & FLAG_ALLOW_NON_IDR_KEYFRAMES) != 0, + (flags & FLAG_DETECT_ACCESS_UNITS) != 0); case TsExtractor.TS_STREAM_TYPE_H265: - return new H265Reader(output.track(trackId), - new SeiReader(output.track(nextEmbeddedTrackId++))); + return new H265Reader(); case TsExtractor.TS_STREAM_TYPE_ID3: - if ((workaroundFlags & WORKAROUND_MAP_BY_TYPE) != 0) { - return id3Reader; - } else { - return new Id3Reader(output.track(nextEmbeddedTrackId++)); - } + return new Id3Reader(); default: return null; } diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/DtsReader.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/DtsReader.java index e2112df755..42223ef285 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/DtsReader.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/DtsReader.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.extractor.ts; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; import com.google.android.exoplayer2.audio.DtsUtil; +import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.TrackOutput; import com.google.android.exoplayer2.util.ParsableByteArray; @@ -37,6 +38,8 @@ import com.google.android.exoplayer2.util.ParsableByteArray; private final ParsableByteArray headerScratchBytes; private final String language; + private TrackOutput output; + private int state; private int bytesRead; @@ -54,20 +57,9 @@ import com.google.android.exoplayer2.util.ParsableByteArray; /** * Constructs a new reader for DTS elementary streams. * - * @param output Track output for extracted samples. - */ - public DtsReader(TrackOutput output) { - this(output, null); - } - - /** - * Constructs a new reader for DTS elementary streams. - * - * @param output Track output for extracted samples. * @param language Track language. */ - public DtsReader(TrackOutput output, String language) { - super(output); + public DtsReader(String language) { headerScratchBytes = new ParsableByteArray(new byte[HEADER_SIZE]); headerScratchBytes.data[0] = (byte) ((SYNC_VALUE >> 24) & 0xFF); headerScratchBytes.data[1] = (byte) ((SYNC_VALUE >> 16) & 0xFF); @@ -84,6 +76,11 @@ import com.google.android.exoplayer2.util.ParsableByteArray; syncBytes = 0; } + @Override + public void init(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) { + output = extractorOutput.track(idGenerator.getNextId()); + } + @Override public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) { timeUs = pesTimeUs; diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/ElementaryStreamReader.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/ElementaryStreamReader.java index 7a220c98b3..e2efbebb43 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/ElementaryStreamReader.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/ElementaryStreamReader.java @@ -33,17 +33,12 @@ public abstract class ElementaryStreamReader { * Returns an {@link ElementaryStreamReader} for a given PMT entry. May return null if the * stream type is not supported or if the stream already has a reader assigned to it. * - * @param pid The pid for the PMT entry. - * @param streamType One of the {@link TsExtractor}{@code .TS_STREAM_TYPE_*} constants defining - * the type of the stream. - * @param esInfo The descriptor information linked to the elementary stream. - * @param output The {@link ExtractorOutput} that provides the {@link TrackOutput}s for the - * created readers. + * @param streamType Stream type value as defined in the PMT entry or associated descriptors. + * @param esInfo Information associated to the elementary stream provided in the PMT. * @return An {@link ElementaryStreamReader} for the elementary streams carried by the provided * pid. {@code null} if the stream is not supported or if it should be ignored. */ - ElementaryStreamReader onPmtEntry(int pid, int streamType, EsInfo esInfo, - ExtractorOutput output); + ElementaryStreamReader createStreamReader(int streamType, EsInfo esInfo); } @@ -70,13 +65,24 @@ public abstract class ElementaryStreamReader { } - protected final TrackOutput output; - /** - * @param output A {@link TrackOutput} to which samples should be written. + * Generates track ids for initializing {@link ElementaryStreamReader}s' {@link TrackOutput}s. */ - protected ElementaryStreamReader(TrackOutput output) { - this.output = output; + public static final class TrackIdGenerator { + + private final int firstId; + private final int idIncrement; + private int generatedIdCount; + + public TrackIdGenerator(int firstId, int idIncrement) { + this.firstId = firstId; + this.idIncrement = idIncrement; + } + + public int getNextId() { + return firstId + idIncrement * generatedIdCount++; + } + } /** @@ -84,6 +90,15 @@ public abstract class ElementaryStreamReader { */ public abstract void seek(); + /** + * Initializes the reader by providing outputs and ids for the tracks. + * + * @param extractorOutput The {@link ExtractorOutput} that receives the extracted data. + * @param idGenerator A {@link TrackIdGenerator} that generates unique track ids for the + * {@link TrackOutput}s. + */ + public abstract void init(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator); + /** * Called when a packet starts. * diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H262Reader.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H262Reader.java index cdbd8e391d..fbfe7e1209 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H262Reader.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H262Reader.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.extractor.ts; import android.util.Pair; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; +import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.TrackOutput; import com.google.android.exoplayer2.util.MimeTypes; import com.google.android.exoplayer2.util.NalUnitUtil; @@ -35,6 +36,8 @@ import java.util.Collections; private static final int START_EXTENSION = 0xB5; private static final int START_GROUP = 0xB8; + private TrackOutput output; + // Maps (frame_rate_code - 1) indices to values, as defined in ITU-T H.262 Table 6-4. private static final double[] FRAME_RATE_VALUES = new double[] { 24000d / 1001, 24, 25, 30000d / 1001, 30, 50, 60000d / 1001, 60}; @@ -58,8 +61,7 @@ import java.util.Collections; private long framePosition; private long frameTimeUs; - public H262Reader(TrackOutput output) { - super(output); + public H262Reader() { prefixFlags = new boolean[4]; csdBuffer = new CsdBuffer(128); } @@ -73,6 +75,11 @@ import java.util.Collections; totalBytesWritten = 0; } + @Override + public void init(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) { + output = extractorOutput.track(idGenerator.getNextId()); + } + @Override public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) { pesPtsUsAvailable = pesTimeUs != C.TIME_UNSET; diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H264Reader.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H264Reader.java index ce7b7e6383..6fee9ea6d7 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H264Reader.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H264Reader.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.extractor.ts; import android.util.SparseArray; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; +import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.TrackOutput; import com.google.android.exoplayer2.util.MimeTypes; import com.google.android.exoplayer2.util.NalUnitUtil; @@ -37,17 +38,20 @@ import java.util.List; private static final int NAL_UNIT_TYPE_SPS = 7; // Sequence parameter set private static final int NAL_UNIT_TYPE_PPS = 8; // Picture parameter set - // State that should not be reset on seek. - private boolean hasOutputFormat; - - // State that should be reset on seek. - private final SeiReader seiReader; - private final boolean[] prefixFlags; - private final SampleReader sampleReader; + private final boolean allowNonIdrKeyframes; + private final boolean detectAccessUnits; private final NalUnitTargetBuffer sps; private final NalUnitTargetBuffer pps; private final NalUnitTargetBuffer sei; private long totalBytesWritten; + private final boolean[] prefixFlags; + + private TrackOutput output; + private SeiReader seiReader; + private SampleReader sampleReader; + + // State that should not be reset on seek. + private boolean hasOutputFormat; // Per packet state that gets reset at the start of each packet. private long pesTimeUs; @@ -56,19 +60,15 @@ import java.util.List; private final ParsableByteArray seiWrapper; /** - * @param output A {@link TrackOutput} to which H.264 samples should be written. - * @param seiReader A reader for CEA-608 samples in SEI NAL units. * @param allowNonIdrKeyframes Whether to treat samples consisting of non-IDR I slices as * synchronization samples (key-frames). * @param detectAccessUnits Whether to split the input stream into access units (samples) based on * slice headers. Pass {@code false} if the stream contains access unit delimiters (AUDs). */ - public H264Reader(TrackOutput output, SeiReader seiReader, boolean allowNonIdrKeyframes, - boolean detectAccessUnits) { - super(output); - this.seiReader = seiReader; + public H264Reader(boolean allowNonIdrKeyframes, boolean detectAccessUnits) { prefixFlags = new boolean[3]; - sampleReader = new SampleReader(output, allowNonIdrKeyframes, detectAccessUnits); + this.allowNonIdrKeyframes = allowNonIdrKeyframes; + this.detectAccessUnits = detectAccessUnits; sps = new NalUnitTargetBuffer(NAL_UNIT_TYPE_SPS, 128); pps = new NalUnitTargetBuffer(NAL_UNIT_TYPE_PPS, 128); sei = new NalUnitTargetBuffer(NAL_UNIT_TYPE_SEI, 128); @@ -85,6 +85,13 @@ import java.util.List; totalBytesWritten = 0; } + @Override + public void init(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) { + output = extractorOutput.track(idGenerator.getNextId()); + sampleReader = new SampleReader(output, allowNonIdrKeyframes, detectAccessUnits); + seiReader = new SeiReader(extractorOutput.track(idGenerator.getNextId())); + } + @Override public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) { this.pesTimeUs = pesTimeUs; diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H265Reader.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H265Reader.java index c8828cefa6..6283371a19 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H265Reader.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/H265Reader.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.extractor.ts; import android.util.Log; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; +import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.TrackOutput; import com.google.android.exoplayer2.util.MimeTypes; import com.google.android.exoplayer2.util.NalUnitUtil; @@ -42,11 +43,13 @@ import java.util.Collections; private static final int PREFIX_SEI_NUT = 39; private static final int SUFFIX_SEI_NUT = 40; + private TrackOutput output; + private SeiReader seiReader; + // State that should not be reset on seek. private boolean hasOutputFormat; // State that should be reset on seek. - private final SeiReader seiReader; private final boolean[] prefixFlags; private final NalUnitTargetBuffer vps; private final NalUnitTargetBuffer sps; @@ -62,13 +65,7 @@ import java.util.Collections; // Scratch variables to avoid allocations. private final ParsableByteArray seiWrapper; - /** - * @param output A {@link TrackOutput} to which H.265 samples should be written. - * @param seiReader A reader for CEA-608 samples in SEI NAL units. - */ - public H265Reader(TrackOutput output, SeiReader seiReader) { - super(output); - this.seiReader = seiReader; + public H265Reader() { prefixFlags = new boolean[3]; vps = new NalUnitTargetBuffer(VPS_NUT, 128); sps = new NalUnitTargetBuffer(SPS_NUT, 128); @@ -91,6 +88,12 @@ import java.util.Collections; totalBytesWritten = 0; } + @Override + public void init(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) { + output = extractorOutput.track(idGenerator.getNextId()); + seiReader = new SeiReader(extractorOutput.track(idGenerator.getNextId())); + } + @Override public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) { this.pesTimeUs = pesTimeUs; diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Id3Reader.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Id3Reader.java index 1001f1a1ae..2c657d4aca 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Id3Reader.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/Id3Reader.java @@ -17,6 +17,7 @@ package com.google.android.exoplayer2.extractor.ts; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; +import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.TrackOutput; import com.google.android.exoplayer2.util.MimeTypes; import com.google.android.exoplayer2.util.ParsableByteArray; @@ -30,6 +31,8 @@ import com.google.android.exoplayer2.util.ParsableByteArray; private final ParsableByteArray id3Header; + private TrackOutput output; + // State that should be reset on seek. private boolean writingSample; @@ -38,10 +41,7 @@ import com.google.android.exoplayer2.util.ParsableByteArray; private int sampleSize; private int sampleBytesRead; - public Id3Reader(TrackOutput output) { - super(output); - output.format(Format.createSampleFormat(null, MimeTypes.APPLICATION_ID3, null, Format.NO_VALUE, - null)); + public Id3Reader() { id3Header = new ParsableByteArray(ID3_HEADER_SIZE); } @@ -50,6 +50,13 @@ import com.google.android.exoplayer2.util.ParsableByteArray; writingSample = false; } + @Override + public void init(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) { + output = extractorOutput.track(idGenerator.getNextId()); + output.format(Format.createSampleFormat(null, MimeTypes.APPLICATION_ID3, null, Format.NO_VALUE, + null)); + } + @Override public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) { if (!dataAlignmentIndicator) { diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/MpegAudioReader.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/MpegAudioReader.java index c78882c2c9..d25d0703ae 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/MpegAudioReader.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/MpegAudioReader.java @@ -17,6 +17,7 @@ package com.google.android.exoplayer2.extractor.ts; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; +import com.google.android.exoplayer2.extractor.ExtractorOutput; import com.google.android.exoplayer2.extractor.MpegAudioHeader; import com.google.android.exoplayer2.extractor.TrackOutput; import com.google.android.exoplayer2.util.ParsableByteArray; @@ -36,6 +37,8 @@ import com.google.android.exoplayer2.util.ParsableByteArray; private final MpegAudioHeader header; private final String language; + private TrackOutput output; + private int state; private int frameBytesRead; private boolean hasOutputFormat; @@ -50,12 +53,11 @@ import com.google.android.exoplayer2.util.ParsableByteArray; // The timestamp to attach to the next sample in the current packet. private long timeUs; - public MpegAudioReader(TrackOutput output) { - this(output, null); + public MpegAudioReader() { + this(null); } - public MpegAudioReader(TrackOutput output, String language) { - super(output); + public MpegAudioReader(String language) { state = STATE_FINDING_HEADER; // The first byte of an MPEG Audio frame header is always 0xFF. headerScratch = new ParsableByteArray(4); @@ -71,6 +73,11 @@ import com.google.android.exoplayer2.util.ParsableByteArray; lastByteWasFF = false; } + @Override + public void init(ExtractorOutput extractorOutput, TrackIdGenerator idGenerator) { + output = extractorOutput.track(idGenerator.getNextId()); + } + @Override public void packetStarted(long pesTimeUs, boolean dataAlignmentIndicator) { timeUs = pesTimeUs; diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/PsExtractor.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/PsExtractor.java index 35eb519f09..b615a3e8ee 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/PsExtractor.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/PsExtractor.java @@ -24,6 +24,7 @@ import com.google.android.exoplayer2.extractor.ExtractorsFactory; import com.google.android.exoplayer2.extractor.PositionHolder; import com.google.android.exoplayer2.extractor.SeekMap; import com.google.android.exoplayer2.extractor.TimestampAdjuster; +import com.google.android.exoplayer2.extractor.ts.ElementaryStreamReader.TrackIdGenerator; import com.google.android.exoplayer2.util.ParsableBitArray; import com.google.android.exoplayer2.util.ParsableByteArray; import java.io.IOException; @@ -49,6 +50,7 @@ public final class PsExtractor implements Extractor { private static final int SYSTEM_HEADER_START_CODE = 0x000001BB; private static final int PACKET_START_CODE_PREFIX = 0x000001; private static final int MPEG_PROGRAM_END_CODE = 0x000001B9; + private static final int MAX_STREAM_ID_PLUS_ONE = 0x100; private static final long MAX_SEARCH_LENGTH = 1024 * 1024; public static final int PRIVATE_STREAM_1 = 0xBD; @@ -189,16 +191,18 @@ public final class PsExtractor implements Extractor { // Private stream, used for AC3 audio. // NOTE: This may need further parsing to determine if its DTS, but that's likely only // valid for DVDs. - elementaryStreamReader = new Ac3Reader(output.track(streamId)); + elementaryStreamReader = new Ac3Reader(); foundAudioTrack = true; } else if (!foundAudioTrack && (streamId & AUDIO_STREAM_MASK) == AUDIO_STREAM) { - elementaryStreamReader = new MpegAudioReader(output.track(streamId)); + elementaryStreamReader = new MpegAudioReader(); foundAudioTrack = true; } else if (!foundVideoTrack && (streamId & VIDEO_STREAM_MASK) == VIDEO_STREAM) { - elementaryStreamReader = new H262Reader(output.track(streamId)); + elementaryStreamReader = new H262Reader(); foundVideoTrack = true; } if (elementaryStreamReader != null) { + TrackIdGenerator idGenerator = new TrackIdGenerator(streamId, MAX_STREAM_ID_PLUS_ONE); + elementaryStreamReader.init(output, idGenerator); payloadReader = new PesReader(elementaryStreamReader, timestampAdjuster); psPayloadReaders.put(streamId, payloadReader); } diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java index 0248db9650..f5a7d090e1 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java @@ -17,6 +17,7 @@ package com.google.android.exoplayer2.extractor.ts; import android.util.Log; import android.util.SparseArray; +import android.util.SparseBooleanArray; import android.util.SparseIntArray; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.extractor.Extractor; @@ -26,6 +27,9 @@ import com.google.android.exoplayer2.extractor.ExtractorsFactory; import com.google.android.exoplayer2.extractor.PositionHolder; import com.google.android.exoplayer2.extractor.SeekMap; import com.google.android.exoplayer2.extractor.TimestampAdjuster; +import com.google.android.exoplayer2.extractor.TrackOutput; +import com.google.android.exoplayer2.extractor.ts.ElementaryStreamReader.EsInfo; +import com.google.android.exoplayer2.extractor.ts.ElementaryStreamReader.TrackIdGenerator; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.ParsableBitArray; import com.google.android.exoplayer2.util.ParsableByteArray; @@ -50,11 +54,6 @@ public final class TsExtractor implements Extractor { }; - private static final String TAG = "TsExtractor"; - - private static final int TS_PACKET_SIZE = 188; - private static final int TS_SYNC_BYTE = 0x47; // First byte of each TS packet. - private static final int TS_PAT_PID = 0; public static final int TS_STREAM_TYPE_MPA = 0x03; public static final int TS_STREAM_TYPE_MPA_LSF = 0x04; @@ -68,6 +67,12 @@ public final class TsExtractor implements Extractor { public static final int TS_STREAM_TYPE_H265 = 0x24; public static final int TS_STREAM_TYPE_ID3 = 0x15; + private static final String TAG = "TsExtractor"; + + private static final int TS_PACKET_SIZE = 188; + private static final int TS_SYNC_BYTE = 0x47; // First byte of each TS packet. + private static final int TS_PAT_PID = 0; + private static final int MAX_PID_PLUS_ONE = 0x2000; private static final long AC3_FORMAT_IDENTIFIER = Util.getIntegerCodeForString("AC-3"); private static final long E_AC3_FORMAT_IDENTIFIER = Util.getIntegerCodeForString("EAC3"); @@ -76,15 +81,18 @@ public final class TsExtractor implements Extractor { private static final int BUFFER_PACKET_COUNT = 5; // Should be at least 2 private static final int BUFFER_SIZE = TS_PACKET_SIZE * BUFFER_PACKET_COUNT; + private final boolean mapByType; private final TimestampAdjuster timestampAdjuster; private final ParsableByteArray tsPacketBuffer; private final ParsableBitArray tsScratch; private final SparseIntArray continuityCounters; private final ElementaryStreamReader.Factory streamReaderFactory; - /* package */ final SparseArray tsPayloadReaders; // Indexed by pid + private final SparseArray tsPayloadReaders; // Indexed by pid + private final SparseBooleanArray trackIds; // Accessed only by the loading thread. private ExtractorOutput output; + private ElementaryStreamReader id3Reader; public TsExtractor() { this(new TimestampAdjuster(0)); @@ -94,19 +102,23 @@ public final class TsExtractor implements Extractor { * @param timestampAdjuster A timestamp adjuster for offsetting and scaling sample timestamps. */ public TsExtractor(TimestampAdjuster timestampAdjuster) { - this(timestampAdjuster, new DefaultStreamReaderFactory()); + this(timestampAdjuster, new DefaultStreamReaderFactory(), false); } /** * @param timestampAdjuster A timestamp adjuster for offsetting and scaling sample timestamps. * @param customReaderFactory Factory for injecting a custom set of elementary stream readers. + * @param mapByType True if {@link TrackOutput}s should be mapped by their type, false to map them + * by their PID. */ public TsExtractor(TimestampAdjuster timestampAdjuster, - ElementaryStreamReader.Factory customReaderFactory) { + ElementaryStreamReader.Factory customReaderFactory, boolean mapByType) { this.timestampAdjuster = timestampAdjuster; this.streamReaderFactory = Assertions.checkNotNull(customReaderFactory); + this.mapByType = mapByType; tsPacketBuffer = new ParsableByteArray(BUFFER_SIZE); tsScratch = new ParsableBitArray(new byte[3]); + trackIds = new SparseBooleanArray(); tsPayloadReaders = new SparseArray<>(); tsPayloadReaders.put(TS_PAT_PID, new PatReader()); continuityCounters = new SparseIntArray(); @@ -413,6 +425,14 @@ public final class TsExtractor implements Extractor { // Skip the descriptors. sectionData.skipBytes(programInfoLength); + if (mapByType && id3Reader == null) { + // Setup an ID3 track regardless of whether there's a corresponding entry, in case one + // appears intermittently during playback. See [Internal: b/20261500]. + EsInfo dummyEsInfo = new EsInfo(TS_STREAM_TYPE_ID3, null, new byte[0]); + id3Reader = streamReaderFactory.createStreamReader(TS_STREAM_TYPE_ID3, dummyEsInfo); + id3Reader.init(output, new TrackIdGenerator(TS_STREAM_TYPE_ID3, MAX_PID_PLUS_ONE)); + } + int remainingEntriesLength = sectionLength - 9 /* Length of fields before descriptors */ - programInfoLength - 4 /* CRC length */; while (remainingEntriesLength > 0) { @@ -422,17 +442,28 @@ public final class TsExtractor implements Extractor { int elementaryPid = pmtScratch.readBits(13); pmtScratch.skipBits(4); // reserved int esInfoLength = pmtScratch.readBits(12); // ES_info_length. - ElementaryStreamReader.EsInfo esInfo = readEsInfo(sectionData, esInfoLength); + EsInfo esInfo = readEsInfo(sectionData, esInfoLength); if (streamType == 0x06) { streamType = esInfo.streamType; } remainingEntriesLength -= esInfoLength + 5; - ElementaryStreamReader pesPayloadReader = streamReaderFactory.onPmtEntry(elementaryPid, - streamType, esInfo, output); + + int trackId = mapByType ? streamType : elementaryPid; + if (trackIds.get(trackId)) { + continue; + } + trackIds.put(trackId, true); + + ElementaryStreamReader pesPayloadReader; + if (mapByType && streamType == TS_STREAM_TYPE_ID3) { + pesPayloadReader = id3Reader; + } else { + pesPayloadReader = streamReaderFactory.createStreamReader(streamType, esInfo); + pesPayloadReader.init(output, new TrackIdGenerator(trackId, MAX_PID_PLUS_ONE)); + } if (pesPayloadReader != null) { - tsPayloadReaders.put(elementaryPid, - new PesReader(pesPayloadReader, timestampAdjuster)); + tsPayloadReaders.put(elementaryPid, new PesReader(pesPayloadReader, timestampAdjuster)); } } @@ -447,7 +478,7 @@ public final class TsExtractor implements Extractor { * @param length The length of descriptors to read from the current position in {@code data}. * @return The stream info read from the available descriptors. */ - private ElementaryStreamReader.EsInfo readEsInfo(ParsableByteArray data, int length) { + private EsInfo readEsInfo(ParsableByteArray data, int length) { int descriptorsStartPosition = data.getPosition(); int descriptorsEndPosition = descriptorsStartPosition + length; int streamType = -1; @@ -479,7 +510,7 @@ public final class TsExtractor implements Extractor { data.skipBytes(positionOfNextDescriptor - data.getPosition()); } data.setPosition(descriptorsEndPosition); - return new ElementaryStreamReader.EsInfo(streamType, language, + return new EsInfo(streamType, language, Arrays.copyOfRange(sectionData.data, descriptorsStartPosition, descriptorsEndPosition)); } diff --git a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java index caca5d9b83..6dade6a02f 100644 --- a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java +++ b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java @@ -369,29 +369,29 @@ import java.util.Locale; } } else if (needNewExtractor) { // MPEG-2 TS segments, but we need a new extractor. - // This flag ensures the change of pid between streams does not affect the sample queues. - @DefaultStreamReaderFactory.WorkaroundFlags - int workaroundFlags = DefaultStreamReaderFactory.WORKAROUND_MAP_BY_TYPE; - String codecs = variants[newVariantIndex].format.codecs; - if (!TextUtils.isEmpty(codecs)) { - // Sometimes AAC and H264 streams are declared in TS chunks even though they don't really - // exist. If we know from the codec attribute that they don't exist, then we can explicitly - // ignore them even if they're declared. - if (!MimeTypes.AUDIO_AAC.equals(MimeTypes.getAudioMediaMimeType(codecs))) { - workaroundFlags |= DefaultStreamReaderFactory.WORKAROUND_IGNORE_AAC_STREAM; - } - if (!MimeTypes.VIDEO_H264.equals(MimeTypes.getVideoMediaMimeType(codecs))) { - workaroundFlags |= DefaultStreamReaderFactory.WORKAROUND_IGNORE_H264_STREAM; - } - } isTimestampMaster = true; if (useInitializedExtractor) { extractor = lastLoadedInitializationChunk.extractor; } else { timestampAdjuster = timestampAdjusterProvider.getAdjuster( segment.discontinuitySequenceNumber, startTimeUs); + // This flag ensures the change of pid between streams does not affect the sample queues. + @DefaultStreamReaderFactory.Flags + int esReaderFactoryFlags = 0; + String codecs = variants[newVariantIndex].format.codecs; + if (!TextUtils.isEmpty(codecs)) { + // Sometimes AAC and H264 streams are declared in TS chunks even though they don't really + // exist. If we know from the codec attribute that they don't exist, then we can + // explicitly ignore them even if they're declared. + if (!MimeTypes.AUDIO_AAC.equals(MimeTypes.getAudioMediaMimeType(codecs))) { + esReaderFactoryFlags |= DefaultStreamReaderFactory.FLAG_IGNORE_AAC_STREAM; + } + if (!MimeTypes.VIDEO_H264.equals(MimeTypes.getVideoMediaMimeType(codecs))) { + esReaderFactoryFlags |= DefaultStreamReaderFactory.FLAG_IGNORE_H264_STREAM; + } + } extractor = new TsExtractor(timestampAdjuster, - new DefaultStreamReaderFactory(workaroundFlags)); + new DefaultStreamReaderFactory(esReaderFactoryFlags), true); } } else { // MPEG-2 TS segments, and we need to continue using the same extractor. From 64262085a71f822741b65d78c0e883abaa15d8c1 Mon Sep 17 00:00:00 2001 From: olly Date: Wed, 12 Oct 2016 08:48:37 -0700 Subject: [PATCH 17/24] Block when surface being replaced is non-null A blocking call is necessary where we want to guarantee that the player wont access the surface after the method call has returned. We currently only do this for the case: Surface->Null But we should also do it for the case: SurfaceA->SurfaceB Since the caller may reasonably do something like destroy SurfaceA immediately after it's been replaced. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=135921296 --- .../com/google/android/exoplayer2/SimpleExoPlayer.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java b/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java index f2c26b9495..e621e19a48 100644 --- a/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java +++ b/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java @@ -619,7 +619,8 @@ public final class SimpleExoPlayer implements ExoPlayer { } private void setVideoSurfaceInternal(Surface surface) { - this.surface = surface; + // Note: We don't turn this method into a no-op if the surface is being replaced with itself + // so as to ensure onRenderedFirstFrame callbacks are still called in this case. ExoPlayerMessage[] messages = new ExoPlayerMessage[videoRendererCount]; int count = 0; for (Renderer renderer : renderers) { @@ -627,12 +628,13 @@ public final class SimpleExoPlayer implements ExoPlayer { messages[count++] = new ExoPlayerMessage(renderer, C.MSG_SET_SURFACE, surface); } } - if (surface == null) { - // Block to ensure that the surface is not accessed after the method returns. + if (this.surface != null && this.surface != surface) { + // We're replacing a surface. Block to ensure that it's not accessed after the method returns. player.blockingSendMessages(messages); } else { player.sendMessages(messages); } + this.surface = surface; } private final class ComponentListener implements VideoRendererEventListener, From ff712aead598515a6c9d47bf3050f185c226774c Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Thu, 13 Oct 2016 04:40:44 -0700 Subject: [PATCH 18/24] Try not adapting before failing with BehindLiveWindowException in Hls Issue:#1782 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=136025847 --- .../exoplayer2/source/hls/HlsChunkSource.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java index 6dade6a02f..53d9e70d76 100644 --- a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java +++ b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java @@ -257,8 +257,16 @@ import java.util.Locale; chunkMediaSequence = getLiveNextChunkSequenceNumber(previous.chunkIndex, oldVariantIndex, newVariantIndex); if (chunkMediaSequence < mediaPlaylist.mediaSequence) { - fatalError = new BehindLiveWindowException(); - return; + // We try getting the next chunk without adapting in case that's the reason for falling + // behind the live window. + newVariantIndex = oldVariantIndex; + mediaPlaylist = variantPlaylists[newVariantIndex]; + chunkMediaSequence = getLiveNextChunkSequenceNumber(previous.chunkIndex, oldVariantIndex, + newVariantIndex); + if (chunkMediaSequence < mediaPlaylist.mediaSequence) { + fatalError = new BehindLiveWindowException(); + return; + } } } } else { From e685edc179837f658bd551992c720dd8dc7bff9f Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Thu, 13 Oct 2016 04:54:13 -0700 Subject: [PATCH 19/24] Make interface implementation consistent among ExtractorOutputs The method track(int id) currently has different behaviours across implementations. This CL maps ids to track outputs, which means that successive calls with the same id will return the same TrackOutput instance. Also fixes TsExtractor inconsistent behavior after a seek. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=136026721 --- .../extractor/ts/AdtsReaderTest.java | 2 +- .../exoplayer2/extractor/Extractor.java | 2 +- .../exoplayer2/extractor/ExtractorOutput.java | 13 ++-- .../exoplayer2/extractor/ts/TsExtractor.java | 34 ++++++--- .../source/ExtractorMediaPeriod.java | 74 +++++++++++-------- .../source/chunk/ChunkExtractorWrapper.java | 4 +- .../testutil/FakeExtractorOutput.java | 11 +-- .../android/exoplayer2/testutil/TestUtil.java | 4 +- 8 files changed, 82 insertions(+), 62 deletions(-) diff --git a/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/AdtsReaderTest.java b/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/AdtsReaderTest.java index d1b5ce600d..e19de76466 100644 --- a/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/AdtsReaderTest.java +++ b/library/src/androidTest/java/com/google/android/exoplayer2/extractor/ts/AdtsReaderTest.java @@ -68,7 +68,7 @@ public class AdtsReaderTest extends TestCase { @Override protected void setUp() throws Exception { - FakeExtractorOutput fakeExtractorOutput = new FakeExtractorOutput(true); + FakeExtractorOutput fakeExtractorOutput = new FakeExtractorOutput(); adtsOutput = fakeExtractorOutput.track(0); id3Output = fakeExtractorOutput.track(1); adtsReader = new AdtsReader(true); diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/Extractor.java b/library/src/main/java/com/google/android/exoplayer2/extractor/Extractor.java index 6bf65710fc..4120110afb 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/Extractor.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/Extractor.java @@ -56,7 +56,7 @@ public interface Extractor { boolean sniff(ExtractorInput input) throws IOException, InterruptedException; /** - * Initializes the extractor with an {@link ExtractorOutput}. + * Initializes the extractor with an {@link ExtractorOutput}. Called at most once. * * @param output An {@link ExtractorOutput} to receive extracted data. */ diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ExtractorOutput.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ExtractorOutput.java index d138c7ce3a..a547f745ca 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ExtractorOutput.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ExtractorOutput.java @@ -21,18 +21,19 @@ package com.google.android.exoplayer2.extractor; public interface ExtractorOutput { /** - * Called when the {@link Extractor} identifies the existence of a track in the stream. + * Called by the {@link Extractor} to get the {@link TrackOutput} for a specific track. *

- * Returns a {@link TrackOutput} that will receive track level data belonging to the track. + * The same {@link TrackOutput} is returned if multiple calls are made with the same + * {@code trackId}. * - * @param trackId A unique track identifier. - * @return The {@link TrackOutput} that should receive track level data belonging to the track. + * @param trackId A track identifier. + * @return The {@link TrackOutput} for the given track identifier. */ TrackOutput track(int trackId); /** - * Called when all tracks have been identified, meaning that {@link #track(int)} will not be - * called again. + * Called when all tracks have been identified, meaning no new {@code trackId} values will be + * passed to {@link #track(int)}. */ void endTracks(); diff --git a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java index f5a7d090e1..bac362d711 100644 --- a/library/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java +++ b/library/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java @@ -54,7 +54,6 @@ public final class TsExtractor implements Extractor { }; - public static final int TS_STREAM_TYPE_MPA = 0x03; public static final int TS_STREAM_TYPE_MPA_LSF = 0x04; public static final int TS_STREAM_TYPE_AAC = 0x0F; @@ -92,6 +91,7 @@ public final class TsExtractor implements Extractor { // Accessed only by the loading thread. private ExtractorOutput output; + private boolean tracksEnded; private ElementaryStreamReader id3Reader; public TsExtractor() { @@ -120,8 +120,8 @@ public final class TsExtractor implements Extractor { tsScratch = new ParsableBitArray(new byte[3]); trackIds = new SparseBooleanArray(); tsPayloadReaders = new SparseArray<>(); - tsPayloadReaders.put(TS_PAT_PID, new PatReader()); continuityCounters = new SparseIntArray(); + resetPayloadReaders(); } // Extractor implementation. @@ -153,11 +153,10 @@ public final class TsExtractor implements Extractor { @Override public void seek(long position) { timestampAdjuster.reset(); - for (int i = 0; i < tsPayloadReaders.size(); i++) { - tsPayloadReaders.valueAt(i).seek(); - } tsPacketBuffer.reset(); continuityCounters.clear(); + // Elementary stream readers' state should be cleared to get consistent behaviours when seeking. + resetPayloadReaders(); } @Override @@ -252,6 +251,13 @@ public final class TsExtractor implements Extractor { // Internals. + private void resetPayloadReaders() { + trackIds.clear(); + tsPayloadReaders.clear(); + tsPayloadReaders.put(TS_PAT_PID, new PatReader()); + id3Reader = null; + } + /** * Parses TS packet payload data. */ @@ -345,7 +351,7 @@ public final class TsExtractor implements Extractor { patScratch.skipBits(13); // network_PID (13) } else { int pid = patScratch.readBits(13); - tsPayloadReaders.put(pid, new PmtReader()); + tsPayloadReaders.put(pid, new PmtReader(pid)); } } } @@ -365,14 +371,16 @@ public final class TsExtractor implements Extractor { private final ParsableBitArray pmtScratch; private final ParsableByteArray sectionData; + private final int pid; private int sectionLength; private int sectionBytesRead; private int crc; - public PmtReader() { + public PmtReader(int pid) { pmtScratch = new ParsableBitArray(new byte[5]); sectionData = new ParsableByteArray(); + this.pid = pid; } @Override @@ -466,8 +474,16 @@ public final class TsExtractor implements Extractor { tsPayloadReaders.put(elementaryPid, new PesReader(pesPayloadReader, timestampAdjuster)); } } - - output.endTracks(); + if (mapByType) { + if (!tracksEnded) { + output.endTracks(); + } + } else { + tsPayloadReaders.remove(TS_PAT_PID); + tsPayloadReaders.remove(pid); + output.endTracks(); + } + tracksEnded = true; } /** diff --git a/library/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java b/library/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java index 49f2cffca5..27bd1f677f 100644 --- a/library/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java +++ b/library/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java @@ -17,6 +17,7 @@ package com.google.android.exoplayer2.source; import android.net.Uri; import android.os.Handler; +import android.util.SparseArray; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; import com.google.android.exoplayer2.FormatHolder; @@ -41,7 +42,6 @@ import com.google.android.exoplayer2.util.ConditionVariable; import com.google.android.exoplayer2.util.Util; import java.io.EOFException; import java.io.IOException; -import java.util.Arrays; /** * A {@link MediaPeriod} that extracts data using an {@link Extractor}. @@ -68,6 +68,7 @@ import java.util.Arrays; private final Runnable maybeFinishPrepareRunnable; private final Runnable onContinueLoadingRequestedRunnable; private final Handler handler; + private final SparseArray sampleQueues; private Callback callback; private SeekMap seekMap; @@ -77,7 +78,6 @@ import java.util.Arrays; private boolean seenFirstTrackSelection; private boolean notifyReset; private int enabledTrackCount; - private DefaultTrackOutput[] sampleQueues; private TrackGroupArray tracks; private long durationUs; private boolean[] trackEnabledStates; @@ -131,7 +131,7 @@ import java.util.Arrays; handler = new Handler(); pendingResetPositionUs = C.TIME_UNSET; - sampleQueues = new DefaultTrackOutput[0]; + sampleQueues = new SparseArray<>(); length = C.LENGTH_UNSET; } @@ -141,8 +141,9 @@ import java.util.Arrays; @Override public void run() { extractorHolder.release(); - for (DefaultTrackOutput sampleQueue : sampleQueues) { - sampleQueue.disable(); + int trackCount = sampleQueues.size(); + for (int i = 0; i < trackCount; i++) { + sampleQueues.valueAt(i).disable(); } } }); @@ -178,7 +179,7 @@ import java.util.Arrays; Assertions.checkState(trackEnabledStates[track]); enabledTrackCount--; trackEnabledStates[track] = false; - sampleQueues[track].disable(); + sampleQueues.valueAt(track).disable(); streams[i] = null; } } @@ -201,9 +202,10 @@ import java.util.Arrays; if (!seenFirstTrackSelection) { // At the time of the first track selection all queues will be enabled, so we need to disable // any that are no longer required. - for (int i = 0; i < sampleQueues.length; i++) { + int trackCount = sampleQueues.size(); + for (int i = 0; i < trackCount; i++) { if (!trackEnabledStates[i]) { - sampleQueues[i].disable(); + sampleQueues.valueAt(i).disable(); } } } @@ -270,11 +272,12 @@ import java.util.Arrays; // Treat all seeks into non-seekable media as being to t=0. positionUs = seekMap.isSeekable() ? positionUs : 0; lastSeekPositionUs = positionUs; + int trackCount = sampleQueues.size(); // If we're not pending a reset, see if we can seek within the sample queues. boolean seekInsideBuffer = !isPendingReset(); - for (int i = 0; seekInsideBuffer && i < sampleQueues.length; i++) { + for (int i = 0; seekInsideBuffer && i < trackCount; i++) { if (trackEnabledStates[i]) { - seekInsideBuffer = sampleQueues[i].skipToKeyframeBefore(positionUs); + seekInsideBuffer = sampleQueues.valueAt(i).skipToKeyframeBefore(positionUs); } } // If we failed to seek within the sample queues, we need to restart. @@ -284,8 +287,8 @@ import java.util.Arrays; if (loader.isLoading()) { loader.cancelLoading(); } else { - for (int i = 0; i < sampleQueues.length; i++) { - sampleQueues[i].reset(trackEnabledStates[i]); + for (int i = 0; i < trackCount; i++) { + sampleQueues.valueAt(i).reset(trackEnabledStates[i]); } } } @@ -296,7 +299,7 @@ import java.util.Arrays; // SampleStream methods. /* package */ boolean isReady(int track) { - return loadingFinished || (!isPendingReset() && !sampleQueues[track].isEmpty()); + return loadingFinished || (!isPendingReset() && !sampleQueues.valueAt(track).isEmpty()); } /* package */ void maybeThrowError() throws IOException { @@ -308,7 +311,8 @@ import java.util.Arrays; return C.RESULT_NOTHING_READ; } - return sampleQueues[track].readData(formatHolder, buffer, loadingFinished, lastSeekPositionUs); + return sampleQueues.valueAt(track).readData(formatHolder, buffer, loadingFinished, + lastSeekPositionUs); } // Loader.Callback implementation. @@ -332,8 +336,9 @@ import java.util.Arrays; long loadDurationMs, boolean released) { copyLengthFromLoader(loadable); if (!released && enabledTrackCount > 0) { - for (int i = 0; i < sampleQueues.length; i++) { - sampleQueues[i].reset(trackEnabledStates[i]); + int trackCount = sampleQueues.size(); + for (int i = 0; i < trackCount; i++) { + sampleQueues.valueAt(i).reset(trackEnabledStates[i]); } callback.onContinueLoadingRequested(this); } @@ -358,11 +363,13 @@ import java.util.Arrays; @Override public TrackOutput track(int id) { - sampleQueues = Arrays.copyOf(sampleQueues, sampleQueues.length + 1); - DefaultTrackOutput sampleQueue = new DefaultTrackOutput(allocator); - sampleQueue.setUpstreamFormatChangeListener(this); - sampleQueues[sampleQueues.length - 1] = sampleQueue; - return sampleQueue; + DefaultTrackOutput trackOutput = sampleQueues.get(id); + if (trackOutput == null) { + trackOutput = new DefaultTrackOutput(allocator); + trackOutput.setUpstreamFormatChangeListener(this); + sampleQueues.put(id, trackOutput); + } + return trackOutput; } @Override @@ -390,18 +397,18 @@ import java.util.Arrays; if (released || prepared || seekMap == null || !tracksBuilt) { return; } - for (DefaultTrackOutput sampleQueue : sampleQueues) { - if (sampleQueue.getUpstreamFormat() == null) { + int trackCount = sampleQueues.size(); + for (int i = 0; i < trackCount; i++) { + if (sampleQueues.valueAt(i).getUpstreamFormat() == null) { return; } } loadCondition.close(); - int trackCount = sampleQueues.length; TrackGroup[] trackArray = new TrackGroup[trackCount]; trackEnabledStates = new boolean[trackCount]; durationUs = seekMap.getDurationUs(); for (int i = 0; i < trackCount; i++) { - trackArray[i] = new TrackGroup(sampleQueues[i].getUpstreamFormat()); + trackArray[i] = new TrackGroup(sampleQueues.valueAt(i).getUpstreamFormat()); } tracks = new TrackGroupArray(trackArray); prepared = true; @@ -455,8 +462,9 @@ import java.util.Arrays; // a new load. lastSeekPositionUs = 0; notifyReset = prepared; - for (int i = 0; i < sampleQueues.length; i++) { - sampleQueues[i].reset(!prepared || trackEnabledStates[i]); + int trackCount = sampleQueues.size(); + for (int i = 0; i < trackCount; i++) { + sampleQueues.valueAt(i).reset(!prepared || trackEnabledStates[i]); } loadable.setLoadPosition(0); } @@ -464,17 +472,19 @@ import java.util.Arrays; private int getExtractedSamplesCount() { int extractedSamplesCount = 0; - for (DefaultTrackOutput sampleQueue : sampleQueues) { - extractedSamplesCount += sampleQueue.getWriteIndex(); + int trackCount = sampleQueues.size(); + for (int i = 0; i < trackCount; i++) { + extractedSamplesCount += sampleQueues.valueAt(i).getWriteIndex(); } return extractedSamplesCount; } private long getLargestQueuedTimestampUs() { long largestQueuedTimestampUs = Long.MIN_VALUE; - for (DefaultTrackOutput sampleQueue : sampleQueues) { + int trackCount = sampleQueues.size(); + for (int i = 0; i < trackCount; i++) { largestQueuedTimestampUs = Math.max(largestQueuedTimestampUs, - sampleQueue.getLargestQueuedTimestampUs()); + sampleQueues.valueAt(i).getLargestQueuedTimestampUs()); } return largestQueuedTimestampUs; } @@ -523,7 +533,7 @@ import java.util.Arrays; @Override public void skipToKeyframeBefore(long timeUs) { - sampleQueues[track].skipToKeyframeBefore(timeUs); + sampleQueues.valueAt(track).skipToKeyframeBefore(timeUs); } } diff --git a/library/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkExtractorWrapper.java b/library/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkExtractorWrapper.java index e316215160..b9aa098b9d 100644 --- a/library/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkExtractorWrapper.java +++ b/library/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkExtractorWrapper.java @@ -59,6 +59,7 @@ public final class ChunkExtractorWrapper implements ExtractorOutput, TrackOutput // Accessed only on the loader thread. private boolean seenTrack; + private int seenTrackId; /** * @param extractor The extractor to wrap. @@ -116,8 +117,9 @@ public final class ChunkExtractorWrapper implements ExtractorOutput, TrackOutput @Override public TrackOutput track(int id) { - Assertions.checkState(!seenTrack); + Assertions.checkState(!seenTrack || seenTrackId == id); seenTrack = true; + seenTrackId = id; return this; } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeExtractorOutput.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeExtractorOutput.java index b0ab90789c..3716c6d37f 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeExtractorOutput.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeExtractorOutput.java @@ -23,7 +23,6 @@ import java.io.File; import java.io.IOException; import java.io.PrintWriter; import junit.framework.Assert; -import junit.framework.TestCase; /** * A fake {@link ExtractorOutput}. @@ -37,8 +36,6 @@ public final class FakeExtractorOutput implements ExtractorOutput, Dumper.Dumpab */ private static final boolean WRITE_DUMP = false; - private final boolean allowDuplicateTrackIds; - public final SparseArray trackOutputs; public int numberOfTracks; @@ -46,11 +43,6 @@ public final class FakeExtractorOutput implements ExtractorOutput, Dumper.Dumpab public SeekMap seekMap; public FakeExtractorOutput() { - this(false); - } - - public FakeExtractorOutput(boolean allowDuplicateTrackIds) { - this.allowDuplicateTrackIds = allowDuplicateTrackIds; trackOutputs = new SparseArray<>(); } @@ -58,11 +50,10 @@ public final class FakeExtractorOutput implements ExtractorOutput, Dumper.Dumpab public FakeTrackOutput track(int trackId) { FakeTrackOutput output = trackOutputs.get(trackId); if (output == null) { + Assert.assertFalse(tracksEnded); numberOfTracks++; output = new FakeTrackOutput(); trackOutputs.put(trackId, output); - } else { - TestCase.assertTrue("Duplicate track id: " + trackId, allowDuplicateTrackIds); } return output; } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java index 7b88062718..6f4578b694 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java @@ -267,8 +267,8 @@ public class TestUtil { */ public static FakeExtractorOutput assertOutput(Extractor extractor, String sampleFile, byte[] fileData, Instrumentation instrumentation, boolean simulateIOErrors, - boolean simulateUnknownLength, - boolean simulatePartialReads) throws IOException, InterruptedException { + boolean simulateUnknownLength, boolean simulatePartialReads) throws IOException, + InterruptedException { FakeExtractorInput input = new FakeExtractorInput.Builder().setData(fileData) .setSimulateIOErrors(simulateIOErrors) .setSimulateUnknownLength(simulateUnknownLength) From a22390c29b253694dd4db326bd5bb77c06b7e703 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 13 Oct 2016 07:35:32 -0700 Subject: [PATCH 20/24] Parse CEA-708 codec for rawCC Note that actually handling CEA-708 is not yet implemented, and so this is a no-op change from a behavior point of view. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=136038439 --- .../exoplayer2/source/dash/manifest/DashManifestParser.java | 4 +++- .../java/com/google/android/exoplayer2/util/MimeTypes.java | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/source/dash/manifest/DashManifestParser.java b/library/src/main/java/com/google/android/exoplayer2/source/dash/manifest/DashManifestParser.java index 78cb8f5c7f..b2f0ae6f98 100644 --- a/library/src/main/java/com/google/android/exoplayer2/source/dash/manifest/DashManifestParser.java +++ b/library/src/main/java/com/google/android/exoplayer2/source/dash/manifest/DashManifestParser.java @@ -655,7 +655,9 @@ public class DashManifestParser extends DefaultHandler return MimeTypes.getVideoMediaMimeType(codecs); } else if (MimeTypes.APPLICATION_RAWCC.equals(containerMimeType)) { if (codecs != null) { - if (codecs.contains("eia608") || codecs.contains("cea608")) { + if (codecs.contains("cea708")) { + return MimeTypes.APPLICATION_CEA708; + } else if (codecs.contains("eia608") || codecs.contains("cea608")) { return MimeTypes.APPLICATION_CEA608; } } diff --git a/library/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java b/library/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java index 561aba0146..4776e4d008 100644 --- a/library/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java +++ b/library/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java @@ -65,6 +65,7 @@ public final class MimeTypes { public static final String APPLICATION_WEBM = BASE_TYPE_APPLICATION + "/webm"; public static final String APPLICATION_ID3 = BASE_TYPE_APPLICATION + "/id3"; public static final String APPLICATION_CEA608 = BASE_TYPE_APPLICATION + "/cea-608"; + public static final String APPLICATION_CEA708 = BASE_TYPE_APPLICATION + "/cea-708"; public static final String APPLICATION_SUBRIP = BASE_TYPE_APPLICATION + "/x-subrip"; public static final String APPLICATION_TTML = BASE_TYPE_APPLICATION + "/ttml+xml"; public static final String APPLICATION_M3U8 = BASE_TYPE_APPLICATION + "/x-mpegURL"; From 6acf59c4fc12bfca9c77bdea3f69cf3b33d66d45 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 13 Oct 2016 08:23:39 -0700 Subject: [PATCH 21/24] Fix Widevine L3 provisioning in V2 1. HttpMediaDrmCallback.executeProvisionRequest needs to specify an empty byte[], else we do a GET instead of a POST. 2. Content-Type should not be set when making the provision request, since there's no body. 3. DataSource implementations must correctly handle a non-null body with zero length. CronetDataSource was not handling this case. DefaultHttpDataSource was, but made a code modification to make it a little clearer. OkHttpDataSource seems to handle the case correctly, and it doens't look like the code can be made clearer. Issue #1925 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=136042641 --- .../exoplayer2/ext/cronet/CronetDataSource.java | 15 ++++++++++----- .../exoplayer2/drm/HttpMediaDrmCallback.java | 5 ++--- .../upstream/DefaultHttpDataSource.java | 15 ++++++++++----- 3 files changed, 22 insertions(+), 13 deletions(-) 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 15ffe5f141..0190668a70 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 @@ -416,8 +416,10 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou executor, cronetEngine); // Set the headers. synchronized (requestProperties) { - if (dataSpec.postBody != null && !requestProperties.containsKey(CONTENT_TYPE)) { - throw new OpenException("POST request must set Content-Type", dataSpec, Status.IDLE); + if (dataSpec.postBody != null && dataSpec.postBody.length != 0 + && !requestProperties.containsKey(CONTENT_TYPE)) { + throw new OpenException("POST request with non-empty body must set Content-Type", dataSpec, + Status.IDLE); } for (Entry headerEntry : requestProperties.entrySet()) { requestBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue()); @@ -434,10 +436,13 @@ public class CronetDataSource extends UrlRequest.Callback implements HttpDataSou } requestBuilder.addHeader("Range", rangeValue.toString()); } - // Set the body. + // Set the method and (if non-empty) the body. if (dataSpec.postBody != null) { - requestBuilder.setUploadDataProvider(new ByteArrayUploadDataProvider(dataSpec.postBody), - executor); + requestBuilder.setHttpMethod("POST"); + if (dataSpec.postBody.length != 0) { + requestBuilder.setUploadDataProvider(new ByteArrayUploadDataProvider(dataSpec.postBody), + executor); + } } return requestBuilder.build(); } diff --git a/library/src/main/java/com/google/android/exoplayer2/drm/HttpMediaDrmCallback.java b/library/src/main/java/com/google/android/exoplayer2/drm/HttpMediaDrmCallback.java index 14329d47ee..65e41dd91e 100644 --- a/library/src/main/java/com/google/android/exoplayer2/drm/HttpMediaDrmCallback.java +++ b/library/src/main/java/com/google/android/exoplayer2/drm/HttpMediaDrmCallback.java @@ -71,7 +71,7 @@ public final class HttpMediaDrmCallback implements MediaDrmCallback { @Override public byte[] executeProvisionRequest(UUID uuid, ProvisionRequest request) throws IOException { String url = request.getDefaultUrl() + "&signedRequest=" + new String(request.getData()); - return executePost(url, null, null); + return executePost(url, new byte[0], null); } @Override @@ -81,6 +81,7 @@ public final class HttpMediaDrmCallback implements MediaDrmCallback { url = defaultUrl; } Map requestProperties = new HashMap<>(); + requestProperties.put("Content-Type", "application/octet-stream"); if (C.PLAYREADY_UUID.equals(uuid)) { requestProperties.putAll(PLAYREADY_KEY_REQUEST_PROPERTIES); } @@ -93,8 +94,6 @@ public final class HttpMediaDrmCallback implements MediaDrmCallback { private byte[] executePost(String url, byte[] data, Map requestProperties) throws IOException { HttpDataSource dataSource = dataSourceFactory.createDataSource(); - // Note: This will be overridden by a Content-Type in requestProperties, if one is set. - dataSource.setRequestProperty("Content-Type", "application/octet-stream"); if (requestProperties != null) { for (Map.Entry requestProperty : requestProperties.entrySet()) { dataSource.setRequestProperty(requestProperty.getKey(), requestProperty.getValue()); diff --git a/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java b/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java index 34ec76b673..b326c41b18 100644 --- a/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java +++ b/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java @@ -413,11 +413,16 @@ public class DefaultHttpDataSource implements HttpDataSource { connection.setInstanceFollowRedirects(followRedirects); connection.setDoOutput(postBody != null); if (postBody != null) { - connection.setFixedLengthStreamingMode(postBody.length); - connection.connect(); - OutputStream os = connection.getOutputStream(); - os.write(postBody); - os.close(); + connection.setRequestMethod("POST"); + if (postBody.length == 0) { + connection.connect(); + } else { + connection.setFixedLengthStreamingMode(postBody.length); + connection.connect(); + OutputStream os = connection.getOutputStream(); + os.write(postBody); + os.close(); + } } else { connection.connect(); } From dca4d16bef3526168a8c24db14ffcbd66ff9f0c4 Mon Sep 17 00:00:00 2001 From: klampert Date: Fri, 14 Oct 2016 09:15:13 -0700 Subject: [PATCH 22/24] Release surfaces created to wrap SurfaceTextures ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=136163292 --- .../android/exoplayer2/SimpleExoPlayer.java | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java b/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java index e621e19a48..4b673d3750 100644 --- a/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java +++ b/library/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java @@ -108,6 +108,7 @@ public final class SimpleExoPlayer implements ExoPlayer { private Format audioFormat; private Surface surface; + private boolean ownsSurface; private SurfaceHolder surfaceHolder; private TextureView textureView; private TextRenderer.Output textOutput; @@ -206,7 +207,7 @@ public final class SimpleExoPlayer implements ExoPlayer { */ public void setVideoSurface(Surface surface) { removeSurfaceCallbacks(); - setVideoSurfaceInternal(surface); + setVideoSurfaceInternal(surface, false); } /** @@ -219,9 +220,9 @@ public final class SimpleExoPlayer implements ExoPlayer { removeSurfaceCallbacks(); this.surfaceHolder = surfaceHolder; if (surfaceHolder == null) { - setVideoSurfaceInternal(null); + setVideoSurfaceInternal(null, false); } else { - setVideoSurfaceInternal(surfaceHolder.getSurface()); + setVideoSurfaceInternal(surfaceHolder.getSurface(), false); surfaceHolder.addCallback(componentListener); } } @@ -246,13 +247,13 @@ public final class SimpleExoPlayer implements ExoPlayer { removeSurfaceCallbacks(); this.textureView = textureView; if (textureView == null) { - setVideoSurfaceInternal(null); + setVideoSurfaceInternal(null, true); } else { if (textureView.getSurfaceTextureListener() != null) { Log.w(TAG, "Replacing existing SurfaceTextureListener."); } SurfaceTexture surfaceTexture = textureView.getSurfaceTexture(); - setVideoSurfaceInternal(surfaceTexture == null ? null : new Surface(surfaceTexture)); + setVideoSurfaceInternal(surfaceTexture == null ? null : new Surface(surfaceTexture), true); textureView.setSurfaceTextureListener(componentListener); } } @@ -468,6 +469,12 @@ public final class SimpleExoPlayer implements ExoPlayer { public void release() { player.release(); removeSurfaceCallbacks(); + if (surface != null) { + if (ownsSurface) { + surface.release(); + } + surface = null; + } } @Override @@ -618,7 +625,7 @@ public final class SimpleExoPlayer implements ExoPlayer { } } - private void setVideoSurfaceInternal(Surface surface) { + private void setVideoSurfaceInternal(Surface surface, boolean ownsSurface) { // Note: We don't turn this method into a no-op if the surface is being replaced with itself // so as to ensure onRenderedFirstFrame callbacks are still called in this case. ExoPlayerMessage[] messages = new ExoPlayerMessage[videoRendererCount]; @@ -629,12 +636,17 @@ public final class SimpleExoPlayer implements ExoPlayer { } } if (this.surface != null && this.surface != surface) { + // If we created this surface, we are responsible for releasing it. + if (this.ownsSurface) { + this.surface.release(); + } // We're replacing a surface. Block to ensure that it's not accessed after the method returns. player.blockingSendMessages(messages); } else { player.sendMessages(messages); } this.surface = surface; + this.ownsSurface = ownsSurface; } private final class ComponentListener implements VideoRendererEventListener, @@ -783,7 +795,7 @@ public final class SimpleExoPlayer implements ExoPlayer { @Override public void surfaceCreated(SurfaceHolder holder) { - setVideoSurfaceInternal(holder.getSurface()); + setVideoSurfaceInternal(holder.getSurface(), false); } @Override @@ -793,14 +805,14 @@ public final class SimpleExoPlayer implements ExoPlayer { @Override public void surfaceDestroyed(SurfaceHolder holder) { - setVideoSurfaceInternal(null); + setVideoSurfaceInternal(null, false); } // TextureView.SurfaceTextureListener implementation @Override public void onSurfaceTextureAvailable(SurfaceTexture surfaceTexture, int width, int height) { - setVideoSurfaceInternal(new Surface(surfaceTexture)); + setVideoSurfaceInternal(new Surface(surfaceTexture), true); } @Override @@ -810,7 +822,7 @@ public final class SimpleExoPlayer implements ExoPlayer { @Override public boolean onSurfaceTextureDestroyed(SurfaceTexture surfaceTexture) { - setVideoSurface(null); + setVideoSurfaceInternal(null, true); return true; } From e873b4b6abf4827b04fcf7074c7c00cd3e5073c4 Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Fri, 14 Oct 2016 11:16:16 -0700 Subject: [PATCH 23/24] Change prepare() for maybePrepare() in HlsSampleStreamWrapper This will allow asynchronous preparation. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=136176854 --- .../android/exoplayer2/source/hls/HlsMediaPeriod.java | 6 +++--- .../exoplayer2/source/hls/HlsSampleStreamWrapper.java | 9 ++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java index 57925ed67a..f4c8177f21 100644 --- a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java +++ b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java @@ -328,7 +328,7 @@ import java.util.List; sampleStreamWrappers = new HlsSampleStreamWrapper[] { buildSampleStreamWrapper(C.TRACK_TYPE_DEFAULT, baseUri, variants, null, null)}; pendingPrepareCount = 1; - sampleStreamWrappers[0].prepare(); + sampleStreamWrappers[0].continuePreparing(); return; } @@ -369,16 +369,16 @@ import java.util.List; selectedVariants.toArray(variants); HlsSampleStreamWrapper sampleStreamWrapper = buildSampleStreamWrapper(C.TRACK_TYPE_DEFAULT, baseUri, variants, masterPlaylist.muxedAudioFormat, masterPlaylist.muxedCaptionFormat); - sampleStreamWrapper.prepare(); sampleStreamWrappers[currentWrapperIndex++] = sampleStreamWrapper; + sampleStreamWrapper.continuePreparing(); } // Build audio stream wrappers. for (int i = 0; i < audioVariants.size(); i++) { HlsSampleStreamWrapper sampleStreamWrapper = buildSampleStreamWrapper(C.TRACK_TYPE_AUDIO, baseUri, new HlsMasterPlaylist.HlsUrl[] {audioVariants.get(i)}, null, null); - sampleStreamWrapper.prepare(); sampleStreamWrappers[currentWrapperIndex++] = sampleStreamWrapper; + sampleStreamWrapper.continuePreparing(); } // Build subtitle stream wrappers. diff --git a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java index 6c698d3c4d..fe756da0ef 100644 --- a/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java +++ b/library/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java @@ -144,8 +144,10 @@ import java.util.LinkedList; pendingResetPositionUs = positionUs; } - public void prepare() { - continueLoading(lastSeekPositionUs); + public void continuePreparing() { + if (!prepared) { + continueLoading(lastSeekPositionUs); + } } /** @@ -154,7 +156,8 @@ import java.util.LinkedList; */ public void prepareSingleTrack(Format format) { track(0).format(format); - endTracks(); + sampleQueuesBuilt = true; + maybeFinishPrepare(); } public void maybeThrowPrepareError() throws IOException { From cecb1f5f765bc834a307d9f65315c880bf7844cd Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 17 Oct 2016 05:03:25 -0700 Subject: [PATCH 24/24] Bump version + update release notes ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=136339035 --- RELEASENOTES.md | 27 +++++++++++++++++++ build.gradle | 2 +- demo/src/main/AndroidManifest.xml | 4 +-- .../exoplayer2/ExoPlayerLibraryInfo.java | 4 +-- playbacktests/src/main/AndroidManifest.xml | 4 +-- 5 files changed, 34 insertions(+), 7 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 150effebb1..9e0439dd12 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,5 +1,25 @@ # Release notes # +### r2.0.3 ### + +This release contains important bug fixes. Users of r2.0.0, r2.0.1 and r2.0.2 +should proactively update to this version. + +* Fixed NullPointerException in ExtractorMediaSource + ([#1914](https://github.com/google/ExoPlayer/issues/1914). +* Fixed NullPointerException in HlsMediaPeriod + ([#1907](https://github.com/google/ExoPlayer/issues/1907). +* Fixed memory leak in PlaybackControlView + ([#1908](https://github.com/google/ExoPlayer/issues/1908). +* Fixed strict mode violation when using + SimpleExoPlayer.setVideoPlayerTextureView(). +* Fixed L3 Widevine provisioning + ([#1925](https://github.com/google/ExoPlayer/issues/1925). +* Fixed hiding of controls with use_controller="false" + ([#1919](https://github.com/google/ExoPlayer/issues/1919). +* Improvements to Cronet network stack extension. +* Misc bug fixes. + ### r2.0.2 ### * Fixes for MergingMediaSource and sideloaded subtitles. @@ -88,6 +108,13 @@ some of the motivations behind ExoPlayer 2.x * Suppressed "Sending message to a Handler on a dead thread" warnings ([#426](https://github.com/google/ExoPlayer/issues/426)). +### r1.5.12 ### + +* Improvements to Cronet network stack extension. +* Fix bug in demo app introduced in r1.5.11 that caused L3 Widevine + provisioning requests to fail. +* Misc bugfixes. + ### r1.5.11 ### * Cronet network stack extension. diff --git a/build.gradle b/build.gradle index 7a76fc92c3..c50dd31b27 100644 --- a/build.gradle +++ b/build.gradle @@ -35,7 +35,7 @@ allprojects { releaseRepoName = 'exoplayer' releaseUserOrg = 'google' releaseGroupId = 'com.google.android.exoplayer' - releaseVersion = 'r2.0.2' + releaseVersion = 'r2.0.3' releaseWebsite = 'https://github.com/google/ExoPlayer' } } diff --git a/demo/src/main/AndroidManifest.xml b/demo/src/main/AndroidManifest.xml index 1bba067c70..7fc0ac3d9c 100644 --- a/demo/src/main/AndroidManifest.xml +++ b/demo/src/main/AndroidManifest.xml @@ -16,8 +16,8 @@ + android:versionCode="2003" + android:versionName="2.0.3"> diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java index 9e212ba2f6..23e6d4d593 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java @@ -23,7 +23,7 @@ public interface ExoPlayerLibraryInfo { /** * The version of the library, expressed as a string. */ - String VERSION = "2.0.2"; + String VERSION = "2.0.3"; /** * The version of the library, expressed as an integer. @@ -32,7 +32,7 @@ public interface ExoPlayerLibraryInfo { * corresponding integer version 1002003 (001-002-003), and "123.45.6" has the corresponding * integer version 123045006 (123-045-006). */ - int VERSION_INT = 2000002; + int VERSION_INT = 2000003; /** * Whether the library was compiled with {@link com.google.android.exoplayer2.util.Assertions} diff --git a/playbacktests/src/main/AndroidManifest.xml b/playbacktests/src/main/AndroidManifest.xml index cd13b96b90..58ede793b2 100644 --- a/playbacktests/src/main/AndroidManifest.xml +++ b/playbacktests/src/main/AndroidManifest.xml @@ -17,8 +17,8 @@ + android:versionCode="2003" + android:versionName="2.0.3">