2

I'm doing a program in which the users can see the video from a camera and record it. I am using the QtMEL library to get the camera feed and record. When the camera starts it calls a function with QTconcurrent::run() to grab the frames. Inside this function there is an infinite loop that checks if the camera feed was shutdown and, if yes, it exits the loop. Something like this:

Q_FOREVER {
    frame = captureFrame();

    //Do stuff

    //check if we must finish grabbing
    if (isStopRequest() || isPauseRequest())
        break;
}

If the user closes the camera (which stops the frame grabbing and ends the thread) and then exists the program everything is fine. The problem is when the user exits the program with the camera feed still on. If this happens the process keeps running forever or throws a segmentation fault error.

I've searched on the documentation and according to it:

Note that the QFuture returned by QtConcurrent::run() does not support canceling, pausing, or progress reporting. The QFuture returned can only be used to query for the running/finished status and the return value of the function.

I came up with a fix for this problem that I don't know if it's the best and I'd like to have a bit more insight from someone more experienced than me.

Basically I re-implemented the closeEvent like this:

void MainWindow::closeEvent(QCloseEvent *event) {
    if(camera1->state() == AbstractGrabber::StoppedState)
        event->accept();
    else {
        camera1->stop();
        connect(camera1, SIGNAL(stateChanged(AbstractGrabber::State)),
            SLOT(exitWindow(AbstractGrabber::State)));
        event->ignore();
    }
}

And on the slot:

void MainWindow::exitWindow(AbstractGrabber::State grabberState) {
    if(grabberState == AbstractGrabber::StoppedState) this->close();
}

Maybe I'm being naive but it seems to me that there is a better solution for this. Could someone with experience help me?

Thanks in advance for your time.

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
Solidus
  • 698
  • 1
  • 9
  • 23

3 Answers3

1

Presumably, stopping the camera thread will be fast, and you shoudln't need to ignore the event. Simply wait until the camera thread finishes, and then return from the closeEvent. The closeEvent doesn't even need to get involved. You don't really care if the window was closed, you care that the application event loop has exited.

It's easy to determine when the camera thread finishes without having to interact with the frame grabber class. QtConcurrent::run executes the functor on a thread taken from the global thread pool, a.k.a. QThreadPool::globalInstance(). The activeThreadCount method returns the number of threads that are busy doing work submitted to them.

You could implement it as follows:

class CameraStopper() {
  Q_DISABLE_COPY(CameraStopper)
  AbstractGrabber m_camera;
public:
  explicit CameraStopper(AbstractGrabber * cam) : m_camera(cam) {}
  ~CameraStopper() {
    m_camera->stop();
    while (QThreadPool::globalInstance()->activeThreadCount())
      QThread::yieldCurrentThread();
  }
}

AbstractGrabber * MainWindow::camera() const { return camera1; }

int main(int argc, char ** argv) {
  QApplication app(argc, argv);
  MainWindow w;
  CameraStopper stopper(w.camera());
  w.show();
  return app.exec();
}

This of course presumes that no other threads from the global pool are stuck doing things that weren't signaled to stop. The CameraStopper would need to issue stop signals to all such asynchronous tasks. Also ensure that if you've reserved any threads via reserveThread, they are properly released before you get to wait for the camera to stop.

Another thing that you can do is to use some per-thread flag to stop the thread. Prior to Qt 5.2, such flag was only available if an QEventLoop was running in the thread - calling QThread::quit() on that thread's instance would end the event loop. Since Qt 5.2, there is another flag that's independent from the event loop. The flag is set via QThread::requestInterruption, and checked via QThread::isInterruptionRequested.

You need to keep a global list of threads, so that you can easily quit them all, since QThreadPool doesn't expose its threads. This could be done so:

class ThreadRegistry {
  Q_DISABLE_COPY(ThreadRegistry)
  friend class ThreadRegistration;
  QMutex m_mutex;
  QList<QThread*> m_threads;
  static QScopedPointer<ThreadRegistry> m_instance;
  static ThreadRegistry * instance() {
    if (!m_instance) m_instance.reset(new ThreadRegistry);
    return m_instance;
  }
public:
  ThreadRegistry() {}
  ~ThreadRegistry() { quitThreads(); waitThreads(); }
  void quitThreads() {
    QMutexLocker lock(&m_mutex);
    QList<QThread*> & threads(m_threads);
    for (auto thread : threads) {
      thread->quit(); // Quit the thread's event loop, if any
      #if QT_VERSION>=QT_VERSION_CHECK(5,2,0)
      thread->requestCancellation();
      #endif
    }
  }
  void waitThreads() {
    forever {
      QMutexLocker lock(&m_mutex);
      int threadCount = m_threads.count();
      lock.unlock();
      if (!threadCount) break;
      QThread::yieldCurrentThread();
    }  
  }
};

class ThreadRegistration {
  Q_DISABLE_COPY(ThreadRegistration)
public:
  ThreadRegistration() {
    QMutexLocker lock(&ThreadRegistry::instance()->m_mutex);
    ThreadRegistry::instance()->m_threads << QThread::currentThread();
  }
  ~ThreadRegistration() {
    QMutexLocker lock(&ThreadRegistry::instance()->m_mutex);
    ThreadRegistry::instance()->m_threads.removeAll(QThread::currentThread());
  }
};

