Syncplay / syncplay

Client/server to synchronize media playback on mpv/VLC/MPC-HC/MPC-BE on many computers
http://syncplay.pl/
Apache License 2.0
2.1k stars 214 forks source link

Transition to pyside2 #147

Closed soredake closed 6 years ago

soredake commented 6 years ago

https://wiki.qt.io/PySide2 Since qt4 will not live forever.

albertosottile commented 6 years ago

@soredake I agree with you and I am quite concerned about running qt4 on future operative systems, especially on macOS (we have all seen what happened just a year ago on 10.12, and 10.13 will be out next week). If you want to help, you could test my PySide2 fork of Syncplay from here: https://github.com/alby128/syncplay/tree/pyside2 . Please, let me know if you find any issues by using the tracker in my repository. Thanks.

soredake commented 6 years ago

@alby128 i'll try when https://bugs.gentoo.org/show_bug.cgi?id=624682 will be fixed.

albertosottile commented 6 years ago

@soredake To be honest, I have only worked with Qt 5.6.x, so you could try with them first. If you do not want to compile PySide2, you can use conda and take their binaries (read here: https://anaconda.org/conda-forge/pyside2 ). This is what I used for tests so far on macOS and Windows.

albertosottile commented 6 years ago

@soredake If you still plan to test Syncplay on PySide2, I would advise you to clone the alby128/qtpy-pyside2 branch (https://github.com/alby128/syncplay/tree/qtpy-pyside2 ), since the native port to PySide2 is now outdated. In addition, the code from that branch works even if you have only PySide1.x installed in your system. Once you install PySide2, it automatically uses the new binding. Thanks.

soredake commented 6 years ago

@alby128 sadly, this preventing me from building pyside2, i'll try the new branch with pyside1 today.

soredake commented 6 years ago

Changed branch and installed, then launched syncplay:

Traceback (most recent call last):
  File "/usr/bin/syncplay", line 19, in <module>
    SyncplayClientManager().run()
  File "/usr/lib/syncplay/syncplay/clientManager.py", line 7, in run
    config = ConfigurationGetter().getConfiguration()
  File "/usr/lib/syncplay/syncplay/ui/ConfigurationGetter.py", line 409, in getConfiguration
    qt5reactor.install()
  File "/usr/lib/syncplay/syncplay/vendor/qt5reactor.py", line 375, in posixinstall
    p = QtReactor()
  File "/usr/lib/syncplay/syncplay/vendor/qt5reactor.py", line 210, in __init__
    posixbase.PosixReactorBase.__init__(self)
  File "/usr/lib64/python2.7/site-packages/twisted/internet/base.py", line 498, in __init__
    self.installWaker()
  File "/usr/lib64/python2.7/site-packages/twisted/internet/posixbase.py", line 291, in installWaker
    self.addReader(self.waker)
  File "/usr/lib/syncplay/syncplay/vendor/qt5reactor.py", line 224, in addReader
    self._add(reader, self._reads, QSocketNotifier.Read)
  File "/usr/lib/syncplay/syncplay/vendor/qt5reactor.py", line 220, in _add
    primary[xer] = TwistedSocketNotifier(None, self, xer, type)
  File "/usr/lib/syncplay/syncplay/vendor/qt5reactor.py", line 127, in __init__
    self.notifier = QSocketNotifier(watcher, socketType, parent)
TypeError: QSocketNotifier(sip.voidptr, QSocketNotifier.Type, parent: QObject = None): argument 1 has unexpected type '_UnixWaker'

qt 5.7.1

albertosottile commented 6 years ago

I guess you also have PyQt or PyQt5 installed on your system. While theoretically Qt.py is compatible also with these bindings, some things need to be specifically coded for each specific binding to fully support Syncplay. Therefore, those branches support only PySide and PySide2. To test the code, please run export QT_PREFERRED_BINDING=PySide in your terminal before running Syncplay.

soredake commented 6 years ago

@alby128

PyQt5

Installed, yes.

export QT_PREFERRED_BINDING=PySide

Syncplay started with this, thanks, maybe better to change order to prefer pyside1 if pyqt5 is not working without additions. PyQT5 support will be VERY nice to me since i can not yet build pyside2.

albertosottile commented 6 years ago

Working on changing the binding order, though we plan to release the code in this branch packed with PySide2 in the future, so I would not worry about this. I understand that you might like PyQt5 support, but I did not focus on it because releasing packed Syncplay apps with embedded PyQt5 is impossible at the moment, due to its GPL license. However, I started this porting using PyQt5, so I know that Syncplay works with Qt.py and PyQt5, and I can assure you that the changes required are minimal, if you want to attempt it privately.

soredake commented 6 years ago

@alby128

embedded PyQt5

Maybe an build/runtime option to use system-provided pyqt5? I see that pyqt5 is present in the homebrew

albertosottile commented 6 years ago

I still think that supporting all the bindings will be too much work for Syncplay. After all, right now it works only with PySide. Even reaching coexisting compatibility with PySide and PySide2 in master would be a great result.

albertosottile commented 6 years ago

For the moment, I am closing this. I encourage you to comment in issue #152.