diff options
author | zachh <zachh@google.com> | 2017-07-10 14:50:37 -0700 |
---|---|---|
committer | Eric Erfanian <erfanian@google.com> | 2017-07-11 08:01:05 -0700 |
commit | 6e7e1b9a79e078d7c77648b414809df79218597e (patch) | |
tree | 67f86db4c4c0144749a7190130c74df46e0f84a9 | |
parent | 7a4b7090da0366e02e742325a6c531c4bdaff0d0 (diff) |
Improved preview scaling in IMS video calls.
This CL attempts to improve many related issues.
The first was that the (post-greenscreen) preview was "squished" when making an outgoing IMS video call:
BEFORE: https://screenshot.googleplex.com/cRcZYmgq1rh
AFTER: https://screenshot.googleplex.com/8cWFQw7Au2U
Another issue is that the preview on the green screen sometimes appears very zoomed:
BEFORE: https://screenshot.googleplex.com/0vyq3U87xVX
AFTER: https://screenshot.googleplex.com/zyhkdATMuUj
These two issues seem to be improved by removing our manual attempts to scale the video via VideoScale. (Note that transforms to fix rotation are still needed in landscape mode and remain.)
Another issue is that when hanging up an unanswered outgoing video call, the preview becomes temporarily extremely zoomed and unblurred:
BEFORE: https://screenshot.googleplex.com/mqwSsTXhwSw
AFTER: https://screenshot.googleplex.com/uKfEpVmd8A2
Another issue is that when rotating the device, the preview would sometimes remain rotated incorrectly.
BEFORE: https://screenshot.googleplex.com/p2mVnPJ7dww
AFTER: https://screenshot.googleplex.com/S8R0FsS0Vsn
I believe that these problems (and possibly other video related flakiness I sometimes observed) happen because we update scaling and views immediately after applying layout changes. Often times, the layout changes haven't taken effect because we're in the middle of a layout pass so the updates are not working correctly. So, I moved most of those updates into specific layout listeners for the preview and remote texture views.
(Note that something similar before was attempted using a global layout listener, but that layout listener removed itself after the first invocation so didn't seem to be working as intended AFAICT.)
The last issue was that when toggling the front/rear camera, when returning to the front camera, the video would appear zoomed. This was fixed by removing the call to setDefaultBufferSize in VideoSurfaceTextureImpl. From the javadoc of that method, it doesn't sound like something that we should need to be doing and it reliably fixes the problem.
BEFORE: https://screenshot.googleplex.com/6j0XDfLGAzk
AFTER: https://screenshot.googleplex.com/Hs7zsbtyjwc
Bug: 62437862
Test: manually placed and received IMS calls and observed improved preview scaling on multiple devices when calling from O to N, O to O, and N to O. Observed that basic features (mute/umute video, swapping cameras, greenscreens, answer screens, hanging up, etc) still behave reasonably. Open to suggestions for automated tests.
PiperOrigin-RevId: 161444534
Change-Id: I4be348875de11b8517feba86da07fe41a3e5351d
-rw-r--r-- | java/com/android/incallui/video/impl/VideoCallFragment.java | 139 | ||||
-rw-r--r-- | java/com/android/incallui/videosurface/impl/VideoSurfaceTextureImpl.java | 3 |
2 files changed, 78 insertions, 64 deletions
diff --git a/java/com/android/incallui/video/impl/VideoCallFragment.java b/java/com/android/incallui/video/impl/VideoCallFragment.java index 67c131af9..4d172235e 100644 --- a/java/com/android/incallui/video/impl/VideoCallFragment.java +++ b/java/com/android/incallui/video/impl/VideoCallFragment.java @@ -21,6 +21,7 @@ import android.content.Context; import android.content.pm.PackageManager; import android.content.res.Resources; import android.graphics.Bitmap; +import android.graphics.Matrix; import android.graphics.Outline; import android.graphics.Point; import android.graphics.drawable.Animatable; @@ -44,11 +45,11 @@ import android.view.Surface; import android.view.TextureView; import android.view.View; import android.view.View.OnClickListener; +import android.view.View.OnLayoutChangeListener; import android.view.View.OnSystemUiVisibilityChangeListener; import android.view.ViewGroup; import android.view.ViewGroup.MarginLayoutParams; import android.view.ViewOutlineProvider; -import android.view.ViewTreeObserver; import android.view.accessibility.AccessibilityEvent; import android.view.animation.AccelerateDecelerateInterpolator; import android.view.animation.Interpolator; @@ -59,7 +60,6 @@ import android.widget.TextView; import com.android.dialer.common.Assert; import com.android.dialer.common.FragmentUtils; import com.android.dialer.common.LogUtil; -import com.android.dialer.common.concurrent.ThreadUtil; import com.android.dialer.compat.ActivityCompat; import com.android.incallui.audioroute.AudioRouteSelectorDialogFragment; import com.android.incallui.audioroute.AudioRouteSelectorDialogFragment.AudioRouteSelectorPresenter; @@ -123,9 +123,6 @@ public class VideoCallFragment extends Fragment } }; - // Must use a named method reference as otherwise they do not match. - // https://stackoverflow.com/questions/28190304/two-exact-method-references-are-not-equal - private final Runnable updatePreviewVideoIfSafe = this::updatePreviewVideoScaling; private InCallScreenDelegate inCallScreenDelegate; private VideoCallScreenDelegate videoCallScreenDelegate; private InCallButtonUiDelegate inCallButtonUiDelegate; @@ -257,25 +254,43 @@ public class VideoCallFragment extends Fragment greenScreenBackgroundView = view.findViewById(R.id.videocall_green_screen_background); fullscreenBackgroundView = view.findViewById(R.id.videocall_fullscreen_background); - // We need the texture view size to be able to scale the remote video. At this point the view - // layout won't be complete so add a layout listener. - ViewTreeObserver observer = remoteTextureView.getViewTreeObserver(); - observer.addOnGlobalLayoutListener( - new ViewTreeObserver.OnGlobalLayoutListener() { + remoteTextureView.addOnLayoutChangeListener( + new OnLayoutChangeListener() { @Override - public void onGlobalLayout() { - LogUtil.i("VideoCallFragment.onGlobalLayout", null); + public void onLayoutChange( + View v, + int left, + int top, + int right, + int bottom, + int oldLeft, + int oldTop, + int oldRight, + int oldBottom) { + LogUtil.i("VideoCallFragment.onLayoutChange", "remoteTextureView layout changed"); updateRemoteVideoScaling(); - updatePreviewVideoScaling(); - updateVideoOffViews(); - // Remove the listener so we don't continually re-layout. - ViewTreeObserver observer = remoteTextureView.getViewTreeObserver(); - if (observer.isAlive()) { - observer.removeOnGlobalLayoutListener(this); - } + updateRemoteOffView(); } }); + previewTextureView.addOnLayoutChangeListener( + new OnLayoutChangeListener() { + @Override + public void onLayoutChange( + View v, + int left, + int top, + int right, + int bottom, + int oldLeft, + int oldTop, + int oldRight, + int oldBottom) { + LogUtil.i("VideoCallFragment.onLayoutChange", "previewTextureView layout changed"); + fixPreviewRotation(); + updatePreviewOffView(); + } + }); return view; } @@ -354,9 +369,6 @@ public class VideoCallFragment extends Fragment super.onPause(); LogUtil.i("VideoCallFragment.onPause", null); inCallScreenDelegate.onInCallScreenPaused(); - - // If this is scheduled we should remove it - ThreadUtil.getUiThreadHandler().removeCallbacks(updatePreviewVideoIfSafe); } @Override @@ -461,7 +473,7 @@ public class VideoCallFragment extends Fragment View view = getView(); if (view != null) { // Code is more expressive with all flags present, even though some may be combined - //noinspection PointlessBitwiseExpression + // noinspection PointlessBitwiseExpression view.setSystemUiVisibility(View.SYSTEM_UI_FLAG_VISIBLE | View.SYSTEM_UI_FLAG_LAYOUT_STABLE); } } @@ -662,15 +674,19 @@ public class VideoCallFragment extends Fragment "showPreview: %b, shouldShowRemote: %b", shouldShowPreview, shouldShowRemote); - this.shouldShowPreview = shouldShowPreview; - this.shouldShowRemote = shouldShowRemote; - this.isRemotelyHeld = isRemotelyHeld; videoCallScreenDelegate.getLocalVideoSurfaceTexture().attachToTextureView(previewTextureView); videoCallScreenDelegate.getRemoteVideoSurfaceTexture().attachToTextureView(remoteTextureView); - updateVideoOffViews(); - updateRemoteVideoScaling(); + this.isRemotelyHeld = isRemotelyHeld; + if (this.shouldShowRemote != shouldShowRemote) { + this.shouldShowRemote = shouldShowRemote; + updateRemoteOffView(); + } + if (this.shouldShowPreview != shouldShowPreview) { + this.shouldShowPreview = shouldShowPreview; + updatePreviewOffView(); + } } @Override @@ -732,7 +748,6 @@ public class VideoCallFragment extends Fragment } else { exitFullscreenMode(); } - updateVideoOffViews(); OnHoldFragment onHoldFragment = ((OnHoldFragment) @@ -949,31 +964,15 @@ public class VideoCallFragment extends Fragment // Do nothing } - private void updatePreviewVideoScaling() { - if (previewTextureView.getWidth() == 0 || previewTextureView.getHeight() == 0) { - LogUtil.i("VideoCallFragment.updatePreviewVideoScaling", "view layout hasn't finished yet"); - return; - } - VideoSurfaceTexture localVideoSurfaceTexture = - videoCallScreenDelegate.getLocalVideoSurfaceTexture(); - Point cameraDimensions = localVideoSurfaceTexture.getSurfaceDimensions(); - if (cameraDimensions == null) { - LogUtil.i( - "VideoCallFragment.updatePreviewVideoScaling", "camera dimensions haven't been set"); - return; - } - if (isLandscape()) { - VideoSurfaceBindings.scaleVideoAndFillView( - previewTextureView, - cameraDimensions.x, - cameraDimensions.y, - videoCallScreenDelegate.getDeviceOrientation()); - } else { - VideoSurfaceBindings.scaleVideoAndFillView( - previewTextureView, - cameraDimensions.y, - cameraDimensions.x, - videoCallScreenDelegate.getDeviceOrientation()); + private void fixPreviewRotation() { + int rotationDegrees = getRotationDegrees(); + if (rotationDegrees == 90 || rotationDegrees == 270) { + int viewWidth = previewTextureView.getWidth(); + int viewHeight = previewTextureView.getHeight(); + Matrix transform = new Matrix(); + // Multiplying by -1 prevents the image from being upside down in landscape mode. + transform.postRotate(rotationDegrees * -1.0f, viewWidth / 2.0f, viewHeight / 2.0f); + previewTextureView.setTransform(transform); } } @@ -1010,6 +1009,22 @@ public class VideoCallFragment extends Fragment return rotation == Surface.ROTATION_90 || rotation == Surface.ROTATION_270; } + private int getRotationDegrees() { + int rotation = getActivity().getWindowManager().getDefaultDisplay().getRotation(); + switch (rotation) { + case Surface.ROTATION_0: + return 0; + case Surface.ROTATION_90: + return 90; + case Surface.ROTATION_180: + return 180; + case Surface.ROTATION_270: + return 270; + default: + throw Assert.createAssertionFailException("unsupported rotation: " + rotation); + } + } + private void enterGreenScreenMode() { LogUtil.i("VideoCallFragment.enterGreenScreenMode", null); RelativeLayout.LayoutParams params = @@ -1019,7 +1034,6 @@ public class VideoCallFragment extends Fragment params.addRule(RelativeLayout.ALIGN_PARENT_TOP); previewTextureView.setLayoutParams(params); previewTextureView.setOutlineProvider(null); - updatePreviewVideoScaling(); updateOverlayBackground(); contactGridManager.setIsMiddleRowVisible(true); updateMutePreviewOverlayVisibility(); @@ -1055,12 +1069,11 @@ public class VideoCallFragment extends Fragment previewOffBlurredImageView.setLayoutParams(params); previewOffBlurredImageView.setOutlineProvider(circleOutlineProvider); previewOffBlurredImageView.setClipToOutline(true); - - // Wait until the layout pass has finished before updating the scaling - ThreadUtil.postOnUiThread(updatePreviewVideoIfSafe); } - private void updateVideoOffViews() { + private void updatePreviewOffView() { + LogUtil.enterBlock("VideoCallFragment.updatePreviewOffView"); + // Always hide the preview off and remote off views in green screen mode. boolean previewEnabled = isInGreenScreenMode || shouldShowPreview; previewOffOverlay.setVisibility(previewEnabled ? View.GONE : View.VISIBLE); @@ -1070,7 +1083,10 @@ public class VideoCallFragment extends Fragment shouldShowPreview, BLUR_PREVIEW_RADIUS, BLUR_PREVIEW_SCALE_FACTOR); + } + private void updateRemoteOffView() { + LogUtil.enterBlock("VideoCallFragment.updateRemoteOffView"); boolean remoteEnabled = isInGreenScreenMode || shouldShowRemote; boolean isResumed = remoteEnabled && !isRemotelyHeld; if (isResumed) { @@ -1097,7 +1113,6 @@ public class VideoCallFragment extends Fragment isRemotelyHeld ? R.string.videocall_remotely_held : R.string.videocall_remote_video_off); remoteVideoOff.setVisibility(View.VISIBLE); } - LogUtil.i("VideoCallFragment.updateVideoOffViews", "calling updateBlurredImageView"); updateBlurredImageView( remoteTextureView, remoteOffBlurredImageView, @@ -1125,6 +1140,8 @@ public class VideoCallFragment extends Fragment int width = Math.round(textureView.getWidth() * scaleFactor); int height = Math.round(textureView.getHeight() * scaleFactor); + LogUtil.i("VideoCallFragment.updateBlurredImageView", "width: %d, height: %d", width, height); + // This call takes less than 10 milliseconds. Bitmap bitmap = textureView.getBitmap(width, height); @@ -1261,4 +1278,4 @@ public class VideoCallFragment extends Fragment } } } -//LINT.ThenChange(//depot/google3/third_party/java_src/android_app/dialer/java/com/android/incallui/video/impl/SurfaceViewVideoCallFragment.java) +// LINT.ThenChange(//depot/google3/third_party/java_src/android_app/dialer/java/com/android/incallui/video/impl/SurfaceViewVideoCallFragment.java) diff --git a/java/com/android/incallui/videosurface/impl/VideoSurfaceTextureImpl.java b/java/com/android/incallui/videosurface/impl/VideoSurfaceTextureImpl.java index 8cac40229..1af7dff4f 100644 --- a/java/com/android/incallui/videosurface/impl/VideoSurfaceTextureImpl.java +++ b/java/com/android/incallui/videosurface/impl/VideoSurfaceTextureImpl.java @@ -67,9 +67,6 @@ public class VideoSurfaceTextureImpl implements VideoSurfaceTexture { "VideoSurfaceTextureImpl.setSurfaceDimensions", "surfaceDimensions: " + surfaceDimensions + " " + toString()); this.surfaceDimensions = surfaceDimensions; - if (surfaceDimensions != null && savedSurfaceTexture != null) { - savedSurfaceTexture.setDefaultBufferSize(surfaceDimensions.x, surfaceDimensions.y); - } } @Override |