From 3b93a85b6a7e1938527bb0986477743478be764d Mon Sep 17 00:00:00 2001
From: Yin-Chia Yeh <yinchiayeh@google.com>
Date: Mon, 9 Sep 2019 14:59:59 -0700
Subject: [PATCH] Camera: fix NPE in buffer error callback

Check if the error stream has been removed first.
Also move some code in submitCaptureRequest so that access to
mConfiguredOutputs is within scope of mInterfaceLock, which
matches all other places accessing mConfiguredOutputs in the file.

Test: Camera CTS
Bug: 140374093, 140527066
Change-Id: I84921eb3a555f67a006548865f34502ab978e33b
---
 .../camera2/impl/CameraDeviceImpl.java        | 56 +++++++++++--------
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
index cc8c182b867e2..06ced7c68467b 100644
--- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
@@ -1026,34 +1026,35 @@ public class CameraDeviceImpl extends CameraDevice
         // callback is valid
         executor = checkExecutor(executor, callback);
 
-        // Make sure that there all requests have at least 1 surface; all surfaces are non-null;
-        // the surface isn't a physical stream surface for reprocessing request
-        for (CaptureRequest request : requestList) {
-            if (request.getTargets().isEmpty()) {
-                throw new IllegalArgumentException(
-                        "Each request must have at least one Surface target");
-            }
+        synchronized(mInterfaceLock) {
+            checkIfCameraClosedOrInError();
 
-            for (Surface surface : request.getTargets()) {
-                if (surface == null) {
-                    throw new IllegalArgumentException("Null Surface targets are not allowed");
+            // Make sure that there all requests have at least 1 surface; all surfaces are non-null;
+            // the surface isn't a physical stream surface for reprocessing request
+            for (CaptureRequest request : requestList) {
+                if (request.getTargets().isEmpty()) {
+                    throw new IllegalArgumentException(
+                            "Each request must have at least one Surface target");
                 }
 
-                for (int i = 0; i < mConfiguredOutputs.size(); i++) {
-                    OutputConfiguration configuration = mConfiguredOutputs.valueAt(i);
-                    if (configuration.isForPhysicalCamera()
-                            && configuration.getSurfaces().contains(surface)) {
-                        if (request.isReprocess()) {
-                            throw new IllegalArgumentException(
-                                    "Reprocess request on physical stream is not allowed");
+                for (Surface surface : request.getTargets()) {
+                    if (surface == null) {
+                        throw new IllegalArgumentException("Null Surface targets are not allowed");
+                    }
+
+                    for (int i = 0; i < mConfiguredOutputs.size(); i++) {
+                        OutputConfiguration configuration = mConfiguredOutputs.valueAt(i);
+                        if (configuration.isForPhysicalCamera()
+                                && configuration.getSurfaces().contains(surface)) {
+                            if (request.isReprocess()) {
+                                throw new IllegalArgumentException(
+                                        "Reprocess request on physical stream is not allowed");
+                            }
                         }
                     }
                 }
             }
-        }
 
-        synchronized(mInterfaceLock) {
-            checkIfCameraClosedOrInError();
             if (repeating) {
                 stopRepeating();
             }
@@ -2343,14 +2344,21 @@ public class CameraDeviceImpl extends CameraDevice
             if (errorCode == ERROR_CAMERA_BUFFER) {
                 // Because 1 stream id could map to multiple surfaces, we need to specify both
                 // streamId and surfaceId.
-                List<Surface> surfaces =
-                        mConfiguredOutputs.get(resultExtras.getErrorStreamId()).getSurfaces();
-                for (Surface surface : surfaces) {
+                OutputConfiguration config = mConfiguredOutputs.get(
+                        resultExtras.getErrorStreamId());
+                if (config == null) {
+                    Log.v(TAG, String.format(
+                            "Stream %d has been removed. Skipping buffer lost callback",
+                            resultExtras.getErrorStreamId()));
+                    return;
+                }
+                for (Surface surface : config.getSurfaces()) {
                     if (!request.containsTarget(surface)) {
                         continue;
                     }
                     if (DEBUG) {
-                        Log.v(TAG, String.format("Lost output buffer reported for frame %d, target %s",
+                        Log.v(TAG, String.format(
+                                "Lost output buffer reported for frame %d, target %s",
                                 frameNumber, surface));
                     }
                     failureDispatch = new Runnable() {