IENT / YUView

The Free and Open Source Cross Platform YUV Viewer with an advanced analytics toolset
http://ient.github.io/YUView
Other
1.9k stars 374 forks source link

Autosave with multiple instance #353

Open nolyn opened 3 years ago

nolyn commented 3 years ago

I often have more than one YUView running. What I noticed is that if there is already an instance running opening a new one will show the crash recovery dialog. That's because crash is detected by the presences of "Autosaveplaylist" in QSettings, put there by the first instance. Looking a the code I think there is also another issue. Every instance will save the playlist every ten seconds. Didn't test this but I would expect with more than one instance it would be chance which playlist you will recover when one crashes. If more than ten seconds pass before you open another one the playlist will be lost.

How about adding the time that YUView was launched to the setting, i.e. something like "Autosaveplaylist-2020.04.16-9:39". Then we'll have more than one saved playlist. Since this is only for crash recovery, playlist older than say a day can be ignored and removed when found. Playlist of the same day can be used to provide a list to choose from.

ChristianFeldmann commented 3 years ago

Hi! Yes I also thought about that problem once but did not address it further because I rarely have two YUView instances open at the same time and when I do I am comparing two videos and don't care much about the playlist being lost.

The idea with the date and time in the name is great! But what if a new instance is launched. How can it tell if the saved playlist is from a crashed YUView or from one that is still running fine? I can think of 2 options:

nolyn commented 3 years ago

Alternatively we can save a UUID for each new instance, along with its process id. New instances can query if there are processes recorded which are no longer running. There is a QUuid class. The process part will be a bit more difficult, probably have to distinguish there between different OSes.

nolyn commented 3 years ago

working on it in #355

ChristianFeldmann commented 3 years ago

Ok but is it possible to get all running process IDs of all currently running instances of YUView on all platforms? What about dockerized environment like Snap? Is this even possible from a security point of view? Let me know if you can make something like this work. I think the inter-process communication version would also be interesing. This should be possible platform independent using Qt. Here is something about inter process communication: https://doc.qt.io/qt-5/ipc.html And the most promising to me right now seems to be shared memory: https://doc.qt.io/qt-5/qsharedmemory.html

nolyn commented 3 years ago

Good point. I think this should work for Linux (tested) and Mac (can't test it), the code for windows will have to be adapted. I'll get it working first, then test with appimage/flatpak (i don't think we have snaps?). From what I found it will probably not work. Changing the backend storage from qsettings to something else shouldn't be difficult though.

ChristianFeldmann commented 3 years ago

Hmm but we should not merge some sort of Hack using QSettings. I still think the better option is to setup some inter process communication. E.g. using Sockets or the Networking library which we are using in YUView already anyways to check for updates.

nolyn commented 3 years ago

One more thing to consider is how long the memory from the other options will persist. Had a quick look at shared memory, it should be persistent until the last instance is closed. However if there was only a single instance I would expect it to be cleaned up after a crash. To get around that we need to create another independent process which keeps the shared memory alive it the main/only instance crashes.

ChristianFeldmann commented 3 years ago

Yep I am now also unsure about the shared memory. Maybe thats not the optimal way. But the Sockets/Network stuff looks more promising. Maybe there we could also use signals/slots and all of that for threading.

nolyn commented 3 years ago

Doesn't that have the same problem if there was/is only one instance?

ChristianFeldmann commented 3 years ago

One idea I had:

Ok this also sounds a bit complicated.

nolyn commented 3 years ago

QSetting based multiples instance work now. Still have to look into other backends