1148179 - [EME] Tests to provide keys before track data

Status: RESOLVED WONTFIX

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

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

To help with test reproducibility, EME tests should be more consistent in how they provide data.

This bug focuses on one combination:
1. Append init segments to source buffers.
2. For each 'encrypted' event, setup the session and provide needed keys.
3. After all init segments have been sucessfully updated, and all keys have been provided, append data segments.

(Other bugs will deal with other combinations)

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 1148179-init-and-keys-before-data.patch

Added playback test that only appends track data after all init and keys have been provided. The updated eme.js routines will help create more of these precise tests.

Keeping old playback test for now, so we can still reproduce issues (in case the new test is "too" good!) It would be good to compare them in the long run.

Preliminary tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=80c986075c03 -- More to come.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Still looking good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=26f2dba3e770
(The 3 errors are out of 78 tests runs are in a totally unrelated area.)

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 1148179-init-and-keys-before-data.patch

Update with more logging; and some timeouts to catch never-ending XHRs, appendBuffer's, and playbacks.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0479f107d0b

Comment 4

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8586730 [details] [diff] [review]
1148179-init-and-keys-before-data.patch

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

::: dom/media/test/test_eme_playback.html
@@ +110,5 @@
> +    });
> +    [ "canplay", "canplaythrough", "error", "loadeddata",
> +      "loadedmetadata", "loadstart", "pause", "play", "playing", "progress",
> +      "stalled", "suspend", "waiting",
> +    ].forEach(function(e) { lastState = e; });

I don't think this forEach does what you intend it to do.

::: dom/media/test/test_eme_playback_init_then_keys_then_data.html
@@ +149,5 @@
> +    });
> +    [ "canplay", "canplaythrough", "error", "loadeddata",
> +      "loadedmetadata", "loadstart", "pause", "play", "playing", "progress",
> +      "stalled", "suspend", "waiting",
> +    ].forEach(function(e) { lastState = e; });

Here as well.

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 1148179-init-and-keys-before-data.patch

Small update as per review in comment 4, carrying r+.

Comment 6

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed9ac648fa89

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b82a84104ae

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

B2G ICS emulator debug mochitest-13 started failing on the checkin-needed push that included this patch. I backed this one (and the one from bug 1146729) out for being the likely causes.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ec4c24bfd48


https://treeherder.mozilla.org/logviewer.html#?job_id=8469225&repo=mozilla-inbound

Comment 9

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

EME tests do not run on B2G, so I do not believe this patch has anything to do with the issue in mochitest-13.
EME tests are actually in mochitest-3, which appeared clean after this checkin -- except for one appearance of an already-known intermittent (bug 1120741), which this patch here will help isolate better.

Could you please try again?

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Looks like this failure IS from this patch, though, as my backout removed the failing test:
https://treeherder.mozilla.org/logviewer.html#?job_id=8469901&repo=mozilla-inbound

Comment 11

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

As explained in comment 9, this is an intermittent issue similar to the one in test_eme_playback.html (bug 1120741), but I was hoping this patch would help gather more information in a better-constrained context. Oh well, I'll have to try other things...

Comment 12

[Chris Peterson [:cpeterson]]

Gerald, cpearce: should we uplift these test changes to Beta 38 once they land?

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Chris Peterson [:cpeterson] from comment #12)
> Gerald, cpearce: should we uplift these test changes to Beta 38 once they
> land?

I think this will be a test only change, so it probably doesn't need uplifting (other than to keep the sheriff's happy by reducing orange).

Comment 14

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Mass change P2 -> P3

Comment 15

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Too old now.