Finally, the runnable would look as follows, for Qt 5.2 and up:

QtConcurrent::run([this]{
  ThreadRegistration reg;
  while (!QThread::currentThread()->isInterruptionRequested()) {
    frame = captureFrame();
    //Do stuff
  }
}

For Qt 5.0 and 5.1, you could leverage a zero-timeout timer to run some code continuously as long as the event loop hasn't been quit:

QtConcurrent::run([this]{
  ThreadRegistration reg;
  QTimer timer;
  timer.setInterval(0);
  timer.setSingleShot(false);
  timer.start();
  QObject::connect(&timer, &QTimer::timeout, [this]{
    frame = captureFrame();
    //Do stuff
  }
  QEventLoop loop;
  loop.run();
}

And the main would looks as follows:

int main(int argc, char ** argv) {
  QApplication app(argc, argv);
  ThreadRegistry registry;
  MainWindow w;
  w.show();
  return app.exec();
  // ThreadRegistry's destructor will stop the threads 
  // and wait for them to finish
}
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
  • Thank you for the thorough reply. Would it be wise to create a class (or several, depending on the complexity) that stops the still running threads like this one, in the way you explained, when the user exits the program? (also making some kind of save, in case it is necessary) – Solidus Aug 07 '14 at 15:54
  • @Solidus Sure. The class needs to know how to signal the thread's "owner" to stop the thread, of course. You can of course run a `QEventLoop` in the thread - this makes stopping a thread trivial - invoke `QThread::stop()` on the thread, and the event loop running in it will exit. This doesn't work if the thread doesn't run an event loop, as it is in your case. – Kuba hasn't forgotten Monica Aug 07 '14 at 15:56
  • I see. So it would make it easier if I use the QThread instead, as JKSH said on his answer? – Solidus Aug 07 '14 at 16:02
  • @KubaOber QtConcurrent deliberately hides the thread management API from the user, to provide a high-level task-based API. Is there a good reason to write all the low-level code in `ThreadRegistry` to un-hide it? Why not just store specialized QThreads in the first place to keep things simple? – JKSH Aug 08 '14 at 08:12
  • @JKSH Because if you run short-lived tasks, the overhead of starting and stopping the threads will bite you, never mind the overhead of having too many threads overall. There's a good technical reason why Apple announced GCD - essentially a thread pool with system-wide coordination of thread count - to such fanfare. What I show is a workaround for `QThreadPool`'s unwillingness to expose a task stopping mechanism through `QFuture`. And, of course, `QThreadPool` on OS X could be implemented on top of GCD (libdispatch), but isn't yet. – Kuba hasn't forgotten Monica Aug 08 '14 at 11:30
  • I should note that on my application the video will not be recording all the time, but the feed from the camera is supposed to be shown, if not all the time, the majority of the time. So the thread would only stop, let's say 90% of the time, when the user closes the program. – Solidus Aug 08 '14 at 14:51
  • @KubaOber Yes, QThreadPool was invented to avoid the overhead of creating/destroying threads at high frequency. However, this question is about long-lived threads so QThreadPool (hence QtConcurrent) should be avoided. Anyway, for short-lived threads, QThreadPool::waitForDone() should be enough -- no need to reach into the threads' guts to stop them. – JKSH Aug 18 '14 at 06:05
1

You can wait in the destructor of your class for the thread to finish. This could be done using QFuture::waitForFinished (). You can have the future for the thread as a class member :

QFuture<void> future;

And run your function in a new thread by :

future = QtConcurrent::run(this,&MyClass::myFunction);

In the destructor after sending the stop request you can wait for the thread to finish :

camera1->stop();

if(future.isRunning())
    future.waitForFinished();
Nejat
  • 31,784
  • 12
  • 106
  • 138
1

Your loop already checks to see if the camera feed has been shut down or paused. Why not just add another check to see if the user has closed the program?

By the way, QtConcurrent::run() and QRunnable are designed for short-term tasks. For infinite loops (permanent threads), the official documentation recommends using QThread instead: http://qt-project.org/doc/qt-5/threads-technologies.html

Here's a reimplementation of your loop in QThread:

// CameraThread subclasses QThread
CameraThread::run()
{
    while (!isStopRequest()
            && !isPauseRequest()
            && !this->isInterruptionRequested())
    {
        frame = captureFrame();

        //Do stuff
    }
}

When you quit, call QThread::requestInterruption() to make isInterruptionRequested() return true and break your loop.

JKSH
  • 2,658
  • 15
  • 33
  • Thanks for the documentation, I did not know that. Would that work/would it be wiser if I call the requestInterruption on the destructor of the class? – Solidus Aug 07 '14 at 15:56
  • @Solidus Do you mean the destructor of `MainWindow`? Yes, that's a good place call `requestInterruption()`. – JKSH Aug 08 '14 at 02:13