Call java SurfaceTextureHelper.dispose from the corresponding C++ destructor.

This makes it clearer that the C++ SurfaceTextureHelper owns its associated java object it.

In addition, arrange so that the SurfaceTextureHelper.stopListening
method (in java) can be called from any thread.

BUG=

Review-Url: https://codereview.webrtc.org/1988043002
Cr-Commit-Position: refs/heads/master@{#12941}
This commit is contained in:
nisse
2016-05-27 00:27:59 -07:00
committed by Commit bot
parent 756f395cbf
commit a44e72c44f
7 changed files with 60 additions and 53 deletions

View File

@ -272,17 +272,6 @@ public final class SurfaceTextureHelperTest extends ActivityTestCase {
surfaceTextureHelper.dispose(); surfaceTextureHelper.dispose();
} }
// Helper method to call stopListening() on correct thread.
private static void stopListeningOnHandlerThread(final SurfaceTextureHelper surfaceTextureHelper)
throws InterruptedException {
ThreadUtils.invokeUninterruptibly(surfaceTextureHelper.getHandler(), new Runnable() {
@Override
public void run() {
surfaceTextureHelper.stopListening();
}
});
}
/** /**
* Call stopListening(), but keep trying to produce more texture frames. No frames should be * Call stopListening(), but keep trying to produce more texture frames. No frames should be
* delivered to the listener. * delivered to the listener.
@ -308,7 +297,7 @@ public final class SurfaceTextureHelperTest extends ActivityTestCase {
surfaceTextureHelper.returnTextureFrame(); surfaceTextureHelper.returnTextureFrame();
// Stop listening - we should not receive any textures after this. // Stop listening - we should not receive any textures after this.
stopListeningOnHandlerThread(surfaceTextureHelper); surfaceTextureHelper.stopListening();
// Draw one frame. // Draw one frame.
GLES20.glClear(GLES20.GL_COLOR_BUFFER_BIT); GLES20.glClear(GLES20.GL_COLOR_BUFFER_BIT);
@ -330,7 +319,7 @@ public final class SurfaceTextureHelperTest extends ActivityTestCase {
"SurfaceTextureHelper test" /* threadName */, null); "SurfaceTextureHelper test" /* threadName */, null);
final MockTextureListener listener = new MockTextureListener(); final MockTextureListener listener = new MockTextureListener();
surfaceTextureHelper.startListening(listener); surfaceTextureHelper.startListening(listener);
stopListeningOnHandlerThread(surfaceTextureHelper); surfaceTextureHelper.stopListening();
surfaceTextureHelper.dispose(); surfaceTextureHelper.dispose();
} }
@ -362,10 +351,13 @@ public final class SurfaceTextureHelperTest extends ActivityTestCase {
stopListeningBarrier.countDown(); stopListeningBarrier.countDown();
stopListeningBarrierDone.await(); stopListeningBarrierDone.await();
// Wait until handler thread is idle to try to catch late startListening() call. // Wait until handler thread is idle to try to catch late startListening() call.
ThreadUtils.invokeUninterruptibly(surfaceTextureHelper.getHandler(), new Runnable() { final CountDownLatch barrier = new CountDownLatch(1);
@Override surfaceTextureHelper.getHandler().post(new Runnable() {
public void run() {} @Override public void run() {
barrier.countDown();
}
}); });
ThreadUtils.awaitUninterruptibly(barrier);
// Previous startListening() call should never have taken place and it should be ok to call it // Previous startListening() call should never have taken place and it should be ok to call it
// again. // again.
surfaceTextureHelper.startListening(listener); surfaceTextureHelper.startListening(listener);
@ -397,7 +389,7 @@ public final class SurfaceTextureHelperTest extends ActivityTestCase {
surfaceTextureHelper.returnTextureFrame(); surfaceTextureHelper.returnTextureFrame();
// Stop listening - |listener1| should not receive any textures after this. // Stop listening - |listener1| should not receive any textures after this.
stopListeningOnHandlerThread(surfaceTextureHelper); surfaceTextureHelper.stopListening();
// Connect different listener. // Connect different listener.
final MockTextureListener listener2 = new MockTextureListener(); final MockTextureListener listener2 = new MockTextureListener();

View File

@ -63,7 +63,7 @@ class SurfaceTextureHelper {
// http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/5.1.1_r1/android/graphics/SurfaceTexture.java#195. // http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/5.1.1_r1/android/graphics/SurfaceTexture.java#195.
// Therefore, in order to control the callback thread on API lvl < 21, the SurfaceTextureHelper // Therefore, in order to control the callback thread on API lvl < 21, the SurfaceTextureHelper
// is constructed on the |handler| thread. // is constructed on the |handler| thread.
return ThreadUtils.invokeUninterruptibly(handler, new Callable<SurfaceTextureHelper>() { return ThreadUtils.invokeAtFrontUninterruptibly(handler, new Callable<SurfaceTextureHelper>() {
@Override @Override
public SurfaceTextureHelper call() { public SurfaceTextureHelper call() {
try { try {
@ -373,17 +373,18 @@ class SurfaceTextureHelper {
/** /**
* Stop listening. The listener set in startListening() is guaranteded to not receive any more * Stop listening. The listener set in startListening() is guaranteded to not receive any more
* onTextureFrameAvailable() callbacks after this function returns. This function must be called * onTextureFrameAvailable() callbacks after this function returns.
* on the getHandler() thread.
*/ */
public void stopListening() { public void stopListening() {
if (handler.getLooper().getThread() != Thread.currentThread()) {
throw new IllegalStateException("Wrong thread.");
}
Logging.d(TAG, "stopListening()"); Logging.d(TAG, "stopListening()");
handler.removeCallbacks(setListenerRunnable); handler.removeCallbacks(setListenerRunnable);
this.listener = null; ThreadUtils.invokeAtFrontUninterruptibly(handler, new Runnable() {
this.pendingListener = null; @Override
public void run() {
listener = null;
pendingListener = null;
}
});
} }
/** /**
@ -431,24 +432,15 @@ class SurfaceTextureHelper {
*/ */
public void dispose() { public void dispose() {
Logging.d(TAG, "dispose()"); Logging.d(TAG, "dispose()");
if (handler.getLooper().getThread() == Thread.currentThread()) { ThreadUtils.invokeAtFrontUninterruptibly(handler, new Runnable() {
@Override
public void run() {
isQuitting = true; isQuitting = true;
if (!isTextureInUse) { if (!isTextureInUse) {
release(); release();
} }
return;
}
final CountDownLatch barrier = new CountDownLatch(1);
handler.postAtFrontOfQueue(new Runnable() {
@Override public void run() {
isQuitting = true;
barrier.countDown();
if (!isTextureInUse) {
release();
}
} }
}); });
ThreadUtils.awaitUninterruptibly(barrier);
} }
public void textureToYUV(ByteBuffer buf, public void textureToYUV(ByteBuffer buf,

View File

@ -53,13 +53,6 @@ AndroidVideoCapturerJni::~AndroidVideoCapturerJni() {
*j_video_capturer_, *j_video_capturer_,
GetMethodID(jni(), *j_video_capturer_class_, "dispose", "()V")); GetMethodID(jni(), *j_video_capturer_class_, "dispose", "()V"));
CHECK_EXCEPTION(jni()) << "error during VideoCapturer.dispose()"; CHECK_EXCEPTION(jni()) << "error during VideoCapturer.dispose()";
if (surface_texture_helper_) {
jni()->CallVoidMethod(
surface_texture_helper_->GetJavaSurfaceTextureHelper(),
GetMethodID(jni(), FindClass(jni(), "org/webrtc/SurfaceTextureHelper"),
"dispose", "()V"));
}
CHECK_EXCEPTION(jni()) << "error during SurfaceTextureHelper.dispose()";
} }
void AndroidVideoCapturerJni::Start(int width, int height, int framerate, void AndroidVideoCapturerJni::Start(int width, int height, int framerate,

View File

@ -14,6 +14,7 @@
#include "webrtc/api/java/jni/classreferenceholder.h" #include "webrtc/api/java/jni/classreferenceholder.h"
#include "webrtc/base/bind.h" #include "webrtc/base/bind.h"
#include "webrtc/base/checks.h" #include "webrtc/base/checks.h"
#include "webrtc/base/logging.h"
namespace webrtc_jni { namespace webrtc_jni {
@ -48,6 +49,14 @@ SurfaceTextureHelper::SurfaceTextureHelper(JNIEnv* jni,
} }
SurfaceTextureHelper::~SurfaceTextureHelper() { SurfaceTextureHelper::~SurfaceTextureHelper() {
LOG(LS_INFO) << "SurfaceTextureHelper dtor";
JNIEnv* jni = AttachCurrentThreadIfNeeded();
jni->CallVoidMethod(
*j_surface_texture_helper_,
GetMethodID(jni, FindClass(jni, "org/webrtc/SurfaceTextureHelper"),
"dispose", "()V"));
CHECK_EXCEPTION(jni) << "error during SurfaceTextureHelper.dispose()";
} }
jobject SurfaceTextureHelper::GetJavaSurfaceTextureHelper() const { jobject SurfaceTextureHelper::GetJavaSurfaceTextureHelper() const {

View File

@ -30,6 +30,8 @@ namespace webrtc_jni {
// SurfaceTextureHelper is reference counted to make sure that it is not // SurfaceTextureHelper is reference counted to make sure that it is not
// destroyed while a VideoFrameBuffer is in use. // destroyed while a VideoFrameBuffer is in use.
// This class is the C++ counterpart of the java class SurfaceTextureHelper. // This class is the C++ counterpart of the java class SurfaceTextureHelper.
// It owns the corresponding java object, and calls the java dispose
// method when destroyed.
// Usage: // Usage:
// 1. Create an instance of this class. // 1. Create an instance of this class.
// 2. Get the Java SurfaceTextureHelper with GetJavaSurfaceTextureHelper(). // 2. Get the Java SurfaceTextureHelper with GetJavaSurfaceTextureHelper().

View File

@ -547,10 +547,10 @@ public class MediaCodecVideoDecoder {
} }
public void release() { public void release() {
// SurfaceTextureHelper.dispose() will block until any onTextureFrameAvailable() in // SurfaceTextureHelper.stopListening() will block until any onTextureFrameAvailable() in
// progress is done. Therefore, the call to dispose() must be outside any synchronized // progress is done. Therefore, the call must be outside any synchronized
// statement that is also used in the onTextureFrameAvailable() above to avoid deadlocks. // statement that is also used in the onTextureFrameAvailable() above to avoid deadlocks.
surfaceTextureHelper.dispose(); surfaceTextureHelper.stopListening();
synchronized (newFrameLock) { synchronized (newFrameLock) {
if (renderedBuffer != null) { if (renderedBuffer != null) {
surfaceTextureHelper.returnTextureFrame(); surfaceTextureHelper.returnTextureFrame();

View File

@ -139,7 +139,20 @@ public class ThreadUtils {
/** /**
* Post |callable| to |handler| and wait for the result. * Post |callable| to |handler| and wait for the result.
*/ */
public static <V> V invokeUninterruptibly(final Handler handler, final Callable<V> callable) { public static <V> V invokeAtFrontUninterruptibly(
final Handler handler, final Callable<V> callable) {
if (handler.getLooper().getThread() == Thread.currentThread()) {
V value;
try {
value = callable.call();
} catch (Exception e) {
final RuntimeException runtimeException =
new RuntimeException("Callable threw exception: " + e);
runtimeException.setStackTrace(e.getStackTrace());
throw runtimeException;
}
return value;
}
class Result { class Result {
public V value; public V value;
} }
@ -163,12 +176,18 @@ public class ThreadUtils {
} }
/** /**
* Post |runner| to |handler| and wait for the result. * Post |runner| to |handler|, at the front, and wait for
* completion.
*/ */
public static void invokeUninterruptibly(final Handler handler, final Runnable runner) { public static void invokeAtFrontUninterruptibly(final Handler handler, final Runnable runner) {
if (handler.getLooper().getThread() == Thread.currentThread()) {
runner.run();
return;
}
final CountDownLatch barrier = new CountDownLatch(1); final CountDownLatch barrier = new CountDownLatch(1);
handler.post(new Runnable() { handler.postAtFrontOfQueue(new Runnable() {
@Override public void run() { @Override
public void run() {
runner.run(); runner.run();
barrier.countDown(); barrier.countDown();
} }