From 6ab31219edf4a65610a9bf2ea46420c0fac453ec Mon Sep 17 00:00:00 2001 From: John Preston Date: Sun, 1 Nov 2020 18:24:21 +0300 Subject: [PATCH] Workaround crash in OpenAL library. Fixes #8887. See https://github.com/kcat/openal-soft/issues/486 --- .../SourceFiles/media/audio/media_audio.cpp | 71 +++++++++++++++++-- .../SourceFiles/media/audio/media_audio.h | 50 +++++++++---- 2 files changed, 102 insertions(+), 19 deletions(-) diff --git a/Telegram/SourceFiles/media/audio/media_audio.cpp b/Telegram/SourceFiles/media/audio/media_audio.cpp index 02dcffd79f..755a25c848 100644 --- a/Telegram/SourceFiles/media/audio/media_audio.cpp +++ b/Telegram/SourceFiles/media/audio/media_audio.cpp @@ -34,6 +34,7 @@ namespace { constexpr auto kSuppressRatioAll = 0.2; constexpr auto kSuppressRatioSong = 0.05; constexpr auto kWaveformCounterBufferSize = 256 * 1024; +constexpr auto kEffectDestructionDelay = crl::time(1000); QMutex AudioMutex; ALCdevice *AudioDevice = nullptr; @@ -179,7 +180,7 @@ void ClosePlaybackDevice(not_null instance) { LOG(("Audio Info: Closing audio playback device.")); if (Player::mixer()) { - Player::mixer()->detachTracks(); + Player::mixer()->prepareToCloseDevice(); } instance->detachTracks(); @@ -383,9 +384,11 @@ void Mixer::Track::resetSpeedEffect() { if (isStreamCreated()) { removeSourceSpeedEffect(); } - OpenAL::alDeleteEffects(1, &speedEffect->effect); - OpenAL::alDeleteAuxiliaryEffectSlots(1, &speedEffect->effectSlot); - OpenAL::alDeleteFilters(1, &speedEffect->filter); + if (Player::mixer()) { + // Don't destroy effect slot immediately. + // See https://github.com/kcat/openal-soft/issues/486 + Player::mixer()->scheduleEffectDestruction(*speedEffect); + } } speedEffect->effect = speedEffect->effectSlot = speedEffect->filter = 0; } @@ -560,6 +563,7 @@ Mixer::Track::~Track() = default; Mixer::Mixer(not_null instance) : _instance(instance) +, _effectsDestructionTimer([=] { destroyStaleEffectsSafe(); }) , _volumeVideo(kVolumeRound) , _volumeSong(kVolumeRound) , _fader(new Fader(&_faderThread)) @@ -622,6 +626,60 @@ void Mixer::onUpdated(const AudioMsgId &audio) { Media::Player::Updated().notify(audio); } +// Thread: Any. Must be locked: AudioMutex. +void Mixer::scheduleEffectDestruction(const SpeedEffect &effect) { + _effectsForDestruction.emplace_back( + crl::now() + kEffectDestructionDelay, + effect); + scheduleEffectsDestruction(); +} + +// Thread: Any. Must be locked: AudioMutex. +void Mixer::scheduleEffectsDestruction() { + if (_effectsForDestruction.empty()) { + return; + } + InvokeQueued(this, [=] { + if (!_effectsDestructionTimer.isActive()) { + _effectsDestructionTimer.callOnce(kEffectDestructionDelay + 1); + } + }); +} + +// Thread: Main. Locks: AudioMutex. +void Mixer::destroyStaleEffectsSafe() { + QMutexLocker lock(&AudioMutex); + destroyStaleEffects(); +} + +// Thread: Main. Must be locked: AudioMutex. +void Mixer::destroyStaleEffects() { + const auto now = crl::now(); + const auto checkAndDestroy = [&]( + const std::pair &pair) { + const auto &[when, effect] = pair; + if (when && when > now) { + return false; + } + OpenAL::alDeleteEffects(1, &effect.effect); + OpenAL::alDeleteAuxiliaryEffectSlots(1, &effect.effectSlot); + OpenAL::alDeleteFilters(1, &effect.filter); + return true; + }; + _effectsForDestruction.erase( + ranges::remove_if(_effectsForDestruction, checkAndDestroy), + end(_effectsForDestruction)); + scheduleEffectsDestruction(); +} + +// Thread: Main. Must be locked: AudioMutex. +void Mixer::destroyEffectsOnClose() { + for (auto &[when, effect] : _effectsForDestruction) { + when = 0; + } + destroyStaleEffects(); +} + void Mixer::onError(const AudioMsgId &audio) { emit stoppedOnError(audio); @@ -823,6 +881,7 @@ void Mixer::forceToBufferExternal(const AudioMsgId &audioId) { _loader->forceToBufferExternal(audioId); } +// Thread: Main. Locks: AudioMutex. void Mixer::setSpeedFromExternal(const AudioMsgId &audioId, float64 speed) { QMutexLocker lock(&AudioMutex); const auto track = trackForType(audioId.type()); @@ -1160,12 +1219,14 @@ void Mixer::setStoppedState(Track *current, State state) { } // Thread: Main. Must be locked: AudioMutex. -void Mixer::detachTracks() { +void Mixer::prepareToCloseDevice() { for (auto i = 0; i != kTogetherLimit; ++i) { trackForType(AudioMsgId::Type::Voice, i)->detach(); trackForType(AudioMsgId::Type::Song, i)->detach(); } _videoTrack.detach(); + + destroyEffectsOnClose(); } // Thread: Main. Must be locked: AudioMutex. diff --git a/Telegram/SourceFiles/media/audio/media_audio.h b/Telegram/SourceFiles/media/audio/media_audio.h index 890570cdb6..090002ab3b 100644 --- a/Telegram/SourceFiles/media/audio/media_audio.h +++ b/Telegram/SourceFiles/media/audio/media_audio.h @@ -11,6 +11,7 @@ https://github.com/telegramdesktop/tdesktop/blob/master/LEGAL #include "ui/chat/attach/attach_prepare.h" #include "core/file_location.h" #include "base/bytes.h" +#include "base/timer.h" #include @@ -145,7 +146,10 @@ public: // External player audio stream interface. void feedFromExternal(ExternalSoundPart &&part); void forceToBufferExternal(const AudioMsgId &audioId); + + // Thread: Main. Locks: AudioMutex. void setSpeedFromExternal(const AudioMsgId &audioId, float64 speed); + Streaming::TimePoint getExternalSyncTimePoint( const AudioMsgId &audio) const; crl::time getExternalCorrectedTime( @@ -158,7 +162,7 @@ public: TrackState currentState(AudioMsgId::Type type); // Thread: Main. Must be locked: AudioMutex. - void detachTracks(); + void prepareToCloseDevice(); // Thread: Main. Must be locked: AudioMutex. void reattachIfNeeded(); @@ -193,11 +197,13 @@ signals: void suppressAll(qint64 duration); private: - bool fadedStop(AudioMsgId::Type type, bool *fadedStart = 0); - void resetFadeStartPosition(AudioMsgId::Type type, int positionInBuffered = -1); - bool checkCurrentALError(AudioMsgId::Type type); - - void externalSoundProgress(const AudioMsgId &audio); + struct SpeedEffect { + uint32 effect = 0; + uint32 effectSlot = 0; + uint32 filter = 0; + int coarseTune = 0; + float64 speed = 1.; + }; class Track { public: @@ -206,8 +212,10 @@ private: // Thread: Any. Must be locked: AudioMutex. void reattach(AudioMsgId::Type type); + // Thread: Main. Must be locked: AudioMutex. void detach(); void clear(); + void started(); bool isStreamCreated() const; @@ -215,6 +223,7 @@ private: int getNotQueuedBufferIndex(); + // Thread: Main. Must be locked: AudioMutex. void setExternalData(std::unique_ptr data); void changeSpeedEffect(float64 speed); @@ -242,13 +251,6 @@ private: Stream stream; std::unique_ptr externalData; - struct SpeedEffect { - uint32 effect = 0; - uint32 effectSlot = 0; - uint32 filter = 0; - int coarseTune = 0; - float64 speed = 1.; - }; std::unique_ptr speedEffect; crl::time lastUpdateWhen = 0; crl::time lastUpdatePosition = 0; @@ -263,6 +265,12 @@ private: }; + bool fadedStop(AudioMsgId::Type type, bool *fadedStart = 0); + void resetFadeStartPosition(AudioMsgId::Type type, int positionInBuffered = -1); + bool checkCurrentALError(AudioMsgId::Type type); + + void externalSoundProgress(const AudioMsgId &audio); + // Thread: Any. Must be locked: AudioMutex. void setStoppedState(Track *current, State state = State::Stopped); @@ -271,7 +279,18 @@ private: int *currentIndex(AudioMsgId::Type type); const int *currentIndex(AudioMsgId::Type type) const; - not_null _instance; + // Thread: Any. Must be locked: AudioMutex. + void scheduleEffectDestruction(const SpeedEffect &effect); + void scheduleEffectsDestruction(); + + // Thread: Main. Must be locked: AudioMutex. + void destroyStaleEffects(); + void destroyEffectsOnClose(); + + // Thread: Main. Locks: AudioMutex. + void destroyStaleEffectsSafe(); + + const not_null _instance; int _audioCurrent = 0; Track _audioTracks[kTogetherLimit]; @@ -281,6 +300,9 @@ private: Track _videoTrack; + std::vector> _effectsForDestruction; + base::Timer _effectsDestructionTimer; + QAtomicInt _volumeVideo; QAtomicInt _volumeSong;