Merge "Camera: Add proper buffer drop errors to LEGACY mode" into nyc-dev

6 files changed, 101 insertions, 43 deletions

diff --git a/core/java/android/hardware/camera2/legacy/CameraDeviceState.java b/core/java/android/hardware/camera2/legacy/CameraDeviceState.java
index 2c2ad1c23ce6..b0b94e3ac009 100644
--- a/core/java/android/hardware/camera2/legacy/CameraDeviceState.java
+++ b/core/java/android/hardware/camera2/legacy/CameraDeviceState.java
@@ -70,7 +70,7 @@ public class CameraDeviceState {
      * CameraDeviceStateListener callbacks to be called after state transitions.
      */
     public interface CameraDeviceStateListener {
-        void onError(int errorCode, RequestHolder holder);
+        void onError(int errorCode, Object errorArg, RequestHolder holder);
         void onConfiguring();
         void onIdle();
         void onBusy();
@@ -162,11 +162,12 @@ public class CameraDeviceState {
      * @param captureError Report a recoverable error for a single buffer or result using a valid
      *                     error code for {@code ICameraDeviceCallbacks}, or
      *                     {@link #NO_CAPTURE_ERROR}.
+     * @param captureErrorArg An argument for some error captureError codes.
      * @return {@code false} if an error has occurred.
      */
     public synchronized boolean setCaptureResult(final RequestHolder request,
-                                             final CameraMetadataNative result,
-                                             final int captureError) {
+            final CameraMetadataNative result,
+            final int captureError, final Object captureErrorArg) {
         if (mCurrentState != STATE_CAPTURING) {
             Log.e(TAG, "Cannot receive result while in state: " + mCurrentState);
             mCurrentError = CameraDeviceImpl.CameraDeviceCallbacks.ERROR_CAMERA_DEVICE;
@@ -179,7 +180,7 @@ public class CameraDeviceState {
                 mCurrentHandler.post(new Runnable() {
                     @Override
                     public void run() {
-                        mCurrentListener.onError(captureError, request);
+                        mCurrentListener.onError(captureError, captureErrorArg, request);
                     }
                 });
             } else {
@@ -194,6 +195,11 @@ public class CameraDeviceState {
         return mCurrentError == NO_CAPTURE_ERROR;
     }
 
+    public synchronized boolean setCaptureResult(final RequestHolder request,
+            final CameraMetadataNative result) {
+        return setCaptureResult(request, result, NO_CAPTURE_ERROR, /*errorArg*/null);
+    }
+
     /**
      * Set the listener for state transition callbacks.
      *
@@ -239,7 +245,7 @@ public class CameraDeviceState {
                     mCurrentHandler.post(new Runnable() {
                         @Override
                         public void run() {
-                            mCurrentListener.onError(mCurrentError, mCurrentRequest);
+                            mCurrentListener.onError(mCurrentError, /*errorArg*/null, mCurrentRequest);
                         }
                     });
                 }
@@ -299,7 +305,7 @@ public class CameraDeviceState {
                         mCurrentHandler.post(new Runnable() {
                             @Override
                             public void run() {
-                                mCurrentListener.onError(error, mCurrentRequest);
+                                mCurrentListener.onError(error, /*errorArg*/null, mCurrentRequest);
                             }
                         });
                     } else {
diff --git a/core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java b/core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java
index d01c275e74dc..f99928a884ee 100644
--- a/core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java
+++ b/core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java
@@ -480,19 +480,15 @@ public class CameraDeviceUserShim implements ICameraDeviceUser {
             throw new ServiceSpecificException(ICameraService.ERROR_DISCONNECTED, err);
         }
 
-        ArrayList<Surface> surfaces = null;
+        SparseArray<Surface> surfaces = null;
         synchronized(mConfigureLock) {
             if (!mConfiguring) {
                 String err = "Cannot end configure, no configuration change in progress.";
                 Log.e(TAG, err);
                 throw new ServiceSpecificException(ICameraService.ERROR_INVALID_OPERATION, err);
             }
-            int numSurfaces = mSurfaces.size();
-            if (numSurfaces > 0) {
-                surfaces = new ArrayList<>();
-                for (int i = 0; i < numSurfaces; ++i) {
-                    surfaces.add(mSurfaces.valueAt(i));
-                }
+            if (mSurfaces != null) {
+                surfaces = mSurfaces.clone();
             }
             mConfiguring = false;
         }
diff --git a/core/java/android/hardware/camera2/legacy/CaptureCollector.java b/core/java/android/hardware/camera2/legacy/CaptureCollector.java
index eb48a01ac103..113927c4c4b2 100644
--- a/core/java/android/hardware/camera2/legacy/CaptureCollector.java
+++ b/core/java/android/hardware/camera2/legacy/CaptureCollector.java
@@ -19,7 +19,7 @@ import android.hardware.camera2.impl.CameraDeviceImpl;
 import android.util.Log;
 import android.util.MutableLong;
 import android.util.Pair;
-
+import android.view.Surface;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.TreeSet;
@@ -95,22 +95,28 @@ public class CaptureCollector {
                     } else {
                         // Send buffer dropped errors for each pending buffer if the request has
                         // started.
-                        if (mFailedPreview) {
-                            Log.w(TAG, "Preview buffers dropped for request: " +
-                                    mRequest.getRequestId());
-                            for (int i = 0; i < mRequest.numPreviewTargets(); i++) {
-                                CaptureCollector.this.mDeviceState.setCaptureResult(mRequest,
-                                    /*result*/null,
-                                        CameraDeviceImpl.CameraDeviceCallbacks.ERROR_CAMERA_BUFFER);
-                            }
-                        }
-                        if (mFailedJpeg) {
-                            Log.w(TAG, "Jpeg buffers dropped for request: " +
-                                    mRequest.getRequestId());
-                            for (int i = 0; i < mRequest.numJpegTargets(); i++) {
-                                CaptureCollector.this.mDeviceState.setCaptureResult(mRequest,
-                                    /*result*/null,
-                                        CameraDeviceImpl.CameraDeviceCallbacks.ERROR_CAMERA_BUFFER);
+                        for (Surface targetSurface : mRequest.getRequest().getTargets() ) {
+                            try {
+                                if (mRequest.jpegType(targetSurface)) {
+                                    if (mFailedJpeg) {
+                                        CaptureCollector.this.mDeviceState.setCaptureResult(mRequest,
+                                                /*result*/null,
+                                                CameraDeviceImpl.CameraDeviceCallbacks.
+                                                        ERROR_CAMERA_BUFFER,
+                                                targetSurface);
+                                    }
+                                } else {
+                                    // preview buffer
+                                    if (mFailedPreview) {
+                                        CaptureCollector.this.mDeviceState.setCaptureResult(mRequest,
+                                                /*result*/null,
+                                                CameraDeviceImpl.CameraDeviceCallbacks.
+                                                        ERROR_CAMERA_BUFFER,
+                                                targetSurface);
+                                    }
+                                }
+                            } catch (LegacyExceptionUtils.BufferQueueAbandonedException e) {
+                                Log.e(TAG, "Unexpected exception when querying Surface: " + e);
                             }
                         }
                     }
diff --git a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
index 661edd734a71..6c9586986b97 100644
--- a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
+++ b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
@@ -36,6 +36,7 @@ import android.os.ServiceSpecificException;
 import android.util.Log;
 import android.util.Pair;
 import android.util.Size;
+import android.util.SparseArray;
 import android.view.Surface;
 
 import java.util.ArrayList;
@@ -64,7 +65,7 @@ public class LegacyCameraDevice implements AutoCloseable {
     private final CameraCharacteristics mStaticCharacteristics;
     private final ICameraDeviceCallbacks mDeviceCallbacks;
     private final CameraDeviceState mDeviceState = new CameraDeviceState();
-    private List<Surface> mConfiguredSurfaces;
+    private SparseArray<Surface> mConfiguredSurfaces;
     private boolean mClosed = false;
 
     private final ConditionVariable mIdle = new ConditionVariable(/*open*/true);
@@ -89,13 +90,29 @@ public class LegacyCameraDevice implements AutoCloseable {
     public static final int NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW = 1;
 
     private CaptureResultExtras getExtrasFromRequest(RequestHolder holder) {
+        return getExtrasFromRequest(holder,
+                /*errorCode*/CameraDeviceState.NO_CAPTURE_ERROR, /*errorArg*/null);
+    }
+
+    private CaptureResultExtras getExtrasFromRequest(RequestHolder holder,
+            int errorCode, Object errorArg) {
+        int errorStreamId = -1;
+        if (errorCode == CameraDeviceImpl.CameraDeviceCallbacks.ERROR_CAMERA_BUFFER) {
+            Surface errorTarget = (Surface) errorArg;
+            int indexOfTarget = mConfiguredSurfaces.indexOfValue(errorTarget);
+            if (indexOfTarget < 0) {
+                Log.e(TAG, "Buffer drop error reported for unknown Surface");
+            } else {
+                errorStreamId = mConfiguredSurfaces.keyAt(indexOfTarget);
+            }
+        }
         if (holder == null) {
             return new CaptureResultExtras(ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE,
                     ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE);
         }
         return new CaptureResultExtras(holder.getRequestId(), holder.getSubsequeceId(),
                 /*afTriggerId*/0, /*precaptureTriggerId*/0, holder.getFrameNumber(),
-                /*partialResultCount*/1, /*errorStreamId*/-1);
+                /*partialResultCount*/1, errorStreamId);
     }
 
     /**
@@ -105,9 +122,9 @@ public class LegacyCameraDevice implements AutoCloseable {
     private final CameraDeviceState.CameraDeviceStateListener mStateListener =
             new CameraDeviceState.CameraDeviceStateListener() {
         @Override
-        public void onError(final int errorCode, final RequestHolder holder) {
+        public void onError(final int errorCode, final Object errorArg, final RequestHolder holder) {
             if (DEBUG) {
-                Log.d(TAG, "onError called, errorCode = " + errorCode);
+                Log.d(TAG, "onError called, errorCode = " + errorCode + ", errorArg = " + errorArg);
             }
             switch (errorCode) {
                 /*
@@ -125,7 +142,7 @@ public class LegacyCameraDevice implements AutoCloseable {
                 }
             }
 
-            final CaptureResultExtras extras = getExtrasFromRequest(holder);
+            final CaptureResultExtras extras = getExtrasFromRequest(holder, errorCode, errorArg);
             mResultHandler.post(new Runnable() {
                 @Override
                 public void run() {
@@ -281,14 +298,17 @@ public class LegacyCameraDevice implements AutoCloseable {
      *
      * <p>Every surface in {@code outputs} must be non-{@code null}.</p>
      *
-     * @param outputs a list of surfaces to set.
+     * @param outputs a list of surfaces to set. LegacyCameraDevice will take ownership of this
+     *          list; it must not be modified by the caller once it's passed in.
      * @return an error code for this binder operation, or {@link NO_ERROR}
      *          on success.
      */
-    public int configureOutputs(List<Surface> outputs) {
+    public int configureOutputs(SparseArray<Surface> outputs) {
         List<Pair<Surface, Size>> sizedSurfaces = new ArrayList<>();
         if (outputs != null) {
-            for (Surface output : outputs) {
+            int count = outputs.size();
+            for (int i = 0; i < count; i++)  {
+                Surface output = outputs.valueAt(i);
                 if (output == null) {
                     Log.e(TAG, "configureOutputs - null outputs are not allowed");
                     return BAD_VALUE;
@@ -353,7 +373,7 @@ public class LegacyCameraDevice implements AutoCloseable {
         }
 
         if (success) {
-            mConfiguredSurfaces = outputs != null ? new ArrayList<>(outputs) : null;
+            mConfiguredSurfaces = outputs;
         } else {
             return LegacyExceptionUtils.INVALID_OPERATION;
         }
@@ -659,6 +679,23 @@ public class LegacyCameraDevice implements AutoCloseable {
         return nativeGetSurfaceId(surface);
     }
 
+    static List<Long> getSurfaceIds(SparseArray<Surface> surfaces) {
+        if (surfaces == null) {
+            throw new NullPointerException("Null argument surfaces");
+        }
+        List<Long> surfaceIds = new ArrayList<>();
+        int count = surfaces.size();
+        for (int i = 0; i < count; i++) {
+            long id = getSurfaceId(surfaces.valueAt(i));
+            if (id == 0) {
+                throw new IllegalStateException(
+                        "Configured surface had null native GraphicBufferProducer pointer!");
+            }
+            surfaceIds.add(id);
+        }
+        return surfaceIds;
+    }
+
     static List<Long> getSurfaceIds(Collection<Surface> surfaces) {
         if (surfaces == null) {
             throw new NullPointerException("Null argument surfaces");
diff --git a/core/java/android/hardware/camera2/legacy/RequestHolder.java b/core/java/android/hardware/camera2/legacy/RequestHolder.java
index 9b628fb72490..476c3debc571 100644
--- a/core/java/android/hardware/camera2/legacy/RequestHolder.java
+++ b/core/java/android/hardware/camera2/legacy/RequestHolder.java
@@ -41,6 +41,8 @@ public class RequestHolder {
     private final int mNumPreviewTargets;
     private volatile boolean mFailed = false;
 
+    private final Collection<Long> mJpegSurfaceIds;
+
     /**
      * A builder class for {@link RequestHolder} objects.
      *
@@ -150,13 +152,13 @@ public class RequestHolder {
          */
         public RequestHolder build(long frameNumber) {
             return new RequestHolder(mRequestId, mSubsequenceId, mRequest, mRepeating, frameNumber,
-                    mNumJpegTargets, mNumPreviewTargets);
+                    mNumJpegTargets, mNumPreviewTargets, mJpegSurfaceIds);
         }
     }
 
     private RequestHolder(int requestId, int subsequenceId, CaptureRequest request,
                           boolean repeating, long frameNumber, int numJpegTargets,
-                          int numPreviewTargets) {
+                          int numPreviewTargets, Collection<Long> jpegSurfaceIds) {
         mRepeating = repeating;
         mRequest = request;
         mRequestId = requestId;
@@ -164,6 +166,7 @@ public class RequestHolder {
         mFrameNumber = frameNumber;
         mNumJpegTargets = numJpegTargets;
         mNumPreviewTargets = numPreviewTargets;
+        mJpegSurfaceIds = jpegSurfaceIds;
     }
 
     /**
@@ -238,6 +241,17 @@ public class RequestHolder {
     }
 
     /**
+     * Returns true if the given surface requires jpeg buffers.
+     *
+     * @param s a {@link android.view.Surface} to check.
+     * @return true if the surface requires a jpeg buffer.
+     */
+    public boolean jpegType(Surface s)
+            throws LegacyExceptionUtils.BufferQueueAbandonedException {
+        return LegacyCameraDevice.containsSurfaceId(s, mJpegSurfaceIds);
+    }
+
+    /**
      * Mark this request as failed.
      */
     public void failRequest() {
diff --git a/core/java/android/hardware/camera2/legacy/RequestThreadManager.java b/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
index e8ce3ec7aaa9..a3fdd56cc65c 100644
--- a/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
+++ b/core/java/android/hardware/camera2/legacy/RequestThreadManager.java
@@ -908,8 +908,7 @@ public class RequestThreadManager {
                         mFaceDetectMapper.mapResultFaces(result, mLastRequest);
 
                         if (!holder.requestFailed()) {
-                            mDeviceState.setCaptureResult(holder, result,
-                                    CameraDeviceState.NO_CAPTURE_ERROR);
+                            mDeviceState.setCaptureResult(holder, result);
                         }
                     }
                     if (DEBUG) {