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

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

media: improve ImageReader/Writer native memory management 

* Hook up the native allocation registration with ImageWriter, such that GC
can get some hint when clean up the large memory object.
* Close all pending images when closing ImageReader. This could avoid native
mem leaks for some bad app practice. For example, some apps may hold images
in background service when activity is paused/destroyed, which could cause
huge native memory leaks even ImageReader is closed.
* make Image close thread safe: it is possible the clients close the image
in listener thread and the client main thread.
* Some minor code refactor to reduce the code duplication.

Bug: 25088440
Change-Id: I37d22b52aeb8d2521bf9c702b0f54c05905473e0
parent 8d19ad44

media/java/android/media/ImageReader.java

+52 −60
Original line number Diff line number Diff line
@@ -17,10 +17,10 @@ 
package android.media;



import android.graphics.ImageFormat;

import android.graphics.PixelFormat;

import android.os.Handler;

import android.os.Looper;

import android.os.Message;

import android.util.Log;

import android.view.Surface;



import dalvik.system.VMRuntime;
@@ -29,6 +29,8 @@ import java.lang.ref.WeakReference; 
import java.nio.ByteBuffer;

import java.nio.ByteOrder;

import java.nio.NioUtils;

import java.util.List;

import java.util.concurrent.CopyOnWriteArrayList;

import java.util.concurrent.atomic.AtomicBoolean;



