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.11k stars 214 forks source link

Python 2 End of Life 2020 #169

Closed FSMaxB closed 6 years ago

FSMaxB commented 6 years ago

Python 2 will stop receiving support in 2020. Are there any plans already to port syncplay to Python 3?

albertosottile commented 6 years ago

Just dropping my 2 cents here, nothing official. We are currently in the middle of another transition, changing the GUI binding to support (eventually) Qt 5. So, in my opinion, it would be better to wait for the official release of PySide2 before thinking to migrate to Python 3. Hopefully, this should happen long before 2020.

That being said, I played a little with 2to3 and syncplay today, and the most annoying compatibility issues that I found so far were caused by the str/Unicode/byte type changes introduced in Python 3. These issues will force to adjust all the player protocols introducing a lot of decode() and encode() in sent or received strings, according to the types that Twisted for Python 3 is now expecting.

This is what I found so far, but maybe someone else is aware of further, much more worrisome, potential problems.

xNinjaKittyx commented 6 years ago

I'm playing around with syncplay on python3.6 and so far I've been able to easily setup the server and have users connect to it. Chat, Hello seems to work, but when the client receives the timestamp of synchronizing, the server gives the client , which I will have to look into.

Edit: Also getting all the dependencies to work with python3.6 is a pain (PySide2 in particular)

Et0h commented 6 years ago

Good question, @FSMaxB. Python 3 support is not something that I have looked into for many years and no recent decisions have been made as to moving Syncplay to Python 3 by any specific date (or whether we would want to support both in parallel by a specific date). Any support people can provide in terms of helping understand and overcome barriers and assessing whether or not there are reasons for urgency or delay would be appreciated.

From the above comments by @alby128 and @xNinjaKittyx it seems clear there are problems with PySide2 on Python 3. Is the situation of PySide for Python 3 any different to Python 2? In Python 2 my understanding is that PySide1 is fully supported (other than a few OS X quirks) and PySide2 is working if you build it yourself but is not yet stable and accessible enough to be depended upon (especially for Windows).

xNinjaKittyx commented 6 years ago

