[Webkit-unassigned] [Bug 34912] audio engine: add ReverbConvolver class
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Mar 23 10:41:37 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=34912
Jeremy Orlow <jorlow at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #51358|review? |review-
Flag| |
--- Comment #22 from Jeremy Orlow <jorlow at chromium.org> 2010-03-23 10:41:37 PST ---
(From update of attachment 51358)
Getting really close on RevertAccumulationBuffer and ReverbInputBuffer. You
might want to split them into their own patch to make reviewing easier. I'll
look at the other half soon.
> --- /dev/null
> +++ b/WebCore/platform/audio/ReverbAccumulationBuffer.cpp
> @@ -0,0 +1,108 @@
> +/*
> + * Copyright (C) 2010 Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> + * its contributors may be used to endorse or promote products derived
> + * from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "config.h"
> +#include "ReverbAccumulationBuffer.h"
> +
> +#include "Accelerate.h"
> +
> +namespace WebCore {
> +
> +ReverbAccumulationBuffer::ReverbAccumulationBuffer(size_t length)
> + : m_buffer(length)
> + , m_readIndex(0)
> + , m_readTimeFrame(0)
> +{
> +}
> +
> +void ReverbAccumulationBuffer::readAndClear(float* destination, size_t numberOfFrames)
Thanks for making these variable names better...but I think the same can be
said for a lot of the variable names in these files. Can you please do a pass?
Spell things out and err on the side of variables being too long to make thing
readable.
> --- /dev/null
> +++ b/WebCore/platform/audio/ReverbConvolverStage.h
> @@ -0,0 +1,84 @@
> +/*
> + * Copyright (C) 2010 Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> + * its contributors may be used to endorse or promote products derived
> + * from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef ReverbConvolverStage_h
> +#define ReverbConvolverStage_h
> +
> +#include "AudioFloatArray.h"
> +#include "FFTFrame.h"
You should include wtf/OwnPtr since it's used below.
> --- /dev/null
> +++ b/WebCore/platform/audio/ReverbInputBuffer.cpp
> @@ -0,0 +1,77 @@
> +/*
> + * Copyright (C) 2010 Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> + * its contributors may be used to endorse or promote products derived
> + * from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "config.h"
> +#include "ReverbInputBuffer.h"
> +
> +namespace WebCore {
> +
> +ReverbInputBuffer::ReverbInputBuffer(size_t length)
> + : m_buffer(length)
> + , m_writeIndex(0)
> +{
> +}
> +
> +void ReverbInputBuffer::write(float* sourceP, size_t numberOfFrames)
> +{
> + size_t bufferLength = m_buffer.size();
> + bool isCopyGood = m_writeIndex + numberOfFrames <= bufferLength;
> + ASSERT(isCopyGood);
> + if (!isCopyGood)
isCopySafe seems like a better name
> + return;
> +
> + memcpy(m_buffer.data() + m_writeIndex, sourceP, sizeof(float) * numberOfFrames);
> +
> + m_writeIndex += numberOfFrames;
> + ASSERT(m_writeIndex <= bufferLength);
> +
> + if (m_writeIndex >= bufferLength)
> + m_writeIndex = 0;
> +}
> +
> +float* ReverbInputBuffer::directReadFrom(int* index, size_t numberOfFrames)
> +{
> + size_t bufferLength = m_buffer.size();
> + bool isPtrGood = *index >= 0 && *index + numberOfFrames <= bufferLength;
> + ASSERT(isPtrGood);
> + float* sourceP = m_buffer;
> + float* p = isPtrGood ? sourceP + *index : 0; // pointer should always be good - force read from 0 to expose bug otherwise
Usually you spell out words like Pointer instead of using Ptr.
If your intention is to crash the program, using CRASH() is better.
--
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