Video is silenced when using createMediaElementSource(videoElem)

RESOLVED FIXED in Firefox 48

Status

()

defect
P1
normal
Rank:
15
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: maushundb, Assigned: pehrsons)

Tracking

({regression, testcase})

48 Branch
mozilla51
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(platform-rel ?, firefox48+ verified, firefox49 verified, firefox-esr45 unaffected, relnote-firefox 48+, firefox50 fixed, firefox51 fixed)

Details

(Whiteboard: [platform-rel-Facebook])

Attachments

(3 attachments)

Reporter

Description

3 years ago
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.
Status: UNCONFIRMED → NEW
Component: Untriaged → Audio/Video
Ever confirmed: true
Product: Firefox → Core
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.
It seems like the audio stream obtained from the video is faulty. Connecting an oscillator to the video works fine.
platform-rel: --- → ?
Whiteboard: [platform-rel-Facebook]
NI'ing, Anthony for relevant re-routing...
Flags: needinfo?(ajones)

Comment 4

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

Updated

3 years ago
Rank: 25
Component: Audio/Video → Web Audio
Priority: -- → P2
Assignee

Comment 5

3 years ago
I could investigate this, I'm doing work in the same area anyway.
Assignee: nobody → pehrson
Flags: needinfo?(ajones)
Reporter

Comment 6

3 years ago
@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.
@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?
Rank: 25 → 17
Priority: P2 → P1
(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)
Reporter

Comment 9

3 years ago
@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.
Has STR: --- → yes
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
Blocks: 1208371
Rank: 17 → 15
Flags: needinfo?(pehrson)
Assignee

Updated

3 years ago
Status: NEW → ASSIGNED
Flags: needinfo?(pehrson)
Assignee

Comment 11

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

Comment 12

3 years ago
(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.
Assignee

Comment 13

3 years ago
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 hidden (mozreview-request)

Comment 17

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

https://reviewboard.mozilla.org/r/72286/#review69878
Attachment #8781989 - Flags: review?(rjesup) → review+

Comment 18

3 years ago
mozreview-review
Comment on attachment 8781990 [details]
Bug 1295296 - Ignore video tracks in MediaStreamAudioSourceNode.

https://reviewboard.mozilla.org/r/72288/#review69880
Attachment #8781990 - Flags: review?(rjesup) → review+

Comment 19

3 years ago
mozreview-review
Comment on attachment 8781988 [details]
Bug 1295296 - Add a mochitest.

https://reviewboard.mozilla.org/r/72284/#review69882
Attachment #8781988 - Flags: review?(rjesup) → review+
Desigan, is it a big deal? thanks
Flags: needinfo?(dchinniah)
pehrsons: would a workaround be to create a new MediaStream from just the audio track, and use that as the input to WebAudio?
Flags: needinfo?(pehrson)
(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.
Flags: needinfo?(dchinniah)
Maire, Randell, should I delay 48.0.1 to take a fix for this issue? Thanks
Flags: needinfo?(mreavy)
Assignee

Comment 24

3 years ago
(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 :-(
Flags: needinfo?(pehrson)
(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.
Flags: needinfo?(mreavy)
Assignee

Comment 26

3 years ago
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
Attachment #8781990 - Flags: approval-mozilla-release?
Attachment #8781990 - Flags: approval-mozilla-beta?
Attachment #8781990 - Flags: approval-mozilla-aurora?
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
Attachment #8781990 - Flags: approval-mozilla-release?
Attachment #8781990 - Flags: approval-mozilla-release+
Attachment #8781990 - Flags: approval-mozilla-beta?
Attachment #8781990 - Flags: approval-mozilla-beta+
Attachment #8781990 - Flags: approval-mozilla-aurora?
Attachment #8781990 - Flags: approval-mozilla-aurora+
Assignee

Updated

3 years ago
Keywords: leave-open

Comment 28

3 years ago
Pushed by kwierso@gmail.com:
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
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. :)
Flags: needinfo?(pehrson)
Marking fixed on all but 51, since we expect to update the mochitest on 51 before marking the whole bug fixed

Comment 31

3 years ago
Backout by rjesup@wgate.com:
https://hg.mozilla.org/integration/fx-team/rev/876756efc390
Backed out changeset 3fb31d11633e
Assignee

Updated

3 years ago
Flags: needinfo?(pehrson)
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.
Flags: needinfo?(pehrson)
Added to the 48.0.1 release notes: "Fix an audio regression impacting some major websites (bug 1295296)"
Assignee

Comment 34

3 years ago
(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/?
Flags: needinfo?(pehrson) → needinfo?(andrei.vaida)
(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.
Flags: needinfo?(andrei.vaida)
Reporter

Comment 36

3 years ago
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)?
(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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Comment 42

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

Comment 44

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/169429641d30
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.