Closed Bug 1074765 Opened 11 years ago Closed 11 years ago

noise in PeriodicWave

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla35

Tracking Flags:

Tracking

Status

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

People

(Reporter: karlt, Assigned: karlt)

References

(
URL
)

Details

Attachments

(4 files)

test PeriodicWave output 11 years ago Karl Tomlinson (:karlt) 3.31 KB, patch	ehsan.akhgari : feedback+	Details \| Diff \| Splinter Review
move const periodicWaveSize multiplier out of loop 11 years ago Karl Tomlinson (:karlt) 1.91 KB, patch	rillian : review+	Details \| Diff \| Splinter Review
increment phase after sampling to begin PeriodicWave at phase = 0 11 years ago Karl Tomlinson (:karlt) 1.99 KB, patch	rillian : review+	Details \| Diff \| Splinter Review
use greater weight for nearer sample in PeriodicWave interpolation 11 years ago Karl Tomlinson (:karlt) 1.80 KB, patch	rillian : review+ abillings : approval-mozilla-aurora+ abillings : approval-mozilla-beta+ abillings : approval-mozilla-esr31+ bajaj : approval-mozilla-b2g30+ bajaj : approval-mozilla-b2g32+	Details \| Diff \| Splinter Review

Karl Tomlinson (:karlt)

Assignee

Description

•

11 years ago

https://bugzilla.mozilla.org/show_bug.cgi?id=865256#c32

Karl Tomlinson (:karlt)

Assignee

Comment 1

•

11 years ago

Attached patch test PeriodicWave output — Details — Splinter Review

Checking that Ehsan is happy for this test to be modified.

Attachment #8497421 - Flags: feedback?(ehsan.akhgari)

Karl Tomlinson (:karlt)

Assignee

Updated

•

11 years ago

Assignee: nobody → karlt

Status: NEW → ASSIGNED

Karl Tomlinson (:karlt)

Assignee

Comment 2

•

11 years ago

Attached patch move const periodicWaveSize multiplier out of loop — Details — Splinter Review

No change in behavior here. The code change is mostly to clarify what can change in the loop.

Attachment #8497422 - Flags: review?(giles)

Karl Tomlinson (:karlt)

Assignee

Comment 3

•

11 years ago

Attached patch increment phase after sampling to begin PeriodicWave at phase = 0 — Details — Splinter Review

This makes phase consistent with the expectations of the test, the sine OscillatorType, and Blink's custom OscillatorType.

Attachment #8497423 - Flags: review?(giles)

Karl Tomlinson (:karlt)

Assignee

Comment 4

•

11 years ago

Attached patch use greater weight for nearer sample in PeriodicWave interpolation — Details — Splinter Review

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)

(no longer active)

Comment 5

•

11 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+

Karl Tomlinson (:karlt)

Assignee

Comment 6

•

11 years ago

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

Ralph Giles (:rillian)

Comment 7

•

11 years ago

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+

Ralph Giles (:rillian)

Updated

•

11 years ago

Attachment #8497423 - Flags: review?(giles) → review+

Ralph Giles (:rillian)

Comment 8

•

11 years ago

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+

Karl Tomlinson (:karlt)

Assignee

Comment 9

•

11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/5330567d85ba https://hg.mozilla.org/integration/mozilla-inbound/rev/7e4ce5cbef3e https://hg.mozilla.org/integration/mozilla-inbound/rev/b083e83ba13a https://hg.mozilla.org/integration/mozilla-inbound/rev/b39629d18e66

Flags: in-testsuite+

Carsten Book [:Tomcat]

Comment 10

•

11 years ago

https://hg.mozilla.org/mozilla-central/rev/5330567d85ba https://hg.mozilla.org/mozilla-central/rev/7e4ce5cbef3e https://hg.mozilla.org/mozilla-central/rev/b083e83ba13a https://hg.mozilla.org/mozilla-central/rev/b39629d18e66

Status: ASSIGNED → RESOLVED

Closed: 11 years ago

Resolution: --- → FIXED

Target Milestone: --- → mozilla35

Karl Tomlinson (:karlt)

Assignee

Comment 12

•

11 years ago

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?

Karl Tomlinson (:karlt)

Assignee

Comment 13

•

11 years ago

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?

Karl Tomlinson (:karlt)

Assignee

Updated

•

11 years ago

Blocks: CVE-2014-1577

Al Billings [:abillings - ex-MoCo]

Updated

•

11 years ago

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+

Ryan VanderMeulen [:RyanVM]

Comment 14

•

11 years ago

status-b2g-v1.4: --- → affected

status-b2g-v2.0: --- → affected

status-b2g-v2.0M: --- → affected

status-b2g-v2.1: --- → fixed

status-b2g-v2.2: --- → fixed

status-firefox32: --- → wontfix

status-firefox33: --- → fixed

status-firefox34: --- → fixed

status-firefox35: --- → fixed

status-firefox-esr31: --- → fixed

bhavana bajaj [:bajaj]

Updated

•

11 years ago

Attachment #8497425 - Flags: approval-mozilla-b2g32?

Attachment #8497425 - Flags: approval-mozilla-b2g32+

Attachment #8497425 - Flags: approval-mozilla-b2g30?

Attachment #8497425 - Flags: approval-mozilla-b2g30+

Ryan VanderMeulen [:RyanVM]

Comment 15

•

11 years ago

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e3a32d59bc92 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/f9262fbb614a https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/4a939bbaa528 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/8f3b3a413900 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/ca0a97fcc525 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/df1d430d494a https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/3f7da5857676 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/34a109087197

status-b2g-v1.4: affected → fixed

status-b2g-v2.0: affected → fixed

Kai-Zhen Li [:seinlin][:kli]

Comment 16

•

11 years ago

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/e3a32d59bc92 http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/f9262fbb614a http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/4a939bbaa528 http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/8f3b3a413900

status-b2g-v2.0M: affected → fixed

Karl Tomlinson (:karlt)

Assignee

Updated

•

10 years ago

Depends on: 1187785

You need to log in before you can comment on or make changes to this bug.