Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a5827141 authored by Zhijun He's avatar Zhijun He
Browse files

ImageWriter: fix and cleanup the closed Image Handling

Attempting to access an Image after it is closed will result in an ISE.

Also fixed some minor doc issues.

Bug: 19872785
Change-Id: I91f037b2b2f243fcbd905d5a646b505bc9c10638
parent 25d4a639
Loading
Loading
Loading
Loading
+23 −1
Original line number Diff line number Diff line
@@ -47,12 +47,22 @@ import android.graphics.Rect;
 * @see ImageReader
 */
public abstract class Image implements AutoCloseable {
    protected boolean mIsImageValid = false;

    /**
     * @hide
     */
    protected Image() {
    }

    /**
     * Throw IllegalStateException if the image is invalid (already closed).
     */
    protected void throwISEIfImageIsInvalid() {
        if (!mIsImageValid) {
            throw new IllegalStateException("Image is already closed");
        }
    }
    /**
     * Get the format for this image. This format determines the number of
     * ByteBuffers needed to represent the image, and the general layout of the
@@ -128,7 +138,7 @@ public abstract class Image implements AutoCloseable {
     * Set the timestamp associated with this frame.
     * <p>
     * The timestamp is measured in nanoseconds, and is normally monotonically
     * increasing. However, However, the behavior of the timestamp depends on
     * increasing. However, the behavior of the timestamp depends on
     * the destination of this image. See {@link android.hardware.Camera Camera}
     * , {@link android.hardware.camera2.CameraDevice CameraDevice},
     * {@link MediaPlayer} and {@link MediaCodec} for more details.
@@ -144,6 +154,7 @@ public abstract class Image implements AutoCloseable {
     * @param timestamp The timestamp to be set for this image.
     */
    public void setTimestamp(long timestamp) {
        throwISEIfImageIsInvalid();
        return;
    }

@@ -155,6 +166,7 @@ public abstract class Image implements AutoCloseable {
     * </p>
     */
    public boolean isOpaque() {
        throwISEIfImageIsInvalid();
        return false;
    }

@@ -167,6 +179,8 @@ public abstract class Image implements AutoCloseable {
     * using coordinates in the largest-resolution plane.
     */
    public Rect getCropRect() {
        throwISEIfImageIsInvalid();

        if (mCropRect == null) {
            return new Rect(0, 0, getWidth(), getHeight());
        } else {
@@ -181,6 +195,8 @@ public abstract class Image implements AutoCloseable {
     * using coordinates in the largest-resolution plane.
     */
    public void setCropRect(Rect cropRect) {
        throwISEIfImageIsInvalid();

        if (cropRect != null) {
            cropRect = new Rect(cropRect);  // make a copy
            cropRect.intersect(0, 0, getWidth(), getHeight());
@@ -228,6 +244,8 @@ public abstract class Image implements AutoCloseable {
     *         a new owner.
     */
    boolean isAttachable() {
        throwISEIfImageIsInvalid();

        return false;
    }

@@ -247,6 +265,8 @@ public abstract class Image implements AutoCloseable {
     * @return The owner of the Image.
     */
    Object getOwner() {
        throwISEIfImageIsInvalid();

        return null;
    }

@@ -262,6 +282,8 @@ public abstract class Image implements AutoCloseable {
     * @return native context associated with this Image.
     */
    long getNativeContext() {
        throwISEIfImageIsInvalid();

        return 0;
    }

+2 −14
Original line number Diff line number Diff line
@@ -368,7 +368,7 @@ public class ImageReader implements AutoCloseable {
        switch (status) {
            case ACQUIRE_SUCCESS:
                si.createSurfacePlanes();
                si.setImageValid(true);
                si.mIsImageValid = true;
            case ACQUIRE_NO_BUFS:
            case ACQUIRE_MAX_IMAGES:
                break;
@@ -444,7 +444,7 @@ public class ImageReader implements AutoCloseable {

        si.clearSurfacePlanes();
        nativeReleaseImage(i);
        si.setImageValid(false);
        si.mIsImageValid = false;
    }

    /**
@@ -686,7 +686,6 @@ public class ImageReader implements AutoCloseable {

    private class SurfaceImage extends android.media.Image {
        public SurfaceImage(int format) {
            mIsImageValid = false;
            mFormat = format;
        }

@@ -784,16 +783,6 @@ public class ImageReader implements AutoCloseable {
            mIsDetached.getAndSet(detached);
        }

        private void setImageValid(boolean isValid) {
            mIsImageValid = isValid;
        }

        private void throwISEIfImageIsInvalid() {
            if (!mIsImageValid) {
                throw new IllegalStateException("Image is already closed");
            }
        }

        private void clearSurfacePlanes() {
            if (mIsImageValid) {
                for (int i = 0; i < mPlanes.length; i++) {
@@ -877,7 +866,6 @@ public class ImageReader implements AutoCloseable {
        private long mTimestamp;

        private SurfacePlane[] mPlanes;
        private boolean mIsImageValid;
        private int mHeight = -1;
        private int mWidth = -1;
        private int mFormat = ImageFormat.UNKNOWN;
+23 −51
Original line number Diff line number Diff line
@@ -29,7 +29,6 @@ import java.nio.ByteOrder;
import java.nio.NioUtils;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;

/**
 * <p>
@@ -204,7 +203,7 @@ public class ImageWriter implements AutoCloseable {
        WriterSurfaceImage newImage = new WriterSurfaceImage(this);
        nativeDequeueInputImage(mNativeContext, newImage);
        mDequeuedImages.add(newImage);
        newImage.setImageValid(true);
        newImage.mIsImageValid = true;
        return newImage;
    }

@@ -260,7 +259,7 @@ public class ImageWriter implements AutoCloseable {
            throw new IllegalArgumentException("image shouldn't be null");
        }
        boolean ownedByMe = isImageOwnedByMe(image);
        if (ownedByMe && !(((WriterSurfaceImage) image).isImageValid())) {
        if (ownedByMe && !(((WriterSurfaceImage) image).mIsImageValid)) {
            throw new IllegalStateException("Image from ImageWriter is invalid");
        }

@@ -312,7 +311,7 @@ public class ImageWriter implements AutoCloseable {
            // Do not call close here, as close is essentially cancel image.
            WriterSurfaceImage wi = (WriterSurfaceImage) image;
            wi.clearSurfacePlanes();
            wi.setImageValid(false);
            wi.mIsImageValid = false;
        }
    }

@@ -555,7 +554,7 @@ public class ImageWriter implements AutoCloseable {

        WriterSurfaceImage wi = (WriterSurfaceImage) image;

        if (!wi.isImageValid()) {
        if (!wi.mIsImageValid) {
            throw new IllegalStateException("Image is invalid");
        }

@@ -568,7 +567,7 @@ public class ImageWriter implements AutoCloseable {
        cancelImage(mNativeContext, image);
        mDequeuedImages.remove(image);
        wi.clearSurfacePlanes();
        wi.setImageValid(false);
        wi.mIsImageValid = false;
    }

    private boolean isImageOwnedByMe(Image image) {
@@ -585,7 +584,6 @@ public class ImageWriter implements AutoCloseable {

    private static class WriterSurfaceImage extends android.media.Image {
        private ImageWriter mOwner;
        private AtomicBoolean mIsImageValid = new AtomicBoolean(false);
        // This field is used by native code, do not access or modify.
        private long mNativeBuffer;
        private int mNativeFenceFd = -1;
@@ -604,9 +602,8 @@ public class ImageWriter implements AutoCloseable {

        @Override
        public int getFormat() {
            if (!mIsImageValid.get()) {
                throw new IllegalStateException("Image is already released");
            }
            throwISEIfImageIsInvalid();

            if (mFormat == -1) {
                mFormat = nativeGetFormat();
            }
@@ -615,9 +612,7 @@ public class ImageWriter implements AutoCloseable {

        @Override
        public int getWidth() {
            if (!mIsImageValid.get()) {
                throw new IllegalStateException("Image is already released");
            }
            throwISEIfImageIsInvalid();

            if (mWidth == -1) {
                mWidth = nativeGetWidth();
@@ -628,9 +623,7 @@ public class ImageWriter implements AutoCloseable {

        @Override
        public int getHeight() {
            if (!mIsImageValid.get()) {
                throw new IllegalStateException("Image is already released");
            }
            throwISEIfImageIsInvalid();

            if (mHeight == -1) {
                mHeight = nativeGetHeight();
@@ -641,36 +634,28 @@ public class ImageWriter implements AutoCloseable {

        @Override
        public long getTimestamp() {
            if (!mIsImageValid.get()) {
                throw new IllegalStateException("Image is already released");
            }
            throwISEIfImageIsInvalid();

            return mTimestamp;
        }

        @Override
        public void setTimestamp(long timestamp) {
            if (!mIsImageValid.get()) {
                throw new IllegalStateException("Image is already released");
            }
            throwISEIfImageIsInvalid();

            mTimestamp = timestamp;
        }

        @Override
        public boolean isOpaque() {
            if (!mIsImageValid.get()) {
                throw new IllegalStateException("Image is already released");
            }
            throwISEIfImageIsInvalid();

            return getFormat() == ImageFormat.PRIVATE;
        }

        @Override
        public Plane[] getPlanes() {
            if (!mIsImageValid.get()) {
                throw new IllegalStateException("Image is already released");
            }
            throwISEIfImageIsInvalid();

            if (mPlanes == null) {
                int numPlanes = ImageUtils.getNumPlanesForFormat(getFormat());
@@ -682,9 +667,7 @@ public class ImageWriter implements AutoCloseable {

        @Override
        boolean isAttachable() {
            if (!mIsImageValid.get()) {
                throw new IllegalStateException("Image is already released");
            }
            throwISEIfImageIsInvalid();
            // Don't allow Image to be detached from ImageWriter for now, as no
            // detach API is exposed.
            return false;
@@ -692,17 +675,21 @@ public class ImageWriter implements AutoCloseable {

        @Override
        ImageWriter getOwner() {
            throwISEIfImageIsInvalid();

            return mOwner;
        }

        @Override
        long getNativeContext() {
            throwISEIfImageIsInvalid();

            return mNativeBuffer;
        }

        @Override
        public void close() {
            if (mIsImageValid.get()) {
            if (mIsImageValid) {
                getOwner().abortImage(this);
            }
        }
@@ -716,16 +703,8 @@ public class ImageWriter implements AutoCloseable {
            }
        }

        private boolean isImageValid() {
            return mIsImageValid.get();
        }

        private void setImageValid(boolean isValid) {
            mIsImageValid.getAndSet(isValid);
        }

        private void clearSurfacePlanes() {
            if (mIsImageValid.get()) {
            if (mIsImageValid) {
                for (int i = 0; i < mPlanes.length; i++) {
                    if (mPlanes[i] != null) {
                        mPlanes[i].clearBuffer();
@@ -756,26 +735,19 @@ public class ImageWriter implements AutoCloseable {

            @Override
            public int getRowStride() {
                if (WriterSurfaceImage.this.isImageValid() == false) {
                    throw new IllegalStateException("Image is already released");
                }
                throwISEIfImageIsInvalid();
                return mRowStride;
            }

            @Override
            public int getPixelStride() {
                if (WriterSurfaceImage.this.isImageValid() == false) {
                    throw new IllegalStateException("Image is already released");
                }
                throwISEIfImageIsInvalid();
                return mPixelStride;
            }

            @Override
            public ByteBuffer getBuffer() {
                if (WriterSurfaceImage.this.isImageValid() == false) {
                    throw new IllegalStateException("Image is already released");
                }

                throwISEIfImageIsInvalid();
                return mBuffer;
            }

+18 −17
Original line number Diff line number Diff line
@@ -1817,7 +1817,6 @@ final public class MediaCodec {
    /** @hide */
    public static class MediaImage extends Image {
        private final boolean mIsReadOnly;
        private boolean mIsValid;
        private final int mWidth;
        private final int mHeight;
        private final int mFormat;
@@ -1830,36 +1829,42 @@ final public class MediaCodec {

        private final static int TYPE_YUV = 1;

        @Override
        public int getFormat() {
            checkValid();
            throwISEIfImageIsInvalid();
            return mFormat;
        }

        @Override
        public int getHeight() {
            checkValid();
            throwISEIfImageIsInvalid();
            return mHeight;
        }

        @Override
        public int getWidth() {
            checkValid();
            throwISEIfImageIsInvalid();
            return mWidth;
        }

        @Override
        public long getTimestamp() {
            checkValid();
            throwISEIfImageIsInvalid();
            return mTimestamp;
        }

        @Override
        @NonNull
        public Plane[] getPlanes() {
            checkValid();
            throwISEIfImageIsInvalid();
            return Arrays.copyOf(mPlanes, mPlanes.length);
        }

        @Override
        public void close() {
            if (mIsValid) {
            if (mIsImageValid) {
                java.nio.NioUtils.freeDirectBuffer(mBuffer);
                mIsValid = false;
                mIsImageValid = false;
            }
        }

@@ -1869,6 +1874,7 @@ final public class MediaCodec {
         * The crop rectangle specifies the region of valid pixels in the image,
         * using coordinates in the largest-resolution plane.
         */
        @Override
        public void setCropRect(@Nullable Rect cropRect) {
            if (mIsReadOnly) {
                throw new ReadOnlyBufferException();
@@ -1876,11 +1882,6 @@ final public class MediaCodec {
            super.setCropRect(cropRect);
        }

        private void checkValid() {
            if (!mIsValid) {
                throw new IllegalStateException("Image is already released");
            }
        }

        private int readInt(@NonNull ByteBuffer buffer, boolean asLong) {
            if (asLong) {
@@ -1895,7 +1896,7 @@ final public class MediaCodec {
                long timestamp, int xOffset, int yOffset, @Nullable Rect cropRect) {
            mFormat = ImageFormat.YUV_420_888;
            mTimestamp = timestamp;
            mIsValid = true;
            mIsImageValid = true;
            mIsReadOnly = buffer.isReadOnly();
            mBuffer = buffer.duplicate();

@@ -1966,20 +1967,20 @@ final public class MediaCodec {

            @Override
            public int getRowStride() {
                checkValid();
                throwISEIfImageIsInvalid();
                return mRowInc;
            }

            @Override
            public int getPixelStride() {
                checkValid();
                throwISEIfImageIsInvalid();
                return mColInc;
            }

            @Override
            @NonNull
            public ByteBuffer getBuffer() {
                checkValid();
                throwISEIfImageIsInvalid();
                return mData;
            }

+1 −1
Original line number Diff line number Diff line
@@ -338,7 +338,7 @@ static void ImageWriter_dequeueImage(JNIEnv* env, jobject thiz, jlong nativeCtx,
    status_t res = anw->dequeueBuffer(anw.get(), &anb, &fenceFd);
    if (res != OK) {
        // TODO: handle different error cases here.
        ALOGE("%s: Set buffer count failed: %s (%d)", __FUNCTION__, strerror(-res), res);
        ALOGE("%s: Dequeue buffer failed: %s (%d)", __FUNCTION__, strerror(-res), res);
        jniThrowRuntimeException(env, "dequeue buffer failed");
        return;
    }