Bug 78854 - DelayNode has inaccurate delay
Summary: DelayNode has inaccurate delay
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 76919
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-16 15:56 PST by Raymond Toy
Modified: 2012-02-23 16:26 PST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Raymond Toy 2012-02-16 15:56:36 PST
In the delaynode.html layout test, if you change delayTimeSeconds (in resources/delay-testingjs), to 0.53, the test fails.

This is because the javascript code expects the delayed signal to start at frame 23373 (= 0.53 * 44100) but the rendered data actually starts at 23372.  I think this is due to roundoff.  The delay time is an AudioParam, which takes float values.  0.53f * 44100 = 23372.998.  Perhaps rounding the starting time to the nearest sample will fix this issue.

(It should probably be a rule in the tests not to use nice "round" numbers for timing values.)
Comment 1 Chris Rogers 2012-02-16 17:48:33 PST
Yes, looks like DelayDSPKernel could use your AudioUtilities function
Comment 2 Raymond Toy 2012-02-16 20:05:29 PST
(In reply to comment #1)
> Yes, looks like DelayDSPKernel could use your AudioUtilities function

That will help, but I will need to take a closer look to see if that is enough.
Comment 3 Raymond Toy 2012-02-17 09:54:08 PST
(In reply to comment #2)
> (In reply to comment #1)
> > Yes, looks like DelayDSPKernel could use your AudioUtilities function
> 
> That will help, but I will need to take a closer look to see if that is enough.

Ok.  Applying timeToSampleFrame to

  double desiredDelayFrames = m_currentDelayTime * sampleRate;

helps with the case of a delay of 0.53 sec.  But consider the case where the delay is 0.005.  This is a delay of 220.5 samples.  Then desiredDelayFrames (after rounding) will be 221.  However, because delayTime comes from an AudioParam that only holds floats, the delayTime is actually 0.004999999888241291 (because 0.005 is converted to a single float which is the value given).  Then the delayTime * sampleRate = 220.49999507144094 which rounds to 220.  Thus, we're off by one.

Maybe the correct solution is to round as single floats instead of doubles?  If we do that delayTime*sampleRate = 220.5 which rounds to 221, as expected.  Would need to investigate if this would cause other problems.

Another solution is to modify AudioParam to use doubles instead of floats.  This might be generally useful if the AudioParam is meant to hold time values as it does for a DelayNode.  This will make webaudio more closely match javascript code.  This would be a fairly invasive change.

Yet another solution is to modify the javascript test code to compute time values in single-precision (by taking a representation (a string or ratio) of the time value and manually converting to a single-precision value that is stored in a double).
Comment 4 Raymond Toy 2012-02-23 16:26:52 PST
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > Yes, looks like DelayDSPKernel could use your AudioUtilities function
> > 
> > That will help, but I will need to take a closer look to see if that is enough.
> 
> Ok.  Applying timeToSampleFrame to
> 
>   double desiredDelayFrames = m_currentDelayTime * sampleRate;
> 
> helps with the case of a delay of 0.53 sec.  But consider the case where the delay is 0.005.  This is a delay of 220.5 samples.  Then desiredDelayFrames (after rounding) will be 221.  However, because delayTime comes from an AudioParam that only holds floats, the delayTime is actually 0.004999999888241291 (because 0.005 is converted to a single float which is the value given).  Then the delayTime * sampleRate = 220.49999507144094 which rounds to 220.  Thus, we're off by one.

The analysis here is not quite right.  AudioParam holds doubles.  But the IDL specifies floats, so the 0.005 (double) from Javascript gets converted to a float which is converted back to a (different) double and stored in AudioParam.

This issue will probably get fixed when bug 76919 is fixed.