To support bug 1542881, move the decoding of WMF and FFmpeg from content process to the RDD process.

It uses data and threads from the RemoteDecoderManagerChild not
RemoteDecoderModule.

Unify the logic to detect and create a GPU based decoder, removing the need for
GpuDecoderModule.

Added DecoderModule type to control which decoders are checked.
(WMFDecoder can handle VPX is some situations and maybe it's desired
to use the agnostic version)

Jean-Yves, before do too deep of a review, can you tell me if this in the right direction?

This is a subset of the parameters passed via CreateDecoderParams and is so
Supports() calls have access to KnowsCompositor and Options when determining
if decoding is supported.

Add a synchronous Supports message to the IPDL PRemoteDecoderManager protocol so
decoder modules can query for playback support in the actual process that will
attempt to do the decoding.

Moved support logic into WMFDecoderModule.

This assumes that access to Windows WMF and DirectX is allowed from RDD process.

This works around failures in the remote process from RecvReadback calls trying
to access image from IPDL thread instead of main thread.

:jya and :mattwoodrow, I've hit a bit of a catch-22. Moving FFmpeg and FFvpx into RDD causes Bug 1597681 (Readback fails in debug because of an ASSERTion that the image is only accessed on main thread, not IPDL thread).

I banned using RDD if basic layers is present, but that causes AV1 to fail because AV1 is only available in RDD.

I spoke to :mattwoodrow about fixing 1597681 and we have a solution. So I jump on that bug and implement it, or remove the exclusion of RDD for basic layers and then fix 1597681?

I'm hoping that fixing bug 1597681 will be quick (just deleting the custom refptr type, and switching the users to use a normal RefPtr), so I think just do that first and then we don't need any exclusions.

For performant video decoding we need access to DXGI/D3D11 similarly to GPU
process.

Disabled by default.

The wrapper will be implemented on the remote side, so we don't need to "double"
wrap in the content process.

