Sometimes tearing is observed on FrameBuffer surface (FB layer) during video playback. It is easy to reproduce with below use-case: 1. Play a video and rotate device by 90 degree. 2. Continuously tap on screen during video playback. Issue is easy to reproduce if we force GPU Composition. It seems FrameBuffer surface is closing / re-using the release fence fd of FB layer before the FB release fence gets signalled (i.e. when driver is still reading it). To verify this, if I add 16.67 msec (1 vsync) wait before setting FB release fence fd then tearing does not happen. This is the patch: @@ -575,6 +575,7 @@ HwcComposer2D::Render(EGLDisplay dpy, EGLSurface sur) // GPU or partial HWC Composition Commit(); + usleep(16666); GetGonkDisplay()->SetFBReleaseFd(mList->hwLayers[mList->numHwLayers - 1].releaseFenceFd); return true; }

Sotaro, can you take a look at this ?

I did the STR in Comment 0 by using nexus-4. I could not reproduce it. Did you use default video app to confirm it. It might be related to vsync and FB layer's fence implementation. In android, SurfaceFlinger does composing at most once in each vsync.

(In reply to Sotaro Ikeda [:sotaro] from comment #2) > I did the STR in Comment 0 by using nexus-4. I could not reproduce it. Did > you use default video app to confirm it. Suhil, did you use default video app to confirm it?

Yes, I checked on default Video App. I checked on reference Copybit device with no H/W overlay. Nexus-4 has H/W overlay.

(In reply to Sotaro Ikeda [:sotaro] from comment #2) > I did the STR in Comment 0 by using nexus-4. I could not reproduce it. Did > you use default video app to confirm it. It might be related to vsync and FB > layer's fence implementation. In android, SurfaceFlinger does composing at > most once in each vsync. No, it is not related to vsync. Because I also checked by adding h/w vsync support and made sure that HwcComposer2D waits for h/w vsync event (coming from HAL) before calling mHwc->prepare (similar to SurfaceFlinger). But it does not fix this issue. If sync-fences are handled correctly, vsync is not needed.

Hey Sotaro, do you have a QRD8x28 device yet? Ref: bug 961246

I do not have QRD8x28 :-(

You probably should get one! Ask your folks for details.

(In reply to Michael Vines [:m1] [:evilmachines] from comment #8) > You probably should get one! Ask your folks for details. Yeah, I am going ask milan or cj about it.

Kevin, do you know how we can get some of these?

Sotaro, This tearing happens on H/W overlay device as well. On adding delay mentioned above, it is not observed. I suspect FB layer's fence implementation. It seems FB Surface is not waiting until FB release fence get signaled. To reproduce this issue, you can also disable H/W Composition and follow STR in Comment 0.

Sotaro, you can check this on v1.3 Nexus-4 with GPU composition.

Sushil, I still can not reproduce the problem on nexus-4. Can you explain more about the symptom and provide the video that the problem happens?

Sotaro, If you are testing on v1.3 Nexus-4, I had recently added a work around to handle this issue by making driver to wait for acquire fence of FB layer to signal. So to reproduce this issue, you need to revert this change in your set-up: https://www.codeaurora.org/cgit/quic/la/platform/hardware/qcom/display/commit/?h=b2g_jb_3.2&id=221706255141c3281901698c52924e0e368214d6 You can play any video (I guess it is easy to reproduce in low resolution video), keep device at 90 degree and keep on tapping the screen so that frames fallback to GPU Composition. Tearing only happens on FB surface. I do not suspect driver since this issue does not happen on Android with same build. I suspect 1 of these possible causes for this issue: 1. FrameBuffer surface is not waiting until FB release fence (set by the driver) gets signalled. 2. We are passing wrong/in-valid acquire fence of FB surface (via FB layer) to driver.

Sushi, I created a nexus-4 build environment by using b2g's default script. hwc_utils.cpp has a following code, but MDP_BUF_SYNC_FLAG_WAIT is not set in my environment. >hwc_sync() >> snip > > if(mdpVersion >= qdutils::MDSS_V5) { > data.flags = MDP_BUF_SYNC_FLAG_WAIT; > }

Since Nexus-4 driver < MDSS_V5 that is why hwc_sync() code in your environment is different. So I would suggest, please check this issue on QRD8x28 which is >= MDSS_V5 on b2g v1.3. Also you need to revert the CAF patch which I mentioned in Comment 14.

Sotaro, If I increase NUM_FRAMEBUFFER_SURFACE_BUFFERS to 3, it is very hard to reproduce tearing in the reported use case. So it gives me hint that eglSwapBuffers() call is not waiting for FB release fence of previous draw cycle to signal. Since, composition is asynchronous. Something like this is happening here in FB Surface: 1. FB[0]: eglSwapBuffers -> FBS::nextBuffer() -> Slot 0 -> hwc_set -> driver -> FB releaseFenceFd is passed to FBS. 2. FB[1]: eglSwapBuffers -> FBS::nextBuffer() -> Slot 1 -> hwc_set -> driver -> FB releaseFenceFd is passed to FBS. 3. FB[0]: eglSwapBuffers -> FBS::nextBuffer() -> Slot 0 -> acquireBufferLocked() -> releaseBufferLocked() -> Tearing It seems [3] is happening when driver is still reading the releaseFenceFd of [1] and it has not been signalled yet. I checked, time taken by acquireBufferLocked() and releaseBufferLocked() is always zero. Where do we wait for FB release fence to signal ?

From, Comment 17, it seems more a release fence problem than an acquire fence problem.

The problem's possible candidate seems to categorized to the following. -[1] FBS's release fence is not correctly delivered to FBS's ANativeWindow -[2] FBS's release fence is not correctly handled by FBS's OpenGL module -[3] FBS's release fence does not work correctly, when multiple rendering is done during one vsync. -[4] FBS's aquire fence is not correctly delivered to HWC -[5] FBS's aquire fence is not correctly handled by HWC

(In reply to Sushil from comment #17) > > It seems [3] is happening when driver is still reading the releaseFenceFd of > [1] and it has not been signalled yet. > I checked, time taken by acquireBufferLocked() and releaseBufferLocked() is > always zero. Where do we wait for FB release fence to signal ? FB release fence should be wait on FBS's OpenGL module. The FBS's OpenGL module gets the release fence by calling ANativeWindow::dequeueBuffer()[Surface::dequeueBuffer()]. If you want to wait the release fence complete somewhere just for testing, FramebufferSurface::setReleaseFenceFd() might be a good place.

The FB's release fence is stored in FramebufferSurface like the following sequence. CompositorOGL::EndFrame() ->GLContextEGL::SwapBuffers() ->HwcComposer2D::Render() ->GonkDisplayJB::SwapBuffers() ->eglSwapBuffers(); ->GonkDisplayJB::Post() ->hwc_composer_device_1_t::prepare() ->hwc_composer_device_1_t::set() ->GonkDisplayJB::SetFBReleaseFd() // set FB's release fence ->FramebufferSurface::setReleaseFenceFd() // set FB's release fence ->ConsumerBase::addReleaseFence() ->ConsumerBase::addReleaseFenceLocked() ->Fence::merge();//merge the fence ->mSlots[slot].mFence = mergedFence; //sotre merged fence in ConsumerBase.

Comment 21, is the call sequence when HWC rendering is disabled.

The release fence that stored by FramebufferSurface(ConsumerBase) is returned to BufferQueue with a gralloc buffer during the next rendering like the following. CompositorOGL::EndFrame() ->GLContextEGL::SwapBuffers() ->HwcComposer2D::Render() ->GonkDisplayJB::SwapBuffers() ->eglSwapBuffers(); // some calls ->BufferQueue::queueBuffer() ->FramebufferSurface::onFrameAvailable() ->FramebufferSurface::nextBuffer() ->ConsumerBase::acquireBufferLocked() ->BufferQueue::acquireBuffer() ->ConsumerBase::releaseBufferLocked() ;// release previous buffer and previous release fence ->BufferQueue::releaseBuffer() // release previous buffer and previous release fence

(In reply to Sushil from comment #14) > frames fallback to GPU Composition. Tearing only happens on FB surface. I do > not suspect driver since this issue does not happen on Android with same > build. I suspect 1 of these possible causes for this issue: > 1. FrameBuffer surface is not waiting until FB release fence (set by the > driver) gets signalled. FrameBuffer surface does not need to wait FB release fence complete. The release fence is got by FB's OpenGL module via ANativeWindow::dequeueBuffer(). And FB's OpenGL module should wait the fence complete.

The following is the call sequence for the FB's acquire fence when HWC composition is disabled(OpenGL composition). CompositorOGL::EndFrame() ->GLContextEGL::SwapBuffers() ->HwcComposer2D::Render() ->GonkDisplayJB::SwapBuffers() ->eglSwapBuffers(); // some calls ->Surface::queueBuffer() //queue a gralloc buffer and a fence for FB ->BufferQueue::queueBuffer() ->FramebufferSurface::onFrameAvailable() ->FramebufferSurface::nextBuffer() ->ConsumerBase::acquireBufferLocked() // got an aquire fence with a gralloc buffer ->BufferQueue::acquireBuffer() // got an aquire fence with a gralloc buffer ->ConsumerBase::releaseBufferLocked() ->//Store the latest aquire fence in FramebufferSurface. // It is used at next hwc_composer_device_1_t::set() lastFenceFD = acquireFence->dup(); ->GonkDisplayJB::Post() ->//Set the lastet FB's aquire fence to HWC mList->hwLayers[1].acquireFenceFd = fence; ->hwc_composer_device_1_t::prepare() ->hwc_composer_device_1_t::set() // composite

(In reply to Sotaro Ikeda [:sotaro] from comment #19) > The problem's possible candidate seems to categorized to the following. > -[1] FBS's release fence is not correctly delivered to FBS's ANativeWindow > -[4] FBS's aquire fence is not correctly delivered to HWC From the analysis, [1] and [4] seems correctly handled.

(In reply to Sotaro Ikeda [:sotaro] from comment #19) > The problem's possible candidate seems to categorized to the following. > -[3] FBS's release fence does not work correctly, when multiple rendering is > done during one vsync. As per Comment 5, I had checked with H/W vsync support, which makes sure that HwComposer2D waits for H/W vsync event from driver which makes sure that multiple rendering does not happen during 1 vsync. So [3] does not seem to be the root cause here.

Sotaro, I haven't gone into details of Comment 20 to 25 yet. But I tried by disabling HwcComposer2D & just use GonkDisplayJB for GPU Composition, I still see the tearing in video use case. In LayerManagerComposite.cpp: +/* if (composer2D && composer2D->TryRender(mRoot, mWorldMatrix)) { mCompositor->EndFrameForExternalComposition(mWorldMatrix); return; } +*/ In GLContextProviderEGL.cpp, comment out this block: if (mHwc) { return mHwc->Render(EGL_DISPLAY(), mSurface); }

Sotaro, I noticed this difference in framework: In GonkBufferQueue.cpp (b2g): mSynchronousMode(true), // GonkBufferQueue always works in sync mode. In BufferQueue.cpp (Android): mSynchronousMode(false) Presently, the composition is asynchronous. Does this flag mean that the composition needs to be synchronous in b2g? If this is the case, then it makes sense that if I make BUFFER_SYNC IOCTL call as synchronous which is patch [1], (which tells driver to wait for acquire fence of FB layer to signal in hwc set call), the tearing is not observed. It is hard to suspect HAL and driver in this issue because there is no tearing on Android with same HAL and driver. [1]: https://www.codeaurora.org/cgit/quic/la/platform/hardware/qcom/display/commit/?h=b2g_jb_3.2&id=221706255141c3281901698c52924e0e368214d6

(In reply to Sushil from comment #29) > Sotaro, I noticed this difference in framework: > > In GonkBufferQueue.cpp (b2g): > mSynchronousMode(true), // GonkBufferQueue always works in sync mode. I do not think it is related to the tearing problem. GonkBufferQueue is used only for video's ANativeWindow and camera hal's ANativeWindow. They are not related to composition. For frame buffer layer's composition, FramebufferSurface is used for it. In JB, FramebufferSurface works as sync mode also in b2g and android. http://mxr.mozilla.org/mozilla-central/source/widget/gonk/libdisplay/FramebufferSurface.cpp#70 http://androidxref.com/4.3_r2.1/xref/frameworks/native/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp#67

Sushil, Can you explain about FrameBuffer surface's tearing problem? Did you saw the problem on thebes layer part? or image layer part(video)?

(In reply to Sotaro Ikeda [:sotaro] from comment #30) > (In reply to Sushil from comment #29) > > Sotaro, I noticed this difference in framework: > > > > In GonkBufferQueue.cpp (b2g): > > mSynchronousMode(true), // GonkBufferQueue always works in sync mode. BufferQueue's sync/async is versatile to how to handle Fence.

Sotaro, I have attached the video clip which shows tearing on FB layer. It is mostly seen on Video part & very little on UI part. Does it matter since it is a single layer?

Sushil, thanks for the video. It is not clear what causes the problem. From the video, I thought another following possibility. - HW Codec start to decode video even when a gralloc buffer is still used by OpenGL.

Sushil, can you test if attachment 8371991 [details] [diff] [review] affect to the tearing?

Sotaro, Yes, tearing is not observed on pulling this patch.

From Comment 37, changed to correct component.

(In reply to Sushil from comment #37) > Sotaro, > > Yes, tearing is not observed on pulling this patch. Thanks for the confirmation. So, this is a video's OpenGL rendering problem. The following thing is happensing. - HW Codec start to decode video even when a gralloc buffer is still used by OpenGL. In current gecko, one video's gralloc buffer is held by compositor side. After the buffer swap, there is a time until the buffer is rendered by Compositor. During this time, previous gralloc buffer is still referenced by OpenGL via EGLImage. This seems to affect to the video rendreing. In ICS, gralloc buffer's ownership between "OpenGL's rendering" and "hw codec decoding" is controlled by genlock. But since JB, genlock is not used anymore and fence is used for it. But release fence is not provided when rendered by OpenGL.

(In reply to Sotaro Ikeda [:sotaro] from comment #35) > Created attachment 8371991 [details] [diff] [review] > temporary patch - hold 2 buffer at ImageClient Just increasing a number ofbuffers holed by ImageClient to 2 could cause a buffer starvation bug at hw codec on some hw like Bug 934870.

hw codec's buffers handling is already very very tight.

OpenGL composition does not provide release fence, we can not use android's fence object to fix the problem.

To solve the problem, returning gralloc buffer to ANativeWindow needs to put off until next composition end. attachment 8371991 [details] [diff] [review] is easier solution. But it increases one gralloc buffer to be held to gecko side.

Increment MIN_UNDEQUEUED_BUFFERS::MIN_UNDEQUEUED_BUFFERS to 3.

(In reply to Sotaro Ikeda [:sotaro] from comment #44) > Created attachment 8375063 [details] [diff] [review] > temporary patch - hold 2 buffer at ImageClient > > Increment MIN_UNDEQUEUED_BUFFERS::MIN_UNDEQUEUED_BUFFERS to 3. When we increases one gralloc buffer to be held to gecko side, we need to increase the number of gralloc buffer allocation at GonkBufferQueue. b2g ics hardeware/platform could not handle this change. So, I limit the change only to more than JB4.3.

Fix nits.

Comment on attachment 8375068 [details] [diff] [review] temporary patch v3 - hold 2 buffer at ImageClient Sushil, is there a problem to increment MIN_UNDEQUEUED_BUFFERS from 2 to 3?

Comment on attachment 8375068 [details] [diff] [review] temporary patch v3 - hold 2 buffer at ImageClient cheng.luo, can I have a feedback about to increment MIN_UNDEQUEUED_BUFFERS from 2 to 3? It could affect to hw codec and camera hal.

(In reply to Sotaro Ikeda [:sotaro] from comment #42) > OpenGL composition does not provide release fence, we can not use android's > fence object to fix the problem. If this is happening, then tearing should be observed on Camera preview (in GPU Composition) as well since it also has a Image layer. But I do not see any tearing on Camera preview's FB layer.

(In reply to Sotaro Ikeda [:sotaro] from comment #47) > Comment on attachment 8375068 [details] [diff] [review] > temporary patch v3 - hold 2 buffer at ImageClient > > Sushil, is there a problem to increment MIN_UNDEQUEUED_BUFFERS from 2 to 3? Not sure. In Android, MIN_UNDEQUEUED_BUFFERS is 2.

(In reply to Sushil from comment #49) > If this is happening, then tearing should be observed on Camera preview (in > GPU Composition) as well since it also has a Image layer. But I do not see > any tearing on Camera preview's FB layer. In attachment 8371859 [details], I saw future frame's color in tearing area. So, return gralloc buffer too early to ANativeWindow(OMXCodec) seems correct assumption. In Bug 901224, I fixed a similar problem. In this case, I saw the problem only on video playback case, not on camera preview. In video playback case, OMXCodec dequeue gralloc buffer from ANativeWindow soon after the buffer is returned to ANativeWindow. But in camera preview case, camera preview request gralloc buffer by the camera hal's own pace. In video playback case, gralloc buffer is much more scarce than camera preview case. This might affect to the differenct.

> > Sushil, is there a problem to increment MIN_UNDEQUEUED_BUFFERS from 2 to 3? > > Not sure. In Android, MIN_UNDEQUEUED_BUFFERS is 2. Actually, it allocate same number of buffers compared to android. In b2g BufferQueue works in sync mode. but in android BufferQueue works in async mode, if the BufferQueue is used for camera preview or media playback. In android, it is set at the following. http://androidxref.com/4.3_r2.1/xref/frameworks/native/services/surfaceflinger/SurfaceTextureLayer.cpp#64 In the sync mode, BufferQueue::getMinUndequeuedBufferCountLocked() return 2. In async mode, it returns 3(2+1). http://androidxref.com/4.3_r2.1/xref/frameworks/native/libs/gui/BufferQueue.cpp#1033 It seems better not to change MIN_UNDEQUEUED_BUFFERS, but to change getMinUndequeuedBufferCountLocked().

Apply Comment 52.

Comment on attachment 8375115 [details] [diff] [review] temporary patch v4 - hold 2 buffer at ImageClient Sushil, can I have a feedback to the updated patch?

Comment on attachment 8375115 [details] [diff] [review] temporary patch v4 - hold 2 buffer at ImageClient Review of attachment 8375115 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gonk/nativewindow/GonkBufferQueue.cpp @@ +1009,5 @@ > } > > int GonkBufferQueue::getMinUndequeuedBufferCountLocked() const { > +#if ANDROID_VERSION >= 18 > + return mMaxAcquiredBufferCount + 1; Will it happen on non-Video and non-Camera use cases as well ? If yes, is it possible to set "mSynchronousMode" to false for Video (Media) and Camera use cases in b2g (for ANDROID_VERSION >= 18) like Android.

(In reply to Sushil from comment #55) > Comment on attachment 8375115 [details] [diff] [review] > temporary patch v4 - hold 2 buffer at ImageClient > > > > int GonkBufferQueue::getMinUndequeuedBufferCountLocked() const { > > +#if ANDROID_VERSION >= 18 > > + return mMaxAcquiredBufferCount + 1; > > Will it happen on non-Video and non-Camera use cases as well ? > > If yes, is it possible to set "mSynchronousMode" to false for Video (Media) > and Camera use cases in b2g (for ANDROID_VERSION >= 18) like Android. GonkBufferQueue is used only for media and camera. For video and camera usage, we can not set to ""mSynchronousMode" to false" on gonk. The way of how to use ANativeWindow is different from android.

> GonkBufferQueue is used only for media and camera. For video and camera > usage, we can not set to ""mSynchronousMode" to false" on gonk. The way of > how to use ANativeWindow is different from android. It was an experience from ICS. It seems better to re-think about it again since JB.

I recall why current GonkNativeWindow is implemented like the current. I already modified the getMinUndequeuedBufferCountLocked() to return same as sync mode. In andorid, getMinUndequeuedBufferCountLocked() return 1 when it is sync, and return 2 when it is async. In current gonk getMinUndequeuedBufferCountLocked() always return 2.

For the time being, Attachment 8371991 [details] [diff] seems better one. But it might cause gralloc buffer starvation at hw codec in some hw like Bug 934870.

On b2g, GonkNativeWindow(BufferQueue)'s sync/async actually make no difference. Only getMinUndequeuedBufferCountLocked() is important. In android, BufferQueue is almost equal to rendering target. But in b2g, gralloc buffers are needed to be forwarded to compositor. Therefore, getMinUndequeuedBufferCountLocked() needs to be bigger than android. In android's video playback and camera preview case, getMinUndequeuedBufferCountLocked() is one in sync, getMinUndequeuedBufferCountLocked() is two in async.

Sotaro, is there any update on this ?

We're focusing on 1.4+ issues for the next three weeks, I expect Sotaro not to be able to look at this issue shortly.

From this week, I am going to work for Bug 974152. But 1.4+ bugs have more high priority.

On nexus-5, the video playback capability was just enabled. On nexus-5, I confirmed the tearing.

From Bug 974152 Comment 15, set to dup of Bug 974152.