| Summary: | Use MathExtras round() in timeToSampleFrame | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Raymond Toy <rtoy> | ||||
| Component: | Web Audio | Assignee: | Raymond Toy <rtoy> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | crogers, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Raymond Toy
2012-02-22 14:50:22 PST
Created attachment 128560 [details]
Patch
Looks like a nice cleanup. This looks good, with the assumption that timeToSampleFrame() is *not* currently being called within any tight loops. That should be the case, but could you please sanity check? (In reply to comment #0) > MathExtras.h includes a round() function. This should be used in the implementation of timeToSampleFrame in AudioUtilities.cpp. round() behaves better (no loss of precision) and works as is on Windows. (All webaudio layout tests pass with round.) I assume you've specifically run the layout tests on Windows? > > This significantly simplifies the code and removes the ugly Windows-specific stuff. (In reply to comment #3) > (In reply to comment #0) > > MathExtras.h includes a round() function. This should be used in the implementation of timeToSampleFrame in AudioUtilities.cpp. round() behaves better (no loss of precision) and works as is on Windows. (All webaudio layout tests pass with round.) > > I assume you've specifically run the layout tests on Windows? Layout tests pass on Windows. And I checked the code and timeToSampleFrame is not used in body of any tight loops. Comment on attachment 128560 [details] Patch Clearing flags on attachment: 128560 Committed r108726: <http://trac.webkit.org/changeset/108726> All reviewed patches have been landed. Closing bug. |