Replaced some mutex locks with atomic variables.

Also removed an SDL audio issue workaround from AudioManager.

es-core/src/AudioManager.cpp

@@ -194,9 +194,7 @@ void AudioManager::mixAudio(void* /*unused*/, Uint8* stream, int len)
     // stream is not played when the video player has been stopped. Otherwise there would
     // be a short time period when the audio would keep playing after the video was stopped
     // and before the stream was cleared in clearStream().
-    std::unique_lock<std::mutex> audioLock{mAudioLock};
     bool muteStream = sMuteStream;
-    audioLock.unlock();
     if (muteStream) {
         SDL_MixAudioFormat(stream, &converted.at(0), sAudioFormat.format, processedLength, 0);
     }
@@ -290,41 +288,4 @@ void AudioManager::clearStream()
     SDL_LockAudioDevice(sAudioDevice);
     SDL_AudioStreamClear(sConversionStream);
     SDL_UnlockAudioDevice(sAudioDevice);
-
-    // TEMPORARY: For evaluating if SDL_AudioStreamClear is stable.
-    return;
-
-    // If sSoundVector is empty it means we are shutting down. In this case don't attempt
-    // to clear the stream as this could lead to a crash.
-    if (sSoundVector.empty())
-        return;
-
-    SDL_LockAudioDevice(sAudioDevice);
-
-    // This code is required as there's seemingly a bug in SDL_AudioStreamAvailable().
-    // The function sometimes returns 0 even if there is data left in the buffer, possibly
-    // because the remaining data is less than the configured sample size. It happens almost
-    // permanently on NetBSD but also on at least Linux from time to time. Adding some data
-    // to the stream buffer to get above this threshold before calling the function will
-    // return the proper number. So adding 10000 as we do here would give a return value of
-    // for instance 10880 instead of 0, assuming there were 880 bytes of data left in the buffer.
-    // Fortunately the SDL_AudioStreamGet() function acts correctly on any arbitrary sample size
-    // so we can actually clear the entire buffer. If this workaround was not implemented, there
-    // would be a sound glitch when some samples from the previous video would play any time a
-    // new video was started (assuming the issue was triggered by some remaining stream data).
-    std::vector<Uint8> writeBuffer(10000);
-    if (SDL_AudioStreamPut(sConversionStream, reinterpret_cast<const void*>(&writeBuffer.at(0)),
-                           10000) == -1) {
-        LOG(LogError) << "Failed to put samples in the conversion stream:";
-        LOG(LogError) << SDL_GetError();
-        SDL_UnlockAudioDevice(sAudioDevice);
-        return;
-    }
-
-    int length = SDL_AudioStreamAvailable(sConversionStream);
-
-    std::vector<Uint8> readBuffer(length);
-    SDL_AudioStreamGet(sConversionStream, static_cast<void*>(&readBuffer.at(0)), length);
-
-    SDL_UnlockAudioDevice(sAudioDevice);
 }

es-core/src/AudioManager.h

@@ -10,8 +10,8 @@
 #define ES_CORE_AUDIO_MANAGER_H
 
 #include <SDL2/SDL_audio.h>
+#include <atomic>
 #include <memory>
-#include <mutex>
 #include <vector>
 
 class Sound;
@@ -36,16 +36,8 @@ public:
     void processStream(const void* samples, unsigned count);
     void clearStream();
 
-    void muteStream()
-    {
-        std::unique_lock<std::mutex> audioLock{mAudioLock};
-        sMuteStream = true;
-    }
-    void unmuteStream()
-    {
-        std::unique_lock<std::mutex> audioLock{mAudioLock};
-        sMuteStream = false;
-    }
+    void muteStream() { sMuteStream = true; }
+    void unmuteStream() { sMuteStream = false; }
 
     bool getHasAudioDevice() { return sHasAudioDevice; }
 
@@ -58,10 +50,9 @@ private:
     static void mixAudio(void* unused, Uint8* stream, int len);
     static void mixAudio2(void* unused, Uint8* stream, int len);
 
-    inline static std::mutex mAudioLock;
     inline static SDL_AudioStream* sConversionStream;
     inline static std::vector<std::shared_ptr<Sound>> sSoundVector;
-    inline static bool sMuteStream = false;
+    inline static std::atomic<bool> sMuteStream = false;
     inline static bool sHasAudioDevice = true;
 };
 

es-core/src/Sound.cpp

@@ -151,8 +151,6 @@ void Sound::play()
 
     SDL_LockAudioDevice(AudioManager::sAudioDevice);
 
