Make image processing multithreaded

[twodoorcoupe]

Summary

• This is a…
  • Bug fix
  • Feature addition
  • Refactoring
  • Minor / simple change (like a typo)
  • Other
• Describe this change in 1-2 sentences:
  Makes Picard run image processors in different threads.

Problem

• JIRA ticket (optional): PICARD-XXX

Solution

The main advantage of this is that when there are multiple images in the CoverArt's queue, the next image's download can start while the previous is still processing.

Then once all images have been downloaded, the Album waits for all images to finish processing before finalizing loading.

I'm still working to make sure exceptions are handled correctly in the threads.

Action

Additional actions required:

• Update Picard documentation (please include a reference to this PR)
• Other (please specify below)

[twodoorcoupe]

Still not ready, as I'm scratching my head on how to deal with the exceptions raised by coverartimage.set_tags_data() in a nice way

[twodoorcoupe]

Ideally this should be callback based and not wait with polling.

Unless it is impossible to achieve, polling should be avoided at all costs IMO.

I understand, for now I changed it so that each CoverArt class has its own thread for image processing, and album.finalize_loading() is called only after each processor has finished, without needing to poll.

My goal is to eventually have the same QThreadPool be shared among all CoverArts, and not having one thread for each. I think I can get something to work using callbacks, I can work on making a general implementation in a later pr.

---

Regarding exceptions, at first I tried using callbacks through the error argument, but this created a race condition when errors needed to be appended to albums. Since callbacks run on the main thread, QThreadPool.waitForDone() does not consider them.

For now there's a decorator function handling these exceptions, but once I settle the "wait for a set of threads to finish" business, I can replace this.

---

Last thing. Before, when an image could not be saved to the temporary folder, CoverArt would not call next_in_queue() any longer. Since now images are saved in different threads, I can't catch this exception as before.

The only way I could think of for achieving a similar behaviour is to stop if the album has already finished loading when next_in_queue() is called, because when an image can't be saved to the temporary folder, the album finalizes loading with error=True. But, I'm still not sure this is the best approach.

[zas]

The patch looks good to me, I tested and didn't find any showstopper.
There's perhaps room for improvements, of course, but the current code looks functional enough to me.

[phw]

Tested this. Patch looks good for what it does and works as expected. LGTM

[Sophist-UK]

I haven't had time to read and digest this code, and AFAICS the image processing code itself is in a separate plugin (and so outside the control of this code), so I have no idea whether the image processing code is going to be CPU intensive (or CPU only) or whether it will be I/O bound.

The reason I mention this is that the GIL implementing in cPython does NOT handle multiple CPU-bound threads (or sometimes even one such thread) particularly well, and this can lead to GIL starvation of the main thread and thus GUI unresponsiveness.

You may therefore wish to add a microsecond sleep to the iterative code so that the GIL is released for a very short period to allow the UI to get some CPU.

(I can reasonably claim some expertise in this area - see faster-cpython/ideas#328.)