Don't crash if camera is reopened or surface is recreated.

briar-android/src/org/briarproject/android/keyagreement/ShowQrCodeFragment.java

@@ -6,7 +6,6 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.graphics.Bitmap;
-import android.hardware.Camera;
 import android.os.AsyncTask;
 import android.os.Bundle;
 import android.support.annotation.Nullable;
@@ -58,7 +57,6 @@ import static android.view.View.INVISIBLE;
 import static android.view.View.VISIBLE;
 import static android.widget.Toast.LENGTH_LONG;
 import static java.util.logging.Level.INFO;
-import static java.util.logging.Level.WARNING;
 
 public class ShowQrCodeFragment extends BaseEventFragment
 		implements QrCodeDecoder.ResultCallback {
@@ -86,7 +84,6 @@ public class ShowQrCodeFragment extends BaseEventFragment
 	private ViewGroup mainProgressContainer;
 
 	private BluetoothStateReceiver receiver;
-	private QrCodeDecoder decoder;
 	private boolean gotRemotePayload, waitingForBluetooth;
 	private KeyAgreementTask task;
 
@@ -136,8 +133,7 @@ public class ShowQrCodeFragment extends BaseEventFragment
 		super.onActivityCreated(savedInstanceState);
 
 		getActivity().setRequestedOrientation(SCREEN_ORIENTATION_NOSENSOR);
-
-		decoder = new QrCodeDecoder(this);
+		cameraView.setPreviewConsumer(new QrCodeDecoder(this));
 	}
 
 	@Override
@@ -163,8 +159,7 @@ public class ShowQrCodeFragment extends BaseEventFragment
 		} else {
 			startListening();
 		}
-
-		openCamera();
+		cameraView.start();
 	}
 
 	@Override
@@ -172,7 +167,7 @@ public class ShowQrCodeFragment extends BaseEventFragment
 		super.onStop();
 		stopListening();
 		if (receiver != null) getActivity().unregisterReceiver(receiver);
-		releaseCamera();
+		cameraView.stop();
 	}
 
 	@UiThread
@@ -200,45 +195,11 @@ public class ShowQrCodeFragment extends BaseEventFragment
 		});
 	}
 
-	@SuppressWarnings("deprecation")
-	@UiThread
-	private void openCamera() {
-		LOG.info("Opening camera");
-		Camera camera;
-		try {
-			camera = Camera.open();
-		} catch (RuntimeException e) {
-			LOG.log(WARNING, e.toString(), e);
-			camera = null;
-		}
-		if (camera == null) {
-			LOG.log(WARNING, "Error opening camera");
-			Toast.makeText(getActivity(), R.string.could_not_open_camera,
-					LENGTH_LONG).show();
-			finish();
-			return;
-		}
-		cameraView.start(camera, decoder, 0);
-	}
-
-	@UiThread
-	private void releaseCamera() {
-		LOG.info("Releasing camera");
-		try {
-			cameraView.stop();
-		} catch (RuntimeException e) {
-			LOG.log(WARNING, "Error releasing camera", e);
-			// TODO better solution
-			finish();
-		}
-	}
-
 	@UiThread
 	private void reset() {
 		statusView.setVisibility(INVISIBLE);
 		cameraView.setVisibility(VISIBLE);
 		gotRemotePayload = false;
-		cameraView.startConsumer();
 		startListening();
 	}
 
@@ -380,7 +341,6 @@ public class ShowQrCodeFragment extends BaseEventFragment
 				LOG.info("Got result from decoder");
 				if (!gotRemotePayload) {
 					gotRemotePayload = true;
-					cameraView.stopConsumer();
 					qrCodeScanned(result.getText());
 				}
 			}

briar-android/src/org/briarproject/android/util/QrCodeDecoder.java

