Use actual mutexes for CameraSource

That homemade spinlock thing was insane, VTune says we were spending 1.5s spinning for no reason when openning a device for example

src/core/coreav.cpp

@@ -395,6 +395,9 @@ void CoreAV::callCallback(ToxAV* toxav, uint32_t friendNum, bool audio, bool vid
 
     if (self->calls.contains(friendNum))
     {
+        /// NOTE: Hanging up from a callback is supposed to be UB,
+        /// but since currently the toxav callbacks are fired from the toxcore thread,
+        /// we'll always reach this point through a non-blocking queud connection, so not in the callback.
         qWarning() << QString("Rejecting call invite from %1, we're already in that call!").arg(friendNum);
         toxav_call_control(toxav, friendNum, TOXAV_CALL_CONTROL_CANCEL, nullptr);
         return;

src/video/camerasource.cpp

@@ -37,7 +37,6 @@ CameraSource* CameraSource::instance{nullptr};
 CameraSource::CameraSource()
     : deviceName{"none"}, device{nullptr}, mode(VideoMode{0,0,0}),
       cctx{nullptr}, cctxOrig{nullptr}, videoStreamIndex{-1},
-      biglock{false}, freelistLock{false},
       _isOpen{false}, subscriptions{0}
 {
     subscriptions = 0;
@@ -73,17 +72,10 @@ void CameraSource::open(const QString deviceName)
 
 void CameraSource::open(const QString DeviceName, VideoMode Mode)
 {
-    {
+    QMutexLocker l{&biglock};
-        bool expected = false;
-        while (!biglock.compare_exchange_weak(expected, true))
-            expected = false;
-    }
 
     if (DeviceName == deviceName && Mode == mode)
-    {
-        biglock = false;
         return;
-    }
 
     if (subscriptions)
         closeDevice();
@@ -94,8 +86,6 @@ void CameraSource::open(const QString DeviceName, VideoMode Mode)
 
     if (subscriptions && _isOpen)
         openDevice();
-
-    biglock = false;
 }
 
 void CameraSource::close()
@@ -110,18 +100,10 @@ bool CameraSource::isOpen()
 
 CameraSource::~CameraSource()
 {
-    // Fast lock, in case our stream thread is running
+    QMutexLocker l{&biglock};
-    {
-        bool expected = false;
-        while (!biglock.compare_exchange_weak(expected, true))
-            expected = false;
-    }
 
     if (!_isOpen)
-    {
-        biglock = false;
         return;
-    }
 
     // Free all remaining VideoFrame
     // Locking must be done precisely this way to avoid races
@@ -144,7 +126,7 @@ CameraSource::~CameraSource()
     device = nullptr;
     // Memfence so the stream thread sees a nullptr device
     std::atomic_thread_fence(std::memory_order_release);
-    biglock=false;
+    l.unlock();
 
     // Synchronize with our stream thread
     while (streamFuture.isRunning())
@@ -153,24 +135,17 @@ CameraSource::~CameraSource()
 
 bool CameraSource::subscribe()
 {
-    // Fast lock
+    QMutexLocker l{&biglock};
-    {
-        bool expected = false;
-        while (!biglock.compare_exchange_weak(expected, true))
-            expected = false;
-    }
 
     if (!_isOpen)
     {
         ++subscriptions;
-        biglock = false;
         return true;
     }
 
     if (openDevice())
     {
         ++subscriptions;
-        biglock = false;
         return true;
     }
     else
@@ -181,7 +156,6 @@ bool CameraSource::subscribe()
         videoStreamIndex = -1;
         // Memfence so the stream thread sees a nullptr device
         std::atomic_thread_fence(std::memory_order_release);
-        biglock = false;
         return false;
     }
 
@@ -189,31 +163,24 @@ bool CameraSource::subscribe()
 
 void CameraSource::unsubscribe()
 {
-    // Fast lock
+    QMutexLocker l{&biglock};
-    {
-        bool expected = false;
-        while (!biglock.compare_exchange_weak(expected, true))
-            expected = false;
-    }
 
     if (!_isOpen)
     {
-        subscriptions--;
+        --subscriptions;
-        biglock = false;
         return;
     }
 
     if (!device)
     {
         qWarning() << "Unsubscribing with zero subscriber";
-        biglock = false;
         return;
     }
 
     if (subscriptions - 1 == 0)
     {
         closeDevice();
-        biglock = false;
+        l.unlock();
 
         // Synchronize with our stream thread
         while (streamFuture.isRunning())
@@ -222,7 +189,6 @@ void CameraSource::unsubscribe()
     else
     {
         device->close();
-        biglock = false;
     }
     subscriptions--;
 
@@ -230,6 +196,8 @@ void CameraSource::unsubscribe()
 
 bool CameraSource::openDevice()
 {
+    qDebug() << "Opening device "<<deviceName;
+
     if (device)
     {
         device->open();
@@ -301,6 +269,8 @@ bool CameraSource::openDevice()
 
 void CameraSource::closeDevice()
 {
+    qDebug() << "Closing device "<<deviceName;
+
     // Free all remaining VideoFrame
     // Locking must be done precisely this way to avoid races
     for (int i = 0; i < freelist.size(); i++)
@@ -346,18 +316,13 @@ void CameraSource::stream()
             if (!frameFinished)
                 return;
 
-            // Broadcast a new VideoFrame, it takes ownership of the AVFrame
+            freelistLock.lock();
-            {
-                bool expected = false;
-                while (!freelistLock.compare_exchange_weak(expected, true))
-                    expected = false;
-            }
 
             int freeFreelistSlot = getFreelistSlotLockless();
             auto frameFreeCb = std::bind(&CameraSource::freelistCallback, this, freeFreelistSlot);
             std::shared_ptr<VideoFrame> vframe = std::make_shared<VideoFrame>(frame, frameFreeCb);
             freelist.append(vframe);
-            freelistLock = false;
+            freelistLock.unlock();
             emit frameAvailable(vframe);
         }
 
@@ -366,39 +331,28 @@ void CameraSource::stream()
     };
 
     forever {
-        // Fast lock
+        biglock.lock();
-        {
-            bool expected = false;
-            while (!biglock.compare_exchange_weak(expected, true))
-                expected = false;
-        }
 
         // When a thread makes device null, it releases it, so we acquire here
         std::atomic_thread_fence(std::memory_order_acquire);
         if (!device)
         {
-            biglock = false;
+            biglock.unlock();
             return;
         }
 
         streamLoop();
 
         // Give a chance to other functions to pick up the lock if needed
-        biglock = false;
+        biglock.unlock();
         QThread::yieldCurrentThread();
     }
 }
 
 void CameraSource::freelistCallback(int freelistIndex)
 {
-    // Fast spinlock
+    QMutexLocker l{&freelistLock};
-    {
-        bool expected = false;
-        while (!freelistLock.compare_exchange_weak(expected, true))
-            expected = false;
-    }
     freelist[freelistIndex].reset();
-    freelistLock = false;
 }
 
 int CameraSource::getFreelistSlotLockless()

src/video/camerasource.h

@@ -91,8 +91,7 @@ private:
     VideoMode mode; ///< What mode we tried to open the device in, all zeros means default mode
     AVCodecContext* cctx, *cctxOrig; ///< Codec context of the camera's selected video stream
     int videoStreamIndex; ///< A camera can have multiple streams, this is the one we're decoding
-    /// TODO: Replace the spinlocks with QMutexS, which spin and sleep
+    QMutex biglock, freelistLock; ///< True when locked. Faster than mutexes for video decoding.
-    std::atomic_bool biglock, freelistLock; ///< True when locked. Faster than mutexes for video decoding.
     std::atomic_bool _isOpen;
     std::atomic_int subscriptions; ///< Remember how many times we subscribed for RAII