Fix issue 514 (#528)
	
		
	
				
					
				
			* Simplify SnapshotGlPictureRecorder * Add size * Remove setDefaultBufferSize * Call onStart at each frame * Add hasOverlay boolean * Use workaround in SnapshotVideoRecorder * Add issue description * Small changes * Move workaround.start() * use glBindTexture instead of workaround * Remove bindTexture from EglViewport * Simplify workaround * Reuse fallback WorkerHandler * Draw overlays on the encoder thread * Use lock instead of afterOverlayGlDrawn * Improve comments and readability * Move blending code to OverlayDrawer * Add OverlayDrawer tests * Improve WorkerHandler testspull/531/head
							parent
							
								
									3e0ae65dad
								
							
						
					
					
						commit
						42de6e30a4
					
				| @ -0,0 +1,120 @@ | ||||
| package com.otaliastudios.cameraview.overlay; | ||||
| 
 | ||||
| 
 | ||||
| import android.content.res.XmlResourceParser; | ||||
| import android.graphics.Canvas; | ||||
| import android.util.AttributeSet; | ||||
| import android.util.Xml; | ||||
| import android.view.View; | ||||
| import android.view.ViewGroup; | ||||
| 
 | ||||
| import androidx.annotation.NonNull; | ||||
| import androidx.test.annotation.UiThreadTest; | ||||
| import androidx.test.ext.junit.runners.AndroidJUnit4; | ||||
| import androidx.test.filters.SmallTest; | ||||
| 
 | ||||
| import com.otaliastudios.cameraview.BaseTest; | ||||
| import com.otaliastudios.cameraview.internal.egl.EglBaseSurface; | ||||
| import com.otaliastudios.cameraview.internal.egl.EglCore; | ||||
| import com.otaliastudios.cameraview.internal.egl.EglViewport; | ||||
| import com.otaliastudios.cameraview.size.Size; | ||||
| 
 | ||||
| import org.hamcrest.BaseMatcher; | ||||
| import org.hamcrest.Description; | ||||
| import org.hamcrest.Matcher; | ||||
| import org.junit.After; | ||||
| import org.junit.Before; | ||||
| import org.junit.Test; | ||||
| import org.junit.runner.RunWith; | ||||
| import org.mockito.Mockito; | ||||
| 
 | ||||
| import static org.junit.Assert.assertFalse; | ||||
| import static org.junit.Assert.assertThat; | ||||
| import static org.junit.Assert.assertTrue; | ||||
| import static org.mockito.ArgumentMatchers.any; | ||||
| import static org.mockito.ArgumentMatchers.anyFloat; | ||||
| import static org.mockito.ArgumentMatchers.anyLong; | ||||
| import static org.mockito.ArgumentMatchers.eq; | ||||
| import static org.mockito.Mockito.mock; | ||||
| import static org.mockito.Mockito.never; | ||||
| import static org.mockito.Mockito.reset; | ||||
| import static org.mockito.Mockito.spy; | ||||
| import static org.mockito.Mockito.times; | ||||
| import static org.mockito.Mockito.verify; | ||||
| import static org.mockito.Mockito.when; | ||||
| 
 | ||||
| @RunWith(AndroidJUnit4.class) | ||||
| @SmallTest | ||||
| public class OverlayDrawerTest extends BaseTest { | ||||
| 
 | ||||
|     private final static int WIDTH = 100; | ||||
|     private final static int HEIGHT = 100; | ||||
| 
 | ||||
|     private EglCore eglCore; | ||||
|     private EglBaseSurface eglSurface; | ||||
| 
 | ||||
|     @Before | ||||
|     public void setUp() { | ||||
|         eglCore = new EglCore(null, EglCore.FLAG_RECORDABLE); | ||||
|         eglSurface = new EglBaseSurface(eglCore); | ||||
|         eglSurface.createOffscreenSurface(WIDTH, HEIGHT); | ||||
|         eglSurface.makeCurrent(); | ||||
|     } | ||||
| 
 | ||||
|     @After | ||||
|     public void tearDown() { | ||||
|         eglSurface.releaseEglSurface(); | ||||
|         eglSurface = null; | ||||
|         eglCore.release(); | ||||
|         eglCore = null; | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|     public void testDraw() { | ||||
|         Overlay overlay = mock(Overlay.class); | ||||
|         OverlayDrawer drawer = new OverlayDrawer(overlay, new Size(WIDTH, HEIGHT)); | ||||
|         drawer.draw(Overlay.Target.PICTURE_SNAPSHOT); | ||||
|         verify(overlay, times(1)).drawOn( | ||||
|                 eq(Overlay.Target.PICTURE_SNAPSHOT), | ||||
|                 any(Canvas.class)); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|     public void testGetTransform() { | ||||
|         // We'll check that the transform is not all zeros, which is highly unlikely
 | ||||
|         // (the default transform should be the identity matrix)
 | ||||
|         OverlayDrawer drawer = new OverlayDrawer(mock(Overlay.class), new Size(WIDTH, HEIGHT)); | ||||
|         drawer.draw(Overlay.Target.PICTURE_SNAPSHOT); | ||||
|         assertThat(drawer.getTransform(), new BaseMatcher<float[]>() { | ||||
|             public void describeTo(Description description) { } | ||||
|             public boolean matches(Object item) { | ||||
|                 float[] array = (float[]) item; | ||||
|                 for (float value : array) { | ||||
|                     if (value != 0.0F) return true; | ||||
|                 } | ||||
|                 return false; | ||||
|             } | ||||
|         }); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|     public void testRender() { | ||||
|         OverlayDrawer drawer = new OverlayDrawer(mock(Overlay.class), new Size(WIDTH, HEIGHT)); | ||||
|         drawer.mViewport = spy(drawer.mViewport); | ||||
|         drawer.draw(Overlay.Target.PICTURE_SNAPSHOT); | ||||
|         drawer.render(); | ||||
|         verify(drawer.mViewport, times(1)).drawFrame( | ||||
|                 drawer.mTextureId, | ||||
|                 drawer.getTransform() | ||||
|         ); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|     public void testRelease() { | ||||
|         OverlayDrawer drawer = new OverlayDrawer(mock(Overlay.class), new Size(WIDTH, HEIGHT)); | ||||
|         EglViewport viewport = spy(drawer.mViewport); | ||||
|         drawer.mViewport = viewport; | ||||
|         drawer.release(); | ||||
|         verify(viewport, times(1)).release(); | ||||
|     } | ||||
| } | ||||
| @ -0,0 +1,119 @@ | ||||
| package com.otaliastudios.cameraview.internal; | ||||
| 
 | ||||
| import android.graphics.Canvas; | ||||
| import android.graphics.Color; | ||||
| import android.graphics.SurfaceTexture; | ||||
| import android.opengl.GLES11Ext; | ||||
| import android.opengl.GLES20; | ||||
| import android.view.Surface; | ||||
| import com.otaliastudios.cameraview.internal.egl.EglViewport; | ||||
| import com.otaliastudios.cameraview.preview.RendererThread; | ||||
| 
 | ||||
| 
 | ||||
| /** | ||||
|  * Fixes an issue for some devices with snapshot picture and video recording. | ||||
|  * This is so unclear that I wanted to have a separate class holding code and comments. | ||||
|  * | ||||
|  * WHEN TO USE THIS CLASS | ||||
|  * There is actually no need of this class in some cases: | ||||
|  * - when we don't have overlays, everything works | ||||
|  * - on the majority of devices, everything works | ||||
|  * But some devices will show the issue #514 and so they need this class to fix it. | ||||
|  * We will use this always since it should have close to none performance impact. | ||||
|  * | ||||
|  * SNAPSHOT PROCEDURE | ||||
|  * The issue is about picture and video snapshots with overlays. In both cases, we: | ||||
|  * 1. Take textureId from the camera preview | ||||
|  * 2. Take EGLContext from the camera preview thread ({@link RendererThread}) | ||||
|  * 3. Create an overlayTextureId | ||||
|  * 4. Create an overlaySurfaceTexture | ||||
|  * 5. Create an overlaySurface | ||||
|  * 6. Move to another thread | ||||
|  * 7. Create a new EGLContext using the old context as a shared context so we have texture data | ||||
|  * 8. Create a new EGLWindow using some surface as output | ||||
|  * 9. For each frame: | ||||
|  *    9A. Draw overlays on the overlaySurface.lockCanvas() / unlockCanvasAndPost() | ||||
|  *    9B. Publish overlays to GL texture using overlaySurfaceTexture.updateTexImage() | ||||
|  *    9C. GLES - draw textureId | ||||
|  *    9D. GLES - draw overlayTextureId | ||||
|  * Both textures are drawn on the same EGLWindow and we manage to overlay them with {@link GLES20#GL_BLEND}. | ||||
|  * This is the whole procedure and it works for the majority of devices and situations. | ||||
|  * | ||||
|  * ISSUE DESCRIPTION | ||||
|  * The #514 issue can be described as follows: | ||||
|  * - Overlays have no transparency: background is {@link Color#BLACK} and covers the video | ||||
|  * - Overlays have distorted colors: {@link Color#RED} becomes greenish, | ||||
|  *                                   {@link Color#GREEN} becomes blueish, | ||||
|  *                                   {@link Color#BLUE} becomes reddish | ||||
|  * | ||||
|  * ISSUE INSIGHTS | ||||
|  * After painful debugging, we have reached these conclusions: | ||||
|  * 1. Overlays are drawn on {@link Canvas} with the correct format | ||||
|  *    This can be checked for example by applying alpha to one overlay. The final color will | ||||
|  *    be faded out, although on a black background. So the {@link Canvas} drawing step works well. | ||||
|  * 2. The GLES shader will always receive pixels in RGBA | ||||
|  *    This seems to be a constant in Android - someone does the conversion for us at a lower level. | ||||
|  *    This was confirmed for example by forcing A=0.5 and seeing the video frames behind the overlay | ||||
|  *    black background, or by forcing to 0.0 some of the channels and seeing the output. | ||||
|  * 3. The {@link Canvas} / {@link Surface} pixels are wrongly treated as YUV! | ||||
|  *    On problematic devices, some component down there thinks that our overlays RGBA are in YUV, | ||||
|  *    and will CONVERT THEM TO RGBA. This means: | ||||
|  *    3A. Original alpha is dropped. The algorithm thinks we have passed YUV. | ||||
|  *    3B. Original colors are messed up. For example, (255,0,0,255,RGBA) is treated as (255,0,0,YUV) | ||||
|  *        and converted back to rgb becoming greenish (74,255,27,255,RGBA). | ||||
|  *        Doing the same conversion for {@link Color#GREEN} and {@link Color#BLUE} confirms what we | ||||
|  *        were seeing in the issue screenshots. | ||||
|  * | ||||
|  * So a pixel format conversion takes place, when it shouldn't happen. We can't solve this: | ||||
|  * - It is done at a lower level, there's no real way for us to specify the surface format, but | ||||
|  *   it seems that these devices will prefer a YUV format and misunderstand our {@link Canvas} pixels. | ||||
|  * - There is also no way to identify which devices will present this issue, it's a bug somewhere | ||||
|  *   and it is implementation specific. | ||||
|  * | ||||
|  * THE MAGIC | ||||
|  * Hard to say why, but using this class fixes the described issue. | ||||
|  * It seems that when the {@link SurfaceTexture#updateTexImage()} method for the overlay surface | ||||
|  * is called - the one that updates the overlayTextureId - we must ensure that the CURRENTLY | ||||
|  * BOUND TEXTURE ID IS NOT 0. The id we choose to apply might be cameraTextureId, or overlayTextureId, | ||||
|  * or probably whatever other valid id, and should be passed to {@link #Issue514Workaround(int)}. | ||||
|  * [Tested with cameraTextureId and overlayTextureId: both do work.] | ||||
|  * [Tested with invalid id like 9999. This won't work.] | ||||
|  * | ||||
|  * This makes no sense, since overlaySurfaceTexture.updateTexImage() is setting it to overlayTextureId | ||||
|  * anyway, but it fixes the issue. Specifically, after any draw operation with {@link EglViewport}, | ||||
|  * the bound texture is reset to 0 so this must be undone here. We offer: | ||||
|  * | ||||
|  * - {@link #beforeOverlayUpdateTexImage()} to be called before the {@link SurfaceTexture#updateTexImage()} call | ||||
|  * - {@link #end()} to release and bring things back to normal state | ||||
|  * | ||||
|  * Since updating and rendering can happen on different threads with a shared EGL context, | ||||
|  * in case they do, the {@link #beforeOverlayUpdateTexImage()}, the actual updateTexImage() and | ||||
|  * finally the {@link EglViewport} drawing operations should be synchronized with a lock. | ||||
|  * | ||||
|  * REFERENCES | ||||
|  * https://github.com/natario1/CameraView/issues/514
 | ||||
|  * https://android.googlesource.com/platform/frameworks/native/+/5c1139f/libs/gui/SurfaceTexture.cpp
 | ||||
|  * I can see here that SurfaceTexture does indeed call glBindTexture with the same parameters whenever | ||||
|  * updateTexImage is called, but it also does other gl stuff first. This other gl stuff might be | ||||
|  * breaking when we don't have a bound texture on some specific hardware implementation. | ||||
|  */ | ||||
| public class Issue514Workaround { | ||||
| 
 | ||||
|     private final int textureId; | ||||
| 
 | ||||
|     public Issue514Workaround(int textureId) { | ||||
|         this.textureId = textureId; | ||||
|     } | ||||
| 
 | ||||
|     public void beforeOverlayUpdateTexImage() { | ||||
|         bindTexture(textureId); | ||||
|     } | ||||
| 
 | ||||
|     public void end() { | ||||
|         bindTexture(0); | ||||
|     } | ||||
| 
 | ||||
|     private void bindTexture(int textureId) { | ||||
|         GLES20.glBindTexture(GLES11Ext.GL_TEXTURE_EXTERNAL_OES, textureId); | ||||
|     } | ||||
| } | ||||
| @ -0,0 +1,131 @@ | ||||
| package com.otaliastudios.cameraview.overlay; | ||||
| 
 | ||||
| import android.graphics.Canvas; | ||||
| import android.graphics.Color; | ||||
| import android.graphics.PorterDuff; | ||||
| import android.graphics.SurfaceTexture; | ||||
| import android.opengl.GLES11Ext; | ||||
| import android.opengl.GLES20; | ||||
| import android.view.Surface; | ||||
| 
 | ||||
| import androidx.annotation.NonNull; | ||||
| import androidx.annotation.VisibleForTesting; | ||||
| 
 | ||||
| import com.otaliastudios.cameraview.CameraLogger; | ||||
| import com.otaliastudios.cameraview.internal.Issue514Workaround; | ||||
| import com.otaliastudios.cameraview.internal.egl.EglViewport; | ||||
| import com.otaliastudios.cameraview.size.Size; | ||||
| 
 | ||||
| import java.nio.Buffer; | ||||
| 
 | ||||
| 
 | ||||
| /** | ||||
|  * Draws overlays through {@link Overlay}. | ||||
|  * | ||||
|  * - Provides a {@link Canvas} to be passed to the Overlay | ||||
|  * - Lets the overlay draw there: {@link #draw(Overlay.Target)} | ||||
|  * - Renders this into the current EGL window: {@link #render()} | ||||
|  * - Applies the {@link Issue514Workaround} the correct way | ||||
|  * | ||||
|  * In the future we might want to use a different approach than {@link EglViewport}, | ||||
|  * {@link SurfaceTexture} and {@link GLES11Ext#GL_TEXTURE_EXTERNAL_OES}, | ||||
|  * for example by using a regular {@link GLES20#GL_TEXTURE_2D} that might | ||||
|  * be filled through {@link GLES20#glTexImage2D(int, int, int, int, int, int, int, int, Buffer)}. | ||||
|  * | ||||
|  * The current approach has some issues, for example see {@link Issue514Workaround}. | ||||
|  */ | ||||
| public class OverlayDrawer { | ||||
| 
 | ||||
|     private static final String TAG = OverlayDrawer.class.getSimpleName(); | ||||
|     private static final CameraLogger LOG = CameraLogger.create(TAG); | ||||
| 
 | ||||
|     private Overlay mOverlay; | ||||
|     @VisibleForTesting int mTextureId; | ||||
|     private SurfaceTexture mSurfaceTexture; | ||||
|     private Surface mSurface; | ||||
|     private float[] mTransform = new float[16]; | ||||
|     @VisibleForTesting EglViewport mViewport; | ||||
|     private Issue514Workaround mIssue514Workaround; | ||||
|     private final Object mIssue514WorkaroundLock = new Object(); | ||||
| 
 | ||||
|     public OverlayDrawer(@NonNull Overlay overlay, @NonNull Size size) { | ||||
|         mOverlay = overlay; | ||||
|         mViewport = new EglViewport(); | ||||
|         mTextureId = mViewport.createTexture(); | ||||
|         mSurfaceTexture = new SurfaceTexture(mTextureId); | ||||
|         mSurfaceTexture.setDefaultBufferSize(size.getWidth(), size.getHeight()); | ||||
|         mSurface = new Surface(mSurfaceTexture); | ||||
|         mIssue514Workaround = new Issue514Workaround(mTextureId); | ||||
|     } | ||||
| 
 | ||||
|     /** | ||||
|      * Should be called to draw the {@link Overlay} on the given {@link Overlay.Target}. | ||||
|      * This will provide a working {@link Canvas} to the overlay and also update the | ||||
|      * drawn contents to a GLES texture. | ||||
|      * @param target the target | ||||
|      */ | ||||
|     public void draw(@NonNull Overlay.Target target) { | ||||
|         try { | ||||
|             final Canvas surfaceCanvas = mSurface.lockCanvas(null); | ||||
|             surfaceCanvas.drawColor(Color.TRANSPARENT, PorterDuff.Mode.CLEAR); | ||||
|             mOverlay.drawOn(target, surfaceCanvas); | ||||
|             mSurface.unlockCanvasAndPost(surfaceCanvas); | ||||
|         } catch (Surface.OutOfResourcesException e) { | ||||
|             LOG.w("Got Surface.OutOfResourcesException while drawing video overlays", e); | ||||
|         } | ||||
|         synchronized (mIssue514WorkaroundLock) { | ||||
|             mIssue514Workaround.beforeOverlayUpdateTexImage(); | ||||
|             mSurfaceTexture.updateTexImage(); | ||||
|         } | ||||
|         mSurfaceTexture.getTransformMatrix(mTransform); | ||||
|     } | ||||
| 
 | ||||
|     /** | ||||
|      * Returns the transform that should be used to render the drawn content. | ||||
|      * This should be called after {@link #draw(Overlay.Target)} and can be modified. | ||||
|      * @return the transform matrix | ||||
|      */ | ||||
|     public float[] getTransform() { | ||||
|         return mTransform; | ||||
|     } | ||||
| 
 | ||||
|     /** | ||||
|      * Renders the drawn content in the current EGL surface, assuming there is one. | ||||
|      * Should be called after {@link #draw(Overlay.Target)} and any {@link #getTransform()} | ||||
|      * modification. | ||||
|      */ | ||||
|     public void render() { | ||||
|         // Enable blending
 | ||||
|         // Reference http://www.learnopengles.com/android-lesson-five-an-introduction-to-blending/
 | ||||
|         GLES20.glDisable(GLES20.GL_CULL_FACE); | ||||
|         GLES20.glDisable(GLES20.GL_DEPTH_TEST); | ||||
|         GLES20.glEnable(GLES20.GL_BLEND); | ||||
|         GLES20.glBlendFunc(GLES20.GL_SRC_ALPHA, GLES20.GL_ONE_MINUS_SRC_ALPHA); | ||||
| 
 | ||||
|         synchronized (mIssue514WorkaroundLock) { | ||||
|             mViewport.drawFrame(mTextureId, mTransform); | ||||
|         } | ||||
|     } | ||||
| 
 | ||||
|     /** | ||||
|      * Releases resources. | ||||
|      */ | ||||
|     public void release() { | ||||
|         if (mIssue514Workaround != null) { | ||||
|             mIssue514Workaround.end(); | ||||
|             mIssue514Workaround = null; | ||||
|         } | ||||
|         if (mSurfaceTexture != null) { | ||||
|             mSurfaceTexture.release(); | ||||
|             mSurfaceTexture = null; | ||||
|         } | ||||
|         if (mSurface != null) { | ||||
|             mSurface.release(); | ||||
|             mSurface = null; | ||||
|         } | ||||
|         if (mViewport != null) { | ||||
|             mViewport.release(); | ||||
|             mViewport = null; | ||||
|         } | ||||
|     } | ||||
| } | ||||
					Loading…
					
					
				
		Reference in new issue