From 9e420afefcb107ac07ffbad283765f71bceb8ea5 Mon Sep 17 00:00:00 2001 From: Alex Glaznev Date: Tue, 7 Apr 2015 10:14:22 -0700 Subject: [PATCH] Fix potential race conditions in Android video renderer. - Check texture properties update flag using the same lock under which the flag value is set. - Adjust texture properties inside frame queue lock. - Plus adding extra logging to track video renderer properties updates. R=wzh@webrtc.org Review URL: https://webrtc-codereview.appspot.com/45929004 Cr-Commit-Position: refs/heads/master@{#8941} --- .../android/org/webrtc/VideoRendererGui.java | 75 +++++++++++-------- 1 file changed, 45 insertions(+), 30 deletions(-) diff --git a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java index 0f616c9475..1c5fda1b47 100644 --- a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java +++ b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java @@ -338,11 +338,10 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { } private void checkAdjustTextureCoords() { - if (!updateTextureProperties || - scalingType == ScalingType.SCALE_FILL) { - return; - } synchronized(updateTextureLock) { + if (!updateTextureProperties || scalingType == ScalingType.SCALE_FILL) { + return; + } // Re - calculate texture vertices to preserve video aspect ratio. float texRight = this.texRight; float texLeft = this.texLeft; @@ -352,9 +351,9 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { float texOffsetV = 0; float displayWidth = (texRight - texLeft) * screenWidth / 2; float displayHeight = (texTop - texBottom) * screenHeight / 2; - Log.d(TAG, "ID: " + id + ". Display: " + displayWidth + + Log.d(TAG, "ID: " + id + ". AdjustTextureCoords. Display: " + displayWidth + " x " + displayHeight + ". Video: " + videoWidth + - " x " + videoHeight); + " x " + videoHeight + ". Rotation: " + rotationDegree); if (displayWidth > 1 && displayHeight > 1 && videoWidth > 1 && videoHeight > 1) { float displayAspectRatio = displayWidth / displayHeight; @@ -427,6 +426,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { directNativeFloatBuffer(textureCoordinatesFloat); } updateTextureProperties = false; + Log.d(TAG, " AdjustTextureCoords done"); } } @@ -469,16 +469,16 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { // No frame received yet - nothing to render. return; } - // Check if texture vertices/coordinates adjustment is required when - // screen orientation changes or video frame size changes. - checkAdjustTextureCoords(); - long now = System.nanoTime(); int currentProgram = 0; I420Frame frameFromQueue; synchronized (frameToRenderQueue) { + // Check if texture vertices/coordinates adjustment is required when + // screen orientation changes or video frame size changes. + checkAdjustTextureCoords(); + frameFromQueue = frameToRenderQueue.peek(); if (frameFromQueue != null && startTimeNs == -1) { startTimeNs = now; @@ -557,7 +557,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { if (frameFromQueue != null) { framesRendered++; drawTimeNs += (System.nanoTime() - now); - if ((framesRendered % 150) == 0) { + if ((framesRendered % 300) == 0) { logStatistics(); } } @@ -579,50 +579,65 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { public void setScreenSize(final int screenWidth, final int screenHeight) { synchronized(updateTextureLock) { + if (screenWidth == this.screenWidth && screenHeight == this.screenHeight) { + return; + } + Log.d(TAG, "ID: " + id + ". YuvImageRenderer.setScreenSize: " + + screenWidth + " x " + screenHeight); this.screenWidth = screenWidth; this.screenHeight = screenHeight; updateTextureProperties = true; } } - public void setPosition(int x, int y, int width, int height, - ScalingType scalingType) { + public void setPosition(int x, int y, int width, int height, ScalingType scalingType) { + float texLeft = (x - 50) / 50.0f; + float texTop = (50 - y) / 50.0f; + float texRight = Math.min(1.0f, (x + width - 50) / 50.0f); + float texBottom = Math.max(-1.0f, (50 - y - height) / 50.0f); synchronized(updateTextureLock) { - texLeft = (x - 50) / 50.0f; - texTop = (50 - y) / 50.0f; - texRight = Math.min(1.0f, (x + width - 50) / 50.0f); - texBottom = Math.max(-1.0f, (50 - y - height) / 50.0f); + if (texLeft == this.texLeft && texTop == this.texTop && texRight == this.texRight && + texBottom == this.texBottom && scalingType == this.scalingType) { + return; + } + Log.d(TAG, "ID: " + id + ". YuvImageRenderer.setPosition: (" + x + ", " + y + + ") " + width + " x " + height + ". Scaling: " + scalingType); + this.texLeft = texLeft; + this.texTop = texTop; + this.texRight = texRight; + this.texBottom = texBottom; this.scalingType = scalingType; updateTextureProperties = true; } } - private void setSize(final int width, final int height, - final int rotation) { - if (width == videoWidth && height == videoHeight && - rotation == rotationDegree) { + private void setSize(final int videoWidth, final int videoHeight, final int rotation) { + if (videoWidth == this.videoWidth && videoHeight == this.videoHeight + && rotation == rotationDegree) { return; } - Log.d(TAG, "ID: " + id + ". YuvImageRenderer.setSize: " + - width + " x " + height + " rotation " + rotation); - - videoWidth = width; - videoHeight = height; - rotationDegree = rotation; - int[] strides = { width, width / 2, width / 2 }; // Frame re-allocation need to be synchronized with copying // frame to textures in draw() function to avoid re-allocating // the frame while it is being copied. synchronized (frameToRenderQueue) { + Log.d(TAG, "ID: " + id + ". YuvImageRenderer.setSize: " + + videoWidth + " x " + videoHeight + " rotation " + rotation); + + this.videoWidth = videoWidth; + this.videoHeight = videoHeight; + rotationDegree = rotation; + int[] strides = { videoWidth, videoWidth / 2, videoWidth / 2 }; + // Clear rendering queue. frameToRenderQueue.poll(); // Re-allocate / allocate the frame. - yuvFrameToRender = new I420Frame(width, height, rotationDegree, + yuvFrameToRender = new I420Frame(videoWidth, videoHeight, rotationDegree, strides, null); - textureFrameToRender = new I420Frame(width, height, rotationDegree, + textureFrameToRender = new I420Frame(videoWidth, videoHeight, rotationDegree, null, -1); updateTextureProperties = true; + Log.d(TAG, " YuvImageRenderer.setSize done."); } }