Closed Bug 1881620 Opened 2 years ago Closed 2 years ago

[wmfme] enable media engine for encypted playback on Nightly

Categories

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

Product:
Core
Component:
Audio/Video: Playback
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
125 Branch
Tracking Flags:
Tracking Status
firefox125 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

Details

Attachments

(1 external file, 6 obsolete files)

No description provided.

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

Attachment #9382155 - Attachment description: Bug 1881620 - enable media engine for encypted playback on Nightly and Dev Edition. → Bug 1881620 - part1 : enable media engine for encypted playback on Nightly and Dev Edition.
Pushed by alwu@mozilla.com: 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

Backed out for causing multiple Media related failures

Backout link

Push with failures

Failure log // Failure log 2

Flags: needinfo?(alwu)
Depends on: 1882342

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

Attached file Bug 1881620 - part4 : add logging. (obsolete) —

Depends on D203062

Attached file Bug 1881620 - part5 : update comment. (obsolete) —

Depends on D203063

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

Flags: needinfo?(alwu)
Attachment #9388760 - Attachment description: Bug 1881620 - part6 : bring the media engine Id check back and provide a dummy Id when checking via a mimetype. → Bug 1881620 - part6 : only create a remote decoder in the MFCDM process if a media engine ID is provided.

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

Pushed by alwu@mozilla.com: 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

Backed out for causing frequent media crashes:

Status: RESOLVED → REOPENED
status-firefox125: fixed → ---
Resolution: FIXED → ---
Target Milestone: 125 Branch → ---

What crashes? Do you have error links?

Flags: needinfo?(ncsoregi)

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

Flags: needinfo?(imoraru)

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

Flags: needinfo?(ncsoregi)
Flags: needinfo?(imoraru)

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

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

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.

Attachment #9382708 - Attachment is obsolete: true

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.

Attachment #9388264 - Attachment is obsolete: true

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

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

Attachment #9388265 - Attachment is obsolete: true

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

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

Attachment #9388266 - Attachment is obsolete: true

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.

Attachment #9388760 - Attachment is obsolete: true

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.

Attachment #9389231 - Attachment is obsolete: true
Depends on: 1884016, 1884029, 1884027, 1884719

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

Backed out for causing frequent media crashes:

== 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

Keywords: perf-alert

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.

Keywords: perf-alert
Attachment #9382155 - Attachment description: Bug 1881620 - part1 : enable media engine for encypted playback on Nightly and Dev Edition. → Bug 1881620 - enable media engine for encypted playback on Nightly and Dev Edition.
Pushed by alwu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/04be4b49f49c enable media engine for encypted playback on Nightly and Dev Edition. r=media-playback-reviewers,padenot

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

Status: REOPENED → RESOLVED
Closed: 2 years ago → 2 years ago
status-firefox125: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 125 Branch
Regressions: 1885350
Blocks: 1885396
Regressions: 1885761

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

Summary: [wmfme] enable media engine for encypted playback on Nightly and Dev Edition → [wmfme] enable media engine for encypted playback on Nightly
Regressions: 1889135
No longer regressions: 1889135
Regressions: 1896948
Blocks: 1901084
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: