1295296 - Video is silenced when using createMediaElementSource(videoElem)

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[maushundb]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36

Steps to reproduce:

Full example available at: https://jsfiddle.net/4k5qw93h/

1. Create a video element using document.createElement('video');
2. Set it's cross origin to anonymous: video.setAttribute('crossorigin', 'anonymous'); Bug still repros if you skip this step, but it was mentioned in previous bugs (Bug 937718).
3. Set a video source: video.src = 'https://dl.dropboxusercontent.com/s/tozkw68tccsp39a/BrianNalumonWindmill360p.mp4';
4. Create a new audioContext, and use it to call createMediaElementSource(video) and connect it with the audioContext.destination
5. Play the video


Actual results:

Observe that the video is silent. Skip step (4) and observe that the video is no longer silent.


Expected results:

The video should play audio.

Comment 1

[Manish Goregaokar [:manishearth]]

When I run it (Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:50.0) Gecko/20100101 Firefox/50.0), I get a second of audio at the end. I think it's the first second of audio.

Comment 2

[Manish Goregaokar [:manishearth]]

It seems like the audio stream obtained from the video is faulty. Connecting an oscillator to the video works fine.

Comment 3

[Desigan Chinniah [:cyberdees] [:dees] [London - GMT]]

NI'ing, Anthony for relevant re-routing...

Comment 4

[Mohamed Hadjoudj]

I've noticed that if you replace var video = document.createElement('video');
with var video = document.createElement('audio');
it plays the audio

Comment 5

[Andreas Pehrson [:pehrsons]]

I could investigate this, I'm doing work in the same area anyway.

Comment 6

[maushundb]

@manishearth Yeah I get a second of audio at the end as well. It's funny that the oscillator works though. This bug report was prompted by my specific use case, which uses a scriptProcessorNode. I found that the input buffer it is receiving from the source node returns all 0's as shown in the console of this fiddle: https://jsfiddle.net/ybw3tvau/2/. So I assumed it was the source node.

Hopefully that helps debugging a tad and it isn't a separate issue.

Comment 7

[Maire Reavy [:mreavy]]

@manishearth - Thanks for the fiddle.  We'll attempt to repro and work to identify the root cause.  It looks like you were using Dev edition 50.  Have you tested with any other versions of Firefox?

Comment 8

[Manish Goregaokar [:manishearth]]

(I'm not the one who posted the fiddles)

It seems to work fine on Firefox 47 (Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:47.0) Gecko/20100101 Firefox/47.0)

Comment 9

[maushundb]

@mreavy It also does not work in the nightly 51.0a1 2016-08-16. I originally encountered it in Firefox 48.

I also remember it (potentially) happening in 47 when the console is opened which is what led me to update to 48, but I'm unable to reproduce it now myself so take that as you will.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

It works in 48 Nightly on Apr 7, fails on Apr 8th.  Likely regression from bug 1208371 (track cloning).  

in mozilla-central:
hg log -r b6683e141c47c022598c0caac3ea8ba8c6236d42:d9b1a9829c8ee2862955043f28183efa07de3d2b

Comment 11

[Andreas Pehrson [:pehrsons]]

For a workaround, it works as expected if you wait for "loadedmetadata" before createMediaElementSource().

Comment 12

[Andreas Pehrson [:pehrsons]]

