From d99dc329b9baefe3d722ccafc0c92461cfef42b1 Mon Sep 17 00:00:00 2001 From: Zhijun He Date: Wed, 13 May 2015 12:11:47 -0700 Subject: [PATCH] Image/Reader/Writer: address API review comments * Remove isOpaque methods * Removew ImageReader#newOpaqueInstance * Rename Writer listener and callback * Clarify the timestamp spec * Fix a bug in ImageUtils Bug: 21044520 Change-Id: Ifb85ef25e4d152c2a445b3369c1e16dc1bc5220c --- api/current.txt | 9 +- api/system-current.txt | 9 +- media/java/android/media/Image.java | 39 ++++---- media/java/android/media/ImageReader.java | 106 ++++++---------------- media/java/android/media/ImageUtils.java | 16 ++-- media/java/android/media/ImageWriter.java | 93 +++++++++---------- 6 files changed, 101 insertions(+), 171 deletions(-) diff --git a/api/current.txt b/api/current.txt index 5efa68503af3a..bda525457463d 100644 --- a/api/current.txt +++ b/api/current.txt @@ -15230,7 +15230,6 @@ package android.media { method public abstract android.media.Image.Plane[] getPlanes(); method public abstract long getTimestamp(); method public abstract int getWidth(); - method public boolean isOpaque(); method public void setCropRect(android.graphics.Rect); method public void setTimestamp(long); } @@ -15250,9 +15249,7 @@ package android.media { method public int getMaxImages(); method public android.view.Surface getSurface(); method public int getWidth(); - method public boolean isOpaque(); method public static android.media.ImageReader newInstance(int, int, int, int); - method public static android.media.ImageReader newOpaqueInstance(int, int, int); method public void setOnImageAvailableListener(android.media.ImageReader.OnImageAvailableListener, android.os.Handler); } @@ -15267,11 +15264,11 @@ package android.media { method public int getMaxImages(); method public static android.media.ImageWriter newInstance(android.view.Surface, int); method public void queueInputImage(android.media.Image); - method public void setImageListener(android.media.ImageWriter.ImageListener, android.os.Handler); + method public void setOnImageReleasedListener(android.media.ImageWriter.OnImageReleasedListener, android.os.Handler); } - public static abstract interface ImageWriter.ImageListener { - method public abstract void onInputImageReleased(android.media.ImageWriter); + public static abstract interface ImageWriter.OnImageReleasedListener { + method public abstract void onImageReleased(android.media.ImageWriter); } public class JetPlayer { diff --git a/api/system-current.txt b/api/system-current.txt index 119295d1dac7e..0e7a1813e7e3c 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -16467,7 +16467,6 @@ package android.media { method public abstract android.media.Image.Plane[] getPlanes(); method public abstract long getTimestamp(); method public abstract int getWidth(); - method public boolean isOpaque(); method public void setCropRect(android.graphics.Rect); method public void setTimestamp(long); } @@ -16487,9 +16486,7 @@ package android.media { method public int getMaxImages(); method public android.view.Surface getSurface(); method public int getWidth(); - method public boolean isOpaque(); method public static android.media.ImageReader newInstance(int, int, int, int); - method public static android.media.ImageReader newOpaqueInstance(int, int, int); method public void setOnImageAvailableListener(android.media.ImageReader.OnImageAvailableListener, android.os.Handler); } @@ -16504,11 +16501,11 @@ package android.media { method public int getMaxImages(); method public static android.media.ImageWriter newInstance(android.view.Surface, int); method public void queueInputImage(android.media.Image); - method public void setImageListener(android.media.ImageWriter.ImageListener, android.os.Handler); + method public void setOnImageReleasedListener(android.media.ImageWriter.OnImageReleasedListener, android.os.Handler); } - public static abstract interface ImageWriter.ImageListener { - method public abstract void onInputImageReleased(android.media.ImageWriter); + public static abstract interface ImageWriter.OnImageReleasedListener { + method public abstract void onImageReleased(android.media.ImageWriter); } public class JetPlayer { diff --git a/media/java/android/media/Image.java b/media/java/android/media/Image.java index 195c98704e4c2..e18e9a395b5a4 100644 --- a/media/java/android/media/Image.java +++ b/media/java/android/media/Image.java @@ -163,10 +163,12 @@ public abstract class Image implements AutoCloseable { * Get the timestamp associated with this frame. *