-    std::unique_lock<std::mutex> playerLock(mMutex);
-
     if (mPlaying)
         // Replay from start. rewind the sample to the beginning.
         mSamplePos = 0;
@@ -160,8 +158,6 @@ void Sound::play()
         // Flag our sample as playing.
         mPlaying = true;
 
-    playerLock.unlock();
-
     SDL_UnlockAudioDevice(AudioManager::sAudioDevice);
     // Tell the AudioManager to start playing samples.
     AudioManager::getInstance().play();
@@ -171,7 +167,6 @@ void Sound::stop()
 {
     // Flag our sample as not playing and rewind its position.
     SDL_LockAudioDevice(AudioManager::sAudioDevice);
-    std::unique_lock<std::mutex> playerLock(mMutex);
     mPlaying = false;
     mSamplePos = 0;
     SDL_UnlockAudioDevice(AudioManager::sAudioDevice);
@@ -182,7 +177,6 @@ void Sound::setPosition(Uint32 newPosition)
     mSamplePos = newPosition;
     if (mSamplePos >= mSampleLength) {
         // Got to or beyond the end of the sample. stop playing.
-        std::unique_lock<std::mutex> playerLock(mMutex);
         mPlaying = false;
         mSamplePos = 0;
     }

es-core/src/Sound.h

@@ -11,9 +11,9 @@
 #define ES_CORE_SOUND_H
 
 #include <SDL2/SDL_audio.h>
+#include <atomic>
 #include <map>
 #include <memory>
-#include <mutex>
 #include <sstream>
 #include <string>
 #include <vector>
@@ -31,11 +31,7 @@ public:
     void loadFile(const std::string& path);
 
     void play();
-    bool isPlaying() const
-    {
-        std::unique_lock<std::mutex> playerLock(mMutex);
-        return mPlaying;
-    }
+    bool isPlaying() const { return mPlaying; }
     void stop();
 
     const Uint8* getData() const { return mSampleData; }
@@ -51,14 +47,13 @@ public:
 private:
     Sound(const std::string& path = "");
 
-    inline static std::mutex mMutex;
     static std::map<std::string, std::shared_ptr<Sound>> sMap;
     std::string mPath;
     SDL_AudioSpec mSampleFormat;
     Uint8* mSampleData;
     Uint32 mSamplePos;
     Uint32 mSampleLength;
-    bool mPlaying;
+    std::atomic<bool> mPlaying;
 };
 
 enum NavigationSoundsID {

es-core/src/Window.cpp

@@ -799,26 +799,14 @@ void Window::closeLaunchScreen()
     mRenderLaunchScreen = false;
 }
 
-void Window::increaseVideoPlayerCount()
-{
-    mVideoCountMutex.lock();
-    mVideoPlayerCount++;
-    mVideoCountMutex.unlock();
-}
+void Window::increaseVideoPlayerCount() { mVideoPlayerCount++; }
 
-void Window::decreaseVideoPlayerCount()
-{
-    mVideoCountMutex.lock();
-    mVideoPlayerCount--;
-    mVideoCountMutex.unlock();
-}
+void Window::decreaseVideoPlayerCount() { mVideoPlayerCount--; }
 
 int Window::getVideoPlayerCount()
 {
     int videoPlayerCount;
-    mVideoCountMutex.lock();
     videoPlayerCount = mVideoPlayerCount;
-    mVideoCountMutex.unlock();
     return videoPlayerCount;
 }
 

es-core/src/Window.h

@@ -16,8 +16,8 @@
 #include "Settings.h"
 #include "resources/TextureResource.h"
 
+#include <atomic>
 #include <memory>
-#include <mutex>
 #include <queue>
 
 class FileData;
@@ -192,8 +192,7 @@ private:
     bool mCachedBackground;
     bool mInvalidatedCachedBackground;
 
-    int mVideoPlayerCount;
-    std::mutex mVideoCountMutex;
+    std::atomic<int> mVideoPlayerCount;
 
     float mTopScale;
     bool mRenderedHelpPrompts;

es-core/src/components/VideoComponent.cpp

@@ -90,12 +90,9 @@ void VideoComponent::setImage(std::string path)
 
 void VideoComponent::onShow()
 {
-    std::unique_lock<std::mutex> playerLock(mPlayerMutex);
     mBlockPlayer = false;
     mPause = false;
     mShowing = true;
-    playerLock.unlock();
-
     manageState();
 }
 
