1307342 - Fast path seems not used for video rendering on WebGL

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect, P2)

Description

[Sotaro Ikeda [:sotaro]]

It was implemented by Bug 814524. It was implemented as WebGLTexture::TexImageFromVideoElement(), but the function was removed by Bug 1221822.

In WebGLTexture::TexOrSubImage(), nsLayoutUtils::SurfaceFromElement() is called to get a surface. nsLayoutUtils::SurfaceFromElement() always return "result.mIsPremultiplied = true". It caused to skip fast path in TexUnpackImage::TexOrSubImage(), then mImage->GetAsSourceSurface() is called. The GetAsSourceSurface() internally does yuv-rgb convert or readback. 
  https://dxr.mozilla.org/mozilla-central/source/dom/canvas/TexUnpackBlob.cpp#499

Comment 1

[Sotaro Ikeda [:sotaro]]

Used the following to check comment 0.
 http://mdn.github.io/webgl-examples/tutorial/sample8/

Comment 2

[Ethan Lin[:ethlin]]

I'll try to fix it first before looking into bug 1246410.

Comment 3

[Ethan Lin[:ethlin]]

Attachment: Make GLBlitHelper accept mismatched premultiply alpha

The fast path was blocked by mismatch premultiply alpha, though the alpha value for video should be 1.0. I let GLBlitHelper can handle premultiply/unpremultply to fix the problem.

Comment 4

[Kelsey Gilbert [:jgilbert]]

(In reply to Ethan Lin[:ethlin] from comment #3)
> Created attachment 8798355 [details] [diff] [review]
> Make GLBlitHelper accept mismatched premultiply alpha
> 
> The fast path was blocked by mismatch premultiply alpha, though the alpha
> value for video should be 1.0. I let GLBlitHelper can handle
> premultiply/unpremultply to fix the problem.

Wait, why is the video being decoded into RGBA? Are you sure it's not RGB/RGBX?

Comment 5

[Ethan Lin[:ethlin]]

Video is decoded into ycbcr. If I do something like "gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, gl.RGBA, gl.UNSIGNED_BYTE, video)", gecko will converts the ycbcr data to RGBA texture with GPU(fast path) or CPU. 
BTW, currently image element also has the same problem due to the premultiply checking.

Comment 6

[Kelsey Gilbert [:jgilbert]]

(In reply to Ethan Lin[:ethlin] from comment #5)
> Video is decoded into ycbcr. If I do something like
> "gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, gl.RGBA, gl.UNSIGNED_BYTE,
> video)", gecko will converts the ycbcr data to RGBA texture with GPU(fast
> path) or CPU. 
> BTW, currently image element also has the same problem due to the
> premultiply checking.

Premultiply mismatches are something we should continue to warn about.

Why does YCbCr coerce into RGBA instead of RGB(X)?

Comment 7

[Ethan Lin[:ethlin]]

Sorry for the late reply. The destination format should be specified by texImage2D's 'internal format'. So if user set the 'internal format' to RGBA, then we'll convert YCbCr to RGBA texture.

Comment 8

[Ethan Lin[:ethlin]]

(In reply to Ethan Lin[:ethlin] from comment #7)
> Sorry for the late reply. The destination format should be specified by
> texImage2D's 'internal format'. So if user set the 'internal format' to
> RGBA, then we'll convert YCbCr to RGBA texture.

I also checked the webgl spec. I didn't see the limitation of the texture format for HTMLVideoElement, so the destination format should respect texImage2D's internal format.

Comment 9

[Kelsey Gilbert [:jgilbert]]

(In reply to Ethan Lin[:ethlin] from comment #7)
> Sorry for the late reply. The destination format should be specified by
> texImage2D's 'internal format'. So if user set the 'internal format' to
> RGBA, then we'll convert YCbCr to RGBA texture.

My question is why we're receiving RGBA from (conceptually) the decoder/converter.
Regardless, it's conceptually 'fine' if we get RGBA, we just need to have *some* fast path choice an app can make to incur no conversion.

I'm less interested in making conversion fast than I am in enabling zero-conversion operations.

Comment 10

[Ethan Lin[:ethlin]]

Per discussion with jgilbert, we should check (srcHasAlpha && dstHasAlpha && srcPremultAlpha != dstPremultAlpha) before BlitImageToFramebuffer. Actually my patch checks the format, but currently BlitImageToFramebuffer doesn't work with D3D11ShareHandleImage and MacIOSurface(2vuy). We should fix it.

Comment 11

[Peter Chang[:pchang]]

Assign myself first to find someone to work on it.

Comment 12

[Julien Cristau [:jcristau]]

Too late for firefox 52, mass-wontfix.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:jgilbert, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 14

[Kelsey Gilbert [:jgilbert]]

I suspect this was fixed at some point, but give a shout if not!