1554699 - Getting HTMLMediaElement's preload, defaultPlaybackRate, playbackRate attributes when playing a MediaStream must return constant values

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, enhancement, P2)

Description

[Andreas Pehrson [:pehrsons]]

This fails a WPT that was updated for this in bug 1498058 with the comments:

Set operations for the preload, defaultPlaybackRate and playbackRate
fields should be ignored when the media element is playing a
MediaStream.
In addition, the preload field must always return "none", and
defaultPlaybackRate and playbackRate must both return 1.

Should be a simple fix.

Comment 1

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - HTMLMediaElement.playbackRate/defaultPlaybackRate: Ignore setting and always get 1.0 when playing a MediaStream. r?jib

Depends on D33094

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - HTMLMediaElement.preload: Ignore setting and always get "none" when playing a MediaStream. r?jib

Depends on D33095

Comment 3

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - Update WPT expectations. r?jib

Depends on D33096

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - Update mochitest framework expectations. r?jib

Depends on D33097

Comment 5

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - Split up and elaborate on MediaStream-MediaElement-srcObject.https.html. r?jib

This does several things:

• Split the test up, so one assert doesn't fail the entire test suite
• Check preload, playbackRate and defaultPlaybackRate attributes after unsetting srcObject
• Check setting currentTime before loading (default playback start position),
  and after loading (official playback position) separately
• Check that duration is changed to a real number on ending
• Check that HAVE_ENOUGH_DATA is reached by only assigning srcObject
• And other minor things and formatting

Depends on D33098

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - Ignore setting currentTime synchronously with assigning a MediaStream. r?jib

Depends on D33647

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - Report real duration after playing a MediaStream ended. r?jib

Depends on D33648

Comment 8

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - Don't fire "timeupdate" in PlayInternal as it's against spec. r?jya!,alwu!

Bug 1279865 introduced this under the premise of
"Run TimeMarchesOn() at the beginning of play.", but it did a bit too much.

This makes us spec compliant for this particular case again.

Depends on D33649

Comment 9

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - Don't fire "timeupdate" in PlayInternal as it's against spec, v2. r?jya

This FireTimeUpdate(false) dates back to bug 611994. Perhaps it was spec
compliant back in the day, but it surely isn't now.

Depends on D33650

Comment 10

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - The loop attribute should not have any effect on a media element playing a MediaStream. r?jib

Depends on D33651

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1554699 - Await play promise in case timeupdate events happened before it resolved. r?jib

Since the media element is playing directly after play(), not waiting for the
readyState to advance before play()ing could leave the play promise from play()
unhandled until cleanup, when it would be rejected by the load algorithm.

This would happen if a timeupdate event is raised before the first frame comes
in to update the readyState, which resolves the play promise.

Waiting for the readyState to advance before play()ing seems like the cleanest
fix wrt testing currentTime after the first timeupdate event.

Depends on D33655

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a97c295e0bcf
HTMLMediaElement.playbackRate/defaultPlaybackRate: Ignore setting and always get 1.0 when playing a MediaStream. r=jib
https://hg.mozilla.org/integration/autoland/rev/8a14b83a8b3b
HTMLMediaElement.preload: Ignore setting and always get "none" when playing a MediaStream. r=jib
https://hg.mozilla.org/integration/autoland/rev/9d311a44d2b3
Update WPT expectations. r=jib
https://hg.mozilla.org/integration/autoland/rev/8bdc867ea3c7
Update mochitest framework expectations. r=jib
https://hg.mozilla.org/integration/autoland/rev/a6187e39bf7c
Split up and elaborate on MediaStream-MediaElement-srcObject.https.html. r=jib
https://hg.mozilla.org/integration/autoland/rev/9f6e9bbf9e59
Ignore setting currentTime synchronously with assigning a MediaStream. r=jib
https://hg.mozilla.org/integration/autoland/rev/78c2ab7785eb
Report real duration after playing a MediaStream ended. r=jib
https://hg.mozilla.org/integration/autoland/rev/7a6d1ef04809
Don't fire "timeupdate" in PlayInternal as it's against spec. r=jya,alwu
https://hg.mozilla.org/integration/autoland/rev/e0a7360fcc7c
Don't fire "timeupdate" in PlayInternal as it's against spec, v2. r=jya
https://hg.mozilla.org/integration/autoland/rev/7727cef089cd
The loop attribute should not have any effect on a media element playing a MediaStream. r=jib
https://hg.mozilla.org/integration/autoland/rev/89350b209bce
Await play promise in case timeupdate events happened before it resolved. r=jib

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17299 for changes under testing/web-platform/tests

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/17299
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/VYPqcYTwQpCWc5QX3bKujA)

Comment 15

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/a97c295e0bcf
https://hg.mozilla.org/mozilla-central/rev/8a14b83a8b3b
https://hg.mozilla.org/mozilla-central/rev/9d311a44d2b3
https://hg.mozilla.org/mozilla-central/rev/8bdc867ea3c7
https://hg.mozilla.org/mozilla-central/rev/a6187e39bf7c
https://hg.mozilla.org/mozilla-central/rev/9f6e9bbf9e59
https://hg.mozilla.org/mozilla-central/rev/78c2ab7785eb
https://hg.mozilla.org/mozilla-central/rev/7a6d1ef04809
https://hg.mozilla.org/mozilla-central/rev/e0a7360fcc7c
https://hg.mozilla.org/mozilla-central/rev/7727cef089cd
https://hg.mozilla.org/mozilla-central/rev/89350b209bce

Comment 16

[Andreas Pehrson [:pehrsons]]

James, here's another upstream PR that needs a nudge.

Comment 17

[James Graham [:jgraham]]

Done, thanks for the request.

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged