Closed Bug 1577505 Opened 1 year ago Closed 4 months ago

HTMLMediaElement playing a MediaStream fires "timeupdate" before it's "potentially playing"

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: pehrsons, Assigned: pehrsons)

References

Details

Attachments

(5 files)

There was an observable change to the timing of the first "timeupdate" in bug 1493613 - it made it occur once both of the following were true:

  • it had seen a live track in the played MediaStream
  • it is not paused

This is close to the definition of "potentially playing", but we need to not only wait for the first track, but until we have received a frame, i.e., once readyState is >= HAVE_FUTURE_DATA.

Before bug 1493613 we fired "timeupdate" much earlier.

See this spec issue for more details.

It was incorrectly included in the renaming part of bug 1454998.

Depends on D49351

This makes all inputs to IsPotentiallyPlayin() Watchable when we're playing a
MediaStream.

Depends on D49352

It started progressing as soon as we set up the rendering of the tracks in the
stream, which is too early according to the HTMLMediaElement spec.

Now it starts progressing when we're potentially playing. The difference being
that we now wait for mReadyState to go beyond HAVE_CURRENT_DATA before hooking
up the time progression mechanism.

Depends on D49353

Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/56eaea06b34a
Test that a media element's currentTime does not start advancing until potentially playing. r=jib
https://hg.mozilla.org/integration/autoland/rev/465c7e6f7572
Rename UpdateSrcTrackTime back to UpdateSrcStreamTime. r=jib
https://hg.mozilla.org/integration/autoland/rev/8529cff7ba28
Make mSrcStreamPlaybackEnded and mReadyState watchable. r=jib
https://hg.mozilla.org/integration/autoland/rev/a81df2957c09
Progress currentTime for a MediaStream only while potentially playing. r=jib
Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.
Flags: needinfo?(wptsync)
Regressions: 1593739
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/20208 for changes under testing/web-platform/tests
Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/20208
* Community-TC (pull_request) (https://community-tc.services.mozilla.com/tasks/groups/KcUO2nagTKewX2WY-rCOsg)
Flags: needinfo?(wptsync) → needinfo?(apehrson)
Flags: needinfo?(apehrson)
Regressions: 1593757

What am I expected to do here? Let the WPT bitrot while I fix gecko in another bug? And if I have WPT changes in that bug, won't there be conflicts in WPT again?

Is there some workflow doc I can read up on?

Flags: needinfo?(james)

There are a couple of options:

  • Push more commits to this bug. They will be picked up and added to the PR. That's a good option if the test is broken or the lint is failing or similar because it means we don't end up committing broken code to master.
  • Comment in the PR that the bug is known and will be fixed, and request an admin merge. That's a good option in the case like this where only Firefox is affected so we aren't making tests unstable in other browsers.

I don't think this is written down anywhere at the moment.

Flags: needinfo?(james)

Thanks James.

I discussed with jib offline and we see this as a bug in the test because currentTime is allowed to change between queueing the task to dispatch canplay and actually dispatching canplay. I'll try to go with your former suggestion of pushing another commit to this bug.

Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e333a13aa5f6
Don't assume media element's canplay task runs immediately. r=jib
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Blocks: 1588840
Upstream PR merged by moz-wptsync-bot
Duplicate of this bug: 1593757
No longer regressions: 1598245
You need to log in before you can comment on or make changes to this bug.