Using QWidget::update() from non-GUI thread

Question

Sometimes my application crashes in QWidget::update() that is performing in non-GUI thread.

I am developing an application in which receives video frames from remote host and display them on QWidget.

For this purpose I use libVLC library that gives to me a decoded image. I receive image in libVLC callback, that is performing in separate libVLC thread. In this callback I'm trying to perform QWidget::update() method. Sometimes application crashes, and callstack is somewhere in this method. Here is the my callback code:

//! Called when a video frame is ready to be displayed, according to the vlc clock. 
//! \c picture is the return value from lockCB().

void VideoWidget::displayCB(void* picture)
{
    QImage* image = reinterpret_cast<QImage*>(picture);

    onScreenPixmapMutex_.lock();
    onScreenPixmap_ = QImage(*image);
    onScreenPixmap_.detach();
    onScreenPixmapMutex_.unlock();

    delete image;

    update();
}

I know that GUI operations outside the main thread are not allowed in Qt. But according documentation QWidget::update() just schedules a paint event for processing when Qt returns to the main event loop and does not cause an immediate repaint.

The questtion is: is the rule "GUI operations outside the main thread are not allowed" appliable for QWidget::update()? Does this operation belong to "GUI operations"?

I use Qt 4.7.3, the crash repoduces on Windows 7 and Linux.

Bullhorn

Are you sure your void* picture is in fact QImage*? – Shaia

What qt version are you using? – Belia

Yes, I pass this pointer to libVLC in lockCB() method. – Bullhorn

Normally application works fine, but sometimes it crashes, and the more such widgets are opened the more frequently crahes are. – Bullhorn

I think is not a good practice delete an object that was reserved in other method. Are using the "void* picture" instance in other site of your code? Try to not delete the "void* picture" received, delete this object in the method where was created. Regards. – Belia

What function from libvlc are you using to get a decoded image ? Thanks ! – Foreskin

Answer 1

The question is: is the rule "GUI operations outside the main thread are not allowed" applicable for QWidget::update()? Does this operation belongs to "GUI operations"?

Yes. Update belongs to GUI operations. According to the documentation, all QWidget and derived classes can only be used by the main thread. This is general, and specific functions may state that they are thread safe, but in this case update() does not, so it is not safe to call from other threads.

The signal/slot mechanism works because Qt will (unless told otherwise) use events to allow slots in one thread to be triggered by signals in another. If you were to use signals/slots and tell Qt not to do the special thread handling, the crashes would reappear.

Louiselouisette

Answer 2

Check out the Mandelbrot Example. In that example a worker thread is generating an image and passing it to rendering widget with signal/slot mechanism. Use the same method!

Instead of implementing a new updatePixmap() slot as given in the example you can directly connect update () slot of your widget as well.

From your code I can see that you have a mutex to provide concurrent access. So it should be easy to directly use your update slot.

Both methods still use signal/slot mechanism because GUI operations outside the main thread are not allowed in Qt.

Wickman

Thank you. Now I emit a signal from Non-GUI thread and connect it QWidget::update() slot for the required widget. Crash does not reproduce. – Bullhorn

Answer 3

All you need to do to update a pixmap from any thread is to execute the relevant code in the main thread. This takes care of all the locking and everything else. Modern multithreaded programming is supposed to be easy: if it isn't, you might be trying too hard :)

For example, supposing that you're using a QLabel to display the image (why reinvent your own widget?!):

/// This function is thread-safe. It can be invoked from any thread.
void setImageOn(const QImage & image, QLabel * label) {
  auto set = [image, label]{
    label->setPixmap(QPixmap::fromImage(image));
  };
  if (label->thread() == QThread::currentThread())
    set();
  else {
    QObject sig;
    sig.connect(&sig, &QObject::destroyed, label, set);
  }
}

Now, you might as well do things right and drop stale images - there's no point to setting an image if there are newer images in the event queue ahead of it. That would be the only reason to use a custom widget. See this answer for details.

Side note (does't apply to your code): If you have to call QWidget::update from outside of the widget's implementation, you're doing something very wrong. If you're using a stock Qt widget, you should never need to do it. If you have your own widget and it needs its user to call update on it, you've designed it wrong. That's all there's to it.

Convex

Answer 4

Check my answer here, if you don't want to use the signal/slot mechanism (which also works).

Remediosremedy