1
0
mirror of https://github.com/qTox/qTox.git synced 2024-03-22 14:00:36 +08:00

fix(video): use a QReadWriteLock to manage camera access

This commit changes the mutex-memfence combination to a pure-mutex
implementation within CameraSource. This should prevent a lot of
pre-existing race conditions from happening.
This commit is contained in:
initramfs 2016-08-04 23:18:37 +08:00
parent 00270ee4d2
commit de6475f3d3
No known key found for this signature in database
GPG Key ID: 78B8BDF87E9EF0AF
2 changed files with 16 additions and 39 deletions

View File

@ -23,7 +23,8 @@ extern "C" {
#include <libavformat/avformat.h> #include <libavformat/avformat.h>
#include <libswscale/swscale.h> #include <libswscale/swscale.h>
} }
#include <QMutexLocker> #include <QWriteLocker>
#include <QReadLocker>
#include <QDebug> #include <QDebug>
#include <QtConcurrent/QtConcurrentRun> #include <QtConcurrent/QtConcurrentRun>
#include <memory> #include <memory>
@ -138,12 +139,10 @@ void CameraSource::open(const QString& deviceName)
void CameraSource::open(const QString& DeviceName, VideoMode Mode) void CameraSource::open(const QString& DeviceName, VideoMode Mode)
{ {
streamBlocker = true; QWriteLocker locker{&streamMutex};
QMutexLocker l{&biglock};
if (DeviceName == deviceName && Mode == mode) if (DeviceName == deviceName && Mode == mode)
{ {
streamBlocker = false;
return; return;
} }
@ -156,8 +155,6 @@ void CameraSource::open(const QString& DeviceName, VideoMode Mode)
if (subscriptions && _isOpen) if (subscriptions && _isOpen)
openDevice(); openDevice();
streamBlocker = false;
} }
/** /**
@ -177,10 +174,12 @@ bool CameraSource::isOpen()
CameraSource::~CameraSource() CameraSource::~CameraSource()
{ {
QMutexLocker l{&biglock}; QWriteLocker locker{&streamMutex};
if (!_isOpen) if (!_isOpen)
{
return; return;
}
// Free all remaining VideoFrame // Free all remaining VideoFrame
VideoFrame::untrackFrames(id, true); VideoFrame::untrackFrames(id, true);
@ -198,9 +197,7 @@ CameraSource::~CameraSource()
device = nullptr; device = nullptr;
} }
// Memfence so the stream thread sees a nullptr device locker.unlock();
std::atomic_thread_fence(std::memory_order_release);
l.unlock();
// Synchronize with our stream thread // Synchronize with our stream thread
while (streamFuture.isRunning()) while (streamFuture.isRunning())
@ -209,7 +206,7 @@ CameraSource::~CameraSource()
bool CameraSource::subscribe() bool CameraSource::subscribe()
{ {
QMutexLocker l{&biglock}; QWriteLocker locker{&streamMutex};
if (!_isOpen) if (!_isOpen)
{ {
@ -228,18 +225,13 @@ bool CameraSource::subscribe()
device = nullptr; device = nullptr;
cctx = cctxOrig = nullptr; cctx = cctxOrig = nullptr;
videoStreamIndex = -1; videoStreamIndex = -1;
// Memfence so the stream thread sees a nullptr device
std::atomic_thread_fence(std::memory_order_release);
return false; return false;
} }
} }
void CameraSource::unsubscribe() void CameraSource::unsubscribe()
{ {
streamBlocker = true; QWriteLocker locker{&streamMutex};
QMutexLocker l{&biglock};
streamBlocker = false;
if (!_isOpen) if (!_isOpen)
{ {
@ -256,11 +248,6 @@ void CameraSource::unsubscribe()
if (subscriptions - 1 == 0) if (subscriptions - 1 == 0)
{ {
closeDevice(); closeDevice();
l.unlock();
// Synchronize with our stream thread
while (streamFuture.isRunning())
QThread::yieldCurrentThread();
} }
else else
{ {
@ -362,7 +349,7 @@ bool CameraSource::openDevice()
*/ */
void CameraSource::closeDevice() void CameraSource::closeDevice()
{ {
qDebug() << "Closing device "<<deviceName; qDebug() << "Closing device " << deviceName;
// Free all remaining VideoFrame // Free all remaining VideoFrame
VideoFrame::untrackFrames(id, true); VideoFrame::untrackFrames(id, true);
@ -374,8 +361,6 @@ void CameraSource::closeDevice()
cctxOrig = nullptr; cctxOrig = nullptr;
while (device && !device->close()) {} while (device && !device->close()) {}
device = nullptr; device = nullptr;
// Memfence so the stream thread sees a nullptr device
std::atomic_thread_fence(std::memory_order_release);
} }
/** /**
@ -413,23 +398,14 @@ void CameraSource::stream()
forever forever
{ {
biglock.lock(); QReadLocker locker{&streamMutex};
// When a thread makes device null, it releases it, so we acquire here // Exit if device is no longer valid
std::atomic_thread_fence(std::memory_order_acquire); if(!device)
if (!device)
{ {
biglock.unlock(); break;
return;
} }
streamLoop(); streamLoop();
// Give a chance to other functions to pick up the lock if needed
biglock.unlock();
while (streamBlocker)
QThread::yieldCurrentThread();
QThread::yieldCurrentThread();
} }
} }

View File

@ -24,6 +24,7 @@
#include <QString> #include <QString>
#include <QFuture> #include <QFuture>
#include <QVector> #include <QVector>
#include <QReadWriteLock>
#include <atomic> #include <atomic>
#include "src/video/videosource.h" #include "src/video/videosource.h"
#include "src/video/videomode.h" #include "src/video/videomode.h"
@ -65,7 +66,7 @@ private:
VideoMode mode; VideoMode mode;
AVCodecContext* cctx, *cctxOrig; AVCodecContext* cctx, *cctxOrig;
int videoStreamIndex; int videoStreamIndex;
QMutex biglock; QReadWriteLock streamMutex;
std::atomic_bool _isOpen; std::atomic_bool _isOpen;
std::atomic_bool streamBlocker; std::atomic_bool streamBlocker;
std::atomic_int subscriptions; std::atomic_int subscriptions;