1541029 - Remove accelerated api use from content - VideoData::CreateAndCopyData

Status: RESOLVED FIXED

(Core :: Audio/Video, defect, P3)

Description

[Jim Mathies [:jimm]]

We're picking up calls like this to d3d11 surfaces in ffmpeg library. For win32k lockdown this will have to move to another process or get removed.

https://searchfox.org/mozilla-central/rev/201450283cddc9e409cec707acb65ba6cf6037b1/dom/media/MediaData.cpp#352

Comment 1

[Jim Mathies [:jimm]]

This appears isolated from the actual decoding library. So maybe there's a way to move away from this without moving/modifying media decoding lobraries. Nils, any thoughts?

typical stack:
win32u!NtGdiDdDDIUnlock
win32u!NtGdiDdDDIUnlock
d3d11!CallAndLogImpl<long
d3d11!NDXGI::CDevice::UnlockCB
vm3dum64_10
vm3dum64_10
vm3dum64_10
d3d11!NDXGI::CDevice::Flush
d3d11!NDXGI::CDevice::ResolveSharedResourceImpl
d3d11!NDXGI::CDevice::DXGIReleaseSync
xul!mozilla::layers::AutoLockD3D11Texture::~AutoLockD3D11Texture
xul!mozilla::layers::D3D11YCbCrImage::SetData
xul!mozilla::VideoData::CreateAndCopyData
xul!mozilla::FFmpegVideoDecoder<46465650>::CreateImage
xul!mozilla::FFmpegVideoDecoder<46465650>::DoDecode
xul!mozilla::FFmpegDataDecoder<46465650>::DoDecode

Comment 2

[Matt Woodrow (:mattwoodrow)]

This code is a performance optimization, since uploading directly to a GPU texture saves a copy (normally we copy to shmem, and then still have the same upload in the compositor process). It has a significant impact when trying to decode 4k video in software.

Depending on what sandboxing we want for other processes, we could do similar things in the RDD process or GPU process, and try decode in those places more often.

We could also just drop it if necessary, but it's definitely not free.

Comment 3

[Nils Ohlmeier [:drno]]

Yeah I think Matt is right that in the long run we want to consider having a fast path between the RDD process and the GPU process. But I think that is still some time out as it will takes us some time to move codecs over the RDD process.

Jean-Yves do you have any thoughts on this?

Comment 4

[Jean-Yves Avenard [:jya]]

Why would you want to remove it? What problems does it cause?

Removing it will be at the expense of speed and CPU usage.

Long term it will no longer be called from the content process, only the GPU and RDD process, but for now must software decoding is done in the content process.

Comment 5

[Jim Mathies [:jimm]]

(In reply to Jean-Yves Avenard [:jya] from comment #4)

Why would you want to remove it? What problems does it cause?

Removing it will be at the expense of speed and CPU usage.

Long term it will no longer be called from the content process, only the GPU
and RDD process, but for now must software decoding is done in the content
process.

These classes use accelerated apis which call win32k sys calls under the hood. When we flip the win32k lockdown switch for content these apis will all fail.

Comment 6

[Bob Owen (:bobowen)]

I think this was fixed by bug 1595994.