Currently, gecko still uses libyuv to convert the video texture to rgb format. We could just pass that ycbcr texture to WR without the color format conversion.

WR supports the planar-ycbcr image format. We could get rid of the software ycbcr to rgb color format conversion(using libyuv) in gecko. MozReview-Commit-ID: 2Fn7cEmc9On

Comment on attachment 8869741 [details] [diff] [review] Use WR PushYCbCrPlanarImage() for software-video-decoded buffer. Review of attachment 8869741 [details] [diff] [review]: ----------------------------------------------------------------- The patch looks good. But it is not clear what we need to do if we want to use wrench. From Bug 1352657, if we want to use wrench, it seems that we need to disable external image id handling locally. Can you explain how we could disable external image id handling with this patch?

I think I could update the wrench to support the external image for some regular gl textures, but I will hit the problem for some special gl texture handles(e.g. the TEXTURE_EXTERNAL texture target). Because we don't know the stride, internal format and size for that special gl texture. So, maybe it's hard to support the external image for wrench recently. So, I will try to do the similar thing as bug 1352657. Just use GetAsSurface() to get the buffer and pass to WR using add_image(). That calls will copy the image buffer to the WR's internal buffer. Then, we don't need to handle the platform specific texture problem. WebRenderTextureHost* wrTexture = texture->AsWebRenderTextureHost(); - if (wrTexture) { + if (false && wrTexture) { Before the dynamic video pipeline id, we should just hardcode the texture format in WebrenderImageLayer. So, I will just use a preference in WebrenderImageLayer to force to put a RGBA image display item at content side, and use GetAsSurface() and add_image() to pass the converted RGBA data to WR. Sotaro, what do you think for this?

(In reply to Jerry Shih[:jerry] (UTC+8) from comment #3) > Before the dynamic video pipeline id, we should just hardcode the texture > format in WebrenderImageLayer. So, I will just use a preference in > WebrenderImageLayer to force to put a RGBA image display item at content > side, and use GetAsSurface() and add_image() to pass the converted RGBA data > to WR. > > Sotaro, what do you think for this? It sounds good!

Bug 1367689 will try to handle the external image with wrench.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=75c321e179c1834b9af9cefc202797d15235fa1b

Hi Sotaro, There are a lot of webm video format test fails in [1]. The problem is that webm decoder is just output the single channel B8G8R8A8 format, but I assume all software decoder will output the 3 channel YCbCr buffer. So, I hit the assert at AddWRImage in [2]. MOZ_ASSERT(aImageKeys.length() == 1); Should we modify the output format as the WR video test workaround until the dynamic pipeline id? Should we wait the dynamic pipeline id patch for this problem? [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=75c321e179c1834b9af9cefc202797d15235fa1b [2] https://bugzilla.mozilla.org/attachment.cgi?id=8869741&action=diff#a/gfx/layers/composite/TextureHost.cpp_sec2

Software decoder does not always output 3 channel YCbCr. If video has alpha, the decoder output 4 channel data. In current gecko, the 4 channel output is converted to B8G8R8A8 format. It was added by Bug 1321076. Can't we add a code to detect format to WebRenderImageLayer?

MozReview-Commit-ID: LcSHVkBbM6G

This patch gets the image format from the image container and push the proper WR commands according to the image format. MozReview-Commit-ID: JrzwvvACGW9

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd339df5d806e7f4c1578b9a4a46f8664ffef90c

Hi Sotaro, There are still some reftests failed(the R6 and R8). It looks like the yuv-to-rgb conversion precision problem. Before this patch, we use [1] to convert yuv to rgb. Now, we are trying to use shader[2] for color conversion. I'm trying to do the similar calculation in shader, but there is a big lookup table used in [1]. It's not a good practice to use the big table in shader. I would like to increase the fuzzy comparing value for R8 and skip some test if we are use webrender for R6. This is the current testing setting: fails-if(layersGPUAccelerated&&!webrender) fails-if(webrender&&browserIsRemote) skip-if(Android) == object-fit-contain-webm-001.html object-fit-contain-webm-001-ref.html I'm trying to set fails-if if we have webrender. Sotaro, what do you think? try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffeabf6ed614ba1840ad3bc3453d624a577684c8 [1] https://dxr.mozilla.org/mozilla-central/source/gfx/ycbcr/yuv_convert.cpp#67 [2] https://dxr.mozilla.org/mozilla-central/source/gfx/webrender/res/ps_yuv_image.fs.glsl?q=path%3Aps_yuv_image.fs.glsl&redirect_type=single#20

(In reply to Jerry Shih[:jerry] (UTC+8) from comment #12) > Hi Sotaro, > > There are still some reftests failed(the R6 and R8). It looks like the > yuv-to-rgb conversion precision problem. Before this patch, we use [1] to > convert yuv to rgb. Now, we are trying to use shader[2] for color conversion. > I'm trying to do the similar calculation in shader, but there is a big > lookup table used in [1]. It's not a good practice to use the big table in > shader. > > I would like to increase the fuzzy comparing value for R8 and skip some test > if we are use webrender for R6. > > Sotaro, what do you think? Yea, it is reasonable.

Handle the ImageFormat::CAIRO_SURFACE case for some videos.

We use gpu for yuv color conversion now. There are some precision problems in gpu path. Mark them fails-if and update the fuzzy-test value. MozReview-Commit-ID: 75KdZmePRzb

rebase

rebase

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8551b2d6debc078ece8ec195d993513cde10f1d1

Wait for bug 1345054.

rebase.

With the video pipeline, we could call the WR image functions in compositor thread.

It's awful to put all combination of image key and WR command settings in the same place. Make the settings go back to textureHosts. Each textureHost should implement GetWRImageKeys() and PushExternalImage() function.

These 2 functions are used for WR. The GetWRImageKeys() will return the proper image keys according to the textureHost format. The PushExternalImage() will put all necessary WR commands into DisplayListBuilder for the textureHost rendering.

WR supports the planar-ycbcr image format. We turn to use the planar-ycbcr image to get rid of the software-ycbcr-to-rgb color format conversion(using libyuv) in gecko. The BufferTextureHost will use 3 image keys for SurfaceFormat::YUV format. The RenderBufferTextureHost will also use 3 DataSourceSurfaces to represent the 3 channel data in planar-ycbcr format.

MozReview-Commit-ID: 5cu8cYoTMxT

Update for DXGITextureHostD3D11 and DXGIYCbCrTextureHostD3D11.

The RenderTextureHost might calls some thread-specific functions(e.g. OpenGL calls) in ~RenderTextureHost(). Add a checking here to prevent this problem.

If we call UnregisterExternalImage() at non-render-thread and decrease the RenderTextureHost's ref-count to zero, the RenderTextureHost will be released in non-render-thread. That will cause some problems if we use some thread-specific functions in ~RenderTextureHost(). This patch uses a message loop in UnregisterExternalImage() to resolve this problem.

rebase.

Comment on attachment 8874371 [details] [diff] [review] P10: Update the thread model for RegisterExternalImage(), UnregisterExternalImage() and GetRenderTexture() call. Review of attachment 8874371 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/webrender_bindings/RenderThread.cpp @@ +289,5 @@ > { > + if (!IsInRenderThread()) { > + Loop()->PostTask(NewRunnableMethod<uint64_t>( > + this, &RenderThread::UnregisterExternalImage, aExternalImageId > + )); In RenderBufferTextureHost case, if RenderThread::UnregisterExternalImage() is called, buffer of RenderBufferTextureHost becomes invalid, since lifetime of buffer is controlled by BufferTextureHost. It means, RenderBufferTextureHost hold invalid buffer until it is unregistered. Can we ensure that the invalid buffer is not used by WebRender?

(In reply to Sotaro Ikeda [:sotaro] from comment #31) > Comment on attachment 8874371 [details] [diff] [review] > P10: Update the thread model for RegisterExternalImage(), > UnregisterExternalImage() and GetRenderTexture() call. > > Review of attachment 8874371 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/webrender_bindings/RenderThread.cpp > @@ +289,5 @@ > > { > > + if (!IsInRenderThread()) { > > + Loop()->PostTask(NewRunnableMethod<uint64_t>( > > + this, &RenderThread::UnregisterExternalImage, aExternalImageId > > + )); > > In RenderBufferTextureHost case, if > RenderThread::UnregisterExternalImage() is called, buffer of > RenderBufferTextureHost becomes invalid, since lifetime of buffer is > controlled by BufferTextureHost. It means, RenderBufferTextureHost hold > invalid buffer until it is unregistered. Can we ensure that the invalid > buffer is not used by WebRender? void TextureParent::Destroy() { if (!mTextureHost) { return; } // ReadUnlock here to make sure the ReadLock's shmem does not outlive the // protocol that created it. mTextureHost->ReadUnlock(); if (mTextureHost->GetFlags() & TextureFlags::DEALLOCATE_CLIENT) { mTextureHost->ForgetSharedData(); } mTextureHost->mActor = nullptr; mTextureHost = nullptr; } void TextureHost::Finalize() { if (!(GetFlags() & TextureFlags::DEALLOCATE_CLIENT)) { DeallocateSharedData(); DeallocateDeviceData(); } } void WebRenderCompositableHolder::Update(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch) { ... // Release TextureHosts based on Epoch while (!holder->mTextureHosts.empty()) { if (aEpoch <= holder->mTextureHosts.front().mEpoch) { break; } holder->mTextureHosts.pop(); } } If the buffer/shmemTextureHost doesn't use TextureFlags::DEALLOCATE_CLIENT flag, it's no affect for its internal buffer data(shmem, raw buffer and hw handle). The textureHost is held by WebRenderCompositableHolder and will be released by the WebRenderCompositableHolder::Update() call. So, the textureHost is only released when the WR doesn't use that textureHost.

If it is not touched by WR, why do we need to destroy RenderBufferTextureHost on Render Thread? I am just not fun of keeping invalid buffer.

(In reply to Sotaro Ikeda [:sotaro] from comment #33) > If it is not touched by WR, why do we need to destroy > RenderBufferTextureHost on Render Thread? I am just not fun of keeping > invalid buffer. The ~RenderTextureHostOGL() will use the thread-specific gl context which should run at render thread. I saw a deadlock problem when we call gl::MakeCurrent() in both render and compositor thread.

Yes, gl related things should be deleted on render thread. But for me, it could be a separate problem. I prefer not to defer unregister ExternalImage. Can we destroy gl related things after unregister ExternalImage?

To simplify the code, if you want to destroy RenderBufferTextureHost on render thread, it might better to add a comment about it and I still perter not to defer unregister ExternalImage.

MozReview-Commit-ID: KjZIGRzRomT

MozReview-Commit-ID: KjZIGRzRomT

Pushed by hshih@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/95f4d82e3d79 Rename mImageClientTypeContainer into mImageClientContainerType. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/2b20aebef47d Update the thread checking for WR image functions. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/82f5a21b53a6 Move the various of image key and WR command settings from WebRenderCompositableHolder into textureHosts. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/d5225d81b832 Add GetWRImageKeys() and PushExternalImage() in textureHost. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/8846dac9ee35 Update BufferTextureHost and RenderBufferTextureHost for video pipeline. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/22408b6a1ad1 Update MacIOSurfaceTextureHostOGL for video pipeline. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/f7fcc15d8f90 Update TextureD3D11 for video pipeline. v2. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/e2f21ee861e5 Update WebRenderTextureHost for video pipeline. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/cf598918bb1b Make sure the RenderTextureHost is released in render thread. r=sotaro. https://hg.mozilla.org/integration/mozilla-inbound/rev/265e39153027 Update the thread model for RegisterExternalImage(), UnregisterExternalImage() and GetRenderTexture() call. v2. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/7f98b7f60e58 update reftest list. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/42350bacb0bc Make sure all wrapped textureHosts doesn't use TextureFlags::DEALLOCATE_CLIENT flag. v2. r=sotaro

Backed out for causing intermittent QuantumRender debug crashtest crashes. https://hg.mozilla.org/integration/mozilla-inbound/rev/b6e861dcd4c25b9dd98bb820e7cad8a475f8e953 https://treeherder.mozilla.org/logviewer.html#?job_id=104983722&repo=mozilla-inbound

Here is another try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=475b157dac42510010dd76f6bfbe642eb63ca8c8&selectedJob=105092098 Only 1 failed from 11 tests. Finally, I know the reason why we have the intermittent failed. The test in [1] will play a big dimension video(9841x705) and check if gecko could handle that. But it also reset the video src to a empty string in onloadedmetadata(). Thus, we can't make sure the first frame of the video is always shown on the screen. Currently, webrender can't handle the image size more than 8192. We have a assertion to check that [2]. I'm going to skip this crash test for webrender. I will also create the follow-up bugs to resolve the intermittent video status and the maximum image size in webrender. [1] https://dxr.mozilla.org/mozilla-central/rev/5801aa478de12a62b2b2982659e787fcc4268d67/dom/media/test/crashtests/789075-1.html [2] https://dxr.mozilla.org/mozilla-central/source/gfx/webrender/src/texture_cache.rs#664

The video size in this case is (9841x705), but the maximun dimension in WR is 8192. So, skip this test here. MozReview-Commit-ID: B9Bg7eeKymj

Pushed by hshih@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fcf47ea8ae7b Rename mImageClientTypeContainer into mImageClientContainerType. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/a03ce839ba8f Update the thread checking for WR image functions. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/abac100c9ba4 Move the various of image key and WR command settings from WebRenderCompositableHolder into textureHosts. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/8aee72988801 Add GetWRImageKeys() and PushExternalImage() in textureHost. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/b46936cf531e Update BufferTextureHost and RenderBufferTextureHost for video pipeline. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/82db60ad7fbc Update MacIOSurfaceTextureHostOGL for video pipeline. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/c998ef6aa6bb Update TextureD3D11 for video pipeline. v2. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/45279b23564d Update WebRenderTextureHost for video pipeline. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/636b348d5129 Make sure the RenderTextureHost is released in render thread. r=sotaro. https://hg.mozilla.org/integration/mozilla-inbound/rev/68f1d270a475 Update the thread model for RegisterExternalImage(), UnregisterExternalImage() and GetRenderTexture() call. v2. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/7627b9aa45d2 update reftest list. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/185a1ac1e281 Make sure all wrapped textureHosts doesn't use TextureFlags::DEALLOCATE_CLIENT flag. v2. r=sotaro https://hg.mozilla.org/integration/mozilla-inbound/rev/00db286bf5b5 Skip dom/media/test/crashtests/789075-1.html for webrender. r=kats

Create bug 1370839 for intermittent video status and bug 1370838 for big size video.

https://hg.mozilla.org/mozilla-central/rev/fcf47ea8ae7b https://hg.mozilla.org/mozilla-central/rev/a03ce839ba8f https://hg.mozilla.org/mozilla-central/rev/abac100c9ba4 https://hg.mozilla.org/mozilla-central/rev/8aee72988801 https://hg.mozilla.org/mozilla-central/rev/b46936cf531e https://hg.mozilla.org/mozilla-central/rev/82db60ad7fbc https://hg.mozilla.org/mozilla-central/rev/c998ef6aa6bb https://hg.mozilla.org/mozilla-central/rev/45279b23564d https://hg.mozilla.org/mozilla-central/rev/636b348d5129 https://hg.mozilla.org/mozilla-central/rev/68f1d270a475 https://hg.mozilla.org/mozilla-central/rev/7627b9aa45d2 https://hg.mozilla.org/mozilla-central/rev/185a1ac1e281 https://hg.mozilla.org/mozilla-central/rev/00db286bf5b5