Bug 79281 - Use MathExtras round() in timeToSampleFrame
Summary: Use MathExtras round() in timeToSampleFrame
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Raymond Toy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-22 14:50 PST by Raymond Toy
Modified: 2012-02-23 21:25 PST (History)
2 users (show)

See Also:

Attachments
Patch (2.71 KB, patch)
2012-02-23 14:44 PST, Raymond Toy 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Raymond Toy 2012-02-22 14:50:22 PST 
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.)

This significantly simplifies the code and removes the ugly Windows-specific stuff.
Comment 1 Raymond Toy 2012-02-23 14:44:18 PST 
Created attachment 128560 [details]
Patch
Comment 2 Chris Rogers 2012-02-23 15:07:25 PST 
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?
Comment 3 Chris Rogers 2012-02-23 15:08:01 PST 
(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.
Comment 4 Raymond Toy 2012-02-23 15:18:55 PST 
(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 5 WebKit Review Bot 2012-02-23 21:25:16 PST 
Comment on attachment 128560 [details]
Patch

Clearing flags on attachment: 128560

Committed r108726: <http://trac.webkit.org/changeset/108726>
Comment 6 WebKit Review Bot 2012-02-23 21:25:21 PST 
All reviewed patches have been landed.  Closing bug.