(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #11)
> For a workaround, it works as expected if you wait for "loadedmetadata"
> before createMediaElementSource().

Well, somewhat. Seeking after this breaks it. As well as changing sources.

Comment 13

[Andreas Pehrson [:pehrsons]]

The problem is that we attach to the video track in some cases. No audio there obviously :-(

I'm adding a test case, an assert and a fix.

Comment 14

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1295296 - Add a mochitest.

Review commit: https://reviewboard.mozilla.org/r/72284/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72284/

Comment 15

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1295296 - Assert that we don't see video tracks in ExternalAudioInputStream.

Review commit: https://reviewboard.mozilla.org/r/72286/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72286/

Comment 16

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1295296 - Ignore video tracks in MediaStreamAudioSourceNode.

Review commit: https://reviewboard.mozilla.org/r/72288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72288/

Comment 17

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8781989 [details]
Bug 1295296 - Assert that we don't see video tracks in ExternalAudioInputStream.

https://reviewboard.mozilla.org/r/72286/#review69878

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8781990 [details]
Bug 1295296 - Ignore video tracks in MediaStreamAudioSourceNode.

https://reviewboard.mozilla.org/r/72288/#review69880

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8781988 [details]
Bug 1295296 - Add a mochitest.

https://reviewboard.mozilla.org/r/72284/#review69882

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Desigan, is it a big deal? thanks

Comment 21

[Randell Jesup [:jesup] (needinfo me)]

pehrsons: would a workaround be to create a new MediaStream from just the audio track, and use that as the input to WebAudio?

Comment 22

[Desigan Chinniah [:cyberdees] [:dees] [London - GMT]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #20)
> Desigan, is it a big deal? thanks

All I can say is that it's been reported by Facebook. So yes. And Maire has prioritised fixing for it.

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Maire, Randell, should I delay 48.0.1 to take a fix for this issue? Thanks

Comment 24

[Andreas Pehrson [:pehrsons]]

(In reply to Randell Jesup [:jesup] from comment #21)
> pehrsons: would a workaround be to create a new MediaStream from just the
> audio track, and use that as the input to WebAudio?

Well, that requires the prefixed and not-up-to-spec mozCaptureStream() to be used. Part of the not-up-to-spec bit is that we end and create new tracks on seeking. You'd have to be aware of that and wait sufficiently long after a seek finishes for the new ones to appear. MediaStream's "addtrack" event would be fine here but it doesn't come until Firefox 50. There are more or less dirty workarounds for that too.. polling the stream comes to mind :-(

Comment 25

[Maire Reavy [:mreavy]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #23)
> Maire, Randell, should I delay 48.0.1 to take a fix for this issue? Thanks

Yes -- jesup, pehrsons and I talked through the options, and the three of us agree that the fix should go into Fx 48.  The fix is very, very safe, and the workarounds are insufficient.  We will ask to uplift the fix itself to 48 since that's all we need for the dot release.

Comment 26

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8781990 [details]
Bug 1295296 - Ignore video tracks in MediaStreamAudioSourceNode.

Approval Request Comment
[Feature/regressing bug #]: bug 1208371
[User impact if declined]: Piping a media file containing both audio and video into WebAudio will most of the time result in silence. Audio only files are fine.
[Describe test coverage new/current, TreeHerder]: Tested manually on Nightly, mochitest in m-c.
[Risks and why]: Very very low risk given the simplicity of the fix.
[String/UUID change made/needed]: None

Comment 27

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8781990 [details]
Bug 1295296 - Ignore video tracks in MediaStreamAudioSourceNode.

Breaks facebook, taking it.
Should be in 48.0.1 as we will do a second build for it

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/3fb31d11633e
Add a mochitest. r=jesup
https://hg.mozilla.org/integration/fx-team/rev/ab241a57d6e8
Assert that we don't see video tracks in ExternalAudioInputStream. r=jesup
https://hg.mozilla.org/integration/fx-team/rev/1a952d5e3046
Ignore video tracks in MediaStreamAudioSourceNode. r=jesup

Comment 29

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/12f39bee3e5a
https://hg.mozilla.org/releases/mozilla-beta/rev/67d8341f1294
https://hg.mozilla.org/releases/mozilla-release/rev/f36f7ace6f48

Unsure what the status flags should end up being set to since this is leave-open. I'll leave that to you. :)

Comment 30

[Randell Jesup [:jesup] (needinfo me)]

Marking fixed on all but 51, since we expect to update the mochitest on 51 before marking the whole bug fixed

Comment 31

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/876756efc390
Backed out changeset 3fb31d11633e

Comment 32

[Andrei Vaida [:avaida]]

This is verified fixed on 48.0.1-build3 (20160817112116) using Windows
7 x64, Window 10 x64, Mac OS X 10.9.5 and Ubuntu 16.04 x86.

There was however a different outcome on one of our Ubuntu test
machines, an HP ProBook 470 G3 with dual GPU (Intel HD 520, AMD Radeon
R7 M340), where 48.0.1-build3 still appeared to be affected, while
Aurora 50.0a2 (2016-08-18) appeared fixed.

      Just to be safe, we checked our other Linux environments,
      specifically:
          - Ubuntu 14.04 x64, 
          - Ubuntu 16.04 x86 
      and on both of them 48.0.1-build3 appeared fixed.

That being said, we suspect there's some sort of environment issue
affecting the HP ProBook, so we think it's safe to say this bug is
verified fixed across platforms. Andreas, if you have any concerns
regarding our approach, please let us know.

Comment 33

[Sylvestre Ledru [:Sylvestre]]

Added to the 48.0.1 release notes: "Fix an audio regression impacting some major websites (bug 1295296)"

Comment 34

[Andreas Pehrson [:pehrsons]]

(In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #32)
> This is verified fixed on 48.0.1-build3 (20160817112116) using Windows
> 7 x64, Window 10 x64, Mac OS X 10.9.5 and Ubuntu 16.04 x86.
> 
> There was however a different outcome on one of our Ubuntu test
> machines, an HP ProBook 470 G3 with dual GPU (Intel HD 520, AMD Radeon
> R7 M340), where 48.0.1-build3 still appeared to be affected, while
> Aurora 50.0a2 (2016-08-18) appeared fixed.
> 
>       Just to be safe, we checked our other Linux environments,
>       specifically:
>           - Ubuntu 14.04 x64, 
>           - Ubuntu 16.04 x86 
>       and on both of them 48.0.1-build3 appeared fixed.
> 
> That being said, we suspect there's some sort of environment issue
> affecting the HP ProBook, so we think it's safe to say this bug is
> verified fixed across platforms. Andreas, if you have any concerns
> regarding our approach, please let us know.

To clarify, on the still affected linux machine do you hear the audio on https://jsfiddle.net/pehrsons/4k5qw93h/17/ but not on https://jsfiddle.net/4k5qw93h/?

Comment 35

[Andrei Vaida [:avaida]]

(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #34)
> To clarify, on the still affected linux machine do you hear the audio on
> https://jsfiddle.net/pehrsons/4k5qw93h/17/ but not on
> https://jsfiddle.net/4k5qw93h/?

I hear the audio on both of them now, with a system restart and a new profile. This was definitely an issue with the way our test environment was set up. 

Thank you for the prompt reply and I apologize for the false alarm here.

Comment 36

[maushundb]

Thanks for the quick fix folks! Two last questions:

1. When is 48.0.01 being released?
2. Can I expect this to be in Nightly anytime soon(er)?

Comment 37

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

https://hg.mozilla.org/mozilla-central/rev/ab241a57d6e8
https://hg.mozilla.org/mozilla-central/rev/1a952d5e3046

Comment 38

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

(In reply to maushundb from comment #36)
> 2. Can I expect this to be in Nightly anytime soon(er)?

Should be in the nightly build that gets built overnight tonight, shipping out tomorrow morning.

Comment 39

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8781988 [details]
Bug 1295296 - Add a mochitest.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72284/diff/1-2/

Comment 40

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8781989 [details]
Bug 1295296 - Assert that we don't see video tracks in ExternalAudioInputStream.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72286/diff/1-2/

Comment 41

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8781990 [details]
Bug 1295296 - Ignore video tracks in MediaStreamAudioSourceNode.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72288/diff/1-2/

Comment 42

[Andreas Pehrson [:pehrsons]]

Test working on android now, needed a simpler encoding profile: https://treeherder.mozilla.org/#/jobs?repo=try&revision=441e772ec38c

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/169429641d30
Add a mochitest. r=jesup

Comment 44

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/169429641d30