From 7bd4a2750253a2049a5ad0cff171eb4f6beb57ff Mon Sep 17 00:00:00 2001 From: "fischman@webrtc.org" Date: Tue, 4 Mar 2014 18:17:55 +0000 Subject: [PATCH] VideoCaptureAndroid: don't deliver frames after stopCapture(). Because stopCapture() and onPreviewFrame() are called on different threads, and are both synchronized, it's possible for onPreviewFrame() to commence execution after stopCapture() has completed, causing a SEGV because the native code is no longer prepared to accept frames. Clarify the contract around synchronized methods in this class to hopefully avoid similar bugs in future. BUG=2947 R=henrike@webrtc.org Review URL: https://webrtc-codereview.appspot.com/9339004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5639 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../videoengine/VideoCaptureAndroid.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/webrtc/modules/video_capture/android/java/src/org/webrtc/videoengine/VideoCaptureAndroid.java b/webrtc/modules/video_capture/android/java/src/org/webrtc/videoengine/VideoCaptureAndroid.java index 691531f873..dfe63adfe6 100644 --- a/webrtc/modules/video_capture/android/java/src/org/webrtc/videoengine/VideoCaptureAndroid.java +++ b/webrtc/modules/video_capture/android/java/src/org/webrtc/videoengine/VideoCaptureAndroid.java @@ -31,7 +31,9 @@ import android.view.SurfaceHolder.Callback; // the entry points to this class are all synchronized. This shouldn't present // a performance bottleneck because only onPreviewFrame() is called more than // once (and is called serially on a single thread), so the lock should be -// uncontended. +// uncontended. Note that each of these synchronized methods must check +// |camera| for null to account for having possibly waited for stopCapture() to +// complete. public class VideoCaptureAndroid implements PreviewCallback, Callback { private final static String TAG = "WEBRTC-JC"; @@ -149,7 +151,13 @@ public class VideoCaptureAndroid implements PreviewCallback, Callback { private native void ProvideCameraFrame( byte[] data, int length, long captureObject); - public synchronized void onPreviewFrame(byte[] data, Camera camera) { + public synchronized void onPreviewFrame(byte[] data, Camera callbackCamera) { + if (camera == null) { + return; + } + if (camera != callbackCamera) { + throw new RuntimeException("Unexpected camera in callback!"); + } ProvideCameraFrame(data, data.length, native_capturer); camera.addCallbackBuffer(data); } @@ -184,10 +192,11 @@ public class VideoCaptureAndroid implements PreviewCallback, Callback { public synchronized void surfaceCreated(SurfaceHolder holder) { Log.d(TAG, "VideoCaptureAndroid::surfaceCreated"); + if (camera == null) { + return; + } try { - if (camera != null) { - camera.setPreviewDisplay(holder); - } + camera.setPreviewDisplay(holder); } catch (IOException e) { throw new RuntimeException(e); } @@ -195,10 +204,11 @@ public class VideoCaptureAndroid implements PreviewCallback, Callback { public synchronized void surfaceDestroyed(SurfaceHolder holder) { Log.d(TAG, "VideoCaptureAndroid::surfaceDestroyed"); + if (camera == null) { + return; + } try { - if (camera != null) { - camera.setPreviewDisplay(null); - } + camera.setPreviewDisplay(null); } catch (IOException e) { throw new RuntimeException(e); }