[Webkit-unassigned] [Bug 46299] Add HRTFPanner files
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Sep 28 15:20:52 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=46299
--- Comment #9 from Chris Rogers <crogers at google.com> 2010-09-28 15:20:51 PST ---
(From update of attachment 69002)
View in context: https://bugs.webkit.org/attachment.cgi?id=69002&action=review
>> WebCore/platform/audio/HRTFPanner.cpp:47
>> +const double HRTFPanner::MaxDelayTime = 0.012; // 12ms
>
> why 12?
>
> add the unit to the name - looks like it's seconds here so perhaps MaxDelayTimeSeconds
FIXED: I actually measured the largest delay time in all the HRTFKernels and picked a good round value higher than this. I added a comment here.
>> WebCore/platform/audio/HRTFPanner.cpp:170
>> + // If we have a stereo input, implement a strange kind of panning with left source processed by left HRTF, and right source by right HRTF.
>
> Should it be an error at a higher level to connect a stereo input to this sort of node? That is an odd sounding pan.
No, I don't think so. Although it is more unusual than panning a mono source, it is expected to be able to pan a stereo source to stereo as well, and it results in a reasonable effect.
>> WebCore/platform/audio/HRTFPanner.h:43
>> + HRTFPanner(double sampleRate)
>
> one-arg constructor needs to be explicit
>
> could the constructor definition be moved to the .cpp? I'm a little concerned about inlining the c'tor for FFTConvolver and DelayDSPKernel without know how big they are
>
> please declare a virtual d'tor and define it in the .cpp
FIXED: I don't think the virtual destructor is necessary since it's empty and will be implicitly generated, but I added it anyway.
>> WebCore/platform/audio/HRTFPanner.h:71
>> + int m_azimuthIndex;
>
> What's de-zippering?
>
> Comments should be sentences
FIXED
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list