Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autosave with multiple instance #353

Open
nolyn opened this issue Apr 16, 2021 · 11 comments
Open

Autosave with multiple instance #353

nolyn opened this issue Apr 16, 2021 · 11 comments
Assignees
Labels

Comments

@nolyn
Copy link
Contributor

nolyn commented Apr 16, 2021

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
Copy link
Member

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:

  • Consider a save state "from a still running YUView" in case it was not modified for more then X (maybe 20) seconds. The YUView would then have to write the update date/time into the save with every update. But this also means that if you restart YUView too quickly it would not show the dialog.
  • We need some inter-process communication for YUView to contact other running instances and ignore saves that are from instances that are still running.

@nolyn
Copy link
Contributor Author

nolyn commented Apr 16, 2021

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 nolyn self-assigned this Apr 17, 2021
@nolyn
Copy link
Contributor Author

nolyn commented Apr 18, 2021

working on it in #355

@ChristianFeldmann
Copy link
Member

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
Copy link
Contributor Author

nolyn commented Apr 19, 2021

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
Copy link
Member

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
Copy link
Contributor Author

nolyn commented Apr 19, 2021

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
Copy link
Member

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
Copy link
Contributor Author

nolyn commented Apr 19, 2021

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

@ChristianFeldmann
Copy link
Member

One idea I had:

  • A new instance tries to connect to an already existing instance. If it is there, communicate with this "central instance" about who is active right now.
  • If no one answers, start a new "central isntance"
  • If the "central instance" quits, another instance takes over (if there is one)

Ok this also sounds a bit complicated.

@nolyn
Copy link
Contributor Author

nolyn commented Apr 21, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants