From 2fd045c092b08772be81a6d58bbb95e5e608403f Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Tue, 14 Jan 2014 15:14:57 -0500 Subject: [PATCH] DO NOT MERGE - merge nativeDecodeFileDescriptor fixes into KLP-MR2 BUG:13191516 Original CLs below: In nativeDecodeFileDescriptor, use fdopen. Instead of attempting to mmap the entire file, create an SkFILEStream from a FILE. BUG:11669944 BUG:11028218 Cherry-pick from: If67da91484acc79f9f3dde6d05201409c0c75e41 ------------------------------------------------------------------------- Decode file descriptor from the correct offset. Fix a few bugs in nativeDecodeFileDescriptor: 1. Restore the FD's offset when exiting the function. 2. Copy the data when potentially using an SkImageRef. The old behavior would have continued to modify the file descriptor's offset each time a new decode was required. The copy ensures that the file descriptor remains unchanged. 3. Buffer the file stream. Prior to this change, if the image was not a PNG, the stream would be rewound to the beginning of the file, even if the file descriptor was passed in with an offset. Thanks to the buffer, the stream is only rewound to the original offset. Depends on https://googleplex-android-review.googlesource.com/#/c/415821/1 in external/skia BUG:12807677 BUG:12895876 Cherry-pick from: I38b8cf5d210dbbc0107e6562e3884867de57fc4b --- core/jni/android/graphics/BitmapFactory.cpp | 69 ++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/core/jni/android/graphics/BitmapFactory.cpp b/core/jni/android/graphics/BitmapFactory.cpp index e66141baf317..e8feacb952a0 100644 --- a/core/jni/android/graphics/BitmapFactory.cpp +++ b/core/jni/android/graphics/BitmapFactory.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -486,6 +487,12 @@ static jobject doDecode(JNIEnv* env, SkStreamRewindable* stream, jobject padding bitmapCreateFlags, ninePatchChunk, layoutBounds, -1); } +// 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 +#define BYTES_TO_BUFFER 64 + static jobject nativeDecodeStream(JNIEnv* env, jobject clazz, jobject is, jbyteArray storage, jobject padding, jobject options) { @@ -493,11 +500,8 @@ static jobject nativeDecodeStream(JNIEnv* env, jobject clazz, jobject is, jbyteA SkAutoTUnref 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 bufferedStream(SkFrontBufferedStream::Create(stream, 64)); + SkAutoTUnref bufferedStream( + SkFrontBufferedStream::Create(stream, BYTES_TO_BUFFER)); SkASSERT(bufferedStream.get() != NULL); // for now we don't allow purgeable with java inputstreams bitmap = doDecode(env, bufferedStream, padding, options, false, false); @@ -518,27 +522,48 @@ static jobject nativeDecodeFileDescriptor(JNIEnv* env, jobject clazz, jobject fi return nullObjectReturn("fstat return -1"); } - bool isPurgeable = optionsPurgeable(env, bitmapFactoryOptions); - bool isShareable = optionsShareable(env, bitmapFactoryOptions); - bool weOwnTheFD = false; - if (isPurgeable && isShareable) { - int newFD = ::dup(descriptor); - if (-1 != newFD) { - weOwnTheFD = true; - descriptor = newFD; - } + // Restore the descriptor's offset on exiting this function. + AutoFDSeek autoRestore(descriptor); + + FILE* file = fdopen(descriptor, "r"); + if (file == NULL) { + return nullObjectReturn("Could not open file"); } - SkAutoTUnref data(SkData::NewFromFD(descriptor)); - if (data.get() == NULL) { - return nullObjectReturn("NewFromFD failed in nativeDecodeFileDescriptor"); + SkAutoTUnref fileStream(new SkFILEStream(file, + SkFILEStream::kCallerRetains_Ownership)); + + SkAutoTUnref stream; + + // Retain the old behavior of allowing purgeable if both purgeable and + // shareable are set to true. + bool isPurgeable = optionsPurgeable(env, bitmapFactoryOptions) + && optionsShareable(env, bitmapFactoryOptions); + if (isPurgeable) { + // Copy the stream, so the image can be decoded multiple times without + // continuing to modify the original file descriptor. + // Copy beginning from the current position. + const size_t fileSize = fileStream->getLength() - fileStream->getPosition(); + void* buffer = sk_malloc_flags(fileSize, 0); + if (buffer == NULL) { + return nullObjectReturn("Could not make a copy for ashmem"); + } + + SkAutoTUnref data(SkData::NewFromMalloc(buffer, fileSize)); + + if (fileStream->read(buffer, fileSize) != fileSize) { + return nullObjectReturn("Could not read the file."); + } + + stream.reset(new SkMemoryStream(data)); + } else { + // Use a buffered stream. Although an SkFILEStream can be rewound, this + // ensures that SkImageDecoder::Factory never rewinds beyond the + // current position of the file descriptor. + stream.reset(SkFrontBufferedStream::Create(fileStream, BYTES_TO_BUFFER)); } - SkAutoTUnref stream(new SkMemoryStream(data)); - /* Allow purgeable iff we own the FD, i.e., in the puregeable and - shareable case. - */ - return doDecode(env, stream, padding, bitmapFactoryOptions, weOwnTheFD); + return doDecode(env, stream, padding, bitmapFactoryOptions, isPurgeable); } static jobject nativeDecodeAsset(JNIEnv* env, jobject clazz, jlong native_asset, -- 2.11.0