As I was working on it a bit more, I finally was able to find a way to get Pyside2 on Python 3.6 through conda packaging (miniconda or anaconda, I used anaconda) And it seems to work fine. It was hell trying to build it myself (as I don't have much experience in that area), and after 2 hours I found out that conda had a package for python 3.6

PySide2 natively (through pip) supports python 3.2, 3.3, 3.4, but it does not support any other versions without having to make your own build, but conda-forge distribution seems to have one that "just works" I was able to start running the client, but.....

There are a lot of issues involving the player APIs. I'm not 100% sure, but in MpcHcApi class, it doesn't seem to like the classes inside classes (it says that __Locks() is not an attribute of class MpcHcApi), but I have to look more into that.

Another issue I was thinking of was py2exe because it never upgraded to python3. But from the top answer from Google, it seems like cx_freeze might be something worth looking into.

From studying the code more, I definitely agree that @alby128 suggestion is considered to move to Qt5 before upgrading to python 3. One issue I ran into was doing a QtDateTime check. You can't use toPython when QtDateTime is Year 0. Nonecheck does not work. False check is working(?).

As for python2 -> python3 syntax changes, here are the ones I found in syncplay's code:

I literally just went around replacing things and doing minor edits. It starts (at least).

albertosottile commented 6 years ago

I was just writing that conda had binaries for PySide2 on Python 2 and 3 for all operative systems. Sorry for the time you wasted trying to compile it, I had to compile PySide2 for macOS myself and I know what you have been through. This will definitely be solved once PySide2 is released and official binaries will be available.

Anyway, it seems that you met the same errors in the player interfaces that I experienced during my tests, so we should probably focus on those when we will attempt this migration. With 2to3 and some minor changes I ran Syncplay and the player (I tried VLC and mpv), but communication between the two was impossible.

xNinjaKittyx commented 6 years ago

Tonight, I'll probably try to do some more debugging as I think it's partially a server issue.

When I was connecting a brand new python3.6 syncplayServer with a 1.5.0 python2.7 syncplayClient, the timestamp given to the client is always None, which is strange to me. I tried looking at the debug logs, but I didn't spend too much time looking at them. I'll probably do a comparison between a 2.7 server and a 3.6 server to find anomales

Another thing that would be nice to add is to add --debug to the server side just like how it is on the client side. Right now, I'm just adding print() statements. And it would be nice to be able to see debug messages on binaries instead of having to grab source.

Et0h commented 6 years ago

@xNinjaKittyx My understanding is that the the server doesn't have debug because (a) we didn't want to encourage invasion of privacy, and (b) for most purposes you can just put debug mode on the client side and get the same info.

xNinjaKittyx commented 6 years ago

That's understandable.

albertosottile commented 6 years ago

I started to do some work about this in my fork, given that the release of PySide2 is approaching (as Python 2 EOL). At this moment, the code contained in https://github.com/albertosottile/syncplay/tree/py3-qtpy-pyside2 works with VLC on macOS 10.12+ with Python 3.6. I tested it locally and using current online servers (running on 2.7) and everything works fine.

This is how I proceeded, you may use the following as guidelines. I used 2to3 to convert the codebase, then I proceeded with the following fixes:

Unfortunately, the code ported in this way is not compatible with 2.7 anymore, but since its EOL is quickly approaching, I think that a net transition would probably be more advisable (and definitely easier, given the codebase).

Now I am working with mplayer.py to support mpv, too. I applied the same method that I used for vlc.py, but for some reason the player does not answer to the client, after loading a file, for the next ~40 seconds, then it answers for multiple times all at once, then it "freezes" again. Any idea why? (@Et0h or anyone else, suggestions are welcome)

Basically, an approximate roadmap for this transition could be (IHMO):

  1. wait for a stable release of PySide2
  2. distribute a new version of Syncplay with embedded Pyside2 and Qt 5.x
  3. start the porting of this codebase to Python 3.x (we have to be careful with dependencies)
  4. test all platform and players (Win, macOS, Linux -- VLC, mpv, MPC)
  5. test packing code for distribution (Win and macOS)
  6. distribute a new version of Syncplay with embedded Python 3.x libraries

Note: vlc.py currently uses asyncore which is included in 3.6 but deprecated so, sooner or later, we should port that to asyncio.

Note2: there are issues with 3.x and both py2exe and py2app, so item 5. will not be trivial.

albertosottile commented 6 years ago

I am using this issue to keep track of the current status of the fork.

Good news:

Bad news:

xNinjaKittyx commented 6 years ago

As seen in the stackoverflow link and what I've mentioned before, cx_freeze could also be an alternative, but I have no experience in executable packaging. cx_freeze is also cross platform, which could simplify workflow if it works.

Edit: Looks like PyInstaller is a lot better documented, maintained, and cross-platform. I think that's probably the better choice.

albertosottile commented 6 years ago

Just a quick update, I wrote a .spec file for PyInstaller and I was able to generate a .exe that works on Windows 10 x64 with VLC and MPC-HC. I still have to push this to the fork, though. This procedure could be viable to pack the portable release, but all the NSIS part of the current packaging script still has to be adapted to generate the setup release.

Side note: with PyInstaller it is impossible to put all the dependencies in a separate 'lib' folder, as we currently do with py2exe. See here for further information. Hence, the portable release looks a little "uglier" compared to what we have now. This should not be a problem for the setup release.

Hummer12007 commented 6 years ago

Re PySide2: it's Qt for Python since a few weeks ago, see http://blog.qt.io/blog/2018/04/13/qt-for-python-is-coming-to-a-computer-near-you/

It seems to me, that they're picking up the pace (5.11 will have it as a technological preview already), so stabilization is not too far away imo.

albertosottile commented 6 years ago

Yes, you are right. As far as I know, they are targeting June for the first release. From our side, we are ready to deploy packed .dmg and .exe versions of Syncplay with embedded PySide2 and Qt 5.9.

albertosottile commented 6 years ago

Great news, everyone. Thanks to @Et0h, now mpv works with Python 3.x on Windows and macOS. I think this completes the player roster for these two platforms, so we can now begin tests on Linux.

PyInstaller for packing the windows distributable works greatly with the --onedir option, not so good with the --onefile option that creates only a giant .exe file with all the libraries embedded. However, since we need to copy some files to other folders and since we want to provide an installer, I would not recommend the --onefile option for distribution. I still have to code and test the packaging of the server .exe.

So, here there is a recap of what has to be done yet:

I will focus on Linux in the next few days, though I can only test with a single distro. Any suggestion on which one I should use?

P.S. Of course, all the codebase will still require extensive and detailed tests from users of all the platforms (and most definitely a beta phase), given that we do not have an automatic test suite to confirm that all the features work.

Et0h commented 6 years ago

Something I was discussing with @albertosottile the other day is that Syncplay's vlc.py currently uses asyncore and asynchat to communicate with VLC via syncplay.lua. According to https://docs.python.org/3/library/asyncore.html and https://docs.python.org/3/library/asynchat.html both asyncore and asynchat these have been deprecated since Python 3.6 in favour of asyncio.

For now that is not an issue as asyncore and asynchat are currently retained for the purposes of backwards compatibility, but in the future we may wish to either go with asyncio for Python 3.4+ or come up with custom threaded STDIN/STDOUT code which does not rely on any of the async* modules.

albertosottile commented 6 years ago

PR #191 ports the codebase to Python 3.x. Everyone is encouraged to test the code in it on all the platforms. If needed, I can provide links for packed binaries for Windows and macOS.

xNinjaKittyx commented 6 years ago

@albertosottile Nice. Does it currently support all players? I may try this later today on Windows 10 w/ MPC-BE/MPC-HC

Binaries would be helpful.

albertosottile commented 6 years ago

It should have exactly the same features of 1.5.5, so try to stress it as much as possible.

@xNinjaKittyx You can download binaries for Windows from here: https://bintray.com/alby128/Syncplay/download_file?file_path=Syncplay-1.5.6-Setup.exe https://bintray.com/alby128/Syncplay/download_file?file_path=Syncplay_1.5.6_Portable.zip

xNinjaKittyx commented 6 years ago

Not sure if it's just my setup, but I get this error:

Traceback (most recent call last):
  File "syncplayClient.py", line 19, in <module>
  File "C:\projects\syncplay\syncplay\clientManager.py", line 7, in run
  File "C:\projects\syncplay\syncplay\ui\ConfigurationGetter.py", line 449, in getConfiguration
  File "C:\projects\syncplay\syncplay\ui\ConfigurationGetter.py", line 356, in _parseConfigFile
  File "C:\Python34\lib\configparser.py", line 735, in readfp
  File "C:\Python34\lib\configparser.py", line 690, in read_file
  File "C:\Python34\lib\configparser.py", line 1043, in _read
configparser.DuplicateSectionError: While reading from 'C:\\Users\\Dan\\AppData\\Roaming\\syncplay.ini' [line 71]: section 'general' already exists

It also doesn't even show syncplay opening, it just crashes immediately.

Am watching a few anime episodes with my friend. So far seems to work well with MPC-BE.

albertosottile commented 6 years ago

Did you reinstall over an old Syncplay installation or try to run portable?

xNinjaKittyx commented 6 years ago

Reinstall on top of an old syncplay.

After installing 1.5.6 and trying to run it, I reinstalled 1.5.5 (It worked), then I reinstalled 1.5.6 again. Still didn't work, so I deleted the config file to make it work.

albertosottile commented 6 years ago

Weird... I am not seeing this on macOS, perhaps @Et0h can provide some more data about Windows.

Et0h commented 6 years ago

The NSIS installer for Syncplay sometimes creates an additional entry for [general] if one already exists and that causes the DuplicateSectionError error in Syncplay.

My understanding this duplicate INI entry occurs when the WriteINIStr commands used at https://github.com/Syncplay/syncplay/blob/master/buildPy2exe.py#L503 are updating a Unicode INI. Maybe it is because it is looking for an ANSI string, I've not tested it.

So we should try and fix the bug with the NSIS installer which allows it to work correctly whether the INI file is ANSI or Unicode.

We also need a way to handle the issue for those who have already installed and have duplicate entries, either through a pre-parser or an except for DuplicateSectionError.

In the meantime, the workaround is to remove the duplicate [general] entry manually.

xNinjaKittyx commented 6 years ago

It's a little odd because the friend that I was testing this with also has a similar setup to mine (Windows 10) but he didn't have any problems, which is interesting.

I do still have a copy of that faulty .ini (from the recycle bin lol), so I'm gonna try later today to replicate it, then I could post its contents and see if it adds more insight.

Et0h commented 6 years ago

@xNinjaKittyx You can check and change the file encoding in Notepad++. I think if you have a Unicode syncplay.ini then causes problems next time you install on that system.

xNinjaKittyx commented 6 years ago

I used the faulty .ini, and i didn't have to change the encoding (Notepad++ says UTF-8 BOM)

[general]
language = en
checkforupdatesautomatically = True
lastcheckedforupdates = 2018-02-10 05:43:18.080000

[client_settings]
name = NinjaKitty
room = SuchKek
playerpath = c:\program files (x86)\kcp\mpc-hc\mpc-hc.exe
perplayerarguments = {}
slowdownthreshold = 1.5
rewindthreshold = 4.0
fastforwardthreshold = 5.0
slowondesync = True
rewindondesync = True
fastforwardondesync = True
dontslowdownwithme = False
forceguiprompt = True
filenameprivacymode = SendRaw
filesizeprivacymode = SendRaw
unpauseaction = IfOthersReady
pauseonleave = False
readyatstart = False
autoplayminusers = -1.0
autoplayinitialstate = None
mediasearchdirectories = {}
sharedplaylistenabled = True
loopatendofplaylist = False
loopsinglefiles = False
onlyswitchtotrusteddomains = True
trusteddomains = [u'youtube.com', u'youtu.be', u'video.xx.fbcdn.net', u'video-lga3-1.xx.fbcdn.net']
publicservers = [u'syncplay.pl:8995', u'syncplay.pl:8996', u'syncplay.pl:8997', u'syncplay.pl:8998', u'syncplay.pl:8999']

[gui]
showosd = True
showosdwarnings = True
showslowdownosd = True
showdifferentroomosd = False
showsameroomosd = True
shownoncontrollerosd = False
showdurationnotification = True
chatinputenabled = True
chatinputfontunderline = False
chatinputfontfamily = sans-serif
chatinputrelativefontsize = 24.0
chatinputfontweight = 1.0
chatinputfontcolor = #FFFF00
chatinputposition = Top
chatdirectinput = False
chatoutputfontfamily = sans-serif
chatoutputrelativefontsize = 24.0
chatoutputfontweight = 1.0
chatoutputfontunderline = False
chatoutputmode = Chatroom
chatmaxlines = 7.0
chattopmargin = 25.0
chatleftmargin = 20.0
chatbottommargin = 30.0
chatmoveosd = True
chatosdmargin = 110.0
notificationtimeout = 3.0
alerttimeout = 5.0
chattimeout = 7.0
chatoutputenabled = True

[server_data]
host = 192.168.5.130
port = 8999
password = 

[general]
language=en
CheckForUpdatesAutomatically=True

Looks like there's a duplicate general that has no spaces?

Deleted the duplicate and it opened correctly.

For some reason on my new .ini file created from scratch, the general section is empty:

[general]
language = 
checkforupdatesautomatically = True
lastcheckedforupdates = 
Et0h commented 6 years ago

It sees to be the same bug as reported at https://sourceforge.net/p/nsis/bugs/787/ which is that if there is a UTF-8 file with a BOM chracter at the start then NSIS does not properly recognise the first group. This means that when [general] is the first group NSIS thinks it is (BOM)[general] and so assumes there is no group called [general] and creates a new one.

Et0h commented 6 years ago

https://docs.python.org/3/library/configparser.html notes that Python 3.2 moved from strict being False by default to being True by default. This change is why the problem has only become apparent now. If we set strict back to False then it will revert to the old behaviour where Syncplay will merge the duplicate entry with the later entries taking precedence, which I think is the behaviour we want.

Et0h commented 6 years ago

Okay, just committed https://github.com/albertosottile/syncplay/commit/76a6390a2b82c7ab022dbf86a5530e9c0b7766e7 which resolves the issue from Syncplay's side.

albertosottile commented 6 years ago

Closed by #191. Thanks to everyone who contributed to this. Long live Syncplay.

Et0h commented 6 years ago

https://syncplay.pl/syncplay-1-5-6-beta-1/

OmarAssadi commented 6 years ago

Someone might want to update the Linux section on the install guide, by the way. Sat here for a while unsure as to why the gui wouldn't work having not realized we transitioned to Python 3.x ;-)