Skip to content

Commit

Permalink
Avoid wrapping byte[] and ByteBuffer in streams.
Browse files Browse the repository at this point in the history
The streams use a buffered stream where the limited size buffer can
cause failures if parsing metadata requries reading past the buffer
size.

This is unnecessary for these two types because both already have all
data in memory or otherwise handle buffering internally. Avoiding the
buffer also saves some memory overhead.
  • Loading branch information
sjudd committed Mar 16, 2021
1 parent a295828 commit 042f6b5
Show file tree
Hide file tree
Showing 8 changed files with 166 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package com.bumptech.glide;

import static com.google.common.truth.Truth.assertThat;

import android.content.ContentResolver;
import android.content.Context;
import android.content.res.Resources;
import android.graphics.Bitmap;
import android.net.Uri;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.bumptech.glide.load.model.UnitModelLoader;
import com.bumptech.glide.test.ConcurrencyHelper;
import com.bumptech.glide.test.ResourceIds;
import com.bumptech.glide.test.TearDownGlide;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.util.Objects;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;
import org.junit.runner.RunWith;

@RunWith(AndroidJUnit4.class)
public class LargeImageTest {
@Rule public final TestRule tearDownGlide = new TearDownGlide();
private final ConcurrencyHelper concurrency = new ConcurrencyHelper();
private final Context context = ApplicationProvider.getApplicationContext();

@Test
public void loadLargeJpeg_asByteArray_succeeds() throws IOException {
byte[] data = getLargeImageBytes();
Bitmap bitmap = concurrency.get(Glide.with(context).asBitmap().load(data).submit());
assertThat(bitmap).isNotNull();
}

@Test
public void loadLargeJpeg_asByteBuffer_succeeds() throws IOException {
// Using UnitModelLoader lets us mimic loading the ByteBuffer from some other data type, which
// reduces the scope of our test.
Glide.get(context)
.getRegistry()
.append(
ByteBuffer.class, ByteBuffer.class, UnitModelLoader.Factory.<ByteBuffer>getInstance());

ByteBuffer buffer = ByteBuffer.wrap(getLargeImageBytes());
Bitmap bitmap = concurrency.get(Glide.with(context).asBitmap().load(buffer).submit());
assertThat(bitmap).isNotNull();
}

private byte[] getLargeImageBytes() throws IOException {
Resources resources = context.getResources();
int resourceId = ResourceIds.raw.canonical_large;
Uri uri =
new Uri.Builder()
.scheme(ContentResolver.SCHEME_ANDROID_RESOURCE)
.authority(resources.getResourcePackageName(resourceId))
.appendPath(resources.getResourceTypeName(resourceId))
.appendPath(resources.getResourceEntryName(resourceId))
.build();

InputStream is = Objects.requireNonNull(context.getContentResolver().openInputStream(uri));
ByteArrayOutputStream os = new ByteArrayOutputStream();
byte[] buffer = new byte[1024 * 1024 * 5];
int read;
while ((read = is.read(buffer, 0, buffer.length)) != -1) {
os.write(buffer, 0, read);
}
return os.toByteArray();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ private ResourceIds() {
public interface raw {
int dl_world_anim = getResourceId("raw", "dl_world_anim");
int canonical = getResourceId("raw", "canonical");
int canonical_large = getResourceId("raw", "canonical_large");
int canonical_png = getResourceId("raw", "canonical_png");
int canonical_transparent_png = getResourceId("raw", "canonical_transparent_png");
int interlaced_transparent_gif = getResourceId("raw", "interlaced_transparent_gif");
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,32 @@ private static ImageType getTypeInternal(
return ImageType.UNKNOWN;
}

/**
* Returns the result from the first of {@code parsers} that returns something other than {@link
* ImageHeaderParser#UNKNOWN_ORIENTATION}.
*
* <p>If {@code buffer} is null, the parers list is empty, or none of the parsers returns a valid
* value, {@link ImageHeaderParser#UNKNOWN_ORIENTATION} is returned.
*/
public static int getOrientation(
@NonNull List<ImageHeaderParser> parsers,
@Nullable final ByteBuffer buffer,
@NonNull final ArrayPool arrayPool)
throws IOException {
if (buffer == null) {
return ImageHeaderParser.UNKNOWN_ORIENTATION;
}

return getOrientationInternal(
parsers,
new OrientationReader() {
@Override
public int getOrientation(ImageHeaderParser parser) throws IOException {
return parser.getOrientation(buffer, arrayPool);
}
});
}

/** Returns the orientation for the given InputStream. */
public static int getOrientation(
@NonNull List<ImageHeaderParser> parsers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
import com.bumptech.glide.load.Options;
import com.bumptech.glide.load.ResourceDecoder;
import com.bumptech.glide.load.engine.Resource;
import com.bumptech.glide.util.ByteBufferUtil;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;

/** Decodes {@link android.graphics.Bitmap Bitmaps} from {@link java.nio.ByteBuffer ByteBuffers}. */
Expand All @@ -27,7 +25,6 @@ public boolean handles(@NonNull ByteBuffer source, @NonNull Options options) {
public Resource<Bitmap> decode(
@NonNull ByteBuffer source, int width, int height, @NonNull Options options)
throws IOException {
InputStream is = ByteBufferUtil.toStream(source);
return downsampler.decode(is, width, height, options);
return downsampler.decode(source, width, height, options);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,21 @@ public Resource<Bitmap> decode(InputStream is, int outWidth, int outHeight, Opti
return decode(is, outWidth, outHeight, options, EMPTY_CALLBACKS);
}

/**
* Identical to {@link #decode(InputStream, int, int, Options)}, except that it accepts a {@link
* ByteBuffer} in place of an {@link InputStream}.
*/
public Resource<Bitmap> decode(
ByteBuffer buffer, int requestedWidth, int requestedHeight, Options options)
throws IOException {
return decode(
new ImageReader.ByteBufferReader(buffer, parsers, byteArrayPool),
requestedWidth,
requestedHeight,
options,
EMPTY_CALLBACKS);
}

/**
* Returns a Bitmap decoded from the given {@link InputStream} that is rotated to match any EXIF
* data present in the stream and that is downsampled according to the given dimensions and any
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,23 @@

import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.graphics.BitmapFactory.Options;
import android.os.Build;
import android.os.ParcelFileDescriptor;
import androidx.annotation.Nullable;
import androidx.annotation.RequiresApi;
import com.bumptech.glide.load.ImageHeaderParser;
import com.bumptech.glide.load.ImageHeaderParser.ImageType;
import com.bumptech.glide.load.ImageHeaderParserUtils;
import com.bumptech.glide.load.data.DataRewinder;
import com.bumptech.glide.load.data.InputStreamRewinder;
import com.bumptech.glide.load.data.ParcelFileDescriptorRewinder;
import com.bumptech.glide.load.engine.bitmap_recycle.ArrayPool;
import com.bumptech.glide.util.ByteBufferUtil;
import com.bumptech.glide.util.Preconditions;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.util.List;

/**
Expand All @@ -31,6 +35,43 @@ interface ImageReader {

void stopGrowingBuffers();

final class ByteBufferReader implements ImageReader {

private final ByteBuffer buffer;
private final List<ImageHeaderParser> parsers;
private final ArrayPool byteArrayPool;

ByteBufferReader(ByteBuffer buffer, List<ImageHeaderParser> parsers, ArrayPool byteArrayPool) {
this.buffer = buffer;
this.parsers = parsers;
this.byteArrayPool = byteArrayPool;
}

@Nullable
@Override
public Bitmap decodeBitmap(Options options) {
return BitmapFactory.decodeStream(stream(), /* outPadding= */ null, options);
}

@Override
public ImageType getImageType() throws IOException {
return ImageHeaderParserUtils.getType(parsers, ByteBufferUtil.rewind(buffer));
}

@Override
public int getImageOrientation() throws IOException {
return ImageHeaderParserUtils.getOrientation(
parsers, ByteBufferUtil.rewind(buffer), byteArrayPool);
}

@Override
public void stopGrowingBuffers() {}

private InputStream stream() {
return ByteBufferUtil.toStream(ByteBufferUtil.rewind(buffer));
}
}

final class InputStreamImageReader implements ImageReader {
private final InputStreamRewinder dataRewinder;
private final ArrayPool byteArrayPool;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static ByteBuffer fromFile(@NonNull File file) throws IOException {
}

public static void toFile(@NonNull ByteBuffer buffer, @NonNull File file) throws IOException {
buffer.position(0);
rewind(buffer);
RandomAccessFile raf = null;
FileChannel channel = null;
try {
Expand Down Expand Up @@ -120,7 +120,7 @@ public static byte[] toBytes(@NonNull ByteBuffer byteBuffer) {
} else {
ByteBuffer toCopy = byteBuffer.asReadOnlyBuffer();
result = new byte[toCopy.limit()];
toCopy.position(0);
rewind(toCopy);
toCopy.get(result);
}
return result;
Expand Down Expand Up @@ -150,7 +150,11 @@ public static ByteBuffer fromStream(@NonNull InputStream stream) throws IOExcept
byte[] bytes = outStream.toByteArray();

// Some resource decoders require a direct byte buffer. Prefer allocateDirect() over wrap()
return (ByteBuffer) ByteBuffer.allocateDirect(bytes.length).put(bytes).position(0);
return rewind(ByteBuffer.allocateDirect(bytes.length).put(bytes));
}

public static ByteBuffer rewind(ByteBuffer buffer) {
return (ByteBuffer) buffer.position(0);
}

@Nullable
Expand Down Expand Up @@ -208,7 +212,7 @@ public boolean markSupported() {
}

@Override
public int read(@NonNull byte[] buffer, int byteOffset, int byteCount) throws IOException {
public int read(@NonNull byte[] buffer, int byteOffset, int byteCount) {
if (!byteBuffer.hasRemaining()) {
return -1;
}
Expand All @@ -227,7 +231,7 @@ public synchronized void reset() throws IOException {
}

@Override
public long skip(long byteCount) throws IOException {
public long skip(long byteCount) {
if (!byteBuffer.hasRemaining()) {
return -1;
}
Expand Down

0 comments on commit 042f6b5

Please sign in to comment.