@@ -113,30 +110,22 @@ void VideoComponent::onStopVideo()
 
 void VideoComponent::onPauseVideo()
 {
-    std::unique_lock<std::mutex> playerLock(mPlayerMutex);
     mBlockPlayer = true;
     mPause = true;
-    playerLock.unlock();
-
     manageState();
 }
 
 void VideoComponent::onUnpauseVideo()
 {
-    std::unique_lock<std::mutex> playerLock(mPlayerMutex);
     mBlockPlayer = false;
     mPause = false;
-    playerLock.unlock();
-
     manageState();
 }
 
 void VideoComponent::onScreensaverActivate()
 {
-    std::unique_lock<std::mutex> playerLock(mPlayerMutex);
     mBlockPlayer = true;
     mPause = true;
-    playerLock.unlock();
 
     if (Settings::getInstance()->getString("ScreensaverType") == "dim")
         stopVideo();
@@ -170,20 +159,16 @@ void VideoComponent::onGameLaunchedDeactivate()
 void VideoComponent::topWindow(bool isTop)
 {
     if (isTop) {
-        std::unique_lock<std::mutex> playerLock(mPlayerMutex);
         mBlockPlayer = false;
         mPause = false;
-        playerLock.unlock();
 
         // Stop video when closing the menu to force a reload of the
         // static image (if the theme is configured as such).
         stopVideo();
     }
     else {
-        std::unique_lock<std::mutex> playerLock(mPlayerMutex);
         mBlockPlayer = true;
         mPause = true;
-        playerLock.unlock();
     }
     manageState();
 }

es-core/src/components/VideoComponent.h

@@ -12,7 +12,7 @@
 #include "GuiComponent.h"
 #include "components/ImageComponent.h"
 
-#include <mutex>
+#include <atomic>
 #include <string>
 
 class MediaViewer;
@@ -110,7 +110,6 @@ private:
 protected:
     Window* mWindow;
     ImageComponent mStaticImage;
-    std::mutex mPlayerMutex;
 
     unsigned mVideoWidth;
     unsigned mVideoHeight;
@@ -124,9 +123,9 @@ protected:
     std::string mPlayingVideoPath;
     unsigned mStartTime;
     bool mStartDelayed;
-    bool mIsPlaying;
-    bool mIsActuallyPlaying;
-    bool mPause;
+    std::atomic<bool> mIsPlaying;
+    std::atomic<bool> mIsActuallyPlaying;
+    std::atomic<bool> mPause;
     bool mShowing;
     bool mDisable;
     bool mMediaViewerMode;

es-core/src/components/VideoFFmpegComponent.cpp

@@ -130,10 +130,8 @@ void VideoFFmpegComponent::render(const glm::mat4& parentTrans)
 
     if (mIsPlaying && mFormatContext) {
         unsigned int color;
-        std::unique_lock<std::mutex> pictureLock(mPictureMutex);
-        bool decodedFrame = mDecodedFrame;
-        pictureLock.unlock();
-        if (decodedFrame && mFadeIn < 1) {
+
+        if (mDecodedFrame && mFadeIn < 1) {
             const unsigned int fadeIn = static_cast<int>(mFadeIn * 255.0f);
             color =
                 Renderer::convertRGBAToABGR((fadeIn << 24) | (fadeIn << 16) | (fadeIn << 8) | 255);
@@ -162,13 +160,11 @@ void VideoFFmpegComponent::render(const glm::mat4& parentTrans)
         for (int i = 0; i < 4; i++)
             vertices[i].pos = glm::round(vertices[i].pos);
 
-        pictureLock.lock();
-
         // This is needed to avoid a slight gap before the video starts playing.
-        if (!mDecodedFrame) {
-            pictureLock.unlock();
+        if (!mDecodedFrame)
             return;
-        }
+
+        std::unique_lock<std::mutex> pictureLock(mPictureMutex);
 
         if (!mOutputPicture.hasBeenRendered) {
             // Move the contents of mOutputPicture to a temporary vector in order to call
@@ -238,7 +234,8 @@ void VideoFFmpegComponent::updatePlayer()
     }
 
     if (mIsActuallyPlaying && mStartTimeAccumulation) {
-        mAccumulatedTime +=
+        mAccumulatedTime =
+            mAccumulatedTime +
             static_cast<double>(std::chrono::duration_cast<std::chrono::nanoseconds>(
                                     std::chrono::high_resolution_clock::now() - mTimeReference)
                                     .count()) /
@@ -268,18 +265,16 @@ void VideoFFmpegComponent::frameProcessing()
     if (mAudioCodecContext)
         audioFilter = setupAudioFilters();
 
-    bool isPlaying = true;
-    bool pause = false;
-
-    while (isPlaying && !pause && videoFilter && (!mAudioCodecContext || audioFilter)) {
+    while (mIsPlaying && !mPause && videoFilter && (!mAudioCodecContext || audioFilter)) {
         readFrames();
-        getProcessedFrames();
-        outputFrames();
+        if (!mIsPlaying)
+            break;
 
-        std::unique_lock<std::mutex> playerLock(mPlayerMutex);
-        isPlaying = mIsPlaying;
-        pause = mPause;
-        playerLock.unlock();
+        getProcessedFrames();
+        if (!mIsPlaying)
+            break;
+
+        outputFrames();
 
         // This 1 ms wait makes sure that the thread does not consume all available CPU cycles.
         SDL_Delay(1);
@@ -672,11 +667,9 @@ void VideoFFmpegComponent::readFrames()
         }
     }
 
-    if (readFrameReturn < 0) {
-        std::unique_lock<std::mutex> playerLock(mPlayerMutex);
+    if (readFrameReturn < 0)
         mEndOfVideo = true;
 }
-}
 
 void VideoFFmpegComponent::getProcessedFrames()
 {
@@ -761,11 +754,7 @@ void VideoFFmpegComponent::outputFrames()
     // Process the audio frames that have a PTS value below mAccumulatedTime (plus a small
     // buffer to avoid underflows).
     while (!mAudioFrameQueue.empty()) {
-        std::unique_lock<std::mutex> audioLock(mAudioMutex);
-        auto accumulatedTime = mAccumulatedTime;
-        audioLock.unlock();
-
-        if (mAudioFrameQueue.front().pts < accumulatedTime + AUDIO_BUFFER) {
+        if (mAudioFrameQueue.front().pts < mAccumulatedTime + AUDIO_BUFFER) {
             // Enable only when needed, as this generates a lot of debug output.
             if (DEBUG_VIDEO) {
                 LOG(LogDebug) << "Processing audio frame with PTS: "
@@ -787,7 +776,7 @@ void VideoFFmpegComponent::outputFrames()
 
             if (outputSound) {
                 // The audio is output to AudioManager from updatePlayer() in the main thread.
-                audioLock.lock();
+                std::unique_lock<std::mutex> audioLock(mAudioMutex);
 
                 mOutputAudio.insert(mOutputAudio.end(),
                                     mAudioFrameQueue.front().resampledData.begin(),
@@ -803,19 +792,11 @@ void VideoFFmpegComponent::outputFrames()
         }
     }
 
-    std::unique_lock<std::mutex> playerLock(mPlayerMutex);
-    bool isActuallyPlaying = mIsActuallyPlaying;
-    playerLock.unlock();
-
     // Process all available video frames that have a PTS value below mAccumulatedTime.
     // But if more than one frame is processed here, it means that the computer can't
     // keep up for some reason.
-    while (isActuallyPlaying && !mVideoFrameQueue.empty()) {
-        std::unique_lock<std::mutex> audioLock(mAudioMutex);
-        double accumulatedTime = mAccumulatedTime;
-        audioLock.unlock();
-
-        if (mVideoFrameQueue.front().pts < accumulatedTime) {
+    while (mIsActuallyPlaying && !mVideoFrameQueue.empty()) {
+        if (mVideoFrameQueue.front().pts < mAccumulatedTime) {
             // Enable only when needed, as this generates a lot of debug output.
             if (DEBUG_VIDEO) {
                 LOG(LogDebug) << "Processing video frame with PTS: "
@@ -843,7 +824,7 @@ void VideoFFmpegComponent::outputFrames()
             // can't keep up. This approach primarily decreases stuttering for videos with frame
             // rates close to, or at, the rendering frame rate, for example 59.94 and 60 FPS.
             if (mDecodedFrame && !mOutputPicture.hasBeenRendered) {
-                double timeDifference = accumulatedTime - mVideoFrameQueue.front().pts -
+                double timeDifference = mAccumulatedTime - mVideoFrameQueue.front().pts -
                                         mVideoFrameQueue.front().frameDuration * 2.0l;
                 if (timeDifference < mVideoFrameQueue.front().frameDuration) {
                     pictureLock.unlock();
@@ -1408,13 +1389,11 @@ void VideoFFmpegComponent::startVideo()
 
 void VideoFFmpegComponent::stopVideo()
 {
-    std::unique_lock<std::mutex> playerLock(mPlayerMutex);
     mIsPlaying = false;
     mIsActuallyPlaying = false;
     mStartDelayed = false;
     mPause = false;
     mEndOfVideo = false;
-    playerLock.unlock();
     mTexture.reset();
 
     if (mFrameProcessingThread) {
@@ -1459,11 +1438,7 @@ void VideoFFmpegComponent::pauseVideo()
 
 void VideoFFmpegComponent::handleLooping()
 {
-    std::unique_lock<std::mutex> playerLock(mPlayerMutex);
-    bool endOfVideo = mEndOfVideo;
-    playerLock.unlock();
-
-    if (mIsPlaying && endOfVideo) {
+    if (mIsPlaying && mEndOfVideo) {
         // If the screensaver video swap time is set to 0, it means we should
         // skip to the next game when the video has finished playing.
         if (mScreensaverMode &&

es-core/src/components/VideoFFmpegComponent.h

@@ -23,6 +23,7 @@ extern "C" {
 #include <libavutil/imgutils.h>
 }
 
+#include <atomic>
 #include <chrono>
 #include <mutex>
 #include <queue>
@@ -161,10 +162,10 @@ private:
     int mVideoFrameReadCount;
     int mVideoFrameDroppedCount;
 
-    double mAccumulatedTime;
-    bool mStartTimeAccumulation;
-    bool mDecodedFrame;
-    bool mEndOfVideo;
+    std::atomic<double> mAccumulatedTime;
+    std::atomic<bool> mStartTimeAccumulation;
+    std::atomic<bool> mDecodedFrame;
+    std::atomic<bool> mEndOfVideo;
     bool mSWDecoder;
 };
 

es-core/src/resources/TextureData.cpp

@@ -179,9 +179,7 @@ bool TextureData::load()
         const ResourceData& data = rm->getFileData(mPath);
         // Is it an SVG?
         if (Utils::String::toLower(mPath.substr(mPath.size() - 4, std::string::npos)) == ".svg") {
-            std::unique_lock<std::mutex> lock(mMutex);
             mScalable = true;
-            lock.unlock();
             std::string dataString;
             dataString.assign(std::string(reinterpret_cast<char*>(data.ptr.get()), data.length));
             retval = initSVGFromMemory(dataString);
@@ -285,11 +283,7 @@ float TextureData::sourceHeight()
 
 void TextureData::setSourceSize(float width, float height)
 {
-    std::unique_lock<std::mutex> lock(mMutex);
-    bool scalable = mScalable;
-    lock.unlock();
-
-    if (scalable) {
+    if (mScalable) {
         if ((mSourceWidth != width) || (mSourceHeight != height)) {
             mSourceWidth = width;
             mSourceHeight = height;

es-core/src/resources/TextureData.h

@@ -11,6 +11,7 @@
 
 #include "utils/MathUtil.h"
 
+#include <atomic>
 #include <cmath>
 #include <mutex>
 #include <string>
@@ -79,7 +80,7 @@ private:
     int mHeight;
     float mSourceWidth;
     float mSourceHeight;
-    bool mScalable;
+    std::atomic<bool> mScalable;
     bool mLinearMagnify;
     bool mReloadable;
     bool mForceRasterization;

es-core/src/resources/TextureDataManager.cpp

@@ -161,10 +161,11 @@ TextureLoader::~TextureLoader()
     std::unique_lock<std::mutex> lock(mMutex);
     mTextureDataQ.clear();
     mTextureDataLookup.clear();
+    lock.unlock();
 
     // Exit the thread.
     mExit = true;
-    lock.unlock();
+
     mEvent.notify_one();
     mThread->join();
     mThread.reset();
@@ -172,9 +173,7 @@ TextureLoader::~TextureLoader()
 
 void TextureLoader::threadProc()
 {
-    bool exit = false;
-
-    while (!exit) {
+    while (!mExit) {
         std::shared_ptr<TextureData> textureData;
         {
             // Wait for an event to say there is something in the queue.
@@ -199,7 +198,6 @@ void TextureLoader::threadProc()
                 mTextureDataLookup.erase(mTextureDataLookup.find(textureData.get()));
             }
         }
-        exit = mExit;
     }
 }
 

es-core/src/resources/TextureDataManager.h

@@ -9,6 +9,7 @@
 #ifndef ES_CORE_RESOURCES_TEXTURE_DATA_MANAGER_H
 #define ES_CORE_RESOURCES_TEXTURE_DATA_MANAGER_H
 
+#include <atomic>
 #include <condition_variable>
 #include <list>
 #include <map>
@@ -41,7 +42,7 @@ private:
     std::unique_ptr<std::thread> mThread;
     std::mutex mMutex;
     std::condition_variable mEvent;
-    bool mExit;
+    std::atomic<bool> mExit;
 };
 
 //