/**
@@ -145,7 +147,7 @@ public class ImageReader implements AutoCloseable { 
                    "NV21 format is not supported");

        }



        mNumPlanes = getNumPlanesFromFormat();

        mNumPlanes = ImageUtils.getNumPlanesForFormat(mFormat);



        nativeInit(new WeakReference<ImageReader>(this), width, height, format, maxImages);
@@ -323,7 +325,7 @@ public class ImageReader implements AutoCloseable { 
     * @see #ACQUIRE_SUCCESS

     */

    private int acquireNextSurfaceImage(SurfaceImage si) {



        synchronized (mCloseLock) {

            int status = nativeImageSetup(si);



            switch (status) {
@@ -337,8 +339,14 @@ public class ImageReader implements AutoCloseable { 
                    throw new AssertionError("Unknown nativeImageSetup return code " + status);

            }



            // Only keep track the successfully acquired image, as the native buffer is only mapped

            // for such case.

            if (status == ACQUIRE_SUCCESS) {

                mAcquiredImages.add(si);

            }

            return status;

        }

    }



    /**

     * <p>
@@ -398,7 +406,11 @@ public class ImageReader implements AutoCloseable { 
                "This image was not produced by an ImageReader");

        }

        SurfaceImage si = (SurfaceImage) i;

        if (si.getReader() != this) {

        if (si.mIsImageValid == false) {

            return;

        }



        if (si.getReader() != this || !mAcquiredImages.contains(i)) {

            throw new IllegalArgumentException(

                "This image was not produced by this ImageReader");

        }
@@ -406,6 +418,7 @@ public class ImageReader implements AutoCloseable { 
        si.clearSurfacePlanes();

        nativeReleaseImage(i);

        si.mIsImageValid = false;

        mAcquiredImages.remove(i);

    }



    /**
@@ -475,7 +488,24 @@ public class ImageReader implements AutoCloseable { 
    public void close() {

        setOnImageAvailableListener(null, null);

        if (mSurface != null) mSurface.release();



        /**

         * Close all outstanding acquired images before closing the ImageReader. It is a good

         * practice to close all the images as soon as it is not used to reduce system instantaneous

         * memory pressure. CopyOnWrite list will use a copy of current list content. For the images

         * being closed by other thread (e.g., GC thread), doubling the close call is harmless. For

         * the image being acquired by other threads, mCloseLock is used to synchronize close and

         * acquire operations.

         */

        synchronized (mCloseLock) {

            for (Image image : mAcquiredImages) {

                image.close();

            }

            mAcquiredImages.clear();



            nativeClose();

        }



        if (mEstimatedNativeAllocBytes > 0) {

            VMRuntime.getRuntime().registerNativeFree(mEstimatedNativeAllocBytes);

            mEstimatedNativeAllocBytes = 0;
@@ -542,45 +572,6 @@ public class ImageReader implements AutoCloseable { 
        si.setDetached(true);

   }



    /**

     * Only a subset of the formats defined in

     * {@link android.graphics.ImageFormat ImageFormat} and

     * {@link android.graphics.PixelFormat PixelFormat} are supported by

     * ImageReader. When reading RGB data from a surface, the formats defined in

     * {@link android.graphics.PixelFormat PixelFormat} can be used, when

     * reading YUV, JPEG or raw sensor data (for example, from camera or video

     * decoder), formats from {@link android.graphics.ImageFormat ImageFormat}

     * are used.

     */

    private int getNumPlanesFromFormat() {

        switch (mFormat) {

            case ImageFormat.YV12:

            case ImageFormat.YUV_420_888:

            case ImageFormat.NV21:

                return 3;

            case ImageFormat.NV16:

                return 2;

            case PixelFormat.RGB_565:

            case PixelFormat.RGBA_8888:

            case PixelFormat.RGBX_8888:

            case PixelFormat.RGB_888:

            case ImageFormat.JPEG:

            case ImageFormat.YUY2:

            case ImageFormat.Y8:

            case ImageFormat.Y16:

            case ImageFormat.RAW_SENSOR:

            case ImageFormat.RAW10:

            case ImageFormat.DEPTH16:

            case ImageFormat.DEPTH_POINT_CLOUD:

                return 1;

            case ImageFormat.PRIVATE:

                return 0;

            default:

                throw new UnsupportedOperationException(

                        String.format("Invalid format specified %d", mFormat));

        }

    }



    private boolean isImageOwnedbyMe(Image image) {

        if (!(image instanceof SurfaceImage)) {

            return false;
@@ -612,7 +603,6 @@ public class ImageReader implements AutoCloseable { 
        }

    }





    private final int mWidth;

    private final int mHeight;

    private final int mFormat;
@@ -622,8 +612,12 @@ public class ImageReader implements AutoCloseable { 
    private int mEstimatedNativeAllocBytes;



    private final Object mListenerLock = new Object();

    private final Object mCloseLock = new Object();

    private OnImageAvailableListener mListener;

    private ListenerHandler mListenerHandler;

    // Keep track of the successfully acquired Images. This need to be thread safe as the images

    // could be closed by different threads (e.g., application thread and GC thread).

    private List<Image> mAcquiredImages = new CopyOnWriteArrayList<Image>();



    /**

     * This field is used by native code, do not access or modify.
@@ -657,10 +651,8 @@ public class ImageReader implements AutoCloseable { 


        @Override

        public void close() {

            if (mIsImageValid) {

            ImageReader.this.releaseImage(this);

        }

        }



        public ImageReader getReader() {

            return ImageReader.this;

media/java/android/media/ImageUtils.java

+3 −0
Original line number Diff line number Diff line
@@ -58,6 +58,9 @@ class ImageUtils { 
            case ImageFormat.Y16:

            case ImageFormat.RAW_SENSOR:

            case ImageFormat.RAW10:

            case ImageFormat.RAW12:

            case ImageFormat.DEPTH16:

            case ImageFormat.DEPTH_POINT_CLOUD:

                return 1;

            case ImageFormat.PRIVATE:

                return 0;

media/java/android/media/ImageWriter.java

+25 −5
Original line number Diff line number Diff line
@@ -18,17 +18,21 @@ package android.media; 


import android.graphics.ImageFormat;

import android.graphics.Rect;

import android.hardware.camera2.utils.SurfaceUtils;

import android.os.Handler;

import android.os.Looper;

import android.os.Message;

import android.util.Size;

import android.view.Surface;



import dalvik.system.VMRuntime;



import java.lang.ref.WeakReference;

import java.nio.ByteBuffer;

import java.nio.ByteOrder;

import java.nio.NioUtils;

import java.util.ArrayList;

import java.util.List;

import java.util.concurrent.CopyOnWriteArrayList;



/**

 * <p>
@@ -83,8 +87,10 @@ public class ImageWriter implements AutoCloseable { 
    private int mWriterFormat;



    private final int mMaxImages;

    // Keep track of the currently dequeued Image.

    private List<Image> mDequeuedImages = new ArrayList<Image>();

    // Keep track of the currently dequeued Image. This need to be thread safe as the images

    // could be closed by different threads (e.g., application thread and GC thread).

    private List<Image> mDequeuedImages = new CopyOnWriteArrayList<Image>();

    private int mEstimatedNativeAllocBytes;



    /**

     * <p>
@@ -128,6 +134,16 @@ public class ImageWriter implements AutoCloseable { 
        // Note that the underlying BufferQueue is working in synchronous mode

        // to avoid dropping any buffers.

        mNativeContext = nativeInit(new WeakReference<ImageWriter>(this), surface, maxImages);



        // Estimate the native buffer allocation size and register it so it gets accounted for

        // during GC. Note that this doesn't include the buffers required by the buffer queue

        // itself and the buffers requested by the producer.

        Size surfSize = SurfaceUtils.getSurfaceSize(surface);

        int format = SurfaceUtils.getSurfaceFormat(surface);

        mEstimatedNativeAllocBytes =

                ImageUtils.getEstimatedNativeAllocBytes(surfSize.getWidth(),surfSize.getHeight(),

                        format, maxImages);

        VMRuntime.getRuntime().registerNativeAllocation(mEstimatedNativeAllocBytes);

    }



    /**
@@ -432,6 +448,11 @@ public class ImageWriter implements AutoCloseable { 
        mDequeuedImages.clear();

        nativeClose(mNativeContext);

        mNativeContext = 0;



        if (mEstimatedNativeAllocBytes > 0) {

            VMRuntime.getRuntime().registerNativeFree(mEstimatedNativeAllocBytes);

            mEstimatedNativeAllocBytes = 0;

        }

    }



    @Override
@@ -569,9 +590,8 @@ public class ImageWriter implements AutoCloseable { 
        }



        WriterSurfaceImage wi = (WriterSurfaceImage) image;



        if (!wi.mIsImageValid) {

            throw new IllegalStateException("Image is invalid");

            return;

        }



        /**

media/jni/android_media_ImageReader.cpp

+3 −2
Original line number Diff line number Diff line
@@ -933,7 +933,7 @@ static void ImageReader_imageRelease(JNIEnv* env, jobject thiz, jobject image) 
        CpuConsumer* consumer = ctx->getCpuConsumer();

        CpuConsumer::LockedBuffer* buffer = Image_getLockedBuffer(env, image);

        if (!buffer) {

            ALOGW("Image already released!!!");

            // Release an already closed image is harmless.

            return;

        }

        consumer->unlockBuffer(*buffer);
@@ -1078,7 +1078,8 @@ static jint ImageReader_imageSetup(JNIEnv* env, jobject thiz, jobject image) { 
    ALOGV("%s:", __FUNCTION__);

    JNIImageReaderContext* ctx = ImageReader_getContext(env, thiz);

    if (ctx == NULL) {

        jniThrowRuntimeException(env, "ImageReaderContext is not initialized");

        jniThrowException(env, "java/lang/IllegalStateException",

                "ImageReader is not initialized or was already closed");

        return -1;

    }

media/jni/android_media_ImageWriter.cpp

+2 −4
Original line number Diff line number Diff line
@@ -403,8 +403,7 @@ static void ImageWriter_cancelImage(JNIEnv* env, jobject thiz, jlong nativeCtx, 
    ALOGV("%s", __FUNCTION__);

    JNIImageWriterContext* const ctx = reinterpret_cast<JNIImageWriterContext *>(nativeCtx);

    if (ctx == NULL || thiz == NULL) {

        jniThrowException(env, "java/lang/IllegalStateException",

                "ImageWriterContext is not initialized");

        ALOGW("ImageWriter#close called before Image#close, consider calling Image#close first");

        return;

    }
@@ -414,8 +413,7 @@ static void ImageWriter_cancelImage(JNIEnv* env, jobject thiz, jlong nativeCtx, 
    int fenceFd = -1;

    Image_getNativeContext(env, image, &buffer, &fenceFd);

    if (buffer == NULL) {

        jniThrowException(env, "java/lang/IllegalStateException",

                "Image is not initialized");

        // Cancel an already cancelled image is harmless.

        return;

    }