+++ This bug was initially created as a clone of Bug #871485 +++ Created this bug from comment #38. During Bug 871485, I often see green video frame drawn especially in following use case. - Bug 871410 - Video playback error after refreshing a direct linked content. The problem is caused by ShadowImageLayerOGL::RenderLayer(). It does not care about following situation. - no graphic buffer for rendering. but try to render a texture. It happened on rendering after VideoFrameContainer::ClearCurrentFrame().

There are two ways to solve the problem -[1] skip the rendering -[2] draw as black video frame

Nominate to leo, because green video frame gives very bad impression to users.

Bug 876838 might happen by this defect.

(In reply to Sotaro Ikeda [:sotaro] from comment #4) > Created attachment 755629 [details] [diff] [review] > patch - skip rendering if there is no image to draw By applying the patch, I confirmed the problem is fixed on v1.1 buri with Bug 871485's patches.

(In reply to Sotaro Ikeda [:sotaro] from comment #1) > There are two ways to solve the problem > -[1] skip the rendering > -[2] draw as black video frame roc, which way is the correct fix in this case? Can I have a comment about it? attachment 755629 [details] [diff] [review] choosed [1] for simplicity.

Can we use the previous frame? That would be best. Otherwise I think black would be best. Showing what's underneath the video would be confusing since that's never normally visible --- there could be strange Web content hidden under there.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #7) > Can we use the previous frame? That would be best. > > Otherwise I think black would be best. Showing what's underneath the video > would be confusing since that's never normally visible --- there could be > strange Web content hidden under there. Thanks for the comment. In this case, previous frame can not be used. It was already send back to content process and destroyed. Then I am going to implement [2](draw black).

Created patch by reusing the code of RenderColorLayer(). Confirmed the patch works on v1.1 buri.

Previous patch was wrong one. Replace it.

Comment on attachment 755693 [details] [diff] [review] patch - draw black when there is no image to draw roc, can you review the path?

Comment on attachment 755693 [details] [diff] [review] patch - draw black when there is no image to draw Review of attachment 755693 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/opengl/ImageLayerOGL.cpp @@ +1018,5 @@ > } while (mTexImage->NextTile()); > } > #ifdef MOZ_WIDGET_GONK > + } else if (mSize == gfxIntSize(0, 0)) { > + // when there is no image for rendering, draw the region by black color "fill the region with black" @@ +1035,5 @@ > + float opacity = GetEffectiveOpacity() * color.a; > + color.r *= opacity; > + color.g *= opacity; > + color.b *= opacity; > + color.a = opacity; why are we doing all this? just do gfxRGBA color(0.0, 0.0, 0.0, GetEffectiveOpacity());

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #12) > Comment on attachment 755693 [details] [diff] [review] > patch - draw black when there is no image to draw > > Review of attachment 755693 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/layers/opengl/ImageLayerOGL.cpp > @@ +1018,5 @@ > > } while (mTexImage->NextTile()); > > } > > #ifdef MOZ_WIDGET_GONK > > + } else if (mSize == gfxIntSize(0, 0)) { > > + // when there is no image for rendering, draw the region by black color > > "fill the region with black" Update in next patch. > > @@ +1035,5 @@ > > + float opacity = GetEffectiveOpacity() * color.a; > > + color.r *= opacity; > > + color.g *= opacity; > > + color.b *= opacity; > > + color.a = opacity; > > why are we doing all this? just do > gfxRGBA color(0.0, 0.0, 0.0, GetEffectiveOpacity()); Yes, I'll update in next patch. Thanks.

Carry "roc: review+". This patch could be applyed only for b2g18. master's source code do not have a same function by gfx layer refactoring.

fix "else if" to following. + } else if (mExternalBufferTexture.IsAllocated() && mSize == gfxIntSize(0, 0)) {

Comment on attachment 755795 [details] [diff] [review] patch v3 for b2g18 - draw black when there is no image to draw roc, can you review again? This patch changes only "else if" line. During rechecking the patch. I thought this change is necessary.

(In reply to Sotaro Ikeda [:sotaro] from comment #17) > https://tbpl.mozilla.org/?tree=Try&rev=35744f50839b reftest failed a log. I am not sure it is related to the change.

(In reply to Sotaro Ikeda [:sotaro] from comment #18) > (In reply to Sotaro Ikeda [:sotaro] from comment #17) > > https://tbpl.mozilla.org/?tree=Try&rev=35744f50839b > > reftest failed a log. I am not sure it is related to the change. The change should affect only to video playback with hw video codec and camera playback.

Following part might be wrong. } else { mSize = gfxIntSize(0, 0); mPictureRect = nsIntRect(0, 0, 0, 0); }

I pushed different patch to try that restricted to affect only to hw video codec rendering and camera preview. but reftest still failed. It seems that base source code has problem. https://tbpl.mozilla.org/?tree=Try&rev=765c22449661

I pushed the no change patch to tryserver and got same result. So the patch make no problem. It seems that some tests do not run correctly on b2g18 https://tbpl.mozilla.org/?tree=Try&rev=9d5b91fd1859

Add header to the patch. Carry "roc: review+".

(In reply to Sotaro Ikeda [:sotaro] from comment #23) > Created attachment 756675 [details] [diff] [review] > patch v4 for b2g18 - draw black when there is no image to draw > > Add header to the patch. Carry "roc: review+". The patch is only for b2g18. There is no same function on master.