Skip to content

alBufferData instead of alBufferDataStatic for small audio file on Apple #19227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 10, 2018
Merged

alBufferData instead of alBufferDataStatic for small audio file on Apple #19227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
967b197
changes AudioCache to use alBufferData instead of alBufferDataStatic
cigumo Nov 24, 2018
f6e4f15
remove hack method "setTimeout" on audio
drelaptop Dec 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 30 additions & 61 deletions cocos/audio/apple/AudioCache.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,22 +51,6 @@ of this software and associated documentation files (the "Software"), to deal
#define INVALID_AL_BUFFER_ID 0xFFFFFFFF
#define PCMDATA_CACHEMAXSIZE 1048576

typedef ALvoid AL_APIENTRY (*alBufferDataStaticProcPtr) (const ALint bid, ALenum format, ALvoid* data, ALsizei size, ALsizei freq);
static ALvoid alBufferDataStaticProc(const ALint bid, ALenum format, ALvoid* data, ALsizei size, ALsizei freq)
{
static alBufferDataStaticProcPtr proc = nullptr;

if (proc == nullptr) {
proc = (alBufferDataStaticProcPtr) alcGetProcAddress(nullptr, (const ALCchar*) "alBufferDataStatic");
}

if (proc){
proc(bid, format, data, size, freq);
}

return;
}

@interface NSTimerWrapper : NSObject
{
std::function<void()> _timeoutCallback;
Expand Down Expand Up @@ -104,11 +88,6 @@ -(void) dealloc

@end

static void setTimeout(double seconds, const std::function<void()>& cb)
{
[[[NSTimerWrapper alloc] initWithTimeInterval:seconds callback:cb] autorelease];
}

using namespace cocos2d;
using namespace cocos2d::experimental;

Expand Down Expand Up @@ -150,35 +129,19 @@ static void setTimeout(double seconds, const std::function<void()>& cb)
}
//wait for the 'readDataTask' task to exit
_readDataTaskMutex.lock();
_readDataTaskMutex.unlock();

if (_pcmData)
if (_state == State::READY)
{
if (_state == State::READY)
{
if (_alBufferId != INVALID_AL_BUFFER_ID && alIsBuffer(_alBufferId))
{
ALOGV("~AudioCache(id=%u), delete buffer: %u", _id, _alBufferId);
alDeleteBuffers(1, &_alBufferId);
_alBufferId = INVALID_AL_BUFFER_ID;
}
}
else
if (_alBufferId != INVALID_AL_BUFFER_ID && alIsBuffer(_alBufferId))
{
ALOGW("AudioCache (%p), id=%u, buffer isn't ready, state=%d", this, _id, _state);
ALOGV("~AudioCache(id=%u), delete buffer: %u", _id, _alBufferId);
alDeleteBuffers(1, &_alBufferId);
_alBufferId = INVALID_AL_BUFFER_ID;
}

// fixed #17494: CrashIfClientProvidedBogusAudioBufferList
// We're using 'alBufferDataStaticProc' for speeding up
// the performance of playing audio without preload, but we need to manage the memory by ourself carefully.
// It's probably that '_pcmData' is freed before OpenAL finishes the audio render task,
// then 'CrashIfClientProvidedBogusAudioBufferList' may be triggered.
// 'cpp-tests/NewAudioEngineTest/AudioSwitchStateTest' can reproduce this issue without the following fix.
// The workaround is delaying 200ms to free pcm data.
char* data = _pcmData;
setTimeout(0.2, [data](){
free(data);
});
}
else
{
ALOGW("AudioCache (%p), id=%u, buffer isn't ready, state=%d", this, _id, _state);
}

if (_queBufferFrames > 0)
Expand All @@ -189,6 +152,7 @@ static void setTimeout(double seconds, const std::function<void()>& cb)
}
}
ALOGVV("~AudioCache() %p, id=%u, end", this, _id);
_readDataTaskMutex.unlock();
}

