diff options
author | Leon Scroggins III <scroggo@google.com> | 2013-09-10 20:26:05 -0400 |
---|---|---|
committer | Leon Scroggins III <scroggo@google.com> | 2013-09-18 12:01:20 -0400 |
commit | 7315f1baee19476363235127bc1438e2a291fa15 (patch) | |
tree | 376c68ec739e970b66165d059cf8e1e8e6d86980 /core/jni | |
parent | c255a7113a6a6b058f1b3b5b128fba1d24bbd3d9 (diff) | |
download | frameworks_base-7315f1baee19476363235127bc1438e2a291fa15.zip frameworks_base-7315f1baee19476363235127bc1438e2a291fa15.tar.gz frameworks_base-7315f1baee19476363235127bc1438e2a291fa15.tar.bz2 |
Use a native buffer for decoding images.
Fixes BUG:10725383
Depends on https://googleplex-android-review.git.corp.google.com/#/c/357300/
in external/skia.
In the previous fix for BUG:8432093 and BUG:6493544
(https://googleplex-android-review.googlesource.com/#/c/346191/),
instead of calling mark on the provided input stream, we
copied the entire stream in native code (except in one case;
more details below), allowing rewind no matter how much of
the stream had been read. This was because two decoders
may rewind after reading an arbitrary amount of the stream:
SkImageDecoder_wbmp and SkImageDecoder_libjpeg.
It turns out that the jpeg decoder does not need this rewind
after arbitrary length (it is a failure recovery case, and
libjpeg has a default recovery we can use - the above referenced
CL in Skia uses the default).
Although the wbmp decoder could read any amount given a
stream with the "right" data, and then return false, such a
stream would not be a valid stream of another format, so it
is okay for this rewind to fail.
Further, the previous fix was inefficient in the common case
where the caller decodes just the bounds, resets, then decodes
the entire image (since we have copied the entire stream twice).
The copy also resulted in the crashes seen in BUG:10725383.
In this CL, buffer only the amount of input needed by
SkImageDecoder::Factory to determine the type of image decoder
needed. Do not mark the input stream provided by the caller,
so their mark (if any) can remain in tact. The new Skia class
SkFrontBufferedStream allows buffering just the beginning
of the stream.
core/jni/android/graphics/BitmapFactory.cpp:
Instead of calling GetRewindableStream (which has been removed),
call CreateJavaInputStreamAdaptor. Then wrap it in an
SkFrontBufferedStream, with a large enough buffer to determine
which type of image is used.
core/jni/android/graphics/CreateJavaOutputStreamAdaptor.h:
core/jni/android/graphics/CreateJavaOutputStreamAdaptor.cpp:
Remove mark, markSupported, and rewind. CreateJavaInputStreamAdaptor
now turns an SkStream which is not rewindable. If the caller
needs rewind that needs to be handled differently (for example,
by using SkFrontBufferedStream, as is done in BitmapFactory and
Movie.
Remove RewindableJavaStream and GetRewindableStream.
Remove code specific to ByteArrayInputStream, which makes slow
JNI calls. Instead, depend on the caller to buffer the input
in the general case. There is no reason to special case this
stream (especially since we already have decodeByteArray).
Remove CheckForAssetStream, which is now always special cased
in Java.
core/jni/android/graphics/Movie.cpp:
Call CreateJavaInputStreamAdaptor and use an SkFrontBufferedStream.
Add a native function for decoding an Asset, and remove old
call to CheckForAssetStream.
graphics/java/android/graphics/BitmapFactory.java:
Write a helper function for decoding a stream to consolidate
common code.
Buffer enough of the input so that SkImageDecoder::Factory
can rewind after having read enough to determine the type.
Unlike the old code, do NOT mark the caller's stream. This is
handled in native code. The caller's mark (if any) is left alone.
graphics/java/android/graphics/Movie.java:
Check for an Asset stream before passing to native, and
call a native function for handling the asset directly.
BUG:6493544
BUG:8432093
BUG:10725383
Change-Id: Ide74d3606ff4bb2a8c6cdbf11bae3f96696f331a
Diffstat (limited to 'core/jni')
-rw-r--r-- | core/jni/android/graphics/BitmapFactory.cpp | 15 | ||||
-rw-r--r-- | core/jni/android/graphics/CreateJavaOutputStreamAdaptor.cpp | 170 | ||||
-rw-r--r-- | core/jni/android/graphics/CreateJavaOutputStreamAdaptor.h | 33 | ||||
-rw-r--r-- | core/jni/android/graphics/Movie.cpp | 33 |
4 files changed, 36 insertions, 215 deletions
diff --git a/core/jni/android/graphics/BitmapFactory.cpp b/core/jni/android/graphics/BitmapFactory.cpp index f9bb233..9c20de2 100644 --- a/core/jni/android/graphics/BitmapFactory.cpp +++ b/core/jni/android/graphics/BitmapFactory.cpp @@ -3,6 +3,7 @@ #include "BitmapFactory.h" #include "NinePatchPeeker.h" #include "SkData.h" +#include "SkFrontBufferedStream.h" #include "SkImageDecoder.h" #include "SkImageRef_ashmem.h" #include "SkImageRef_GlobalPool.h" @@ -120,7 +121,7 @@ static void scaleNinePatchChunk(android::Res_png_9patch* chunk, float scale) { } } -static SkPixelRef* installPixelRef(SkBitmap* bitmap, SkStream* stream, +static SkPixelRef* installPixelRef(SkBitmap* bitmap, SkStreamRewindable* stream, int sampleSize, bool ditherImage) { SkImageRef* pr; @@ -465,13 +466,17 @@ static jobject nativeDecodeStream(JNIEnv* env, jobject clazz, jobject is, jbyteA jobject padding, jobject options) { jobject bitmap = NULL; - SkAutoTUnref<SkStreamRewindable> stream(GetRewindableStream(env, is, storage)); + SkAutoTUnref<SkStream> stream(CreateJavaInputStreamAdaptor(env, is, storage)); if (stream.get()) { + // Need to buffer enough input to be able to rewind as much as might be read by a decoder + // trying to determine the stream's format. Currently the most is 64, read by + // SkImageDecoder_libwebp. + // FIXME: Get this number from SkImageDecoder + SkAutoTUnref<SkStreamRewindable> bufferedStream(SkFrontBufferedStream::Create(stream, 64)); + SkASSERT(bufferedStream.get() != NULL); // for now we don't allow purgeable with java inputstreams - // FIXME: GetRewindableStream may have made a copy, in which case - // purgeable should be allowed. - bitmap = doDecode(env, stream, padding, options, false, false); + bitmap = doDecode(env, bufferedStream, padding, options, false, false); } return bitmap; } diff --git a/core/jni/android/graphics/CreateJavaOutputStreamAdaptor.cpp b/core/jni/android/graphics/CreateJavaOutputStreamAdaptor.cpp index f773f59..da8f083 100644 --- a/core/jni/android/graphics/CreateJavaOutputStreamAdaptor.cpp +++ b/core/jni/android/graphics/CreateJavaOutputStreamAdaptor.cpp @@ -5,18 +5,12 @@ #include "SkStream.h" #include "SkTypes.h" #include "Utils.h" -#include <androidfw/Asset.h> -static jmethodID gInputStream_resetMethodID; -static jmethodID gInputStream_markMethodID; -static jmethodID gInputStream_markSupportedMethodID; static jmethodID gInputStream_readMethodID; static jmethodID gInputStream_skipMethodID; -class RewindableJavaStream; - /** - * Non-rewindable wrapper for a Java InputStream. + * Wrapper for a Java InputStream. */ class JavaInputStreamAdaptor : public SkStream { public: @@ -64,25 +58,6 @@ public: } private: - // Does not override rewind, since a JavaInputStreamAdaptor's interface - // does not support rewinding. RewindableJavaStream, which is a friend, - // will be able to call this method to rewind. - bool doRewind() { - JNIEnv* env = fEnv; - - fBytesRead = 0; - fIsAtEnd = false; - - env->CallVoidMethod(fJavaInputStream, gInputStream_resetMethodID); - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - env->ExceptionClear(); - SkDebugf("------- reset threw an exception\n"); - return false; - } - return true; - } - size_t doRead(void* buffer, size_t size) { JNIEnv* env = fEnv; size_t bytesRead = 0; @@ -148,9 +123,6 @@ private: size_t fCapacity; size_t fBytesRead; bool fIsAtEnd; - - // Allows access to doRewind and fBytesRead. - friend class RewindableJavaStream; }; SkStream* CreateJavaInputStreamAdaptor(JNIEnv* env, jobject stream, @@ -190,123 +162,6 @@ SkStreamRewindable* CopyJavaInputStream(JNIEnv* env, jobject stream, return adaptor_to_mem_stream(adaptor.get()); } -/** - * Wrapper for a Java InputStream which is rewindable and - * has a length. - */ -class RewindableJavaStream : public SkStreamRewindable { -public: - // RewindableJavaStream takes ownership of adaptor. - RewindableJavaStream(JavaInputStreamAdaptor* adaptor, size_t length) - : fAdaptor(adaptor) - , fLength(length) { - SkASSERT(fAdaptor != NULL); - } - - virtual ~RewindableJavaStream() { - fAdaptor->unref(); - } - - virtual bool rewind() { - return fAdaptor->doRewind(); - } - - virtual size_t read(void* buffer, size_t size) { - return fAdaptor->read(buffer, size); - } - - virtual bool isAtEnd() const { - return fAdaptor->isAtEnd(); - } - - virtual size_t getLength() const { - return fLength; - } - - virtual bool hasLength() const { - return true; - } - - virtual SkStreamRewindable* duplicate() const { - // Duplicating this stream requires rewinding and - // reading, which modify this Stream (and could - // fail, leaving this one invalid). - SkASSERT(false); - return NULL; - } - -private: - JavaInputStreamAdaptor* fAdaptor; - const size_t fLength; -}; - -static jclass gByteArrayInputStream_Clazz; -static jfieldID gCountField; -static jfieldID gPosField; - -/** - * If jstream is a ByteArrayInputStream, return its remaining length. Otherwise - * return 0. - */ -static size_t get_length_from_byte_array_stream(JNIEnv* env, jobject jstream) { - if (env->IsInstanceOf(jstream, gByteArrayInputStream_Clazz)) { - // Return the remaining length, to keep the same behavior of using the rest of the - // stream. - return env->GetIntField(jstream, gCountField) - env->GetIntField(jstream, gPosField); - } - return 0; -} - -/** - * If jstream is a class that has a length, return it. Otherwise - * return 0. - * Only checks for a set of subclasses. - */ -static size_t get_length_if_supported(JNIEnv* env, jobject jstream) { - size_t len = get_length_from_byte_array_stream(env, jstream); - if (len > 0) { - return len; - } - return 0; -} - -SkStreamRewindable* GetRewindableStream(JNIEnv* env, jobject stream, - jbyteArray storage) { - SkAutoTUnref<SkStream> adaptor(CreateJavaInputStreamAdaptor(env, stream, storage)); - if (NULL == adaptor.get()) { - return NULL; - } - - const size_t length = get_length_if_supported(env, stream); - if (length > 0 && env->CallBooleanMethod(stream, gInputStream_markSupportedMethodID)) { - // Set the readLimit for mark to the end of the stream, so it can - // be rewound regardless of how much has been read. - env->CallVoidMethod(stream, gInputStream_markMethodID, length); - // RewindableJavaStream will unref adaptor when it is destroyed. - return new RewindableJavaStream(static_cast<JavaInputStreamAdaptor*>(adaptor.detach()), - length); - } - - return adaptor_to_mem_stream(adaptor.get()); -} - -static jclass gAssetInputStream_Clazz; -static jmethodID gGetAssetIntMethodID; - -android::AssetStreamAdaptor* CheckForAssetStream(JNIEnv* env, jobject jstream) { - if (!env->IsInstanceOf(jstream, gAssetInputStream_Clazz)) { - return NULL; - } - - jint jasset = env->CallIntMethod(jstream, gGetAssetIntMethodID); - android::Asset* a = reinterpret_cast<android::Asset*>(jasset); - if (NULL == a) { - jniThrowNullPointerException(env, "NULL native asset"); - return NULL; - } - return new android::AssetStreamAdaptor(a); -} - /////////////////////////////////////////////////////////////////////////////// static jmethodID gOutputStream_writeMethodID; @@ -382,13 +237,6 @@ static jclass findClassCheck(JNIEnv* env, const char classname[]) { return clazz; } -static jfieldID getFieldIDCheck(JNIEnv* env, jclass clazz, - const char fieldname[], const char type[]) { - jfieldID id = env->GetFieldID(clazz, fieldname, type); - SkASSERT(!env->ExceptionCheck()); - return id; -} - static jmethodID getMethodIDCheck(JNIEnv* env, jclass clazz, const char methodname[], const char type[]) { jmethodID id = env->GetMethodID(clazz, methodname, type); @@ -398,25 +246,9 @@ static jmethodID getMethodIDCheck(JNIEnv* env, jclass clazz, int register_android_graphics_CreateJavaOutputStreamAdaptor(JNIEnv* env) { jclass inputStream_Clazz = findClassCheck(env, "java/io/InputStream"); - gInputStream_resetMethodID = getMethodIDCheck(env, inputStream_Clazz, "reset", "()V"); - gInputStream_markMethodID = getMethodIDCheck(env, inputStream_Clazz, "mark", "(I)V"); - gInputStream_markSupportedMethodID = getMethodIDCheck(env, inputStream_Clazz, "markSupported", "()Z"); gInputStream_readMethodID = getMethodIDCheck(env, inputStream_Clazz, "read", "([BII)I"); gInputStream_skipMethodID = getMethodIDCheck(env, inputStream_Clazz, "skip", "(J)J"); - gByteArrayInputStream_Clazz = findClassCheck(env, "java/io/ByteArrayInputStream"); - // Ref gByteArrayInputStream_Clazz so we can continue to refer to it when - // calling IsInstance. - gByteArrayInputStream_Clazz = (jclass) env->NewGlobalRef(gByteArrayInputStream_Clazz); - gCountField = getFieldIDCheck(env, gByteArrayInputStream_Clazz, "count", "I"); - gPosField = getFieldIDCheck(env, gByteArrayInputStream_Clazz, "pos", "I"); - - gAssetInputStream_Clazz = findClassCheck(env, "android/content/res/AssetManager$AssetInputStream"); - // Ref gAssetInputStream_Clazz so we can continue to refer to it when - // calling IsInstance. - gAssetInputStream_Clazz = (jclass) env->NewGlobalRef(gAssetInputStream_Clazz); - gGetAssetIntMethodID = getMethodIDCheck(env, gAssetInputStream_Clazz, "getAssetInt", "()I"); - jclass outputStream_Clazz = findClassCheck(env, "java/io/OutputStream"); gOutputStream_writeMethodID = getMethodIDCheck(env, outputStream_Clazz, "write", "([BII)V"); gOutputStream_flushMethodID = getMethodIDCheck(env, outputStream_Clazz, "flush", "()V"); diff --git a/core/jni/android/graphics/CreateJavaOutputStreamAdaptor.h b/core/jni/android/graphics/CreateJavaOutputStreamAdaptor.h index fcc0c9a..ecd270f 100644 --- a/core/jni/android/graphics/CreateJavaOutputStreamAdaptor.h +++ b/core/jni/android/graphics/CreateJavaOutputStreamAdaptor.h @@ -4,10 +4,6 @@ //#include <android_runtime/AndroidRuntime.h> #include "jni.h" -namespace android { - class AssetStreamAdaptor; -} - class SkMemoryStream; class SkStream; class SkStreamRewindable; @@ -15,6 +11,7 @@ class SkWStream; /** * Return an adaptor from a Java InputStream to an SkStream. + * Does not support rewind. * @param env JNIEnv object. * @param stream Pointer to Java InputStream. * @param storage Java byte array for retrieving data from the @@ -28,7 +25,7 @@ SkStream* CreateJavaInputStreamAdaptor(JNIEnv* env, jobject stream, jbyteArray storage); /** - * Copy a Java InputStream. + * Copy a Java InputStream. The result will be rewindable. * @param env JNIEnv object. * @param stream Pointer to Java InputStream. * @param storage Java byte array for retrieving data from the @@ -39,32 +36,6 @@ SkStream* CreateJavaInputStreamAdaptor(JNIEnv* env, jobject stream, SkStreamRewindable* CopyJavaInputStream(JNIEnv* env, jobject stream, jbyteArray storage); -/** - * Get a rewindable stream from a Java InputStream. - * @param env JNIEnv object. - * @param stream Pointer to Java InputStream. - * @param storage Java byte array for retrieving data from the - * Java InputStream. - * @return SkStreamRewindable Either a wrapper around the Java - * InputStream, if possible, or a copy which is rewindable. - * Since it may be a wrapper, must not be used after the - * caller returns, like the result of CreateJavaInputStreamAdaptor. - */ -SkStreamRewindable* GetRewindableStream(JNIEnv* env, jobject stream, - jbyteArray storage); - -/** - * If the Java InputStream is an AssetInputStream, return an adaptor. - * This should not be used after the calling function returns, since - * the caller may close the asset. Returns NULL if the stream is - * not an AssetInputStream. - * @param env JNIEnv object. - * @param stream Pointer to Java InputStream. - * @return AssetStreamAdaptor representing the InputStream, or NULL. - * Must not be held onto. - */ -android::AssetStreamAdaptor* CheckForAssetStream(JNIEnv* env, jobject stream); - SkWStream* CreateJavaOutputStreamAdaptor(JNIEnv* env, jobject stream, jbyteArray storage); #endif diff --git a/core/jni/android/graphics/Movie.cpp b/core/jni/android/graphics/Movie.cpp index 2eae841..feb2dec 100644 --- a/core/jni/android/graphics/Movie.cpp +++ b/core/jni/android/graphics/Movie.cpp @@ -1,4 +1,5 @@ #include "ScopedLocalRef.h" +#include "SkFrontBufferedStream.h" #include "SkMovie.h" #include "SkStream.h" #include "GraphicsJNI.h" @@ -81,23 +82,33 @@ static void movie_draw(JNIEnv* env, jobject movie, jobject canvas, c->drawBitmap(b, sx, sy, p); } +static jobject movie_decodeAsset(JNIEnv* env, jobject clazz, jint native_asset) { + android::Asset* asset = reinterpret_cast<android::Asset*>(native_asset); + if (asset == NULL) return NULL; + SkAutoTUnref<SkStreamRewindable> stream (new android::AssetStreamAdaptor(asset)); + SkMovie* moov = SkMovie::DecodeStream(stream.get()); + return create_jmovie(env, moov); +} + static jobject movie_decodeStream(JNIEnv* env, jobject clazz, jobject istream) { NPE_CHECK_RETURN_ZERO(env, istream); - SkStreamRewindable* strm = CheckForAssetStream(env, istream); - jbyteArray byteArray = NULL; - ScopedLocalRef<jbyteArray> scoper(env, NULL); - if (NULL == strm) { - byteArray = env->NewByteArray(16*1024); - scoper.reset(byteArray); - strm = GetRewindableStream(env, istream, byteArray); - } + jbyteArray byteArray = env->NewByteArray(16*1024); + ScopedLocalRef<jbyteArray> scoper(env, byteArray); + SkStream* strm = CreateJavaInputStreamAdaptor(env, istream, byteArray); if (NULL == strm) { return 0; } - SkMovie* moov = SkMovie::DecodeStream(strm); + // Need to buffer enough input to be able to rewind as much as might be read by a decoder + // trying to determine the stream's format. The only decoder for movies is GIF, which + // will only read 6. + // FIXME: Get this number from SkImageDecoder + SkAutoTUnref<SkStreamRewindable> bufferedStream(SkFrontBufferedStream::Create(strm, 6)); + SkASSERT(bufferedStream.get() != NULL); + + SkMovie* moov = SkMovie::DecodeStream(bufferedStream); strm->unref(); return create_jmovie(env, moov); } @@ -135,7 +146,9 @@ static JNINativeMethod gMethods[] = { { "setTime", "(I)Z", (void*)movie_setTime }, { "draw", "(Landroid/graphics/Canvas;FFLandroid/graphics/Paint;)V", (void*)movie_draw }, - { "decodeStream", "(Ljava/io/InputStream;)Landroid/graphics/Movie;", + { "nativeDecodeAsset", "(I)Landroid/graphics/Movie;", + (void*)movie_decodeAsset }, + { "nativeDecodeStream", "(Ljava/io/InputStream;)Landroid/graphics/Movie;", (void*)movie_decodeStream }, { "nativeDestructor","(I)V", (void*)movie_destructor }, { "decodeByteArray", "([BII)Landroid/graphics/Movie;", |