* The timestamp is measured in nanoseconds, and is normally monotonically - * increasing. However, the behavior of the timestamp depends on the source - * of this image. See {@link android.hardware.Camera Camera}, - * {@link android.hardware.camera2.CameraDevice CameraDevice}, {@link MediaPlayer} and - * {@link MediaCodec} for more details. + * increasing. The timestamps for the images from different sources may have + * different timebases therefore may not be comparable. The specific meaning and + * timebase of the timestamp depend on the source providing images. See + * {@link android.hardware.Camera Camera}, + * {@link android.hardware.camera2.CameraDevice CameraDevice}, + * {@link MediaPlayer} and {@link MediaCodec} for more details. *

*/ public abstract long getTimestamp(); @@ -175,9 +177,11 @@ public abstract class Image implements AutoCloseable { * Set the timestamp associated with this frame. *

* The timestamp is measured in nanoseconds, and is normally monotonically - * 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}, + * increasing. The timestamps for the images from different sources may have + * different timebases therefore may not be comparable. The specific meaning and + * timebase of the timestamp depend on the source providing images. See + * {@link android.hardware.Camera Camera}, + * {@link android.hardware.camera2.CameraDevice CameraDevice}, * {@link MediaPlayer} and {@link MediaCodec} for more details. *

*

@@ -195,18 +199,6 @@ public abstract class Image implements AutoCloseable { return; } - /** - *

Check if the image is opaque.

- * - *

The pixel data of opaque images are not accessible to the application, - * and therefore {@link #getPlanes} will return an empty array for an opaque image. - *

- */ - public boolean isOpaque() { - throwISEIfImageIsInvalid(); - return false; - } - private Rect mCropRect; /** @@ -243,10 +235,11 @@ public abstract class Image implements AutoCloseable { /** * Get the array of pixel planes for this Image. The number of planes is - * determined by the format of the Image. The application will get an - * empty array if the image is opaque because the opaque image pixel data - * is not directly accessible. The application can check if an image is - * opaque by calling {@link Image#isOpaque}. + * determined by the format of the Image. The application will get an empty + * array if the image format is {@link android.graphics.ImageFormat#PRIVATE + * PRIVATE}, because the image pixel data is not directly accessible. The + * application can check the image format by calling + * {@link Image#getFormat()}. */ public abstract Plane[] getPlanes(); diff --git a/media/java/android/media/ImageReader.java b/media/java/android/media/ImageReader.java index 6d30208087581..f5222645f6591 100644 --- a/media/java/android/media/ImageReader.java +++ b/media/java/android/media/ImageReader.java @@ -67,77 +67,46 @@ public class ImageReader implements AutoCloseable { */ private static final int ACQUIRE_MAX_IMAGES = 2; - /** - *

Create a new reader for images of the desired size and format.

- * - *

The {@code maxImages} parameter determines the maximum number of {@link Image} - * objects that can be be acquired from the {@code ImageReader} - * simultaneously. Requesting more buffers will use up more memory, so it is - * important to use only the minimum number necessary for the use case.

- * - *

The valid sizes and formats depend on the source of the image - * data.

- * - * @param width - * The default width in pixels of the Images that this reader will produce. - * @param height - * The default height in pixels of the Images that this reader will produce. - * @param format - * The format of the Image that this reader will produce. This - * must be one of the {@link android.graphics.ImageFormat} or - * {@link android.graphics.PixelFormat} constants. Note that - * not all formats is supported, like ImageFormat.NV21. - * @param maxImages - * The maximum number of images the user will want to - * access simultaneously. This should be as small as possible to limit - * memory use. Once maxImages Images are obtained by the user, one of them - * has to be released before a new Image will become available for access - * through {@link #acquireLatestImage()} or {@link #acquireNextImage()}. - * Must be greater than 0. - * - * @see Image - */ - public static ImageReader newInstance(int width, int height, int format, int maxImages) { - if (format == ImageFormat.PRIVATE) { - throw new IllegalArgumentException("To obtain an opaque ImageReader, please use" - + " newOpaqueInstance rather than newInstance"); - } - return new ImageReader(width, height, format, maxImages); - } - /** *

- * Create a new opaque reader for images of the desired size. - *

- *

- * An opaque {@link ImageReader} produces images that are not directly - * accessible by the application. The application can still acquire images - * from an opaque image reader, and send them to the - * {@link android.hardware.camera2.CameraDevice camera} for reprocessing via - * {@link ImageWriter} interface. However, the {@link Image#getPlanes() - * getPlanes()} will return an empty array for opaque images. The - * application can check if an existing reader is an opaque reader by - * calling {@link #isOpaque()}. + * Create a new reader for images of the desired size and format. *

*

* The {@code maxImages} parameter determines the maximum number of * {@link Image} objects that can be be acquired from the * {@code ImageReader} simultaneously. Requesting more buffers will use up - * more memory, so it is important to use only the minimum number necessary. + * more memory, so it is important to use only the minimum number necessary + * for the use case. *

*

* The valid sizes and formats depend on the source of the image data. *

*

- * Opaque ImageReaders are more efficient to use when application access to - * image data is not necessary, compared to ImageReaders using a non-opaque - * format such as {@link ImageFormat#YUV_420_888 YUV_420_888}. + * If the {@code format} is {@link ImageFormat#PRIVATE PRIVATE}, the created + * {@link ImageReader} will produce images that are not directly accessible + * by the application. The application can still acquire images from this + * {@link ImageReader}, and send them to the + * {@link android.hardware.camera2.CameraDevice camera} for reprocessing via + * {@link ImageWriter} interface. However, the {@link Image#getPlanes() + * getPlanes()} will return an empty array for {@link ImageFormat#PRIVATE + * PRIVATE} format images. The application can check if an existing reader's + * format by calling {@link #getImageFormat()}. + *

+ *

+ * {@link ImageFormat#PRIVATE PRIVATE} format {@link ImageReader + * ImageReaders} are more efficient to use when application access to image + * data is not necessary, compared to ImageReaders using other format such + * as {@link ImageFormat#YUV_420_888 YUV_420_888}. *

* * @param width The default width in pixels of the Images that this reader * will produce. * @param height The default height in pixels of the Images that this reader * will produce. + * @param format The format of the Image that this reader will produce. This + * must be one of the {@link android.graphics.ImageFormat} or + * {@link android.graphics.PixelFormat} constants. Note that not + * all formats are supported, like ImageFormat.NV21. * @param maxImages The maximum number of images the user will want to * access simultaneously. This should be as small as possible to * limit memory use. Once maxImages Images are obtained by the @@ -147,8 +116,8 @@ public class ImageReader implements AutoCloseable { * Must be greater than 0. * @see Image */ - public static ImageReader newOpaqueInstance(int width, int height, int maxImages) { - return new ImageReader(width, height, ImageFormat.PRIVATE, maxImages); + public static ImageReader newInstance(int width, int height, int format, int maxImages) { + return new ImageReader(width, height, format, maxImages); } /** @@ -247,23 +216,6 @@ public class ImageReader implements AutoCloseable { return mMaxImages; } - /** - *

- * Check if the {@link ImageReader} is an opaque reader. - *

- *

- * An opaque image reader produces opaque images, see {@link Image#isOpaque} - * for more details. - *

- * - * @return true if the ImageReader is opaque. - * @see Image#isOpaque - * @see ImageReader#newOpaqueInstance - */ - public boolean isOpaque() { - return mFormat == ImageFormat.PRIVATE; - } - /** *

Get a {@link Surface} that can be used to produce {@link Image Images} for this * {@code ImageReader}.

@@ -540,11 +492,11 @@ public class ImageReader implements AutoCloseable { *

*

* This method can be used to achieve zero buffer copy for use cases like - * {@link android.hardware.camera2.CameraDevice Camera2 API} OPAQUE and YUV + * {@link android.hardware.camera2.CameraDevice Camera2 API} PRIVATE and YUV * reprocessing, where the application can select an output image from * {@link ImageReader} and transfer this image directly to * {@link ImageWriter}, where this image can be consumed by camera directly. - * For OPAQUE reprocessing, this is the only way to send input buffers to + * For PRIVATE reprocessing, this is the only way to send input buffers to * the {@link android.hardware.camera2.CameraDevice camera} for * reprocessing. *

@@ -745,12 +697,6 @@ public class ImageReader implements AutoCloseable { return mPlanes.clone(); } - @Override - public boolean isOpaque() { - throwISEIfImageIsInvalid(); - return mFormat == ImageFormat.PRIVATE; - } - @Override protected final void finalize() throws Throwable { try { diff --git a/media/java/android/media/ImageUtils.java b/media/java/android/media/ImageUtils.java index 89313bf30e319..c312525e0cd49 100644 --- a/media/java/android/media/ImageUtils.java +++ b/media/java/android/media/ImageUtils.java @@ -57,7 +57,7 @@ class ImageUtils { case ImageFormat.RAW_SENSOR: case ImageFormat.RAW10: return 1; - case PixelFormat.OPAQUE: + case ImageFormat.PRIVATE: return 0; default: throw new UnsupportedOperationException( @@ -70,10 +70,11 @@ class ImageUtils { * Copy source image data to destination Image. *

*

- * Only support the copy between two non-opaque images with same properties - * (format, size, etc.). The data from the source image will be copied to - * the byteBuffers from the destination Image starting from position zero, - * and the destination image will be rewound to zero after copy is done. + * Only support the copy between two non-{@link ImageFormat#PRIVATE PRIVATE} format + * images with same properties (format, size, etc.). The data from the + * source image will be copied to the byteBuffers from the destination Image + * starting from position zero, and the destination image will be rewound to + * zero after copy is done. *

* * @param src The source image to be copied from. @@ -88,8 +89,9 @@ class ImageUtils { if (src.getFormat() != dst.getFormat()) { throw new IllegalArgumentException("Src and dst images should have the same format"); } - if (src.isOpaque() || dst.isOpaque()) { - throw new IllegalArgumentException("Opaque image is not copyable"); + if (src.getFormat() == ImageFormat.PRIVATE || + dst.getFormat() == ImageFormat.PRIVATE) { + throw new IllegalArgumentException("PRIVATE format images are not copyable"); } if (!(dst.getOwner() instanceof ImageWriter)) { throw new IllegalArgumentException("Destination image is not from ImageWriter. Only" diff --git a/media/java/android/media/ImageWriter.java b/media/java/android/media/ImageWriter.java index f80533914b576..2ef251959a4d3 100644 --- a/media/java/android/media/ImageWriter.java +++ b/media/java/android/media/ImageWriter.java @@ -33,8 +33,8 @@ import java.util.List; /** *

* The ImageWriter class allows an application to produce Image data into a - * {@link android.view.Surface}, and have it be consumed by another component like - * {@link android.hardware.camera2.CameraDevice CameraDevice}. + * {@link android.view.Surface}, and have it be consumed by another component + * like {@link android.hardware.camera2.CameraDevice CameraDevice}. *

*

* Several Android API classes can provide input {@link android.view.Surface @@ -54,21 +54,21 @@ import java.util.List; *

* If the application already has an Image from {@link ImageReader}, the * application can directly queue this Image into ImageWriter (via - * {@link #queueInputImage}), potentially with zero buffer copies. For the opaque - * Images produced by an opaque ImageReader (created by - * {@link ImageReader#newOpaqueInstance}), this is the only way to send Image - * data to ImageWriter, as the Image data aren't accessible by the application. + * {@link #queueInputImage}), potentially with zero buffer copies. For the + * {@link ImageFormat#PRIVATE PRIVATE} format Images produced by + * {@link ImageReader}, this is the only way to send Image data to ImageWriter, + * as the Image data aren't accessible by the application. *

- * Once new input Images are queued into an ImageWriter, it's up to the downstream - * components (e.g. {@link ImageReader} or + * Once new input Images are queued into an ImageWriter, it's up to the + * downstream components (e.g. {@link ImageReader} or * {@link android.hardware.camera2.CameraDevice}) to consume the Images. If the * downstream components cannot consume the Images at least as fast as the - * ImageWriter production rate, the {@link #dequeueInputImage} call will eventually - * block and the application will have to drop input frames.

+ * ImageWriter production rate, the {@link #dequeueInputImage} call will + * eventually block and the application will have to drop input frames.

*/ public class ImageWriter implements AutoCloseable { private final Object mListenerLock = new Object(); - private ImageListener mListener; + private OnImageReleasedListener mListener; private ListenerHandler mListenerHandler; private long mNativeContext; @@ -168,32 +168,34 @@ public class ImageWriter implements AutoCloseable { * This call will block if all available input images have been queued by * the application and the downstream consumer has not yet consumed any. * When an Image is consumed by the downstream consumer and released, an - * {@link ImageListener#onInputImageReleased} callback will be fired, which - * indicates that there is one input Image available. For non-opaque formats - * (({@link ImageWriter#getFormat()} != {@link ImageFormat#PRIVATE})), it is + * {@link OnImageReleasedListener#onImageReleased} callback will be fired, + * which indicates that there is one input Image available. For non- + * {@link ImageFormat#PRIVATE PRIVATE} formats ( + * {@link ImageWriter#getFormat()} != {@link ImageFormat#PRIVATE}), it is * recommended to dequeue the next Image only after this callback is fired, * in the steady state. *

*

- * If the ImageWriter is opaque ({@link ImageWriter#getFormat()} == - * {@link ImageFormat#PRIVATE}), the image buffer is inaccessible to - * the application, and calling this method will result in an - * {@link IllegalStateException}. Instead, the application should acquire - * opaque images from some other component (e.g. an opaque + * If the format of ImageWriter is {@link ImageFormat#PRIVATE PRIVATE} ( + * {@link ImageWriter#getFormat()} == {@link ImageFormat#PRIVATE}), the + * image buffer is inaccessible to the application, and calling this method + * will result in an {@link IllegalStateException}. Instead, the application + * should acquire images from some other component (e.g. an * {@link ImageReader}), and queue them directly to this ImageWriter via the * {@link ImageWriter#queueInputImage queueInputImage()} method. *

* * @return The next available input Image from this ImageWriter. * @throws IllegalStateException if {@code maxImages} Images are currently - * dequeued, or the ImageWriter is opaque. + * dequeued, or the ImageWriter format is + * {@link ImageFormat#PRIVATE PRIVATE}. * @see #queueInputImage * @see Image#close */ public Image dequeueInputImage() { if (mWriterFormat == ImageFormat.PRIVATE) { throw new IllegalStateException( - "Opaque ImageWriter doesn't support this operation since opaque images are" + "PRIVATE format ImageWriter doesn't support this operation since the images are" + " inaccessible to the application!"); } @@ -229,10 +231,10 @@ public class ImageWriter implements AutoCloseable { *

*

* After this method is called and the downstream consumer consumes and - * releases the Image, an {@link ImageListener#onInputImageReleased - * onInputImageReleased()} callback will fire. The application can use this - * callback to avoid sending Images faster than the downstream consumer - * processing rate in steady state. + * releases the Image, an {@link OnImageReleasedListener#onImageReleased} + * callback will fire. The application can use this callback to avoid + * sending Images faster than the downstream consumer processing rate in + * steady state. *

*

* Passing in an Image from some other component (e.g. an @@ -271,12 +273,12 @@ public class ImageWriter implements AutoCloseable { } ImageReader prevOwner = (ImageReader) image.getOwner(); - // Only do the image attach for opaque images for now. Do the image + // Only do the image attach for PRIVATE format images for now. Do the image // copy for other formats. TODO: use attach for other formats to // improve the performance, and fall back to copy when attach/detach // fails. Right now, detach is guaranteed to fail as the buffer is // locked when ImageReader#acquireNextImage is called. See bug 19962027. - if (image.isOpaque()) { + if (image.getFormat() == ImageFormat.PRIVATE) { prevOwner.detachImage(image); attachAndQueueInputImage(image); // This clears the native reference held by the original owner. @@ -319,8 +321,9 @@ public class ImageWriter implements AutoCloseable { * Get the ImageWriter format. *

* This format may be different than the Image format returned by - * {@link Image#getFormat()}. However, if the ImageWriter is opaque (format - * == {@link ImageFormat#PRIVATE}) , the images from it will also be opaque. + * {@link Image#getFormat()}. However, if the ImageWriter format is + * {@link ImageFormat#PRIVATE PRIVATE}, calling {@link #dequeueInputImage()} + * will result in an {@link IllegalStateException}. *

* * @return The ImageWriter format. @@ -333,7 +336,7 @@ public class ImageWriter implements AutoCloseable { * ImageWriter callback interface, used to to asynchronously notify the * application of various ImageWriter events. */ - public interface ImageListener { + public interface OnImageReleasedListener { /** *

* Callback that is called when an input Image is released back to @@ -361,7 +364,7 @@ public class ImageWriter implements AutoCloseable { * @see ImageWriter * @see Image */ - void onInputImageReleased(ImageWriter writer); + void onImageReleased(ImageWriter writer); } /** @@ -375,7 +378,7 @@ public class ImageWriter implements AutoCloseable { * @throws IllegalArgumentException If no handler specified and the calling * thread has no looper. */ - public void setImageListener(ImageListener listener, Handler handler) { + public void setOnImageReleasedListener(OnImageReleasedListener listener, Handler handler) { synchronized (mListenerLock) { if (listener != null) { Looper looper = handler != null ? handler.getLooper() : Looper.myLooper(); @@ -408,7 +411,7 @@ public class ImageWriter implements AutoCloseable { */ @Override public void close() { - setImageListener(null, null); + setOnImageReleasedListener(null, null); for (Image image : mDequeuedImages) { image.close(); } @@ -431,19 +434,18 @@ public class ImageWriter implements AutoCloseable { * Attach and queue input Image to this ImageWriter. *

*

- * When an Image is from an opaque source (e.g. an opaque ImageReader - * created by {@link ImageReader#newOpaqueInstance}), or the source Image is - * so large that copying its data is too expensive, this method can be used - * to migrate the source Image into ImageWriter without a data copy, and - * then queue it to this ImageWriter. The source Image must be detached from - * its previous owner already, or this call will throw an + * When the format of an Image is {@link ImageFormat#PRIVATE PRIVATE}, or + * the source Image is so large that copying its data is too expensive, this + * method can be used to migrate the source Image into ImageWriter without a + * data copy, and then queue it to this ImageWriter. The source Image must + * be detached from its previous owner already, or this call will throw an * {@link IllegalStateException}. *

*

* After this call, the ImageWriter takes ownership of this Image. This * ownership will automatically be removed from this writer after the * consumer releases this Image, that is, after - * {@link ImageListener#onInputImageReleased}. The caller is responsible for + * {@link OnImageReleasedListener#onImageReleased}. The caller is responsible for * closing this Image through {@link Image#close()} to free up the resources * held by this Image. *

@@ -492,12 +494,12 @@ public class ImageWriter implements AutoCloseable { @Override public void handleMessage(Message msg) { - ImageListener listener; + OnImageReleasedListener listener; synchronized (mListenerLock) { listener = mListener; } if (listener != null) { - listener.onInputImageReleased(ImageWriter.this); + listener.onImageReleased(ImageWriter.this); } } } @@ -646,13 +648,6 @@ public class ImageWriter implements AutoCloseable { mTimestamp = timestamp; } - @Override - public boolean isOpaque() { - throwISEIfImageIsInvalid(); - - return getFormat() == ImageFormat.PRIVATE; - } - @Override public Plane[] getPlanes() { throwISEIfImageIsInvalid();