Closed
Bug 1513733
Opened 6 years ago
Closed 6 years ago
start blocked AudioContext when it's source media element starts
Categories
(Core :: Web Audio, defect, P2)
Core
Web Audio
Tracking
()
RESOLVED
FIXED
mozilla66
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox64 | --- | unaffected |
firefox65 | --- | unaffected |
firefox66 | + | fixed |
People
(Reporter: alwu, Assigned: alwu)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(5 files)
STR.
1. set "media.autoplay.block-webaudio=true"
2. go to https://alastor0325.github.io/htmltests/non_mse_tests/audioAsSourceToWebAudio.html
3. click play button
Expect.
4. audio is playing and web audio should have sound
Actual
4. audio is playing and web audio doesn't have sound
---
In this case, when user starts MediaElement which is used as a source for AudioContext, I think we should also resume AudioContext.
Assignee | ||
Comment 1•6 years ago
|
||
Hi, Paul,
May I have your opinion about resuming AudioContext when its source starts?
Thank you!
Flags: needinfo?(padenot)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(padenot)
Assignee | ||
Comment 2•6 years ago
|
||
In order to call this method on other situations, rename it to 'StartBlockedAudioContextIfAllowed()'.
Assignee | ||
Comment 3•6 years ago
|
||
If media element is used as a source for AudioContext, we would try to start AudioContext which was not allowed
to start when media element starts playing.
Assignee | ||
Comment 4•6 years ago
|
||
Updated•6 years ago
|
Summary: start blocked AudioContext when it's source starts → start blocked AudioContext when it's source media element starts
Assignee | ||
Updated•6 years ago
|
status-firefox65:
--- → unaffected
status-firefox66:
--- → affected
tracking-firefox66:
--- → +
Keywords: regression
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6d105dcaa3df
part1 : rename 'NotifyScheduledSourceNodeStarted()' r=karlt
https://hg.mozilla.org/integration/autoland/rev/7ab6eb45e6b8
part2 : try to start AudioContext when media element which is as a source for web audio starts r=cpearce,karlt
https://hg.mozilla.org/integration/autoland/rev/5ce7c992bd81
part3 : add test. r=karlt
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6a89226d8c15
Backed out 3 changesets for frequent failures at browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js on a CLOSED TREE
Comment 8•6 years ago
|
||
Backed out 3 changesets (bug 1513733) for frequent failures at browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js on a CLOSED TREE
Backout link: https://hg.mozilla.org/integration/autoland/rev/6a89226d8c15f0b8b9dbcb23f76f371fdefc289b
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&selectedJob=219708943&revision=5ce7c992bd81c2426c5be3c3be57e7dfa72fdd9c
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=219708943&repo=autoland&lineNumber=5980
Log snippet:
02:48:23 INFO - TEST-START | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js
02:49:09 INFO - TEST-INFO | started process screenshot
02:49:09 INFO - TEST-INFO | screenshot: exit 0
02:49:09 INFO - Buffered messages logged at 02:48:23
02:49:09 INFO - Entering test bound start_tests
02:49:09 INFO - - setup test preference -
02:49:09 INFO - - start 'testResumeAudioContextWhenMediaElementSourceStarted' -
02:49:09 INFO - Buffered messages logged at 02:48:24
02:49:09 INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "https://example.com/browser/toolkit/content/tests/browser/file_video.html" line: 0}]
02:49:09 INFO - - create audio context -
02:49:09 INFO - - check AudioContext status -
02:49:09 INFO - TEST-PASS | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | AudioContext is not started yet. - true == true -
02:49:09 INFO - - create and start MediaElementAudioSourceNode -
02:49:09 INFO - TEST-PASS | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | video started playing - true == true -
02:49:09 INFO - - AudioContext should be resumed after MediaElementAudioSourceNode started -
02:49:09 INFO - Buffered messages finished
02:49:09 INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | Test timed out -
02:49:09 INFO - GECKO(5632) | MEMORY STAT | vsize 1707MB | vsizeMaxContiguous 130580191MB | residentFast 223MB | heapAllocated 71MB
02:49:09 INFO - TEST-OK | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | took 45034ms
02:49:09 INFO - Not taking screenshot here: see the one that was previously logged
02:49:09 INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | Found a tab after previous test timed out: https://example.com/browser/toolkit/content/tests/browser/file_video.html -
02:49:09 INFO - GECKO(5632) | JavaScript error: , line 0: InvalidStateError: An attempt was made to use an object that is not, or is no longer, usable
02:49:09 INFO - checking window state
Flags: needinfo?(alwu)
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Comment 9•6 years ago
|
||
When we block AudioContext in the time of construction, we would have a internal suspend call which is
used to stop AudioContext from starting. The suspend call would eventually dispatch state change task,
but we don't need that in this situation, because AudioContext would keep staying at 'suspended' state.
In this situation, if we dispatch the state change task for 'suspend' in the beginning, the AudioContext's
state would be changed incorrectly when we resume the AudioContext soon after we block it. The reason is
that the state change task for 'suspend' which is called in the time of contruction would be executed after
the state change task for 'resume' because MSG finishs 'resume' operation faster than finishs 'suspend' operation
even if the suspend operation has come before the resume operation.
Assignee | ||
Comment 10•6 years ago
|
||
From the log, we can see the reason of intermittent test fail is because the state change task for 'suspend' which is called at the time of AudioContext contruction came late after the state change task for 'resume'.
It causes that even we have resumed AudioContext successfully, its state is still keeping at 'suspended' state.
Flags: needinfo?(alwu)
Assignee | ||
Comment 11•6 years ago
|
||
I found that even the new patch can fix the test's timeout problem, but there is still a leaking issue [1] which needs to be fixed...
[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=e918caccf8a85b92ffad67334ff6f3506d30e9d2&selectedJob=219863559
Assignee | ||
Comment 12•6 years ago
|
||
Hi, Karlt,
Do you have any idea about the leaking in comment11?
I suspect that is related with the lambda capturing, but I have no further evidence which can prove that assumption...
Thank you!
Flags: needinfo?(karlt)
Comment 13•6 years ago
|
||
I don't see any new strong references added in the patch here and so I guess it is something new in the test that is triggering the leak.
I would reduce what the test is doing as an experiment to find out which part exactly is triggering the leak. Hopefully that might provide some clues.
Flags: needinfo?(karlt)
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 14•6 years ago
|
||
I filed leaking issue in bug1518213, and I'm going to change the test to simple plain-mochitest.
Updated•6 years ago
|
Attachment #9034268 -
Attachment description: Bug 1513733 - part4 : do not dispatch state change task for interal suspend call. → Bug 1513733 - part4 : do not call suspendInternal() when constructing AudioContext
Comment 15•6 years ago
|
||
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05005b52bb56
part1 : rename 'NotifyScheduledSourceNodeStarted()' r=karlt
https://hg.mozilla.org/integration/autoland/rev/c34e287f2f7c
part2 : try to start AudioContext when media element which is as a source for web audio starts r=cpearce,karlt
https://hg.mozilla.org/integration/autoland/rev/22ce92cbdf64
part3 : add test. r=baku,karlt
https://hg.mozilla.org/integration/autoland/rev/8699d4c48838
part4 : do not call suspendInternal() when constructing AudioContext r=karlt
Comment 16•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/05005b52bb56
https://hg.mozilla.org/mozilla-central/rev/c34e287f2f7c
https://hg.mozilla.org/mozilla-central/rev/22ce92cbdf64
https://hg.mozilla.org/mozilla-central/rev/8699d4c48838
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Updated•6 years ago
|
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•