From 6217d8c5ca71bd0d0542922d69c76574f2ae62b1 Mon Sep 17 00:00:00 2001 From: James Chen Date: Mon, 27 Mar 2017 17:28:39 +0800 Subject: [PATCH] fixed #17591: [ios, mac, win32] Audio could not be played in the callback of AudioEngine::setFinishedCallback (#17593) * Moves the implementation of AudioEngine::AudioInfo to cpp file. * fixed #17591: [ios, mac, win32] Audio could not be played in the callback of AudioEngine::setFinishedCallback * fixed #17591: Adds test case. --- cocos/audio/AudioEngine.cpp | 15 +++++ cocos/audio/apple/AudioEngine-inl.mm | 19 +++++-- cocos/audio/include/AudioEngine.h | 14 ++--- cocos/audio/win32/AudioEngine-win32.cpp | 19 +++++-- .../NewAudioEngineTest/NewAudioEngineTest.cpp | 56 +++++++++++++++++++ .../NewAudioEngineTest/NewAudioEngineTest.h | 17 ++++++ 6 files changed, 121 insertions(+), 19 deletions(-) diff --git a/cocos/audio/AudioEngine.cpp b/cocos/audio/AudioEngine.cpp index 9f9b7c04d7..d23cc70a3e 100644 --- a/cocos/audio/AudioEngine.cpp +++ b/cocos/audio/AudioEngine.cpp @@ -67,6 +67,21 @@ AudioEngineImpl* AudioEngine::_audioEngineImpl = nullptr; AudioEngine::AudioEngineThreadPool* AudioEngine::s_threadPool = nullptr; +AudioEngine::AudioInfo::AudioInfo() +: filePath(nullptr) +, profileHelper(nullptr) +, volume(1.0f) +, loop(false) +, duration(TIME_UNKNOWN) +, state(AudioState::INITIALIZING) +{ + +} + +AudioEngine::AudioInfo::~AudioInfo() +{ +} + class AudioEngine::AudioEngineThreadPool { public: diff --git a/cocos/audio/apple/AudioEngine-inl.mm b/cocos/audio/apple/AudioEngine-inl.mm index 5e1e2bcdd1..0da35dda6a 100644 --- a/cocos/audio/apple/AudioEngine-inl.mm +++ b/cocos/audio/apple/AudioEngine-inl.mm @@ -588,37 +588,44 @@ void AudioEngineImpl::update(float dt) ALint sourceState; int audioID; AudioPlayer* player; + ALuint alSource; // ALOGV("AudioPlayer count: %d", (int)_audioPlayers.size()); for (auto it = _audioPlayers.begin(); it != _audioPlayers.end(); ) { audioID = it->first; player = it->second; - alGetSourcei(player->_alSource, AL_SOURCE_STATE, &sourceState); + alSource = player->_alSource; + alGetSourcei(alSource, AL_SOURCE_STATE, &sourceState); if (player->_removeByAudioEngine) { - _alSourceUsed[player->_alSource] = false; - AudioEngine::remove(audioID); _threadMutex.lock(); it = _audioPlayers.erase(it); _threadMutex.unlock(); delete player; + _alSourceUsed[alSource] = false; } else if (player->_ready && sourceState == AL_STOPPED) { - _alSourceUsed[player->_alSource] = false; + std::string filePath; if (player->_finishCallbak) { auto& audioInfo = AudioEngine::_audioIDInfoMap[audioID]; - player->_finishCallbak(audioID, *audioInfo.filePath); //FIXME: callback will delay 50ms + filePath = *audioInfo.filePath; } AudioEngine::remove(audioID); - delete player; _threadMutex.lock(); it = _audioPlayers.erase(it); _threadMutex.unlock(); + + if (player->_finishCallbak) { + player->_finishCallbak(audioID, filePath); //FIXME: callback will delay 50ms + } + + delete player; + _alSourceUsed[alSource] = false; } else{ ++it; diff --git a/cocos/audio/include/AudioEngine.h b/cocos/audio/include/AudioEngine.h index 87d0e64420..77399f3751 100644 --- a/cocos/audio/include/AudioEngine.h +++ b/cocos/audio/include/AudioEngine.h @@ -321,13 +321,13 @@ protected: float duration; AudioState state; - AudioInfo() - : profileHelper(nullptr) - , duration(TIME_UNKNOWN) - , state(AudioState::INITIALIZING) - { - - } + AudioInfo(); + ~AudioInfo(); + private: + AudioInfo(const AudioInfo& info); + AudioInfo(AudioInfo&& info); + AudioInfo& operator=(const AudioInfo& info); + AudioInfo& operator=(AudioInfo&& info); }; //audioID,audioAttribute diff --git a/cocos/audio/win32/AudioEngine-win32.cpp b/cocos/audio/win32/AudioEngine-win32.cpp index 7d4551bf87..313695eff5 100644 --- a/cocos/audio/win32/AudioEngine-win32.cpp +++ b/cocos/audio/win32/AudioEngine-win32.cpp @@ -444,37 +444,44 @@ void AudioEngineImpl::update(float dt) ALint sourceState; int audioID; AudioPlayer* player; + ALuint alSource; // ALOGV("AudioPlayer count: %d", (int)_audioPlayers.size()); for (auto it = _audioPlayers.begin(); it != _audioPlayers.end(); ) { audioID = it->first; player = it->second; - alGetSourcei(player->_alSource, AL_SOURCE_STATE, &sourceState); + alSource = player->_alSource; + alGetSourcei(alSource, AL_SOURCE_STATE, &sourceState); if (player->_removeByAudioEngine) { - _alSourceUsed[player->_alSource] = false; - AudioEngine::remove(audioID); _threadMutex.lock(); it = _audioPlayers.erase(it); _threadMutex.unlock(); delete player; + _alSourceUsed[alSource] = false; } else if (player->_ready && sourceState == AL_STOPPED) { - _alSourceUsed[player->_alSource] = false; + std::string filePath; if (player->_finishCallbak) { auto& audioInfo = AudioEngine::_audioIDInfoMap[audioID]; - player->_finishCallbak(audioID, *audioInfo.filePath); //FIXME: callback will delay 50ms + filePath = *audioInfo.filePath; } AudioEngine::remove(audioID); - delete player; + _threadMutex.lock(); it = _audioPlayers.erase(it); _threadMutex.unlock(); + + if (player->_finishCallbak) { + player->_finishCallbak(audioID, filePath); //FIXME: callback will delay 50ms + } + delete player; + _alSourceUsed[alSource] = false; } else{ ++it; diff --git a/tests/cpp-tests/Classes/NewAudioEngineTest/NewAudioEngineTest.cpp b/tests/cpp-tests/Classes/NewAudioEngineTest/NewAudioEngineTest.cpp index c0fc194fe8..2e0d557cb0 100644 --- a/tests/cpp-tests/Classes/NewAudioEngineTest/NewAudioEngineTest.cpp +++ b/tests/cpp-tests/Classes/NewAudioEngineTest/NewAudioEngineTest.cpp @@ -47,6 +47,7 @@ AudioEngineTests::AudioEngineTests() ADD_TEST_CASE(AudioPreloadSameFileMultipleTimes); ADD_TEST_CASE(AudioPlayFileInWritablePath); ADD_TEST_CASE(AudioIssue16938Test); + ADD_TEST_CASE(AudioPlayInFinishedCB); //FIXME: Please keep AudioSwitchStateTest to the last position since this test case doesn't work well on each platforms. ADD_TEST_CASE(AudioSwitchStateTest); @@ -985,3 +986,58 @@ std::string AudioPlayFileInWritablePath::subtitle() const { return "Could play audio"; } + +// +void AudioPlayInFinishedCB::onEnter() +{ + AudioEngineTestDemo::onEnter(); + + auto item = MenuItemFont::create("Play 3 files one by one", [this](Ref* sender){ + playMusic("background.mp3"); + playMusic("background.mp3"); + playMusic("background.mp3"); + }); + + item->setPosition(VisibleRect::center()); + + auto menu = Menu::create(item, nullptr); + menu->setPosition(Vec2::ANCHOR_BOTTOM_LEFT); + addChild(menu); +} + +void AudioPlayInFinishedCB::onExit() +{ + AudioEngineTestDemo::onExit(); +} + +std::string AudioPlayInFinishedCB::title() const +{ + return "Click menu item to play 3 audio files"; +} + +std::string AudioPlayInFinishedCB::subtitle() const +{ + return "After played over, click again, should also hear 3 audios"; +} + +void AudioPlayInFinishedCB::doPlay(const std::string& filename) +{ + int playID = AudioEngine::play2d(filename, false, 1); + AudioEngine::setFinishCallback(playID, [this](int finishID, const std::string& file){ + _playList.pop_front(); + log("finish music %s",file.c_str()); + if (!_playList.empty()) { + const std::string& name = _playList.front(); + doPlay(name); + } + }); +} + +void AudioPlayInFinishedCB::playMusic(const std::string& filename) +{ + _playList.push_back(filename); + if (_playList.size() == 1) { + doPlay(filename); + } +} + diff --git a/tests/cpp-tests/Classes/NewAudioEngineTest/NewAudioEngineTest.h b/tests/cpp-tests/Classes/NewAudioEngineTest/NewAudioEngineTest.h index 54ec8ec3a0..e63d2e2276 100644 --- a/tests/cpp-tests/Classes/NewAudioEngineTest/NewAudioEngineTest.h +++ b/tests/cpp-tests/Classes/NewAudioEngineTest/NewAudioEngineTest.h @@ -271,4 +271,21 @@ private: std::vector _oldSearchPaths; }; +class AudioPlayInFinishedCB : public AudioEngineTestDemo +{ +public: + CREATE_FUNC(AudioPlayInFinishedCB); + + virtual void onEnter() override; + virtual void onExit() override; + + virtual std::string title() const override; + virtual std::string subtitle() const override; + +private: + void doPlay(const std::string& filename); + void playMusic(const std::string& filename); + std::list _playList; +}; + #endif /* defined(__NEWAUDIOENGINE_TEST_H_) */