1881620 - [wmfme] enable media engine for encypted playback on Nightly

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, task, P1)

Comment 1

[Alastor Wu [:alwu]]

Attachment: Bug 1881620 - enable media engine for encypted playback on Nightly and Dev Edition.

Depends on D202510

Comment 2

[Alastor Wu [:alwu]]

We should modify this as well before changing the pref the media engine.

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1881620 - part2 : use ExternalEngineStateMachine for non-mse media only when the media engine pref allows non-encrypted playback.

Depends on D202511

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b6bef839c33
part1 : enable media engine for encypted playback on Nightly and Dev Edition. r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/2bd01d77bc4d
part2 : use ExternalEngineStateMachine for non-mse media only when the media engine pref allows non-encrypted playback. r=jolin

Comment 5

[Norisz Fay [:noriszfay]]

Backed out for causing multiple Media related failures

Backout link

Push with failures

Failure log // Failure log 2

Comment 6

[Alastor Wu [:alwu]]

Attachment: Bug 1881620 - part3 : do not re-inform the decoder owner about `loadedmetadata` if we've loaded metadata before.

When enalbing media engine playback, we would always start with the
external state machine that would communicate with the media engine.

If the media format or key system is not supported by the media engine,
we would fallback to the MediaDecoderStateMachine (MDSM). In this
situation, we want this transition invisible to users.

Therefore, if the external state machine has loaded and informed the
loadedmetadata to the media element, we shouldn't do it again.
According to the spec [1], the loadedmetadata should only be
dispatched once.

[1] https://html.spec.whatwg.org/multipage/media.html#event-media-loadedmetadata

Depends on D202758

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1881620 - part4 : add logging.

Depends on D203062

Comment 8

[Alastor Wu [:alwu]]

Attachment: Bug 1881620 - part5 : update comment.

Depends on D203063

Comment 9

[Alastor Wu [:alwu]]

Attachment: Bug 1881620 - part6 : only create a remote decoder in the MFCDM process if a media engine ID is provided.

In bug 1853285, we remove the media engine Id check in order to let the
MFCDM process be able to report a correct support list for codecs, which
can be known via RemoteDecoderManagerChild::Supports(). [1]

However, that causes a problem where we would incorrectly use the MFCDM
process for playback which shouldn't be used via the media engine. This
issue was discovered by browser_utility_multipleAudio.js.

Therefore, we still need that Id check to ensure that only using media
engine on correct playback type, and keep the MFCDM process reporting
the correct support list.

[1] https://searchfox.org/mozilla-central/rev/6bc0f370cc459bf79e1330ef74b21009a9848c91/dom/media/ipc/RemoteDecoderManagerChild.cpp#242,244

Depends on D203064

Comment 10

[Alastor Wu [:alwu]]

Attachment: Bug 1881620 - part7 : avoid re-initialize the video bridge.

There is a timing issue where we would re-initialize the video bridge
causing an assertion, which is discovered by
browser_utility_multipleAudio.js.

Depends on D203307

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a2fb26876e1
part1 : enable media engine for encypted playback on Nightly and Dev Edition. r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/dcafb1fadf9b
part2 : use ExternalEngineStateMachine for non-mse media only when the media engine pref allows non-encrypted playback. r=jolin
https://hg.mozilla.org/integration/autoland/rev/fab988ca73ff
part3 : do not re-inform the decoder owner about `loadedmetadata` if we've loaded metadata before. r=padenot
https://hg.mozilla.org/integration/autoland/rev/1d7ec3e9b00f
part4 : add logging. r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/5f23b23fc038
part5 : update comment. r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/c58d5ba5c317
part6 : only create a remote decoder in the MFCDM process if a media engine ID is provided. r=jolin
https://hg.mozilla.org/integration/autoland/rev/d07acf022aad
part7 : avoid re-initialize the video bridge. r=gerard-majax

Comment 12

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/7a2fb26876e1
https://hg.mozilla.org/mozilla-central/rev/dcafb1fadf9b
https://hg.mozilla.org/mozilla-central/rev/fab988ca73ff
https://hg.mozilla.org/mozilla-central/rev/1d7ec3e9b00f
https://hg.mozilla.org/mozilla-central/rev/5f23b23fc038
https://hg.mozilla.org/mozilla-central/rev/c58d5ba5c317
https://hg.mozilla.org/mozilla-central/rev/d07acf022aad

Comment 13

[Natalia Csoregi [:nataliaCs]]

