Closed Bug 1405562 Opened 4 years ago Closed 4 years ago

RenderDXGIYCbCrTextureHostOGL is not used with FFmpegVideoDecoder

Categories

(Core :: Graphics: WebRender, defect, P1)

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

References

Details

(Whiteboard: [wr-mvp])

Attachments

(2 files, 4 obsolete files)

RenderDXGIYCbCrTextureHostOGL was added by Bug 1388240. But RenderDXGIYCbCrTextureHostOGL was not used when video was decoded by FFmpegVideoDecoder.
When I tested attachment 8915024 [details] [diff] [review] on my Win PC(P50) with youtube https://www.youtube.com/watch?v=3tmd-ClpJxA , I faced the following problems.
- Green flickering occasionally happened to video area.
- AcquireSync failed with the log.
   + GP+[GFX1]: RenderDXGIYCbCrTextureHostOGL AcquireSync timeout, hr=0x80070057
- The performance seemed not to become better than without attachment 8915024 [details] [diff] [review].

I did not see the problem on another Win desktop pc. The problem seems to related to cpu/GPU performance.
:jerry, is RenderDXGIYCbCrTextureHostOGL actually used on current gecko? In which use case? Do you notice the problem like Comment 2?
Flags: needinfo?(hshih)
Whiteboard: [wr-mvp] [triage]
Priority: -- → P2
Whiteboard: [wr-mvp] [triage] → [wr-mvp]
Hi sotaro,
With this patch, we could test the d3d-ycbcr path from a regular h.264 video(which will use nv12 format).
I still don't know which video format could go through the d3d-ycbcr directly.
Flags: needinfo?(hshih)
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Jerry Shih[:jerry] (UTC+8) from comment #4)
> Created attachment 8915950 [details] [diff] [review]
> Test for d3d-ycbcr format texture.
> 
> Hi sotaro,
> With this patch, we could test the d3d-ycbcr path from a regular h.264
> video(which will use nv12 format).
> I still don't know which video format could go through the d3d-ycbcr
> directly.

One user seems like FFmpegVideoDecoder<LIBAV_VER>::DoDecode(). It calls VideoData::CreateAndCopyData() and the CreateAndCopyData() creates D3D11YCbCrImage() on windows.
   https://dxr.mozilla.org/mozilla-central/source/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp#399
   https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaData.cpp#324

I confirmed that FFmpegVideoDecoder is used for youtube playback on my laptop P50. FFmpegVideoDecoder seems to be used for VP8/VP9 if hw accelerated video decoding is not used.
Flags: needinfo?(sotaro.ikeda.g)
See Also: → 1388240
Priority: P2 → P3
Whiteboard: [wr-mvp] → [wr-reserve]
Attachment #8915024 - Attachment is obsolete: true
Assignee: nobody → sotaro.ikeda.g
With latest m-c, attachment 8933183 [details] [diff] [review] works well. With https://www.youtube.com/watch?v=0wCC3aLXdOw , I confirmed that performance of software decoded video was improved on windows with the patch.

[4k video on P50(Win10) default]
- FPS: 26fps
- WebRender cpu(compositor) time: 40ms-37ms

[4k video on P50(Win10) with attachment 8933183 [details] [diff] [review]]
- FPS: 32fps
- WebRender cpu(compositor) time: 20ms
Attachment #8933183 - Flags: review?(matt.woodrow)
Depends on: 1421939
Comment on attachment 8933183 [details] [diff] [review]
patch - Enable D3D11YCbCrImage usage with WebRender

Review of attachment 8933183 [details] [diff] [review]:
-----------------------------------------------------------------

This works, it's pretty painful for media code to have to understand these details though.

Would be nice if aAllocator had a function that returned what the underlying OS API we're targeting is, and have us return LAYERS_D3D11 from that for the WR+ANGLE case.

Advanced Layers (LayerManagerMLGPU) identifies as LAYERS_D3D11 already, for this reason. Most of the callers don't care what algorithm we're using to batch drawing, and only want to know what surface types they can use.

Can we make WR return LAYERS_D3D11 (or whatever is appropriate) from GetCompositorBackendType? Maybe add something new to identify WR for the callers that really need to know.
(In reply to Matt Woodrow (:mattwoodrow) from comment #8)
> Comment on attachment 8933183 [details] [diff] [review]
> patch - Enable D3D11YCbCrImage usage with WebRender
> 
> 
> Can we make WR return LAYERS_D3D11 (or whatever is appropriate) from
> GetCompositorBackendType? Maybe add something new to identify WR for the
> callers that really need to know.

We could not change as to return LAYERS_D3D11, since it is already used in another place that expects WR.

Instead, we could add a function like SupportsD3D11() to KnowsCompositor.
Addressed the comment.
Attachment #8933183 - Attachment is obsolete: true
Attachment #8933183 - Flags: review?(matt.woodrow)
Attachment #8933265 - Attachment is obsolete: true
Attachment #8933266 - Flags: review?(matt.woodrow)
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [wr-reserve] → [wr-mvp]
Attachment #8933266 - Flags: review?(matt.woodrow) → review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/994147e6fd70
Enable D3D11YCbCrImage usage with WebRender r=mattwoodrow
Oh, I checked build only on windows :(
Flags: needinfo?(sotaro.ikeda.g)
Fixed build failure.
Attachment #8933266 - Attachment is obsolete: true
Attachment #8933560 - Flags: review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/53ae8f24dcc5
Enable D3D11YCbCrImage usage with WebRender r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/53ae8f24dcc5
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.