Skip to content

Commit

Permalink
Revert "Clear video surface without re-initializing EGL. (youtube#1413)…
Browse files Browse the repository at this point in the history
…" (youtube#1730)

This reverts commit 7eff466.

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
  • Loading branch information
jellefoks authored Oct 5, 2023
1 parent 0fdb82f commit fff6f07
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 208 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,7 +34,6 @@
public class VideoSurfaceView extends SurfaceView {

private static Surface currentSurface = null;
private SurfaceHolder.Callback mSurfaceHolderCallback = null;

private static final Set<String> needResetSurfaceList = new HashSet<>();

Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion starboard/android/shared/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
146 changes: 0 additions & 146 deletions starboard/android/shared/system_egl.cc

This file was deleted.

100 changes: 88 additions & 12 deletions starboard/android/shared/video_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<EGLNativeWindowType>(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;
Expand All @@ -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,
Expand Down Expand Up @@ -152,7 +228,7 @@ void VideoSurfaceHolder::ClearVideoWindow(bool force_reset_surface) {
return;
}
}
env->CallStarboardVoidMethodOrAbort("clearVideoSurface", "()V");
ClearNativeWindow(g_native_video_window);
}

} // namespace shared
Expand Down

0 comments on commit fff6f07

Please sign in to comment.