79281 2012-02-22 14:50:22 -0800 Use MathExtras round() in timeToSampleFrame 2012-02-23 21:25:21 -0800 1 1 1 Unclassified WebKit Web Audio 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 rtoy rtoy crogers webkit.review.bot oldest_to_newest 562410 0 rtoy 2012-02-22 14:50:22 -0800 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. 563505 1 128560 rtoy 2012-02-23 14:44:18 -0800 Created attachment 128560 Patch 563528 2 crogers 2012-02-23 15:07:25 -0800 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? 563530 3 crogers 2012-02-23 15:08:01 -0800 (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. 563538 4 rtoy 2012-02-23 15:18:55 -0800 (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. 563903 5 128560 webkit.review.bot 2012-02-23 21:25:16 -0800 Comment on attachment 128560 Patch Clearing flags on attachment: 128560 Committed r108726: <http://trac.webkit.org/changeset/108726> 563904 6 webkit.review.bot 2012-02-23 21:25:21 -0800 All reviewed patches have been landed. Closing bug. 128560 2012-02-23 14:44:18 -0800 2012-02-23 21:25:16 -0800 Patch bug-79281-20120223144417.patch text/plain 2780 rtoy U3VidmVyc2lvbiBSZXZpc2lvbjogMTA4NjE5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTdkNDY4ZDQyODkxMzc5 YjNlNjA3MTU2N2ViMzNhNmYwZTY4ZWM1ZS4uZmQ4Y2YxZjI5OWVlMWZmZGFlMjNjNWY4YmEzYzBk YjQ1NWMzNTllOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDEyLTAyLTIzICBSYXlt b25kIFRveSAgPHJ0b3lAZ29vZ2xlLmNvbT4KKworICAgICAgICBVc2UgTWF0aEV4dHJhcyByb3Vu ZCgpIGluIHRpbWVUb1NhbXBsZUZyYW1lCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD03OTI4MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gRXhpc3RpbmcgdGVzdHMgY292ZXIgdGhpcyBj aGFuZ2UuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9hdWRpby9BdWRpb1V0aWxpdGllcy5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpBdWRpb1V0aWxpdGllczo6dGltZVRvU2FtcGxlRnJhbWUpOiBVc2Ug cm91bmQoKS4gCisKIDIwMTItMDItMjIgIElnb3IgT2xpdmVpcmEgIDxpZ29yLm9Ac2lzYS5zYW1z dW5nLmNvbT4KIAogICAgICAgICBFdmVyeSBjYWxsIHRvIFJlbmRlck9iamVjdDo6c2V0QW5pbWF0 YWJsZVN0eWxlKCkgaXRlcmF0ZXMgdGhyb3VnaCBhbGwgbV9jb21wb3NpdGVBbmltYXRpb25zOiBw b3RlbnRpYWxseSBPKE5eMikKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2F1 ZGlvL0F1ZGlvVXRpbGl0aWVzLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2F1ZGlvL0F1 ZGlvVXRpbGl0aWVzLmNwcAppbmRleCBmZjE5Y2VlNjk3M2Q5NzY0MmQwMDFjNTQ5NzZkYWIxNTVk Mjg2OTNiLi43ZWM4MzNjY2Q1NDRjZTYyNjgwZTBkNjNkZWY5ZTk4NDU2ZDY1Yjc0IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9hdWRpby9BdWRpb1V0aWxpdGllcy5jcHAKKysr IGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vYXVkaW8vQXVkaW9VdGlsaXRpZXMuY3BwCkBAIC01 NiwzOCArNTYsMTAgQEAgZmxvYXQgZGlzY3JldGVUaW1lQ29uc3RhbnRGb3JTYW1wbGVSYXRlKGZs b2F0IHRpbWVDb25zdGFudCwgZmxvYXQgc2FtcGxlUmF0ZSkKICAgICByZXR1cm4gMSAtIHBvd2Yo MSAvIDIuNzE4MjgyZiwgMSAvIChzYW1wbGVSYXRlICogdGltZUNvbnN0YW50KSk7CiB9CiAKLSNp ZiBPUyhXSU5ET1dTKSAmJiBDT01QSUxFUihNU1ZDKSAmJiAhX01fSVg4Nl9GUAotLy8gV2hlbiBj b21waWxpbmcgd2l0aCBNU1ZDIHdpdGggeDg3IEZQVSBpbnN0cnVjdGlvbnMgdXNpbmcgODAtYml0 Ci0vLyBmbG9hdHMsIHdlIHdhbnQgdmVyeSBwcmVjaXNlIGNvbnRyb2wgb3ZlciB0aGUgYXJpdGht ZXRpYyBzbyB0aGF0Ci0vLyByb3VuZGluZyBpcyBkb25lIGFjY29yZGluZyB0byB0aGUgSUVFRSA3 NTQgc3BlY2lmaWNhdGlvbiBmb3IKLS8vIHNpbmdsZS0gYW5kIGRvdWJsZS1wcmVjaXNpb24gZmxv YXRzLiBXZSB3YW50IGVhY2ggb3BlcmF0aW9uIHRvIGJlCi0vLyBkb25lIHdpdGggc3BlY2lmaWVk IGFyaXRobWV0aWMgcHJlY2lzaW9uIGFuZCByb3VuZGluZyBjb25zaXN0ZW50Ci0vLyB3aXRoIGdj Yywgbm90IGV4dGVuZGVkIHRvIDgwIGJpdHMgYXV0b21hdGljYWxseS4KLS8vCi0vLyBUaGVzZSBw cmFnbWFzIGFyZSBlcXVpdmFsZW50IHRvIC9mcDpzdHJpY3QgZmxhZywgYnV0IHdlIG9ubHkgbmVl ZAotLy8gaXQgZm9yIHRoZSBmdW5jdGlvbiBoZXJlLiAgKFVzaW5nIGZwOnN0cmljdCBldmVyeXdo ZXJlIGNhbiBoYXZlCi0vLyBzZXZlcmUgaW1wYWN0IG9uIGZsb2F0aW5nIHBvaW50IHBlcmZvcm1h bmNlLikKLSNwcmFnbWEgZmxvYXRfY29udHJvbChwdXNoKQotI3ByYWdtYSBmbG9hdF9jb250cm9s KHByZWNpc2UsIG9uKQotI3ByYWdtYSBmZW52X2FjY2VzcyhvbikKLSNwcmFnbWEgZmxvYXRfY29u dHJvbChleGNlcHQsIG9uKQotI2VuZGlmCi0KIHNpemVfdCB0aW1lVG9TYW1wbGVGcmFtZShkb3Vi bGUgdGltZSwgZG91YmxlIHNhbXBsZVJhdGUpCiB7Ci0gICAgLy8gRE8gTk9UIENPTlNPTElEQVRF IFRIRVNFIEFTU0lHTk1FTlRTIElOVE8gT05FISBXaGVuIGNvbXBpbGluZwotICAgIC8vIHdpdGgg VmlzdWFsIFN0dWRpbywgdGhlc2UgYXNzaWdubWVudHMgZm9yY2UgdGhlIHJvdW5kaW5nIG9mCi0g ICAgLy8gZWFjaCBvcGVyYXRpb24gYWNjb3JkaW5nIHRvIElFRUUgNzU0LCBpbnN0ZWFkIG9mIGxl YXZpbmcKLSAgICAvLyBpbnRlcm1lZGlhdGUgcmVzdWx0cyBpbiA4MC1iaXQgcHJlY2lzaW9uIHdo aWNoIGlzIG5vdAotICAgIC8vIGNvbnNpc3RlbnQgd2l0aCBJRUVFIDc1NCBkb3VibGUtcHJlY2lz aW9uIHJvdW5kaW5nLgotICAgIGRvdWJsZSByID0gdGltZSAqIHNhbXBsZVJhdGU7Ci0gICAgciAr PSAwLjU7Ci0gICAgcmV0dXJuIHN0YXRpY19jYXN0PHNpemVfdD4ocik7CisgICAgcmV0dXJuIHN0 YXRpY19jYXN0PHNpemVfdD4ocm91bmQodGltZSAqIHNhbXBsZVJhdGUpKTsKIH0KLSNpZiBPUyhX SU5ET1dTKSAmJiBDT01QSUxFUihNU1ZDKSAmJiAhX01fSVg4Nl9GUAotLy8gUmVzdG9yZSBub3Jt YWwgZmxvYXRpbmctcG9pbnQgc2VtYW50aWNzLgotI3ByYWdtYSBmbG9hdF9jb250cm9sKHBvcCkK LSNlbmRpZgogfSAvLyBBdWRpb1V0aWxpdGVzCiAKIH0gLy8gV2ViQ29yZQo=