Closed Bug 1275596 Opened 8 years ago Closed 8 years ago

createMediaElementSource on YouTube video turns off the audio of any video played thereafter

Categories

(Core :: Audio/Video: MediaStreamGraph, defect, P1)

48 Branch
x86_64
Windows 7
defect

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox47 --- unaffected
firefox48 + verified
firefox49 + verified

People

(Reporter: epinal99-bugzilla2, Assigned: pehrsons)

References

Details

(Keywords: regression)

Attachments

(1 file)

Bug found by testing bug 1275393.

STR:
1) Open YT video https://www.youtube.com/watch?v=jI-kpVh6e1U
2) Run in web console:
context = new window.AudioContext();
source = context.createMediaElementSource($('video'));
source.connect(context.destination);
3) Clik on any video link on the page

Result: audio of 2nd video (and next ones) is off.

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f5fe9e219441413c6fc8429050ca19c910b7c62a&tochange=da8d6c4eab6142c98b04706255498750076fee9d
Blocks: 1208371
Has Regression Range: --- → yes
Has STR: --- → yes
Flags: needinfo?(pehrsons)
Keywords: regression
Andreas -- I believe this was introduced by your landing and that you're planning to take this (based on irc conversations).  If I'm wrong, please needinfo me back so I can find an owner.  We want to fix this (if we can) before Fx48 goes to Beta.  Thanks.
Assignee: nobody → pehrsons
Rank: 15
Priority: -- → P1
Side note: it also mutes if you drag the seekbar after step 2).
I'll investigate now.
Status: NEW → ASSIGNED
Flags: needinfo?(pehrsons)
I'm pretty sure this revision is causing the problems we see: https://hg.mozilla.org/mozilla-central/rev/1e531196e085

MediaElementAudioSourceNode is a sub class of MediaStreamAudioSourceNode: https://dxr.mozilla.org/mozilla-central/rev/8d0aadfe7da782d415363880008b4ca027686137/dom/media/webaudio/MediaElementAudioSourceNode.h#15

I changed MediaStreamAudioSourceNode to lock on to the first track. This was fine for MediaElementAudioSourceNode as well since we don't support multiple AudioTracks in a single HTMLMediaElement yet. The only problem is -- if the first track has ended there will be no more data, so we can skip to the next track automatically. This is not really defined in the spec, but automatically switching to the next track is more in line with how we behaved before my refactor.
This is how it was meant to work when the refactor landed in Bug 1208371.
We have no test coverage of seeking apparently.

Review commit: https://reviewboard.mozilla.org/r/55446/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55446/
Attachment #8756852 - Flags: review?(padenot)
Comment on attachment 8756852 [details]
MozReview Request: Bug 1275596 - Ignore ended tracks when selecting new track in MediaStreamAudioSourceNode and MediaElementSourceNode. r?padenot

https://reviewboard.mozilla.org/r/55446/#review52190
Attachment #8756852 - Flags: review?(padenot) → review+
Tracking this regression for 48/49 based on the fact that the audio is turned off.
That try looks bad. Here's a new one rebased: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd792d2f9daa

Maire, if that looks equally bad, could you decide how to handle those intermittents while I'm off for the night?

I don't see any relation at all between my changes and the oranges that show up on try.
Flags: needinfo?(mreavy)
I see the same failure in try pushes that don't include your patch; I think it's a timing issue related to the speed of the test VM.  Perhaps due to the track cloning, but not your current patch, so you can land.  

See https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd4b1ee34e18b7691193ee6d0c29da8de9941690 (and I've seen it on other Try pushes, but not on inbound)
Flags: needinfo?(mreavy) → needinfo?(pehrsons)
Ok, great!
Flags: needinfo?(pehrsons)
Comment on attachment 8756852 [details]
MozReview Request: Bug 1275596 - Ignore ended tracks when selecting new track in MediaStreamAudioSourceNode and MediaElementSourceNode. r?padenot

Approval Request Comment
[Feature/regressing bug #]: Bug 1208371
[User impact if declined]: Using MediaStreams or media elements as input to WebAudio might make it go silent when it shouldn't.
[Describe test coverage new/current, TreeHerder]: Tested manually.
[Risks and why]: Low. Change is simple.
[String/UUID change made/needed]: None.
Attachment #8756852 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/0fa7d5fb96af
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8756852 [details]
MozReview Request: Bug 1275596 - Ignore ended tracks when selecting new track in MediaStreamAudioSourceNode and MediaElementSourceNode. r?padenot

Fix a recent regression, taking it
Attachment #8756852 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
Verified as Fixed in Firefox Beta 48.0b2 (20160620091522) and Firefox DevEdition 49.0a2 (20160622004014) on Windows 7 x64 and Windows 10 x64.
Status: RESOLVED → VERIFIED
Updating tracking flags, based on the above comment.
You need to log in before you can comment on or make changes to this bug.