-1

I am taking photos using gphoto2 and would like to them to a list widget asynchronously as the photos are taken but for some reason it isn't working as intended. It takes the photo on a QThread but is not adding the photo to the list until all photos have been taken (like a bulk add). How would I go about this?

Here is the relevant source code (it won't compile because as there is too many dependencies to fit within the question):

class DownloadThread(QThread):

    data_downloaded = Signal(object)

    def __init__(self, photo_name):
        QThread.__init__(self)
        self.photo_name = photo_name

    def run(self):
        image_location = capture_image.take_photo(self.photo_name)
        image = QImage(image_location)
        to_pixmap = QPixmap.fromImage(image).scaled(200, 200)
        to_qicon = QIcon(to_pixmap)

        self.data_downloaded.emit(QListWidgetItem(to_qicon, image_location))


class MainWindow(QMainWindow, Ui_MainWindow):

    def take_photo(self):
        import time
        for x in range(2):
            photo_name = str(x) +'.jpg'   
            downloader = DownloadThread(photo_name)
            downloader.data_downloaded.connect(self.on_photo_ready)
            downloader.start()
            time.sleep(5)

    def on_photo_ready(self, photo):
        print "WHY"
        self.listWidget.addItem(photo)

I have some simple print statement in the function being called so the terminal looks like this:

Photo Photo Photo Photo Photo Photo WHY WHY WHY WHY WHY WHY

Meaning it waits to actually call emit until the for loop is complete and not on its own thread as intended. Any help would be AWESOME!

Obj3ctiv3_C_88
  • 1,478
  • 1
  • 17
  • 29
  • Your thread does not have a running event loop, and queued signals require one. You need to use the worker model. See [this recent question](http://stackoverflow.com/questions/37195489/migrating-from-inherited-qthread-to-worker-model) for loads of background and implementation info. – jonspaceharper May 17 '16 at 23:17
  • Specifically, unless you really, really know what you are doing, do not subclass `QThread`. – jonspaceharper May 17 '16 at 23:18
  • 1
    @JonHarper I don't think that is the problem actually. The issue is more likely the `time.sleep(5)` which is going to block the main Qt event loop. However removing it will cause the thread to be garbage collected before it has finished running, so really the whole code needs to be refactored to not launch multiple threads since there doesn't seem to be a need to do so. A worker model as you suggest where the `photo_name` is sent via a signal to the thread, and the image returned to the main thread, is probably preferred. – three_pineapples May 18 '16 at 00:53
  • @Obj3ctiv3_C_88 [Here](http://stackoverflow.com/q/35527439/1994235) is another recent question that demonstrates 2-way communication between a worker thread and the main thread that you could adapt (See previous comment for why you would want to do this) – three_pineapples May 18 '16 at 00:56
  • Thanks @three_pineapples I'm not at work right now but will give this a shot tomorrow! – Obj3ctiv3_C_88 May 18 '16 at 01:35

2 Answers2

1

There are a few problems

1. Your thread isn't actually running

You need to call QThread.start() to actually run QThread.run(). That being said, you probably don't want to design your application like this. There's no reason to create dozens or hundreds of different threads -- one for each image download. It would be far more efficient to create one worker thread that downloads all the images in a queue. See below for an example.

2. You can't create QPixmaps or GUI items in a secondary thread

You can't create QPixmap's outside the main thread. You can't create QListWidgetItem's either, or any GUI element for that matter; they can only be created (and safely manipulated) in the main thread. You can use other similar elements (like QImage), but really, the only thing you need to pass back to the main thread is the downloaded filepath; the main thread can handle the QPixmap and item creation.

class DownloadWorker(QObject):

    data_downloaded = Signal(object)

    @QtCore.Slot(str)
    def download_image(self, name):
        image_location = capture_image.take_photo(name)
        self.data_downloaded.emit(image_location)


class MainWindow(QMainWindow, Ui_MainWindow):

    request_download = QtCore.Signal(str)

    def __init__(self, ...)
        ...
        self.worker = DownloadWorker()
        self.thread = QThread(self)
        self.request_download.connect(self.worker.download_image)
        self.worker.data_downloaded.connect(self.on_photo_ready)
        self.worker.moveToThread(self.thread)
        self.thread.start()

        self.timer = QTimer(self)
        self.timer.timeout.connect(self.take_photo)
        self.timer.start(5000)

    def take_photo(self):
        import time
        photo_name = str(time.time()) +'.jpg'  
        self.request_download.emit(photo_name) 

    @QtCore.Slot(str)
    def on_photo_ready(self, filepath):
        item = QListWidgetItem(QIcon(filepath))
        self.listWidget.addItem(item)
Brendan Abel
  • 35,343
  • 14
  • 88
  • 118
  • #2 is false. Creating a `QPixmap` or `QListWidgetItem` In a secondary thread is fine. The file system engine relies on `QPixmap` to asynchronously fetch icons. List widget items are just data containers. Just don't try and paint in a secondary thread! – jonspaceharper May 18 '16 at 11:09
1

You are telling the main thread to sleep while the secondary thread works. This queues all of your signals so that they arrive at once. Remove time.sleep(5) and change

 downloader = ...

To

 self.downloader = ...

And you should be fine.

That said, the worker model is a Good Thing. See this question or this one for details.

Community
  • 1
  • 1
jonspaceharper
  • 4,207
  • 2
  • 22
  • 42