Signal/Slot Multithreaded With GUI Displaying OpenCV Mat

pistorinoj

I am using QT 5.10.11 with VS2015 and OpenCV 3.4.1 in C++ on a Win10 platform.

I am a Qt newbie so this may be something simple.
My app is crashing as a result of a problem that may be related to inter-thread signals/slots.

I my app, I am trying to display images stored in an OpenCV mat. The mat is prepared on one thread and then sent as a signal to a slot on the GUI thread. On the GUI thread, the mat is converted into a pixmap and displayed on a label.

In order to test things, I put a sample image into a mat on the GUI thread and used my slot function to display it. That worked fine. If I send the same image as a parameter to a signal, I can see that the slot is called and that the image is there. However, once the call to display the image is made using setPixmap/setScaledContents, the app locks up. If I do everything but call those two lines, the app runs.

The statement that connects the signal/slot looks like:

tresult = connect(&(WorkThreadChannelArray[x]).wChannel, &Channel::ImageChanged, this, &SV23::onImageChanged, Qt::QueuedConnection);
	if (!tresult)
		QMessageBox::critical(this, "#3", "Error");

No error is reported.

My slot function looks like:

void SV23::onImageChanged(int tChannelNumber, cv::Mat tmat, bool MainWinCond)
{
    QPixmap pixmap;
    if (tmat.type() == CV_8UC1)
    {
	// Set the color table (used to translate colour indexes to qRgb values)
	QVector<QRgb> colorTable;
	for (int i = 0; i<256; i++)
		colorTable.push_back(qRgb(i, i, i));
	// Copy input Mat
	const uchar *qImageBuffer = (const uchar*)tmat.data;
	// Create QImage with same dimensions as input Mat
	QImage img(qImageBuffer, tmat.cols, tmat.rows, tmat.step, QImage::Format_Indexed8);
	img.setColorTable(colorTable);
	pixmap = QPixmap::fromImage(img);
    }
    // 8-bits unsigned, NO. OF CHANNELS=3
    if (tmat.type() == CV_8UC3)
    {
	// Copy input Mat
	const uchar *qImageBuffer = (const uchar*)tmat.data;
	// Create QImage with same dimensions as input Mat
	QImage img(qImageBuffer, tmat.cols, tmat.rows, tmat.step, QImage::Format_RGB888);
	pixmap = QPixmap::fromImage(img.rgbSwapped());
    }
    else
    {
	QMessageBox::critical(this,"ERROR","PixMap Error");
	return;
    }
    switch (tChannelNumber)
    {
	case CHANNEL_SELECT0:
		ui.ChannelBox0->setPixmap(pixmap);
		ui.ChannelBox0->setScaledContents(true);
		break;
	case CHANNEL_SELECT1:
		ui.ChannelBox1->setPixmap(pixmap);
		ui.ChannelBox1->setScaledContents(true);
		break;
	case CHANNEL_SELECT2:
		ui.ChannelBox2->setPixmap(pixmap);
		ui.ChannelBox2->setScaledContents(true);
		break;
	default:
		break;
    }
}

Is there something else I should be calling? Is there a problem accessing the labels from the calling thread on the GUI thread? Should I be doing some sort of mutex on them?

Any help appreciated.

SGaist

Hi and welcome to devnet,

Can you show how you are building your cv::Mat ?

You might need to trigger a deep copy before emitting the signal. Otherwise you likely are just sending a wrapper on top of your data that might not be valid anymore once you start processing them.

pistorinoj

I am not sure how you mean.

In this case, the mat is loaded from a file like this:

Mat tFrame = cv::imread(COLOR_TEST_PATTERN);
onImageChanged(0, tFrame, false);

Using ImageWatch in VS2015, I can see that when I am in the slot function, the mat is properly there. I do not delete the mat on the signal side.

SGaist

IIRC, by default cv::Mat does a shallow copy, so you are likely working on a cv::Mat that is pointing to a now unallocated memory region. If onImageChanged is is the last thing called in your function, tFrame is destroyed then and the memory it points to is not valid anymore.

pistorinoj