(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #22)

Created attachment 9115374 [details]
Bug 1595994 - PC: Loosen RDD sandbox.

For performant video decoding we need access to DXGI/D3D11 similarly to GPU
process.

Unfortunately, this fundamentally weakens the sandbox and we don't want to weaken it for the decoders that already run in it (or even really for the new decoders).
Is this just for uploading frames to the GPU?

Ideally we would want to move the GPU access to the GPU process.
Perhaps as an interim solution we could have two RDD processes, one with win32k enabled for the decoders that need it.

I discussed with :mattwoodrow and we decided the best move here would be to move FFmpeg/FFvpx to the GPU process since the goal is to lock down the Content process.

With that said, if we can't open up the RDD process to allow D3D/DXGI access then I can't see Bug 1542881 being resolved unless the GPU process is available with these patches. Bug 1542881 states that WMF calls are happening in the Content process which shouldn't occur if the GPU process is enabled.

(In reply to Dan Glastonbury (:kamidphish) | needinfo me from comment #26)

I discussed with :mattwoodrow and we decided the best move here would be to move FFmpeg/FFvpx to the GPU process since the goal is to lock down the Content process.

With that said, if we can't open up the RDD process to allow D3D/DXGI access then I can't see Bug 1542881 being resolved unless the GPU process is available with these patches. Bug 1542881 states that WMF calls are happening in the Content process which shouldn't occur if the GPU process is enabled.

If enabling win32k on the RDD process is the only way of fixing bug 1542881 and this was required for all decoders (so a second RDD process wouldn't help) then we'll have to weigh that up against win32k lockdown for the content process.
It's been a while since I looked at bug 1542881, but I think the slow path could be speeded up considerably, although I don't know whether it would be enough.

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:kamidphish, could you have a look please?
For more information, please visit auto_nag documentation.

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #28)

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:kamidphish, could you have a look please?
For more information, please visit auto_nag documentation.

Thanks auto-nag. :dminor is continuing the work on this bug and sorting out the last few try failures.

During shutdown, nullptr is possible here while running Talos tests.

Depends on D56858

With the performance concerns with previous changes to RDD, I've spent some time looking at Talos results. It was suggested to avoid noise in the results, I do two try runs, one with the changes applied, and the other with the base revision of mozilla-central. Doing that, I'm not seeing any significant differences: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=3d9dbf25792ffc5fa4579443c8033b8c1bfc565e&newProject=try&newRevision=c241c31b3419fce5a2d4f6e9262ed90a4f50d4ae&framework=1. I find this surprising, but without any clear indication that these changes are going to cause performance regressions, my plan is to get the remaining patches up for review and then land, dealing with any regressions after the fact.

I'm resetting the assignee because I haven't looked at this since the end of March, I've been busy with wfh/WebRTC stuff.

This is no longer required now that the RemoteDecoderManagerChild manages decoder creation

Depends on D56858

Ultimately, we should be able to remove everything that got added to the RDD sandbox from the content's one.

Fly-by fix; allow checking if AVX512 is supported in content sandbox.

Depends on D91688

Depends on D91689

Depends on D91690

Depends on D91691

Depends on D91692

Attempting to initialize a PDM can cause the process to be killed due to sandbox violations.
For now, only FFVPX decoders is working and enabled in the RDD process.

One downside is that changing the ffvpx/ffmpeg rdd pref now requires a restart.

Will be able to remove such issue once all sandbox issues have been resolved.

Depends on D91693

GPUVideoImage can be used on other platform than Windows; if so it will be made of an opaque handle referencing the texture as known by the PVideoBridge.
Only if a sub-description is set can you directly read the image from the content process.

Depends on D91694

When using a software decoder, the IOSurface type is using a YUV422 before which is only usable with webrender.
For now, disable the test that attempts to check if a fast read is possible when not running webrender.

These tests can be re-enabled once WebGL works in the GPU process.

Depends on D91695

Depends on D91696

Seen while blocking the parent in the RDD with a debugger, this delayed the startup task which cause the RemoteDecoderManagerChild singleton to not be set yet as it will only be so when the IPC binding has completed.

Depends on D91697

We don't always know what the compositor/content device is going to be; we need to separate both cases.

Depends on D91698

Depends on D91699

We unfortunately must skip the fastpath tests on mac and linux as they will succeed on M-gli platform (WebGL in GPU) and we can't filter those out.

Depends on D91700

The code always assumed that the size of the image with the Y plane dimensions, which, while often the case, isn't correct.
We remove the assertions that the display offset was always (0,0) and properly carry the actual data over IPC.

Until bug 1668840 is fixed, we can't allocate a SharedRGBImage without a PImageBridge

Until bug 1668840 is fixed, we can't allocate a SharedRGBImage without a PImageBridge

Depends on D92233

Following bug 1566389, an AudioBuffer can be created only to be then truncated if we need to trim it back to a size of 0.

When the resulting AudioBuffer is transferred over IPC it will be rebuilt, and be null then making EnsureAudioBuffer fail.

Depends on D92345

As we are just 2 weeks away from beta, we made the decision to only allow the new RDD decoders in nightly for now.

This remove the need for a sync dispatch to the main thread, that lead to deadlocks.

Depends on D93316

And we remove unnecessary checks, BackgroundParent only run in the parent process and if e10s is on. Also RecvLauchRDDProcess will only ever be called if the rdd pref is on already.

By streamlining the call we also reduce the number of sync dispatch to 1.

Remove unnecessary virtual for method not used as such.

Depends on D93476

Depends on D93476

This patch series is causing build errors for me

94:50.98 ld.lld: error: undefined hidden symbol: icu_67::Calendar::createInstance(icu_67::TimeZone*, icu_67::Locale const&, UErrorCode&)
94:50.98 >>> referenced by DateTimeFormat.cpp:181 (/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/intl/locale/DateTimeFormat.cpp:181)
94:50.98 >>>               lto.tmp:(mozilla::DateTimeFormat::GetCalendarSymbol(mozilla::DateTimeFormat::Field, mozilla::DateTimeFormat::Style, PRExplodedTime const*, nsTSubstring<char16_t>&))
94:50.99 ld.lld: error: undefined hidden symbol: icu_67::Calendar::setTimeInMillis(double, UErrorCode&)
94:50.99 >>> referenced by calendar.h:441 (/usr/include/unicode/calendar.h:441)
94:50.99 >>>               lto.tmp:(mozilla::DateTimeFormat::GetCalendarSymbol(mozilla::DateTimeFormat::Field, mozilla::DateTimeFormat::Style, PRExplodedTime const*, nsTSubstring<char16_t>&))
94:50.99 ld.lld: error: undefined hidden symbol: icu_67::DateFormatSymbols::createForLocale(icu_67::Locale const&, UErrorCode&)
94:50.99 >>> referenced by DateTimeFormat.cpp:192 (/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/intl/locale/DateTimeFormat.cpp:192)
94:50.99 >>>               lto.tmp:(mozilla::DateTimeFormat::GetCalendarSymbol(mozilla::DateTimeFormat::Field, mozilla::DateTimeFormat::Style, PRExplodedTime const*, nsTSubstring<char16_t>&))
94:50.99 ld.lld: error: undefined hidden symbol: icu_67::DateFormatSymbols::~DateFormatSymbols()
94:50.99 >>> referenced by unique_ptr.h:85 (/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../include/c++/10.2.0/bits/unique_ptr.h:85)
94:50.99 >>>               lto.tmp:(mozilla::DateTimeFormat::GetCalendarSymbol(mozilla::DateTimeFormat::Field, mozilla::DateTimeFormat::Style, PRExplodedTime const*, nsTSubstring<char16_t>&))
94:50.99 ld.lld: error: undefined hidden symbol: icu_67::UMemory::operator delete(void*)
94:50.99 >>> referenced by unique_ptr.h:85 (/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../include/c++/10.2.0/bits/unique_ptr.h:85)
94:50.99 >>>               lto.tmp:(mozilla::DateTimeFormat::GetCalendarSymbol(mozilla::DateTimeFormat::Field, mozilla::DateTimeFormat::Style, PRExplodedTime const*, nsTSubstring<char16_t>&))
94:50.99 >>> referenced by FluentBundle.cpp:265 (/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/intl/l10n/FluentBundle.cpp:265)
94:50.99 >>>               lto.tmp:(FluentBuiltInNumberFormatterDestroy)
94:50.99 ld.lld: error: undefined hidden symbol: icu_67::Calendar::get(UCalendarDateFields, UErrorCode&) const
94:50.99 >>> referenced by DateTimeFormat.cpp:198 (/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/intl/locale/DateTimeFormat.cpp:198)
94:50.99 >>>               lto.tmp:(mozilla::DateTimeFormat::GetCalendarSymbol(mozilla::DateTimeFormat::Field, mozilla::DateTimeFormat::Style, PRExplodedTime const*, nsTSubstring<char16_t>&))
94:50.99 >>> referenced by DateTimeFormat.cpp:209 (/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/intl/locale/DateTimeFormat.cpp:209)
94:51.00 >>>               lto.tmp:(mozilla::DateTimeFormat::GetCalendarSymbol(mozilla::DateTimeFormat::Field, mozilla::DateTimeFormat::Style, PRExplodedTime const*, nsTSubstring<char16_t>&))
94:51.00 ld.lld: error: undefined hidden symbol: icu_67::DateFormatSymbols::getMonths(int&, icu_67::DateFormatSymbols::DtContextType, icu_67::DateFormatSymbols::DtWidthType) const
94:51.00 >>> referenced by DateTimeFormat.cpp:202 (/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/intl/locale/DateTimeFormat.cpp:202)
94:51.00 >>>               lto.tmp:(mozilla::DateTimeFormat::GetCalendarSymbol(mozilla::DateTimeFormat::Field, mozilla::DateTimeFormat::Style, PRExplodedTime const*, nsTSubstring<char16_t>&))
94:51.00 ld.lld: error: undefined hidden symbol: icu_67::DateFormatSymbols::getWeekdays(int&, icu_67::DateFormatSymbols::DtContextType, icu_67::DateFormatSymbols::DtWidthType) const
94:51.00 >>> referenced by DateTimeFormat.cpp:213 (/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/intl/locale/DateTimeFormat.cpp:213)
94:51.00 >>>               lto.tmp:(mozilla::DateTimeFormat::GetCalendarSymbol(mozilla::DateTimeFormat::Field, mozilla::DateTimeFormat::Style, PRExplodedTime const*, nsTSubstring<char16_t>&))
94:51.74 clang-10: error: linker command failed with exit code 1 (use -v to see invocation)
94:51.74 make[4]: *** [/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/config/rules.mk:545: libxul.so] Error 1
94:51.74 make[3]: *** [/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/config/recurse.mk:72: toolkit/library/build/target] Error 2
94:51.74 make[2]: *** [/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/config/recurse.mk:34: compile] Error 2
94:51.74 make[1]: *** [/mnt/BIG/francois/tmp/firefox-wayland-hg/src/mozilla-unified/config/rules.mk:355: default] Error 2
94:51.75 make: *** [client.mk:125: build] Error 2```

So, my build error was probably unrelated, I manged to build successfully again by removing --with-system-icu from my mozconfig.

However, with all these patches applied, I still get sandbox violations when playing video content (Bug #1619585), is this expected? Is there any pref I should be paying attention to?

https://hg.mozilla.org/mozilla-central/rev/c910232fb2e8
https://hg.mozilla.org/mozilla-central/rev/44471f99280c
https://hg.mozilla.org/mozilla-central/rev/e3f920da2232
https://hg.mozilla.org/mozilla-central/rev/9e0809ed54cd
https://hg.mozilla.org/mozilla-central/rev/0606794a9e8c
https://hg.mozilla.org/mozilla-central/rev/1932f2f34993
https://hg.mozilla.org/mozilla-central/rev/57607f8b6d8e
https://hg.mozilla.org/mozilla-central/rev/c177bcba1c07
https://hg.mozilla.org/mozilla-central/rev/fca05e774f88
https://hg.mozilla.org/mozilla-central/rev/c5dac46ef11d
https://hg.mozilla.org/mozilla-central/rev/73415132bcec
https://hg.mozilla.org/mozilla-central/rev/d0c9c64867cc
https://hg.mozilla.org/mozilla-central/rev/d158b20d9a65
https://hg.mozilla.org/mozilla-central/rev/85b9abba021d
https://hg.mozilla.org/mozilla-central/rev/9914d2e0f6ca
https://hg.mozilla.org/mozilla-central/rev/d82b6476a18d
https://hg.mozilla.org/mozilla-central/rev/84b82a60dc74
https://hg.mozilla.org/mozilla-central/rev/5194f8d4d589
https://hg.mozilla.org/mozilla-central/rev/705422df443e
https://hg.mozilla.org/mozilla-central/rev/661cb6c32fd7
https://hg.mozilla.org/mozilla-central/rev/24d845bcdcde
https://hg.mozilla.org/mozilla-central/rev/0e4fc20bf2d3
https://hg.mozilla.org/mozilla-central/rev/1e465667a9d0
https://hg.mozilla.org/mozilla-central/rev/33281bbafab7
https://hg.mozilla.org/mozilla-central/rev/47976a6ef68d
https://hg.mozilla.org/mozilla-central/rev/a9ef204e1554
https://hg.mozilla.org/mozilla-central/rev/f1ffe5e08ef1
https://hg.mozilla.org/mozilla-central/rev/4c7f671c7b69
https://hg.mozilla.org/mozilla-central/rev/7a990fa02246
https://hg.mozilla.org/mozilla-central/rev/f9337268523c
https://hg.mozilla.org/mozilla-central/rev/277d8c145ed4
https://hg.mozilla.org/mozilla-central/rev/3412eec07786
https://hg.mozilla.org/mozilla-central/rev/15bab2afca7a
https://hg.mozilla.org/mozilla-central/rev/92697e760c82
https://hg.mozilla.org/mozilla-central/rev/084a23683a4e
https://hg.mozilla.org/mozilla-central/rev/3dd1e88edaf1
https://hg.mozilla.org/mozilla-central/rev/4bd5056a4a6f
https://hg.mozilla.org/mozilla-central/rev/9b7e2bccf383
https://hg.mozilla.org/mozilla-central/rev/4c3e6fb95aa6

Today TB on Windows is flickering and showing black window background/graphics errors. Disabling the HW acceleration or a back-out of this bug fixes it. Mac and Linux aren't affected.

What could be the cause of this? What could TB need to work again with HW acceleration? TB, and especially under Windows, had already problems in bug 1665476.

When I enable media.rdd-ffmpeg.enabled (to get VAAPI working), the RDD process crashes(1). Is this currently expected?

(1): https://crash-stats.mozilla.org/report/index/3ce29381-6239-44ee-bf67-be1320201021

(In reply to tgn-ff from comment #67)

When I enable media.rdd-ffmpeg.enabled (to get VAAPI working), the RDD process crashes(1). Is this currently expected?

(1): https://crash-stats.mozilla.org/report/index/3ce29381-6239-44ee-bf67-be1320201021

Please file a new bug about it - we should not call gfxPlatform::Init() there.

(In reply to tgn-ff from comment #67)

When I enable media.rdd-ffmpeg.enabled (to get VAAPI working), the RDD process crashes(1). Is this currently expected?

(1): https://crash-stats.mozilla.org/report/index/3ce29381-6239-44ee-bf67-be1320201021

Yes, that's why it's disabled by default.

(In reply to Richard Marti (:Paenglab) from comment #66)

Today TB on Windows is flickering and showing black window background/graphics errors. Disabling the HW acceleration or a back-out of this bug fixes it. Mac and Linux aren't affected.

What could be the cause of this? What could TB need to work again with HW acceleration? TB, and especially under Windows, had already problems in bug 1665476.

What is TB?
Please open a new bug with step to reproduce.

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

What is TB?

Thunderbird

(In reply to tgn-ff from comment #67)

When I enable media.rdd-ffmpeg.enabled (to get VAAPI working), the RDD process crashes(1). Is this currently expected?

(1): https://crash-stats.mozilla.org/report/index/3ce29381-6239-44ee-bf67-be1320201021

You do NOT need to set this pref to get VAAPI going. It's totally unrelated, and ffmpeg will not work in the RDD processes at this stage.

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

You do NOT need to set this pref to get VAAPI going. It's totally unrelated, and ffmpeg will not work in the RDD processes at this stage.

VAAPI doesn't work with ffvpx; we have to set media.ffvpx.enabled=false to work in the content process. That's why I tried that pref to enable ffmpeg in RDD. The title of the bug and one of the patches (P10) gave me the wrong impression that it should work. I asked here to make sure it shouldn't yet work to avoid filing an invalid bug. Thanks for confirming.