Backed out for causing frequent media crashes:

• backout: https://hg.mozilla.org/integration/autoland/rev/86d7c9df102479f43cb8818a301d254323412462

Comment 14

[Alastor Wu [:alwu]]

What crashes? Do you have error links?

Comment 15

[Iulian Moraru]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/236a38c8865a

Comment 16

[Alastor Wu [:alwu]]

Can anyone provide me error links so that I can know what happened?

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Bug 1824294 had a spike and there were also Microsoft::WRL::ComPtr<T>::InternalRelease crashes.

Comment 18

[Alastor Wu [:alwu]]

Okay thanks, I will take a look on them tomorrow.

Comment 19

[Alastor Wu [:alwu]]

I'm going to land those fixes and improvements first in bug 1884016, and left this bug for only switching the pref.

Comment 20

[Phabricator Automation]

Comment on attachment 9382708 [details]
Bug 1881620 - part2 : use ExternalEngineStateMachine for non-mse media only when the media engine pref allows non-encrypted playback.

Revision D202758 was moved to bug 1884016. Setting attachment 9382708 [details] to obsolete.

Comment 21

[Phabricator Automation]

Comment on attachment 9388264 [details]
Bug 1881620 - part3 : do not re-inform the decoder owner about loadedmetadata if we've loaded metadata before.

Revision D203062 was moved to bug 1884016. Setting attachment 9388264 [details] to obsolete.

Comment 22

[Phabricator Automation]

Comment on attachment 9388265 [details]
Bug 1881620 - part4 : add logging.

Revision D203063 was moved to bug 1884016. Setting attachment 9388265 [details] to obsolete.

Comment 23

[Phabricator Automation]

Comment on attachment 9388266 [details]
Bug 1881620 - part5 : update comment.

Revision D203064 was moved to bug 1884016. Setting attachment 9388266 [details] to obsolete.

Comment 24

[Phabricator Automation]

Comment on attachment 9388760 [details]
Bug 1881620 - part6 : only create a remote decoder in the MFCDM process if a media engine ID is provided.

Revision D203307 was moved to bug 1884016. Setting attachment 9388760 [details] to obsolete.

Comment 25

[Phabricator Automation]

Comment on attachment 9389231 [details]
Bug 1881620 - part7 : avoid re-initialize the video bridge.

Revision D203518 was moved to bug 1884016. Setting attachment 9389231 [details] to obsolete.

Comment 26

[Acasandrei Beatrice (needinfo me)]

(In reply to Natalia Csoregi [:nataliaCs] from comment #13)

Backed out for causing frequent media crashes:

• backout: https://hg.mozilla.org/integration/autoland/rev/86d7c9df102479f43cb8818a301d254323412462

== Change summary for alert #41758 (as of Tue, 12 Mar 2024 08:02:15 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
53%	youtube fcp	windows10-64-shippable-qr	fission warm webrender	117.40 -> 180.03	Before/After
52%	youtube fcp	windows10-64-shippable-qr	fission warm webrender	118.15 -> 179.72	Before/After
21%	youtube FirstVisualChange	windows10-64-shippable-qr	fission warm webrender	117.24 -> 141.95	Before/After
10%	instagram loadtime	windows10-64-shippable-qr	cold fission webrender	1,408.17 -> 1,548.66	Before/After
8%	instagram FirstVisualChange	linux1804-64-shippable-qr	cold fission webrender	271.78 -> 292.59	Before/After
5%	youtube largestContentfulPaint	windows10-64-shippable-qr	fission warm webrender	984.56 -> 1,037.43	Before/After
5%	youtube loadtime	windows10-64-shippable-qr	fission warm webrender	993.71 -> 1,045.79	Before/After
4%	instagram PerceptualSpeedIndex	linux1804-64-shippable-qr	cold fission webrender	761.96 -> 789.79	Before/After

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
18%	facebook-nav.landing LastVisualChange	windows10-64-shippable-qr	cold fission webrender	1,866.89 -> 1,521.72	Before/After

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=41758

Comment 27

[Alastor Wu [:alwu]]

We should remove the perf-alert because the patch hasn't landed yet. If this pref flip causes any performance regression in the future, then please add this keyword back.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/04be4b49f49c
enable media engine for encypted playback on Nightly and Dev Edition. r=media-playback-reviewers,padenot

Comment 29

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/04be4b49f49c

Comment 30

[Alastor Wu [:alwu]]

We disabled Dev Edition in bug 1885396, so this is only on Nightly.