From fff6f07bebcd18a7cecdb8bb54f4eec3a879918c Mon Sep 17 00:00:00 2001 From: Jelle Foks Date: Thu, 5 Oct 2023 12:35:51 -0700 Subject: [PATCH] Revert "Clear video surface without re-initializing EGL. (#1413)" (#1730) This reverts commit 7eff4666ef9e3042b0841570e438218608624ea0. This reverts the switch from clearing the video window using low level EGL and GLES to using ClearVideoWindow because it appeared to have adverse effects on related video surface state management in the Android framework. b/302589325 b/303316973 --- .../java/dev/cobalt/coat/CobaltActivity.java | 4 - .../java/dev/cobalt/coat/StarboardBridge.java | 9 -- .../dev/cobalt/media/VideoSurfaceView.java | 37 +---- starboard/android/shared/BUILD.gn | 2 +- starboard/android/shared/system_egl.cc | 146 ------------------ starboard/android/shared/video_window.cc | 100 ++++++++++-- 6 files changed, 90 insertions(+), 208 deletions(-) delete mode 100644 starboard/android/shared/system_egl.cc diff --git a/starboard/android/apk/app/src/main/java/dev/cobalt/coat/CobaltActivity.java b/starboard/android/apk/app/src/main/java/dev/cobalt/coat/CobaltActivity.java index cb3a65b67c2d..2a763280dd1d 100644 --- a/starboard/android/apk/app/src/main/java/dev/cobalt/coat/CobaltActivity.java +++ b/starboard/android/apk/app/src/main/java/dev/cobalt/coat/CobaltActivity.java @@ -328,10 +328,6 @@ public void onRequestPermissionsResult( getStarboardBridge().onRequestPermissionsResult(requestCode, permissions, grantResults); } - public void clearVideoSurface() { - if (videoSurfaceView != null) videoSurfaceView.clearSurface(); - } - public void resetVideoSurface() { runOnUiThread( new Runnable() { diff --git a/starboard/android/apk/app/src/main/java/dev/cobalt/coat/StarboardBridge.java b/starboard/android/apk/app/src/main/java/dev/cobalt/coat/StarboardBridge.java index dacb2dcc9b7a..d32cf59f29d7 100644 --- a/starboard/android/apk/app/src/main/java/dev/cobalt/coat/StarboardBridge.java +++ b/starboard/android/apk/app/src/main/java/dev/cobalt/coat/StarboardBridge.java @@ -712,15 +712,6 @@ void onRequestPermissionsResult(int requestCode, String[] permissions, int[] gra audioPermissionRequester.onRequestPermissionsResult(requestCode, permissions, grantResults); } - @SuppressWarnings("unused") - @UsedByNative - public void clearVideoSurface() { - Activity activity = activityHolder.get(); - if (activity instanceof CobaltActivity) { - ((CobaltActivity) activity).clearVideoSurface(); - } - } - @SuppressWarnings("unused") @UsedByNative public void resetVideoSurface() { diff --git a/starboard/android/apk/app/src/main/java/dev/cobalt/media/VideoSurfaceView.java b/starboard/android/apk/app/src/main/java/dev/cobalt/media/VideoSurfaceView.java index 816c6495af14..a4ef40ea4b1f 100644 --- a/starboard/android/apk/app/src/main/java/dev/cobalt/media/VideoSurfaceView.java +++ b/starboard/android/apk/app/src/main/java/dev/cobalt/media/VideoSurfaceView.java @@ -17,10 +17,7 @@ import static dev.cobalt.media.Log.TAG; import android.content.Context; -import android.graphics.Canvas; import android.graphics.Color; -import android.graphics.PixelFormat; -import android.graphics.PorterDuff; import android.os.Build; import android.util.AttributeSet; import android.view.Surface; @@ -37,7 +34,6 @@ public class VideoSurfaceView extends SurfaceView { private static Surface currentSurface = null; - private SurfaceHolder.Callback mSurfaceHolderCallback = null; private static final Set needResetSurfaceList = new HashSet<>(); @@ -72,46 +68,15 @@ public VideoSurfaceView(Context context, AttributeSet attrs, int defStyleAttr, i private void initialize(Context context) { setBackgroundColor(Color.TRANSPARENT); - mSurfaceHolderCallback = new SurfaceHolderCallback(); - getHolder().addCallback(mSurfaceHolderCallback); + getHolder().addCallback(new SurfaceHolderCallback()); // TODO: Avoid recreating the surface when the player bounds change. // Recreating the surface is time-consuming and complicates synchronizing // punch-out video when the position / size is animated. } - public void clearSurface() { - SurfaceHolder holder = getHolder(); - if (holder == null) { - return; - } - Surface surface = holder.getSurface(); - if ((surface != null) && surface.isValid()) { - Canvas canvas = holder.lockCanvas(); - if (canvas != null) { - canvas.drawColor(Color.BLACK, PorterDuff.Mode.CLEAR); - holder.unlockCanvasAndPost(canvas); - } - } - // Trigger a surface changed event to prevent 'already connected'. - // But disable the callback to prevent it from making calls to the locking - // nativeOnVideoSurfaceChanged because we already are holding the same lock. - if (mSurfaceHolderCallback != null) { - holder.removeCallback(mSurfaceHolderCallback); - } - holder.setFormat(PixelFormat.TRANSPARENT); - holder.setFormat(PixelFormat.OPAQUE); - currentSurface = holder.getSurface(); - nativeOnVideoSurfaceChangedLocked(currentSurface); - if (mSurfaceHolderCallback != null) { - holder.addCallback(mSurfaceHolderCallback); - } - } - private static native void nativeOnVideoSurfaceChanged(Surface surface); - private static native void nativeOnVideoSurfaceChangedLocked(Surface surface); - private static native void nativeSetNeedResetSurface(); private class SurfaceHolderCallback implements SurfaceHolder.Callback { diff --git a/starboard/android/shared/BUILD.gn b/starboard/android/shared/BUILD.gn index 96130a4a6995..db75e51032f8 100644 --- a/starboard/android/shared/BUILD.gn +++ b/starboard/android/shared/BUILD.gn @@ -55,6 +55,7 @@ action("game_activity_sources") { static_library("starboard_platform") { sources = [ + "//starboard/shared/egl/system_egl.cc", "//starboard/shared/gcc/atomic_gcc_public.h", "//starboard/shared/gles/gl_call.h", "//starboard/shared/gles/system_gles2.cc", @@ -367,7 +368,6 @@ static_library("starboard_platform") { "speech_synthesis_internal.cc", "speech_synthesis_is_supported.cc", "speech_synthesis_speak.cc", - "system_egl.cc", "system_get_connection_type.cc", "system_get_device_type.cc", "system_get_extensions.cc", diff --git a/starboard/android/shared/system_egl.cc b/starboard/android/shared/system_egl.cc deleted file mode 100644 index 9a81788e27bf..000000000000 --- a/starboard/android/shared/system_egl.cc +++ /dev/null @@ -1,146 +0,0 @@ -// Copyright 2019 The Cobalt Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include -#include -#include - -#include "starboard/egl.h" -#include "starboard/gles.h" - -#if !defined(EGL_VERSION_1_0) || !defined(EGL_VERSION_1_1) || \ - !defined(EGL_VERSION_1_2) || !defined(EGL_VERSION_1_3) || \ - !defined(EGL_VERSION_1_4) -#error "EGL version must be >= 1.4" -#endif - -namespace { - -bool first_make_current = false; - -EGLBoolean SbEglInitialize(EGLDisplay dpy, EGLint* major, EGLint* minor) { - first_make_current = true; - return eglInitialize(dpy, major, minor); -} - -EGLBoolean SbEglTerminate(EGLDisplay dpy) { - first_make_current = false; - return eglTerminate(dpy); -} - -EGLBoolean SbEglMakeCurrent(EGLDisplay dpy, - EGLSurface draw, - EGLSurface read, - EGLContext ctx) { - EGLBoolean result = eglMakeCurrent(dpy, draw, read, ctx); - if (first_make_current && (dpy != EGL_NO_DISPLAY) && - (draw != EGL_NO_SURFACE) && (eglGetError() == EGL_SUCCESS)) { - first_make_current = false; - // Start by showing a black surface immediately. - const SbGlesInterface* gles = SbGetGlesInterface(); - gles->glClearColor(0, 0, 0, 1); - gles->glClear(GL_COLOR_BUFFER_BIT); - gles->glFlush(); - if (glGetError() == GL_NO_ERROR) { - eglSwapBuffers(dpy, draw); - } - } - return result; -} - -// Convenience functions that redirect to the intended function but "cast" the -// type of the SbEglNative*Type parameter into the desired type. Depending on -// the platform, the type of cast to use is different so either C-style casts or -// constructor-style casts are needed to work across platforms (or provide -// implementations for these functions for each platform). - -SbEglBoolean SbEglCopyBuffers(SbEglDisplay dpy, - SbEglSurface surface, - SbEglNativePixmapType target) { - return eglCopyBuffers(dpy, surface, (EGLNativePixmapType)target); -} - -SbEglSurface SbEglCreatePixmapSurface(SbEglDisplay dpy, - SbEglConfig config, - SbEglNativePixmapType pixmap, - const SbEglInt32* attrib_list) { - return eglCreatePixmapSurface(dpy, config, (EGLNativePixmapType)pixmap, - attrib_list); -} - -SbEglSurface SbEglCreateWindowSurface(SbEglDisplay dpy, - SbEglConfig config, - SbEglNativeWindowType win, - const SbEglInt32* attrib_list) { - return eglCreateWindowSurface(dpy, config, (EGLNativeWindowType)win, - attrib_list); -} - -SbEglDisplay SbEglGetDisplay(SbEglNativeDisplayType display_id) { - return eglGetDisplay((EGLNativeDisplayType)display_id); -} - -const SbEglInterface g_sb_egl_interface = { - &eglChooseConfig, - &SbEglCopyBuffers, - &eglCreateContext, - &eglCreatePbufferSurface, - &SbEglCreatePixmapSurface, - &SbEglCreateWindowSurface, - &eglDestroyContext, - &eglDestroySurface, - &eglGetConfigAttrib, - &eglGetConfigs, - &eglGetCurrentDisplay, - &eglGetCurrentSurface, - &SbEglGetDisplay, - &eglGetError, - &eglGetProcAddress, - &SbEglInitialize, - &SbEglMakeCurrent, - &eglQueryContext, - &eglQueryString, - &eglQuerySurface, - &eglSwapBuffers, - &SbEglTerminate, - &eglWaitGL, - &eglWaitNative, - &eglBindTexImage, - &eglReleaseTexImage, - &eglSurfaceAttrib, - &eglSwapInterval, - &eglBindAPI, - &eglQueryAPI, - &eglCreatePbufferFromClientBuffer, - &eglReleaseThread, - &eglWaitClient, - &eglGetCurrentContext, - - nullptr, // eglCreateSync - nullptr, // eglDestroySync - nullptr, // eglClientWaitSync - nullptr, // eglGetSyncAttrib - nullptr, // eglCreateImage - nullptr, // eglDestroyImage - nullptr, // eglGetPlatformDisplay - nullptr, // eglCreatePlatformWindowSurface - nullptr, // eglCreatePlatformPixmapSurface - nullptr, // eglWaitSync -}; - -} // namespace - -const SbEglInterface* SbGetEglInterface() { - return &g_sb_egl_interface; -} diff --git a/starboard/android/shared/video_window.cc b/starboard/android/shared/video_window.cc index 43b2d726ff82..d6998e35793e 100644 --- a/starboard/android/shared/video_window.cc +++ b/starboard/android/shared/video_window.cc @@ -44,13 +44,98 @@ VideoSurfaceHolder* g_video_surface_holder = NULL; // Global boolean to indicate if we need to reset SurfaceView after playing // vertical video. bool g_reset_surface_on_clear_window = false; + +void ClearNativeWindow(ANativeWindow* native_window) { + EGLDisplay display = eglGetDisplay(EGL_DEFAULT_DISPLAY); + eglInitialize(display, NULL, NULL); + if (display == EGL_NO_DISPLAY) { + SB_DLOG(ERROR) << "Found no EGL display in ClearVideoWindow"; + return; + } + + const EGLint kAttributeList[] = { + EGL_RED_SIZE, + 8, + EGL_GREEN_SIZE, + 8, + EGL_BLUE_SIZE, + 8, + EGL_ALPHA_SIZE, + 8, + EGL_RENDERABLE_TYPE, + EGL_OPENGL_ES2_BIT, + EGL_NONE, + 0, + EGL_NONE, + }; + + // First, query how many configs match the given attribute list. + EGLint num_configs = 0; + EGL_CALL(eglChooseConfig(display, kAttributeList, NULL, 0, &num_configs)); + SB_DCHECK(num_configs != 0); + + // Allocate space to receive the matching configs and retrieve them. + EGLConfig* configs = new EGLConfig[num_configs]; + EGL_CALL(eglChooseConfig(display, kAttributeList, configs, num_configs, + &num_configs)); + + EGLNativeWindowType egl_native_window = + static_cast(native_window); + EGLConfig config; + + // Find the first config that successfully allow a window surface to be + // created. + EGLSurface surface; + for (int config_number = 0; config_number < num_configs; ++config_number) { + config = configs[config_number]; + surface = eglCreateWindowSurface(display, config, egl_native_window, NULL); + if (eglGetError() == EGL_SUCCESS) + break; + } + if (surface == EGL_NO_SURFACE) { + SB_DLOG(ERROR) << "Found no EGL surface in ClearVideoWindow"; + return; + } + SB_DCHECK(surface != EGL_NO_SURFACE); + + delete[] configs; + + // Create an OpenGL ES 2.0 context. + EGLContext context = EGL_NO_CONTEXT; + EGLint context_attrib_list[] = { + EGL_CONTEXT_CLIENT_VERSION, + 2, + EGL_NONE, + }; + context = + eglCreateContext(display, config, EGL_NO_CONTEXT, context_attrib_list); + SB_DCHECK(eglGetError() == EGL_SUCCESS); + SB_DCHECK(context != EGL_NO_CONTEXT); + + /* connect the context to the surface */ + EGL_CALL(eglMakeCurrent(display, surface, surface, context)); + + GL_CALL(glClearColor(0, 0, 0, 1)); + GL_CALL(glClear(GL_COLOR_BUFFER_BIT)); + GL_CALL(glFlush()); + EGL_CALL(eglSwapBuffers(display, surface)); + + // Cleanup all used resources. + EGL_CALL( + eglMakeCurrent(display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT)); + EGL_CALL(eglDestroyContext(display, context)); + EGL_CALL(eglDestroySurface(display, surface)); + EGL_CALL(eglTerminate(display)); +} + } // namespace extern "C" SB_EXPORT_PLATFORM void -Java_dev_cobalt_media_VideoSurfaceView_nativeOnVideoSurfaceChangedLocked( +Java_dev_cobalt_media_VideoSurfaceView_nativeOnVideoSurfaceChanged( JNIEnv* env, jobject unused_this, jobject surface) { + ScopedLock lock(*GetViewSurfaceMutex()); if (g_video_surface_holder) { g_video_surface_holder->OnSurfaceDestroyed(); g_video_surface_holder = NULL; @@ -66,19 +151,10 @@ Java_dev_cobalt_media_VideoSurfaceView_nativeOnVideoSurfaceChangedLocked( if (surface) { g_j_video_surface = env->NewGlobalRef(surface); g_native_video_window = ANativeWindow_fromSurface(env, surface); + ClearNativeWindow(g_native_video_window); } } -extern "C" SB_EXPORT_PLATFORM void -Java_dev_cobalt_media_VideoSurfaceView_nativeOnVideoSurfaceChanged( - JNIEnv* env, - jobject j_this, - jobject surface) { - ScopedLock lock(*GetViewSurfaceMutex()); - Java_dev_cobalt_media_VideoSurfaceView_nativeOnVideoSurfaceChangedLocked( - env, j_this, surface); -} - extern "C" SB_EXPORT_PLATFORM void Java_dev_cobalt_media_VideoSurfaceView_nativeSetNeedResetSurface( JNIEnv* env, @@ -152,7 +228,7 @@ void VideoSurfaceHolder::ClearVideoWindow(bool force_reset_surface) { return; } } - env->CallStarboardVoidMethodOrAbort("clearVideoSurface", "()V"); + ClearNativeWindow(g_native_video_window); } } // namespace shared