Thanks. I think you must be right/close. As I say, the on the worker thread, I do not change the Mat that is being referenced and that Mat persists beyond the calling function. Nevertheless, just after my emit call, I put in a thread.msleep(20) call and the app did not crash.

That leads me to believe that it is something along the lines you are saying. Obviously, putting a sleep call in is less than desirable. Is there someway of passing a deep copy that you know of? Given that my current implementation does not alter/destroy the Mat even after the call, what I already have would have seemed to work.

ambershark

@pistorinoj From the cv docs:

Use a copy constructor or assignment operator where there can be an array or expression on the right side (see below). As noted in the introduction, the array assignment is an O(1) operation because it only copies the header and increases the reference counter. The Mat::clone() method can be used to get a full (deep) copy of the array when you need it

You can always but tFrame on the heap and clean it up when it's done in the thread. That would prevent it from going out of scope and causing the issue with the shallow copy in your thread.

VRonin

You can try onImageChanged(0, tFrame.clone(), false);

pistorinoj

Thanks.

I tried something similar but no luck. Instead of passing an OpenCV Mat through the signal, I processed the Mat into a QImage and passed that as the parameter of the signal. I assume that that would make a local copy of the QImage on the GUI thread. But, again, the app crashes unless you delay the worker thread.

J.Hilk

@pistorinoj do you, by any Chance, pass your connect Statement, Qt::DirectConnection as 5th paarmeter?

pistorinoj

No. The Connect statement uses QueuedConnection.

SGaist

Depending on how you build your QImage it will have the same issue of needing the underlying data's existence to be guaranteed.

pistorinoj

I built it using the same code above. All I did was move everything before the pixmap call onto the worker thread and pass the img.

SGaist

Sorry, I understood that you changed your code to emit a QImage rather than a cv::Mat.

pistorinoj

Right. That is what I did.
My signal code now looks like what I say below. But I have to put in the msleep statements in the app crashes.

bool Channel::ImageDisp(int tChannelNumber, Mat tmat, bool MainWinCond)
{
   if (tmat.type() == CV_8UC1)
   {
	// Set the color table (used to translate colour indexes to qRgb values)
        QVector<QRgb> colorTable;
	for (int i = 0; i<256; i++)
		colorTable.push_back(qRgb(i, i, i));
	// Copy input Mat
	const uchar *qImageBuffer = (const uchar*)tmat.data;
	// Create QImage with same dimensions as input Mat
	QImage img(qImageBuffer, tmat.cols, tmat.rows, tmat.step, QImage::Format_Indexed8);
	img.setColorTable(colorTable);
	emit ImageChanged(tChannelNumber, img , false, MainWinCond);
	QThread::msleep(15);
    }
    else if (tmat.type() == CV_8UC3)
    {
	// 8-bits unsigned, NO. OF CHANNELS=3
	// Copy input Mat
	const uchar *qImageBuffer = (const uchar*)tmat.data;
	// Create QImage with same dimensions as input Mat
	QImage img(qImageBuffer, tmat.cols, tmat.rows, tmat.step, QImage::Format_RGB888);
	emit ImageChanged(tChannelNumber, img , true, MainWinCond);
	QThread::msleep(15);
    }
    else
	return (false);

    return(true);
}

jsulm

@pistorinoj Please read http://doc.qt.io/qt-5/qimage.html#QImage-3
"The buffer must remain valid throughout the life of the QImage and all copies that have not been modified or otherwise detached from the original buffer."
Your buffer is tmat.data. tmat is a local variable and is destroyed when ImageDisp finishes. That's why QThread::msleep(15) helps. But this isn't a solution but a dirty work around.
You need to make sure the buffer is alive as long as you use the QImage. Maybe changing "Mat tmat" to "Mat &tmat" will already be anough.

SGaist

Then it's exactly the problem I described, you have to trigger a copy of your QImage so it will deep copy the data from your underlying buffer.

Or create a QImage of the right size and format and then do a memcpy of your buffer into the QImage internal buffer. Whatever you prefer.

pistorinoj

Thanks all.

I did not realize that even QImage was making a shallow copy.
I changed this so that the first thing the slot does is make a deep copy of the QImage and that seems to have addressed the issue.

Sorry for being so slow.

Thanks again.