Bug 95792 - WebAudio: limit output level to 0db
Summary: WebAudio: limit output level to 0db
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac All
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2012-09-04 15:59 PDT by Jer Noble
Modified: 2012-10-16 08:51 PDT (History)
4 users (show)

See Also:

Attachments
Patch (6.76 KB, patch)
2012-09-04 16:19 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Patch (6.76 KB, patch)
2012-09-04 16:21 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Patch (6.87 KB, patch)
2012-09-05 10:01 PDT, Jer Noble 		no flags Details | Formatted Diff | Diff
Patch (7.43 KB, patch)
2012-10-15 13:39 PDT, Jer Noble 		crogers: review+
Details | Formatted Diff | Diff
Show Obsolete (3) 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 Jer Noble 2012-09-04 15:59:53 PDT 
WebAudio: limit output level to 0db
Comment 1 Jer Noble 2012-09-04 16:06:02 PDT 
<rdar://problem/11966135>
Comment 2 Jer Noble 2012-09-04 16:19:56 PDT 
Created attachment 162124 [details]
Patch
Comment 3 Jer Noble 2012-09-04 16:21:13 PDT 
Created attachment 162125 [details]
Patch

Set frequency of test to within the average human hearing range.
Comment 4 Alexey Proskuryakov 2012-09-05 09:54:31 PDT 
Comment on attachment 162125 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162125&action=review

> Source/WebCore/ChangeLog:4
> +        WebAudio: limit output level to 0db
> +        https://bugs.webkit.org/show_bug.cgi?id=95792

Could you please add Radar link here, too? It's always a good idea to have these readily accessible.
Comment 5 Jer Noble 2012-09-05 10:01:59 PDT 
Created attachment 162273 [details]
Patch

Sure thing.  Also added a couple of missing ASSERTS to AudioDestinationMac::configure()
Comment 6 Chris Rogers 2012-10-02 14:12:14 PDT 
Hi Jer, I highly recommend against using AUPeakLimiter in this case.  It's more of a mastering effect and will "touch" the audio signal in ways that would be undesirable.  It also has a processing latency.

Instead, I recommend looking at AudioDestinationMac::render() after this call:

    m_callback.render(0, &m_renderBus, numberOfFrames);

I would simply go through each AudioChannel of m_renderBus and "clamp" all the PCM data to be within the range -1 -> +1
Comment 7 Jer Noble 2012-10-15 13:39:19 PDT 
Created attachment 168771 [details]
Patch
Comment 8 Chris Rogers 2012-10-15 17:40:14 PDT 
Comment on attachment 168771 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168771&action=review

Looks fine to me with some small nits.

> Source/WebCore/ChangeLog:12
> +        >0db buffers and effects.

I'd update the ChangeLog comment here

> Source/WebCore/platform/audio/VectorMath.cpp:115
> +void vclip(const float * sourceP, int sourceStride, const float* lowThresholdP, const float* highThresholdP, float* destP, int destStride, size_t framesToProcess)

style nit: "float *" -> "float*"

> Source/WebCore/platform/audio/VectorMath.cpp:651
> +void vclip(const float * sourceP, int sourceStride, const float* lowThresholdP, const float* highThresholdP, float* destP, int destStride, size_t framesToProcess)

style nit: "float *" -> "float*"

> Source/WebCore/platform/audio/VectorMath.h:53
> +void vclip(const float * sourceP, int sourceStride, const float* lowThresholdP, const float* highThresholdP, float* destP, int destStride, size_t framesToProcess);

style nit: "float *" -> "float*"

> ManualTests/webaudio/limit-level-0db.html:36
> +    </p>

Maybe update comment - Strictly speaking, you'll hear the "clipping", thus the sine wave will be clipped into the form of a square wave, but it shouldn't get radically louder than the full-scale sine wave
Comment 9 Jer Noble 2012-10-16 08:51:34 PDT 
Committed r131459: <http://trac.webkit.org/changeset/131459>