From d3f2b4e9e3bab7410775993ea2d0497d246d71ca Mon Sep 17 00:00:00 2001 From: Maurice Lam Date: Thu, 17 May 2018 14:54:09 -0700 Subject: [PATCH] Release IllustrationVideoView in onStop. DO NOT MERGE Based on the view's visibility, release or reattach IllustrationVideoView as soon as it becomes invisible to avoid media player resources being held on for a long time, and to avoid multiple video views being released all at once at the end of setup wizard. Test: ./gradlew test connectedAndroidTest Bug: 78360819 Bug: 117581782 Change-Id: Idc5af901122562d12ff3d7b1ee46012b7a60cf25 (cherry picked from commit 6ffb792f4620dcab24f71b24536c1c2d90996a0e) --- .../view/IllustrationVideoView.java | 55 +++++++++++++++++-- .../view/IllustrationVideoViewTest.java | 28 ++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/library/main/src/com/android/setupwizardlib/view/IllustrationVideoView.java b/library/main/src/com/android/setupwizardlib/view/IllustrationVideoView.java index e5c2fb1..fd1e176 100644 --- a/library/main/src/com/android/setupwizardlib/view/IllustrationVideoView.java +++ b/library/main/src/com/android/setupwizardlib/view/IllustrationVideoView.java @@ -64,6 +64,8 @@ public class IllustrationVideoView extends TextureView implements Animatable, @VisibleForTesting Surface mSurface; + protected int mWindowVisibility; + public IllustrationVideoView(Context context, AttributeSet attrs) { super(context, attrs); final TypedArray a = context.obtainStyledAttributes(attrs, @@ -124,7 +126,7 @@ public class IllustrationVideoView extends TextureView implements Animatable, * Creates a media player for the current URI. The media player will be started immediately if * the view's window is visible. If there is an existing media player, it will be released. */ - private void createMediaPlayer() { + protected void createMediaPlayer() { if (mMediaPlayer != null) { mMediaPlayer.release(); } @@ -149,11 +151,35 @@ public class IllustrationVideoView extends TextureView implements Animatable, } else { Log.wtf(TAG, "Unable to initialize media player for video view"); } - if (getWindowVisibility() == View.VISIBLE) { + if (mWindowVisibility == View.VISIBLE) { start(); } } + protected void createSurface() { + if (mSurface != null) { + mSurface.release(); + mSurface = null; + } + // Reattach only if it has been previously released + SurfaceTexture surfaceTexture = getSurfaceTexture(); + if (surfaceTexture != null) { + setVisibility(View.INVISIBLE); + mSurface = new Surface(surfaceTexture); + } + } + + @Override + protected void onWindowVisibilityChanged(int visibility) { + super.onWindowVisibilityChanged(visibility); + mWindowVisibility = visibility; + if (visibility == View.VISIBLE) { + reattach(); + } else { + release(); + } + } + /** * Whether the media player should play the video in a continuous loop. The default value is * true. @@ -178,14 +204,34 @@ public class IllustrationVideoView extends TextureView implements Animatable, } } + private void reattach() { + if (mSurface == null) { + initVideo(); + } + } + + private void initVideo() { + if (mWindowVisibility != View.VISIBLE) { + return; + } + createSurface(); + if (mSurface != null) { + createMediaPlayer(); + } else { + Log.w("IllustrationVideoView", "Surface creation failed"); + } + } + + protected void onRenderingStart() { + } + /* SurfaceTextureListener methods */ @Override public void onSurfaceTextureAvailable(SurfaceTexture surfaceTexture, int width, int height) { // Keep the view hidden until video starts setVisibility(View.INVISIBLE); - mSurface = new Surface(surfaceTexture); - createMediaPlayer(); + initVideo(); } @Override @@ -230,6 +276,7 @@ public class IllustrationVideoView extends TextureView implements Animatable, if (what == MediaPlayer.MEDIA_INFO_VIDEO_RENDERING_START) { // Video available, show view now setVisibility(View.VISIBLE); + onRenderingStart(); } return false; } diff --git a/library/test/robotest/src/com/android/setupwizardlib/view/IllustrationVideoViewTest.java b/library/test/robotest/src/com/android/setupwizardlib/view/IllustrationVideoViewTest.java index 21822a4..a501506 100644 --- a/library/test/robotest/src/com/android/setupwizardlib/view/IllustrationVideoViewTest.java +++ b/library/test/robotest/src/com/android/setupwizardlib/view/IllustrationVideoViewTest.java @@ -16,6 +16,8 @@ package com.android.setupwizardlib.view; +import static com.google.common.truth.Truth.assertThat; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; @@ -31,6 +33,7 @@ import android.media.MediaPlayer; import android.os.Build.VERSION_CODES; import android.support.annotation.RawRes; import android.view.Surface; +import android.view.View; import com.android.setupwizardlib.R; import com.android.setupwizardlib.robolectric.SuwLibRobolectricTestRunner; @@ -90,6 +93,30 @@ public class IllustrationVideoViewTest { } } + @Test + public void onVisibilityChanged_notVisible_shouldRelease() { + createDefaultView(); + mView.onWindowVisibilityChanged(View.GONE); + + verify(ShadowMockMediaPlayer.sMediaPlayer).release(); + assertThat(mView.mSurface).isNull(); + assertThat(mView.mMediaPlayer).isNull(); + } + + @Test + public void onVisibilityChanged_visible_shouldPlay() { + createDefaultView(); + + mView.onWindowVisibilityChanged(View.GONE); + assertThat(mView.mSurface).isNull(); + assertThat(mView.mMediaPlayer).isNull(); + + mView.onWindowVisibilityChanged(View.VISIBLE); + + assertThat(mView.mSurface).isNotNull(); + assertThat(mView.mMediaPlayer).isNotNull(); + } + @Test public void testPausedWhenWindowFocusLost() { createDefaultView(); @@ -148,6 +175,7 @@ public class IllustrationVideoViewTest { // Any resource attribute should work, since the media player is mocked .addAttribute(R.attr.suwVideo, "@android:color/white") .build()); + mView.setSurfaceTexture(mock(SurfaceTexture.class)); mView.onSurfaceTextureAvailable(mSurfaceTexture, 500, 500); }