From d5031fcf928b615a1c9aa9f15e60d1c946e6d456 Mon Sep 17 00:00:00 2001 From: magjed Date: Fri, 14 Aug 2015 03:13:05 -0700 Subject: [PATCH] Android VideoRendererGui: Add dispose function There is currently no way to dispose VideoRendererGui or VideoRendererGui.YuvImageRenderer. This CL adds functions to do so. BUG=webrtc:4892 Review URL: https://codereview.webrtc.org/1273803002 Cr-Commit-Position: refs/heads/master@{#9710} --- .../android/org/webrtc/VideoRendererGui.java | 140 ++++++++++++------ .../java/src/org/webrtc/VideoRenderer.java | 17 ++- .../src/org/appspot/apprtc/CallActivity.java | 1 + 3 files changed, 104 insertions(+), 54 deletions(-) diff --git a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java index 26285252f3..9dd2f02c4b 100644 --- a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java +++ b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java @@ -55,6 +55,8 @@ import org.webrtc.VideoRenderer.I420Frame; * Only one instance of the class can be created. */ public class VideoRendererGui implements GLSurfaceView.Renderer { + // |instance|, |instance.surface|, |eglContext|, and |eglContextReady| are synchronized on + // |VideoRendererGui.class|. private static VideoRendererGui instance = null; private static Runnable eglContextReady = null; private static final String TAG = "VideoRendererGui"; @@ -68,7 +70,8 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { private int screenWidth; private int screenHeight; // List of yuv renderers. - private ArrayList yuvImageRenderers; + private final ArrayList yuvImageRenderers; + // |drawer| is synchronized on |yuvImageRenderers|. private GlRectDrawer drawer; // The minimum fraction of the frame content that will be shown for |SCALE_ASPECT_BALANCED|. // This limits excessive cropping when adjusting display size. @@ -101,12 +104,32 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { yuvImageRenderers = new ArrayList(); } + public static synchronized void dispose() { + if (instance == null){ + return; + } + synchronized (instance.yuvImageRenderers) { + for (YuvImageRenderer yuvImageRenderer : instance.yuvImageRenderers) { + yuvImageRenderer.release(); + } + instance.yuvImageRenderers.clear(); + if (instance.drawer != null) { + instance.drawer.release(); + } + } + instance.surface = null; + instance.eglContext = null; + instance.eglContextReady = null; + instance = null; + } + /** * Class used to display stream of YUV420 frames at particular location * on a screen. New video frames are sent to display using renderFrame() * call. */ private static class YuvImageRenderer implements VideoRenderer.Callbacks { + // |surface| is synchronized on |this|. private GLSurfaceView surface; private int id; private int[] yuvTextures = { -1, -1, -1 }; @@ -116,8 +139,8 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { // an offer (writing I420Frame to render) and early-returns (recording // a dropped frame) if that queue is full. draw() call does a peek(), // copies frame to texture and then removes it from a queue using poll(). - LinkedBlockingQueue frameToRenderQueue; - // Local copy of incoming video frame. + private final LinkedBlockingQueue frameToRenderQueue; + // Local copy of incoming video frame. Synchronized on |frameToRenderQueue|. private I420Frame yuvFrameToRender; private I420Frame textureFrameToRender; // Type of video frame used for recent frame rendering. @@ -178,6 +201,15 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { rotationDegree = 0; } + private synchronized void release() { + surface = null; + synchronized (frameToRenderQueue) { + frameToRenderQueue.clear(); + yuvFrameToRender = null; + textureFrameToRender = null; + } + } + private void createTextures() { Log.d(TAG, " YuvImageRenderer.createTextures " + id + " on GL thread:" + Thread.currentThread().getId()); @@ -425,46 +457,52 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { @Override public synchronized void renderFrame(I420Frame frame) { + if (surface == null) { + // This object has been released. + return; + } setSize(frame.width, frame.height, frame.rotationDegree); long now = System.nanoTime(); framesReceived++; - // Skip rendering of this frame if setSize() was not called. - if (yuvFrameToRender == null || textureFrameToRender == null) { - framesDropped++; - return; - } - // Check input frame parameters. - if (frame.yuvFrame) { - if (frame.yuvStrides[0] < frame.width || - frame.yuvStrides[1] < frame.width / 2 || - frame.yuvStrides[2] < frame.width / 2) { - Log.e(TAG, "Incorrect strides " + frame.yuvStrides[0] + ", " + - frame.yuvStrides[1] + ", " + frame.yuvStrides[2]); + synchronized (frameToRenderQueue) { + // Skip rendering of this frame if setSize() was not called. + if (yuvFrameToRender == null || textureFrameToRender == null) { + framesDropped++; return; } - // Check incoming frame dimensions. - if (frame.width != yuvFrameToRender.width || - frame.height != yuvFrameToRender.height) { - throw new RuntimeException("Wrong frame size " + - frame.width + " x " + frame.height); + // Check input frame parameters. + if (frame.yuvFrame) { + if (frame.yuvStrides[0] < frame.width || + frame.yuvStrides[1] < frame.width / 2 || + frame.yuvStrides[2] < frame.width / 2) { + Log.e(TAG, "Incorrect strides " + frame.yuvStrides[0] + ", " + + frame.yuvStrides[1] + ", " + frame.yuvStrides[2]); + return; + } + // Check incoming frame dimensions. + if (frame.width != yuvFrameToRender.width || + frame.height != yuvFrameToRender.height) { + throw new RuntimeException("Wrong frame size " + + frame.width + " x " + frame.height); + } } - } - if (frameToRenderQueue.size() > 0) { - // Skip rendering of this frame if previous frame was not rendered yet. - framesDropped++; - return; - } + if (frameToRenderQueue.size() > 0) { + // Skip rendering of this frame if previous frame was not rendered yet. + framesDropped++; + return; + } - // Create a local copy of the frame. - if (frame.yuvFrame) { - yuvFrameToRender.copyFrom(frame); - rendererType = RendererType.RENDERER_YUV; - frameToRenderQueue.offer(yuvFrameToRender); - } else { - textureFrameToRender.copyFrom(frame); - rendererType = RendererType.RENDERER_TEXTURE; - frameToRenderQueue.offer(textureFrameToRender); + // Create a local copy of the frame. + if (frame.yuvFrame) { + yuvFrameToRender.copyFrom(frame); + rendererType = RendererType.RENDERER_YUV; + frameToRenderQueue.offer(yuvFrameToRender); + } else { + textureFrameToRender.copyFrom(frame); + rendererType = RendererType.RENDERER_TEXTURE; + frameToRenderQueue.offer(textureFrameToRender); + } } copyTimeNs += (System.nanoTime() - now); seenFrame = true; @@ -481,14 +519,14 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { } /** Passes GLSurfaceView to video renderer. */ - public static void setView(GLSurfaceView surface, + public static synchronized void setView(GLSurfaceView surface, Runnable eglContextReadyCallback) { Log.d(TAG, "VideoRendererGui.setView"); instance = new VideoRendererGui(surface); eglContextReady = eglContextReadyCallback; } - public static EGLContext getEGLContext() { + public static synchronized EGLContext getEGLContext() { return eglContext; } @@ -514,7 +552,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { * resolution (width, height). All parameters are in percentage of * screen resolution. */ - public static YuvImageRenderer create(int x, int y, int width, int height, + public static synchronized YuvImageRenderer create(int x, int y, int width, int height, ScalingType scalingType, boolean mirror) { // Check display region parameters. if (x < 0 || x > 100 || y < 0 || y > 100 || @@ -557,7 +595,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { return yuvImageRenderer; } - public static void update( + public static synchronized void update( VideoRenderer.Callbacks renderer, int x, int y, int width, int height, ScalingType scalingType, boolean mirror) { Log.d(TAG, "VideoRendererGui.update"); @@ -574,15 +612,18 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { } } - public static void remove(VideoRenderer.Callbacks renderer) { + public static synchronized void remove(VideoRenderer.Callbacks renderer) { Log.d(TAG, "VideoRendererGui.remove"); if (instance == null) { throw new RuntimeException( "Attempt to remove yuv renderer before setting GLSurfaceView"); } synchronized (instance.yuvImageRenderers) { - if (!instance.yuvImageRenderers.remove(renderer)) { + final int index = instance.yuvImageRenderers.indexOf(renderer); + if (index == -1) { Log.w(TAG, "Couldn't remove renderer (not present in current list)"); + } else { + instance.yuvImageRenderers.remove(index).release(); } } } @@ -593,14 +634,15 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { Log.d(TAG, "VideoRendererGui.onSurfaceCreated"); // Store render EGL context. if (CURRENT_SDK_VERSION >= EGL14_SDK_VERSION) { - eglContext = EGL14.eglGetCurrentContext(); - Log.d(TAG, "VideoRendererGui EGL Context: " + eglContext); + synchronized (VideoRendererGui.class) { + eglContext = EGL14.eglGetCurrentContext(); + Log.d(TAG, "VideoRendererGui EGL Context: " + eglContext); + } } - // Create drawer for YUV/OES frames. - drawer = new GlRectDrawer(); - synchronized (yuvImageRenderers) { + // Create drawer for YUV/OES frames. + drawer = new GlRectDrawer(); // Create textures for all images. for (YuvImageRenderer yuvImageRenderer : yuvImageRenderers) { yuvImageRenderer.createTextures(); @@ -612,8 +654,10 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { GLES20.glClearColor(0.15f, 0.15f, 0.15f, 1.0f); // Fire EGL context ready event. - if (eglContextReady != null) { - eglContextReady.run(); + synchronized (VideoRendererGui.class) { + if (eglContextReady != null) { + eglContextReady.run(); + } } } diff --git a/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java b/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java index 401640b463..596d3779a9 100644 --- a/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java +++ b/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java @@ -177,9 +177,9 @@ public class VideoRenderer { } // |this| either wraps a native (GUI) renderer or a client-supplied Callbacks - // (Java) implementation; so exactly one of these will be non-0/null. - final long nativeVideoRenderer; - private final Callbacks callbacks; + // (Java) implementation; this is indicated by |isWrappedVideoRenderer|. + long nativeVideoRenderer; + private final boolean isWrappedVideoRenderer; public static VideoRenderer createGui(int x, int y) { long nativeVideoRenderer = nativeCreateGuiVideoRenderer(x, y); @@ -191,20 +191,25 @@ public class VideoRenderer { public VideoRenderer(Callbacks callbacks) { nativeVideoRenderer = nativeWrapVideoRenderer(callbacks); - this.callbacks = callbacks; + isWrappedVideoRenderer = true; } private VideoRenderer(long nativeVideoRenderer) { this.nativeVideoRenderer = nativeVideoRenderer; - callbacks = null; + isWrappedVideoRenderer = false; } public void dispose() { - if (callbacks == null) { + if (nativeVideoRenderer == 0) { + // Already disposed. + return; + } + if (!isWrappedVideoRenderer) { freeGuiVideoRenderer(nativeVideoRenderer); } else { freeWrappedVideoRenderer(nativeVideoRenderer); } + nativeVideoRenderer = 0; } private static native long nativeCreateGuiVideoRenderer(int x, int y); diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java index d46f9a3d59..a033c2f1ce 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java @@ -279,6 +279,7 @@ public class CallActivity extends Activity logToast.cancel(); } activityRunning = false; + VideoRendererGui.dispose(); } // CallFragment.OnCallEvents interface implementation.