@@ -28,7 +28,7 @@ public class QrCodeDecoder implements PreviewConsumer, PreviewCallback {
 	private final Reader reader = new QRCodeReader();
 	private final ResultCallback callback;
 
-	private boolean stopped = false;
+	private Camera camera = null;
 
 	public QrCodeDecoder(ResultCallback callback) {
 		this.callback = callback;
@@ -36,37 +36,33 @@ public class QrCodeDecoder implements PreviewConsumer, PreviewCallback {
 
 	@Override
 	public void start(Camera camera) {
-		stopped = false;
-		askForPreviewFrame(camera);
+		this.camera = camera;
+		askForPreviewFrame();
 	}
 
 	@Override
 	public void stop() {
-		stopped = true;
+		camera = null;
 	}
 
 	@UiThread
-	private void askForPreviewFrame(Camera camera) {
-		if (!stopped) camera.setOneShotPreviewCallback(this);
+	private void askForPreviewFrame() {
+		if (camera != null) camera.setOneShotPreviewCallback(this);
 	}
 
 	@UiThread
 	@Override
 	public void onPreviewFrame(byte[] data, Camera camera) {
-		if (!stopped) {
-			Size size = camera.getParameters().getPreviewSize();
-			new DecoderTask(camera, data, size.width, size.height).execute();
-		}
+		Size size = camera.getParameters().getPreviewSize();
+		new DecoderTask(data, size.width, size.height).execute();
 	}
 
 	private class DecoderTask extends AsyncTask<Void, Void, Void> {
 
-		private final Camera camera;
 		private final byte[] data;
 		private final int width, height;
 
-		DecoderTask(Camera camera, byte[] data, int width, int height) {
-			this.camera = camera;
+		DecoderTask(byte[] data, int width, int height) {
 			this.data = data;
 			this.width = width;
 			this.height = height;
@@ -84,7 +80,7 @@ public class QrCodeDecoder implements PreviewConsumer, PreviewCallback {
 			} catch (ReaderException e) {
 				return null; // No barcode found
 			} catch (RuntimeException e) {
-				return null; // Decoding failed due to bug in decoder
+				return null; // Preview data did not match width and height
 			} finally {
 				reader.reset();
 			}
@@ -97,7 +93,7 @@ public class QrCodeDecoder implements PreviewConsumer, PreviewCallback {
 
 		@Override
 		protected void onPostExecute(Void result) {
-			askForPreviewFrame(camera);
+			askForPreviewFrame();
 		}
 	}
 

briar-android/src/org/briarproject/android/view/CameraView.java

@@ -14,6 +14,8 @@ import android.view.SurfaceHolder;
 import android.view.SurfaceView;
 
 import org.briarproject.android.util.PreviewConsumer;
+import org.briarproject.api.nullsafety.MethodsNotNullByDefault;
+import org.briarproject.api.nullsafety.ParametersNotNullByDefault;
 
 import java.io.IOException;
 import java.util.List;
@@ -33,6 +35,8 @@ import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 
 @SuppressWarnings("deprecation")
+@MethodsNotNullByDefault
+@ParametersNotNullByDefault
 public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 		AutoFocusCallback {
 
@@ -44,7 +48,7 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 	private PreviewConsumer previewConsumer = null;
 	private Surface surface = null;
 	private int displayOrientation = 0, surfaceWidth = 0, surfaceHeight = 0;
-	private boolean autoFocus = false;
+	private boolean previewStarted = false, autoFocus = false;
 
 	public CameraView(Context context) {
 		super(context);
@@ -58,6 +62,12 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 		super(context, attrs, defStyleAttr);
 	}
 
+	@UiThread
+	public void setPreviewConsumer(PreviewConsumer previewConsumer) {
+		LOG.info("Setting preview consumer");
+		this.previewConsumer = previewConsumer;
+	}
+
 	@Override
 	protected void onAttachedToWindow() {
 		super.onAttachedToWindow();
@@ -70,15 +80,18 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 		super.onDetachedFromWindow();
 		setKeepScreenOn(false);
 		getHolder().removeCallback(this);
-		if (surface != null) surface.release();
 	}
 
 	@UiThread
-	public void start(Camera camera, PreviewConsumer previewConsumer,
-			int rotationDegrees) {
-		this.camera = camera;
-		this.previewConsumer = previewConsumer;
-		setDisplayOrientation(rotationDegrees);
+	public void start() {
+		try {
+			LOG.info("Opening camera");
+			camera = Camera.open();
+		} catch (RuntimeException e) {
+			LOG.log(WARNING, "Error opening camera", e);
+			return;
+		}
+		setDisplayOrientation(0);
 		// Use barcode scene mode if it's available
 		Parameters params = camera.getParameters();
 		params = setSceneMode(camera, params);
@@ -96,14 +109,17 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 		enableAutoFocus(params.getFocusMode());
 		// Log the parameters that are being used (maybe not what we asked for)
 		logCameraParameters();
-		if (surface != null) startPreview(getHolder());
+		// Start the preview when the camera and the surface are both ready
+		if (surface != null && !previewStarted) startPreview(getHolder());
 	}
 
 	@UiThread
 	public void stop() {
+		if (camera == null) return;
 		stopPreview();
 		try {
-			if (camera != null) camera.release();
+			LOG.info("Releasing camera");
+			camera.release();
 		} catch (RuntimeException e) {
 			LOG.log(WARNING, "Error releasing camera", e);
 		}
@@ -112,9 +128,11 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 
 	@UiThread
 	private void startPreview(SurfaceHolder holder) {
+		LOG.info("Starting preview");
 		try {
 			camera.setPreviewDisplay(holder);
 			camera.startPreview();
+			previewStarted = true;
 			startConsumer();
 		} catch (IOException | RuntimeException e) {
 			LOG.log(WARNING, "Error starting camera preview", e);
@@ -123,24 +141,26 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 
 	@UiThread
 	private void stopPreview() {
+		LOG.info("Stopping preview");
 		try {
 			stopConsumer();
-			if (camera != null) camera.stopPreview();
+			camera.stopPreview();
 		} catch (RuntimeException e) {
 			LOG.log(WARNING, "Error stopping camera preview", e);
 		}
+		previewStarted = false;
 	}
 
 	@UiThread
-	public void startConsumer() {
+	private void startConsumer() {
 		if (autoFocus) camera.autoFocus(this);
 		previewConsumer.start(camera);
 	}
 
 	@UiThread
-	public void stopConsumer() {
-		if (previewConsumer != null) previewConsumer.stop();
+	private void stopConsumer() {
 		if (autoFocus) camera.cancelAutoFocus();
+		previewConsumer.stop();
 	}
 
 	@UiThread
@@ -295,9 +315,13 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 	@UiThread
 	private void surfaceCreatedUi(SurfaceHolder holder) {
 		LOG.info("Surface created");
-		if (surface != null) throw new IllegalStateException();
+		if (surface != null && surface != holder.getSurface()) {
+			LOG.info("Releasing old surface");
+			surface.release();
+		}
 		surface = holder.getSurface();
-		if (camera != null) startPreview(holder);
+		// Start the preview when the camera and the surface are both ready
+		if (camera != null && !previewStarted) startPreview(holder);
 	}
 
 	@Override
@@ -314,9 +338,10 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 	@UiThread
 	private void surfaceChangedUi(SurfaceHolder holder, int w, int h) {
 		if (LOG.isLoggable(INFO)) LOG.info("Surface changed: " + w + "x" + h);
-		// Release the previous surface if necessary
-		if (surface != null && surface != holder.getSurface())
+		if (surface != null && surface != holder.getSurface()) {
+			LOG.info("Releasing old surface");
 			surface.release();
+		}
 		surface = holder.getSurface();
 		surfaceWidth = w;
 		surfaceHeight = h;
@@ -346,8 +371,12 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback,
 	@UiThread
 	private void surfaceDestroyedUi(SurfaceHolder holder) {
 		LOG.info("Surface destroyed");
-		if (holder.getSurface() != surface) throw new IllegalStateException();
-		if (surface != null) surface.release();
+		if (surface != null && surface != holder.getSurface()) {
+			LOG.info("Releasing old surface");
+			surface.release();
+		}
+		surface = null;
+		holder.getSurface().release();
 	}
 
 	@Override