Status

()

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla35
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox32 wontfix, firefox33 fixed, firefox34 fixed, firefox35 fixed, firefox-esr31 fixed, b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

()

Attachments

(4 attachments)

Checking that Ehsan is happy for this test to be modified.
Attachment #8497421 - Flags: feedback?(ehsan.akhgari)
Assignee: nobody → karlt
Status: NEW → ASSIGNED
No change in behavior here.
The code change is mostly to clarify what can change in the loop.
Attachment #8497422 - Flags: review?(giles)
This makes phase consistent with the expectations of the test, the sine
OscillatorType, and Blink's custom OscillatorType.
Attachment #8497423 - Flags: review?(giles)
This is the main fix.
The improvement can be seen on Ralph's nice demo at
https://people.mozilla.org/~rgiles/2013/osc07.html
Attachment #8497425 - Flags: review?(giles)

Comment 5

5 years ago
Comment on attachment 8497421 [details] [diff] [review]
test PeriodicWave output

Review of attachment 8497421 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!
Attachment #8497421 - Flags: feedback?(ehsan.akhgari) → feedback+
I tweaked some of the wave coefficients in the test to give enough high frequency components so that fuzzyCompare() in webaudio.js noticed the improvement in interpolation.

https://tbpl.mozilla.org/?tree=Try&rev=881c65f85bdb
https://tbpl.mozilla.org/?tree=Try&rev=7b489e208387
Comment on attachment 8497422 [details] [diff] [review]
move const periodicWaveSize multiplier out of loop

Review of attachment 8497422 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webaudio/OscillatorNode.cpp
@@ +383,1 @@
>   

Might as well remove the trailing whitespace while you're here.
Attachment #8497422 - Flags: review?(giles) → review+
Attachment #8497423 - Flags: review?(giles) → review+
Comment on attachment 8497425 [details] [diff] [review]
use greater weight for nearer sample in PeriodicWave interpolation

Review of attachment 8497425 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch.
Attachment #8497425 - Flags: review?(giles) → review+
Comment on attachment 8497425 [details] [diff] [review]
use greater weight for nearer sample in PeriodicWave interpolation

Please see comment 11.
Attachment #8497425 - Flags: approval-mozilla-esr31?
Attachment #8497425 - Flags: approval-mozilla-beta?
Attachment #8497425 - Flags: approval-mozilla-b2g30?
Attachment #8497425 - Flags: approval-mozilla-b2g28?
Attachment #8497425 - Flags: approval-mozilla-aurora?
Comment on attachment 8497425 [details] [diff] [review]
use greater weight for nearer sample in PeriodicWave interpolation

28 is EOL.
Attachment #8497425 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g32?
Attachment #8497425 - Flags: approval-mozilla-esr31?
Attachment #8497425 - Flags: approval-mozilla-esr31+
Attachment #8497425 - Flags: approval-mozilla-beta?
Attachment #8497425 - Flags: approval-mozilla-beta+
Attachment #8497425 - Flags: approval-mozilla-aurora?
Attachment #8497425 - Flags: approval-mozilla-aurora+
Attachment #8497425 - Flags: approval-mozilla-b2g32?
Attachment #8497425 - Flags: approval-mozilla-b2g32+
Attachment #8497425 - Flags: approval-mozilla-b2g30?
Attachment #8497425 - Flags: approval-mozilla-b2g30+
Depends on: 1187785
You need to log in before you can comment on or make changes to this bug.