void AudioCache::readDataTask(unsigned int selfId)
Expand Down Expand Up @@ -261,35 +225,23 @@ static void setTimeout(double seconds, const std::function<void()>& cb)

_pcmData = (char*)malloc(dataSize);
memset(_pcmData, 0x00, dataSize);
ALOGV(" id=%u _pcmData alloc: %p", selfId, _pcmData);

if (adjustFrames > 0)
{
memcpy(_pcmData + (dataSize - adjustFrameBuf.size()), adjustFrameBuf.data(), adjustFrameBuf.size());
}

alGenBuffers(1, &_alBufferId);
auto alError = alGetError();
if (alError != AL_NO_ERROR) {
ALOGE("%s: attaching audio to buffer fail: %x", __PRETTY_FUNCTION__, alError);
break;
}

if (*_isDestroyed)
break;

alBufferDataStaticProc(_alBufferId, _format, _pcmData, (ALsizei)dataSize, (ALsizei)sampleRate);

framesRead = decoder.readFixedFrames(std::min(framesToReadOnce, remainingFrames), _pcmData + _framesRead * bytesPerFrame);
_framesRead += framesRead;
remainingFrames -= framesRead;

if (*_isDestroyed)
break;

_state = State::READY;

invokingPlayCallbacks();

uint32_t frames = 0;
while (!*_isDestroyed && _framesRead < originalTotalFrames)
{
Expand All @@ -309,9 +261,22 @@ static void setTimeout(double seconds, const std::function<void()>& cb)
{
memset(_pcmData + _framesRead * bytesPerFrame, 0x00, (totalFrames - _framesRead) * bytesPerFrame);
}
ALOGV("pcm buffer was loaded successfully, total frames: %u, total read frames: %u, adjust frames: %u, remainingFrames: %u", totalFrames, _framesRead, adjustFrames, remainingFrames);

ALOGV("pcm buffer was loaded successfully, total frames: %u, total read frames: %u, adjust frames: %u, remainingFrames: %u", totalFrames, _framesRead, adjustFrames, remainingFrames);
_framesRead += adjustFrames;

alGenBuffers(1, &_alBufferId);
auto alError = alGetError();
if (alError != AL_NO_ERROR) {
ALOGE("%s: attaching audio to buffer fail: %x", __PRETTY_FUNCTION__, alError);
break;
}
ALOGV(" id=%u generated alGenBuffers: %u for _pcmData: %p", selfId, _alBufferId, _pcmData);
ALOGV(" id=%u _pcmData alBufferData: %p", selfId, _pcmData);
alBufferData(_alBufferId, _format, _pcmData, (ALsizei)dataSize, (ALsizei)sampleRate);
_state = State::READY;
invokingPlayCallbacks();

}
else
{
Expand All @@ -333,6 +298,10 @@ static void setTimeout(double seconds, const std::function<void()>& cb)

} while (false);

if (_pcmData != nullptr){
CC_SAFE_FREE(_pcmData);
}

decoder.close();

//FIXME: Why to invoke play callback first? Should it be after 'load' callback?
Expand All @@ -345,7 +314,7 @@ static void setTimeout(double seconds, const std::function<void()>& cb)
_state = State::FAILED;
if (_alBufferId != INVALID_AL_BUFFER_ID && alIsBuffer(_alBufferId))
{
ALOGV("readDataTask failed, delete buffer: %u", _alBufferId);
ALOGV(" id=%u readDataTask failed, delete buffer: %u", selfId, _alBufferId);
alDeleteBuffers(1, &_alBufferId);
_alBufferId = INVALID_AL_BUFFER_ID;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/cpp-tests/Classes/NewAudioEngineTest/NewAudioEngineTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ bool AudioSwitchStateTest::init()
AudioEngine::play2d("audio/SoundEffectsFX009/FX082.mp3");
AudioEngine::play2d("audio/LuckyDay.mp3");

}, 0.1f, "AudioSwitchStateTest");
}, 0.01f, "AudioSwitchStateTest");

return true;
}
Expand Down