Closed Bug 1258567 Opened 8 years ago Closed 8 years ago

Sound volume is very small

Categories

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

46 Branch
Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
mozilla48
Tracking Status
firefox45 --- unaffected
firefox46 + verified
firefox47 --- verified
firefox48 --- verified
firefox-esr45 --- unaffected

People

(Reporter: alice0775, Assigned: jwwang)

References

Details

(Keywords: regression)

Attachments

(2 files)

[Tracking Requested - why for this release]: regression since 46

See http://forums.mozillazine.org/viewtopic.php?p=14549165#p14549165

https://bitcoinwisdom.com/markets/bitfinex/btcusd
alarm sounds dont work with FF 46 beta 2 and beta 1

Just go to this site above, click alarm, type in a value below the current price. The beeping should be loud and long, and you can barely hear it.

STR:
1. Open https://bitcoinwisdom.com/markets/bitfinex/btcusd
2. Click "TOOLS" and click "Alarm"
3. type in a value below the current price

Actual Results:
The beeping very small, and you can barely hear it

Expected Results:
The beeping should be loud


Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=151695836c37eb591dab55cdb696d620b7092039&tochange=0af7319a6a4169d94453f8fcfd78384459c343db

Regressed by: Bug 948267
Flags: needinfo?(kinetik)
Flags: needinfo?(jwwang)
OS: Unspecified → Windows
via local build
Last good: 9ce51229f0f9
First bad: 0af7319a6a41

Triggered by: 0af7319a6a41	JW Wang — Bug 948267. Part 3 - remove MDSM::AdjustAudioThresholds() to ensure we have enough data for AudioStream::DataCallback() to consume in the audio-only case. Also increase the amount of prerolling audio to 1s (which is the size of the circular buffer of AudioStream) to ensure a smooth start of playback. r=kinetik.
checking...
Flags: needinfo?(kinetik)
Flags: needinfo?(jwwang)
This bug is not regressed from Bug 948267 which just makes it more obvious.

See the loop at [1]. The volume will apply to |output| multiple times [2] if |audio.Length() > 1|. We should move |  aOutput->ApplyVolume(aVolume)| from SendStreamAudio() to SendAudio() to ensure the volume is only applied once.

Somehow, the volume of https://bitcoinwisdom.com/markets/bitfinex/btcusd is set to 0.8 and the final volume will approximate zero after applying volume=0.8 many times.

[1] https://hg.mozilla.org/mozilla-central/file/4037eb98974db1b1e0b5012c8a7f3a36428eaa11/dom/media/mediasink/DecodedStream.cpp#l521
[2] https://hg.mozilla.org/mozilla-central/file/4037eb98974db1b1e0b5012c8a7f3a36428eaa11/dom/media/mediasink/DecodedStream.cpp#l497
Attachment #8733213 - Flags: review?(kinetik) → review+
Comment on attachment 8733213 [details]
MozReview Request: Bug 1258567 - per comment 3, ensure volume is only applied once to the AudioSegment. r=kinetik.

https://reviewboard.mozilla.org/r/41651/#review38343
Thanks!
Need to uplift to 46.
Flags: needinfo?(jwwang)
Assignee: nobody → jwwang
Flags: needinfo?(jwwang)
Flags: needinfo?(jwwang)
https://hg.mozilla.org/mozilla-central/rev/8ba88b6c9d7d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment on attachment 8733213 [details]
MozReview Request: Bug 1258567 - per comment 3, ensure volume is only applied once to the AudioSegment. r=kinetik.

Approval Request Comment
[Feature/regressing bug #]:unknown
[User impact if declined]:audio volume might become very small when WebAudio is used to play music
[Describe test coverage new/current, TreeHerder]:manual test
[Risks and why]: very low for the change is very simple
[String/UUID change made/needed]:none
Attachment #8733213 - Flags: approval-mozilla-aurora?
Approval Request Comment
[Feature/regressing bug #]:unknown
[User impact if declined]:audio volume might become very small when WebAudio is used to play music
[Describe test coverage new/current, TreeHerder]:manual test
[Risks and why]: very low for the change is very simple
[String/UUID change made/needed]:none
Flags: needinfo?(jwwang)
Attachment #8734228 - Flags: review+
Attachment #8734228 - Flags: approval-mozilla-beta?
Comment on attachment 8733213 [details]
MozReview Request: Bug 1258567 - per comment 3, ensure volume is only applied once to the AudioSegment. r=kinetik.

recent regression, Aurora47+
Attachment #8733213 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8734228 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Alice0775 White, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(alice0775)
I can reproduce on
https://hg.mozilla.org/mozilla-central/rev/531d1f6d1cde1182e9f7f9dff81a4fc5abc0a601
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 ID:20160113025830

And I cannot reproduce the problem on latest Nightly
https://hg.mozilla.org/mozilla-central/rev/63be002b4a803df1122823841ef7633b7561d873
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 ID:20160328030215

Verified fixed.
Flags: needinfo?(alice0775)
(In reply to Alice0775 White from comment #15)
> I can reproduce on
> https://hg.mozilla.org/mozilla-central/rev/
> 531d1f6d1cde1182e9f7f9dff81a4fc5abc0a601
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
> ID:20160113025830
> 
> And I cannot reproduce the problem on latest Nightly
> https://hg.mozilla.org/mozilla-central/rev/
> 63be002b4a803df1122823841ef7633b7561d873
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
> ID:20160328030215
> 
> Verified fixed.

Awesome! You are amazing. :)
Flags: qe-verify+
Verified fixed on Windows 7 64bit, Mac OSX 10.9.5 and Ubuntu 14.04 64bit using Firefox 46 Beta 6 (buildID: 20160328182534) and latest Aurora 47.0a2 (buildID: 20160401004045).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.