Closed Bug 1267579 Opened 4 years ago Closed 4 years ago

Unexpected result when using OscillatorNode with custom wave shape

Categories

(Core :: Web Audio, defect, P2)

45 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox46 --- wontfix
firefox47 + fixed
firefox48 + fixed
firefox49 --- fixed

People

(Reporter: sebpiq, Assigned: dminor)

References

Details

(Keywords: regression, testcase)

Attachments

(3 files)

Attached file bugreport-ff.html
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160325004402

Steps to reproduce:

Try to get a Cos wave of frequency 0 with web audio OscillatorNode, in order to have a node outputting only ones. See attachment for a test case.


Actual results:

The OscillatorNode outputs only zeros.


Expected results:

The OscillatorNode should have output ones.
Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=74166e
0168385eba427642965b93c7ad010662ca&tochange=3785489fa210d38b890ab73f21202b94c552
8c1f

Regressed by Bug 1222405.

Dan, your thoughts?
Blocks: 1222405
Component: Untriaged → Web Audio
Flags: needinfo?(dminor)
Keywords: regression, testcase
Product: Firefox → Core
Having a look now.
Assignee: nobody → dminor
Flags: needinfo?(dminor)
Thanks for your report!
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
This fixes some division by zero errors introduced by Bug 1222405 when the
fundamental frequency is zero.

Review commit: https://reviewboard.mozilla.org/r/48987/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48987/
Attachment #8745347 - Flags: review?(padenot)
Attachment #8745348 - Flags: review?(padenot)
Attachment #8745347 - Flags: review?(padenot) → review+
Comment on attachment 8745347 [details]
MozReview Request: Bug 1267579 - Unexpected result when using OscillatorNode with custom wave shape; r?padenot

https://reviewboard.mozilla.org/r/48987/#review45819
Comment on attachment 8745348 [details]
MozReview Request: Bug 1267579 - Add test case; r?padenot

https://reviewboard.mozilla.org/r/48989/#review45821
Attachment #8745348 - Flags: review?(padenot) → review+
Rank: 23
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/b165d4c2ee46
https://hg.mozilla.org/mozilla-central/rev/a6e1c4c2b2c4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Target Milestone: mozilla48 → mozilla49
Dan, Paul -- Does it make sense to uplift this?  If so, how far?  I'm marking Fx46 as wontfix, but Fx47 is still very early in the cycle -- so it would get basically the entire Beta cycle to bake there if we ask for uplift this week.  Let me know your thoughts.  Thanks!
Flags: needinfo?(padenot)
Flags: needinfo?(dminor)
This is an edge case that only occurs when the fundamental frequency is set to zero, which I don't think is likely to affect that many people. On the other hand, it's also low risk. I'll defer to Paul :)
Flags: needinfo?(dminor)
It's still fixing a division by zero.

Also, admittedly, to have a DC offset, it's more efficient to have an AudioBufferSourceNode with a buffer with a length of 1, with the only element set to 1.

Very low-risk indeed, maybe it's worth it because it's fixing a correctness issue.
Flags: needinfo?(padenot)
Comment on attachment 8745347 [details]
MozReview Request: Bug 1267579 - Unexpected result when using OscillatorNode with custom wave shape; r?padenot

Given this is low risk, I think it makes sense to ask for uplift to 48.

Approval Request Comment
[Feature/regressing bug #]: Bug 1267579
[User impact if declined]: Minimal, as Paul mentions there is a more efficient way of generating the desired signal.
[Describe test coverage new/current, TreeHerder]: A new test case has been written.
[Risks and why]: Very low risk - this only affects cases where the requested fundamental frequency is zero, which isn't a common use case.
[String/UUID change made/needed]: None
Attachment #8745347 - Flags: approval-mozilla-aurora?
Flags: in-testsuite+
Attachment #8745347 - Flags: approval-mozilla-beta?
Comment on attachment 8745347 [details]
MozReview Request: Bug 1267579 - Unexpected result when using OscillatorNode with custom wave shape; r?padenot

Recent regression, Beta47+, Aurora48+
Attachment #8745347 - Flags: approval-mozilla-beta?
Attachment #8745347 - Flags: approval-mozilla-beta+
Attachment #8745347 - Flags: approval-mozilla-aurora?
Attachment #8745347 - Flags: approval-mozilla-aurora+
Hello sebpiq, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(sebpiq)
Yep works in latest nightly (49.0a1). Thanks for fixing this!
Flags: needinfo?(sebpiq)
You need to log in before you can comment on or make changes to this bug.