From bc10185fa1669a151568feb72277785d323fc344 Mon Sep 17 00:00:00 2001 From: Chong Zhang Date: Thu, 14 May 2015 10:08:08 -0700 Subject: [PATCH] MediaDataSource: address API council comments - throw IOExecption on fatal errors - add offset argument to readAt - and fix a crash in MediaExtractor bug: 21045118 bug: 21163225 Change-Id: I3c0ff42e539868b9374a4f1f3a9852143f68ba68 --- api/current.txt | 7 ++++--- api/system-current.txt | 7 ++++--- media/java/android/media/MediaDataSource.java | 21 +++++++++---------- media/jni/android_media_MediaDataSource.cpp | 15 ++++++++----- media/jni/android_media_MediaExtractor.cpp | 5 +++++ 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/api/current.txt b/api/current.txt index ac623acd11465..39e5e34931dea 100644 --- a/api/current.txt +++ b/api/current.txt @@ -15701,9 +15701,10 @@ package android.media { ctor public MediaCryptoException(java.lang.String); } - public abstract interface MediaDataSource implements java.io.Closeable { - method public abstract long getSize(); - method public abstract int readAt(long, byte[], int); + public abstract class MediaDataSource implements java.io.Closeable { + ctor public MediaDataSource(); + method public abstract long getSize() throws java.io.IOException; + method public abstract int readAt(long, byte[], int, int) throws java.io.IOException; } public class MediaDescription implements android.os.Parcelable { diff --git a/api/system-current.txt b/api/system-current.txt index 8ff6f6b6bf3d3..3c7a755879bf0 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -16923,9 +16923,10 @@ package android.media { ctor public MediaCryptoException(java.lang.String); } - public abstract interface MediaDataSource implements java.io.Closeable { - method public abstract long getSize(); - method public abstract int readAt(long, byte[], int); + public abstract class MediaDataSource implements java.io.Closeable { + ctor public MediaDataSource(); + method public abstract long getSize() throws java.io.IOException; + method public abstract int readAt(long, byte[], int, int) throws java.io.IOException; } public class MediaDescription implements android.os.Parcelable { diff --git a/media/java/android/media/MediaDataSource.java b/media/java/android/media/MediaDataSource.java index 246c0ef0a3d04..948da0b97910b 100644 --- a/media/java/android/media/MediaDataSource.java +++ b/media/java/android/media/MediaDataSource.java @@ -18,6 +18,7 @@ package android.media; import java.io.Closeable; +import java.io.IOException; /** * For supplying media data to the framework. Implement this if your app has @@ -29,34 +30,32 @@ import java.io.Closeable; * you don't need to do your own synchronization unless you're modifying the * MediaDataSource from another thread while it's being used by the framework.

*/ -public interface MediaDataSource extends Closeable { +public abstract class MediaDataSource implements Closeable { /** * Called to request data from the given position. * * Implementations should should write up to {@code size} bytes into * {@code buffer}, and return the number of bytes written. * - * Return {@code 0} to indicate that {@code position} is at, or beyond, the - * end of the source. + * Return {@code 0} if size is zero (thus no bytes are read). * - * Return {@code -1} to indicate that a fatal error occurred. The failed - * read will not be retried, so transient errors should be handled - * internally. - * - * Throwing an exception from this method will have the same effect as - * returning {@code -1}. + * Return {@code -1} to indicate that end of stream is reached. * * @param position the position in the data source to read from. * @param buffer the buffer to read the data into. + * @param offset the offset within buffer to read the data into. * @param size the number of bytes to read. + * @throws IOException on fatal errors. * @return the number of bytes read, or -1 if there was an error. */ - public int readAt(long position, byte[] buffer, int size); + public abstract int readAt(long position, byte[] buffer, int offset, int size) + throws IOException; /** * Called to get the size of the data source. * + * @throws IOException on fatal errors * @return the size of data source in bytes, or -1 if the size is unknown. */ - public long getSize(); + public abstract long getSize() throws IOException; } diff --git a/media/jni/android_media_MediaDataSource.cpp b/media/jni/android_media_MediaDataSource.cpp index 1e6d2afa84b84..025133f3ff009 100644 --- a/media/jni/android_media_MediaDataSource.cpp +++ b/media/jni/android_media_MediaDataSource.cpp @@ -39,7 +39,7 @@ JMediaDataSource::JMediaDataSource(JNIEnv* env, jobject source) ScopedLocalRef mediaDataSourceClass(env, env->GetObjectClass(mMediaDataSourceObj)); CHECK(mediaDataSourceClass.get() != NULL); - mReadMethod = env->GetMethodID(mediaDataSourceClass.get(), "readAt", "(J[BI)I"); + mReadMethod = env->GetMethodID(mediaDataSourceClass.get(), "readAt", "(J[BII)I"); CHECK(mReadMethod != NULL); mGetSizeMethod = env->GetMethodID(mediaDataSourceClass.get(), "getSize", "()J"); CHECK(mGetSizeMethod != NULL); @@ -80,7 +80,7 @@ ssize_t JMediaDataSource::readAt(off64_t offset, size_t size) { JNIEnv* env = AndroidRuntime::getJNIEnv(); jint numread = env->CallIntMethod(mMediaDataSourceObj, mReadMethod, - (jlong)offset, mByteArrayObj, (jint)size); + (jlong)offset, mByteArrayObj, (jint)0, (jint)size); if (env->ExceptionCheck()) { ALOGW("An exception occurred in readAt()"); LOGW_EX(env); @@ -89,9 +89,14 @@ ssize_t JMediaDataSource::readAt(off64_t offset, size_t size) { return -1; } if (numread < 0) { - ALOGW("An error occurred in readAt()"); - mJavaObjStatus = UNKNOWN_ERROR; - return -1; + if (numread != -1) { + ALOGW("An error occurred in readAt()"); + mJavaObjStatus = UNKNOWN_ERROR; + return -1; + } else { + // numread == -1 indicates EOF + return 0; + } } if ((size_t)numread > size) { ALOGE("readAt read too many bytes."); diff --git a/media/jni/android_media_MediaExtractor.cpp b/media/jni/android_media_MediaExtractor.cpp index b6b7a80e6a3f7..4e9b72685223a 100644 --- a/media/jni/android_media_MediaExtractor.cpp +++ b/media/jni/android_media_MediaExtractor.cpp @@ -715,6 +715,11 @@ static void android_media_MediaExtractor_setDataSourceCallback( status_t err = extractor->setDataSource(bridge); if (err != OK) { + // Clear bridge so that JMediaDataSource::close() is called _before_ + // we throw the IOException. + // Otherwise close() gets called when we go out of scope, it calls + // Java with a pending exception and crashes the process. + bridge.clear(); jniThrowException( env, "java/io/IOException",