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

VLC 3.0.0 is not working with syncplay #108

Closed ghost closed 7 years ago

ghost commented 8 years ago

Hi,

recently I've updated my VLC to 3.0.0. Since then the Syncplay is not working. I've tried both 1.3.4 and 1.4.0 RC2. Also I've tried with the latest syncplay.lua.

After opening the player the GUI for the syncplayClient shows me as not ready. Pausing/unpausing from VLC does not change my syncplay status to ready (but starts the actual playback on my end). When I check the I'm ready checkbox in the interface the others are seeing me as ready and the video starts on their side, but the local VLC is not starting the playback.

Please advise me how to fix this.

System info OS: Fedora 24 VLC: 3.0.0-git Vetinari (revision 2b82c9f) Syncplay: tried with both 1.3.4 and 1.4.0 RC2

Et0h commented 8 years ago

A quick workaround is to delete lines 320-331 inclusive of Syncplay's vlc.py (the 'for line in iter(self.__process.stderr.readline, ''):' stuff).

It appears to be due to a change in VLC, as VLC doesn't appear to be outputting Syncplay lua intf messages via the STDOUT or STDERR pipes any more.

Ideally I want to still be able to check stderr to find error messages, but if it isn't easily fixable then I may have to produce a more complicated workaround.

ghost commented 8 years ago

Hi,

removing the mentioned lines causes:

error: uncaptured python exception, closing channel <__Listener(VLC Listener, started daemon 139845843293952)> (<class 'socket.error'>:[Errno 111] Connection refused [/usr/lib64/python2.7/asyncore.py|read|83] [/usr/lib64/python2.7/asyncore.py|handle_read_event|446] [/usr/lib64/python2.7/asyncore.py|handle_connect_event|454])

Also when I'm the only one in the room everything works as expected (the VLC pause/unpause changes the ready state of the Syncplay GUI), but there is something strange with the times:

[10:34:32 PM] Attempting to connect to 127.0.0.1:8999 [10:34:32 PM] Successfully connected to server [10:34:33 PM] is playing 'file.mkv' (42:28) in room: 'master' [10:34:34 PM] jumped from 00:00 to 2d, 11:26:41 [10:34:35 PM] jumped from 2d, 11:26:41 to 2d, 12:25:45 [10:34:35 PM] unpaused [10:34:35 PM] You're alone in the room [10:34:35 PM] jumped from 2d, 12:25:45 to 3d, 16:27:53 [10:34:35 PM] jumped from 3d, 16:27:53 to 6d, 11:27:06 [10:34:36 PM] jumped from 6d, 11:27:06 to 10:19:56 (Title 1) [10:34:36 PM] jumped from 10:19:56 (Title 1) to 2d, 20:17:55 (Title 1) [10:34:36 PM] jumped from 2d, 20:17:55 (Title 1) to 3d, 07:51:06 (Title 1) [10:34:36 PM] jumped from 3d, 07:51:06 (Title 1) to 5d, 06:28:55 (Title 1) [10:34:36 PM] jumped from 5d, 06:28:55 (Title 1) to 6d, 05:29:49 (Title 1) [10:34:36 PM] jumped from 6d, 05:29:49 (Title 1) to 2d, 02:56:29 (Title 2) [10:34:36 PM] jumped from 2d, 02:56:29 (Title 2) to 4d, 01:33:51 (Title 2) [10:34:37 PM] jumped from 4d, 01:33:51 (Title 2) to 5d, 00:22:30 (Title 2) [10:34:37 PM] jumped from 5d, 00:22:30 (Title 2) to 10:26:34 (Title 3) [10:34:37 PM] jumped from 10:26:34 (Title 3) to 21:55:00 (Title 3) [10:34:37 PM] paused

If there is a second person in the room, the Syncplay interface is detached from VLC and play/pause don't influence the Syncplay ready state.

Et0h commented 8 years ago

You may need to add some wait code depending on your system so it doesn't connect before the script has loaded.

On 3 Aug 2016 22:09, "Mincho Gaydarov" notifications@github.com wrote:

Hi,

removing the mentioned lines cause:

error: uncaptured python exception, closing channel <__Listener(VLC Listener, started daemon 139845843293952)> (:[Errno 111] Connection refused [/usr/lib64/python2.7/asyncore.py|read|83] [/usr/lib64/python2.7/asyncore.py|handle_read_event|446] [/usr/lib64/python2.7/asyncore.py|handle_connect_event|454])

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Syncplay/syncplay/issues/108#issuecomment-237374581, or mute the thread https://github.com/notifications/unsubscribe-auth/ACR1REqn8oYOoZuPL_jOKSmyyaNpDb8Jks5qcQOMgaJpZM4JbGi- .

ghost commented 8 years ago

Hi,

I want to bring your attention again on the fact that the problem is appearing when there are two or more people in the room. Also when I'm alone in the room there are some strange messages about where did my player 'jumped' which I already pasted in my previous comment (jumped from 00:00 to 2d, 11:26:41 when the file is 42:28 long).

The proposed workarounds doesn't fix the issue.

first time I've tried the following

diff --git a/syncplay/players/vlc.py b/syncplay/players/vlc.py
index 331fcbf..e50019d 100644
--- a/syncplay/players/vlc.py
+++ b/syncplay/players/vlc.py
@@ -314,18 +314,6 @@ class VlcPlayer(BasePlayer):
             self._vlcready = vlcReady
             self._vlcclosed = vlcClosed
             self.__process = subprocess.Popen(call, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
-            for line in iter(self.__process.stderr.readline, ''):
-                if "[syncplay]" in line:
-                    if "Listening on host" in line:
-                        break
-                    if "Hosting Syncplay" in line:
-                        break
-                    elif "Couldn't find lua interface" in line:
-                        playerController._client.ui.showErrorMessage(getMessage("vlc-failed-noscript").format(line), True)
-                        break
-                    elif "lua interface error" in line:
-                        playerController._client.ui.showErrorMessage(getMessage("media-player-error").format(line), True)
-                        break
             self.__process.stderr = None
             threading.Thread.__init__(self, name="VLC Listener")
             asynchat.async_chat.__init__(self)

Today I've tried the following

diff --git a/syncplay/players/vlc.py b/syncplay/players/vlc.py
index 331fcbf..fa2ae4a 100644
--- a/syncplay/players/vlc.py
+++ b/syncplay/players/vlc.py
@@ -314,18 +314,7 @@ class VlcPlayer(BasePlayer):
             self._vlcready = vlcReady
             self._vlcclosed = vlcClosed
             self.__process = subprocess.Popen(call, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
-            for line in iter(self.__process.stderr.readline, ''):
-                if "[syncplay]" in line:
-                    if "Listening on host" in line:
-                        break
-                    if "Hosting Syncplay" in line:
-                        break
-                    elif "Couldn't find lua interface" in line:
-                        playerController._client.ui.showErrorMessage(getMessage("vlc-failed-noscript").format(line), True)
-                        break
-                    elif "lua interface error" in line:
-                        playerController._client.ui.showErrorMessage(getMessage("media-player-error").format(line), True)
-                        break
+            time.sleep(3)
             self.__process.stderr = None
             threading.Thread.__init__(self, name="VLC Listener")
             asynchat.async_chat.__init__(self)

Also I'm not sure if this is relevant, but checking around in the code I saw that the --data-path option is passed to the player, but this option is no longer available in version 3.0.0 (Warning: option --data-path no longer exists.).

Et0h commented 8 years ago

So are you saying VLC 3 works with the workarounds, it just has the weird jumping timestamp issue?

As far as I know the --data-path stuff can be removed as it is no longer used.

Et0h commented 8 years ago

It looks like the issue with the timing is that VLC has moved from decimal seconds to microseconds, so will need to update get_time() and set_time() functions to do a conversion on VLC 3 (but not VLC 2).

ghost commented 8 years ago

Hi,

If I don't patch anything and I'm the only person in the room, VLC works with the weird timings.

If there are 2 people in the room VLC does not work neither without neither with the workarounds.

ghost commented 8 years ago

Solution for the timing can be

diff --git a/resources/lua/intf/syncplay.lua b/resources/lua/intf/syncplay.lua
index 2cbeee0..32e9048 100644
--- a/resources/lua/intf/syncplay.lua
+++ b/resources/lua/intf/syncplay.lua
@@ -268,6 +268,10 @@ function get_time()
         return errormsg
     end

+    if string.sub(vlcversion,1,2) == "3." then
+        realtime = realtime / 1000000
+    end
+
     title = get_var("title", 0)

     if errormsg ~= nil and errormsg ~= "" then
@@ -285,6 +289,9 @@ function set_time ( timetoset)
         realtime = timetoset % titlemultiplier
         oldtitle = radixsafe_tonumber(get_var("title", 0))
         newtitle = (timetoset - realtime) / titlemultiplier
+        if string.sub(vlcversion,1,2) == "3." then
+            newtitle = newtitle * 1000000
+        end
         if oldtitle ~= newtitle and newtitle > -1 then
             set_var("title", radixsafe_tonumber(newtitle))
         end

But still when we are two people in the room the VLC and Syncplay GUI seems to be detached from each other.

ghost commented 8 years ago

When it works: 1) I join the room first 2) Somebody else joins the room second 3) I unpause the player (the Syncplay changes state to ready) 4) The second person in the room updauses 5) The video starts 6) I am able to pause/play through the player (and this is affecting the Syncplay and the second person in the room)

When it doesn't work: 1) Somebody is already in the room 2) I'm joining as second person 3) Play/Pause the player does not change the Syncplay status (only the player on my side starts/stops to play)

1) I join the room first 2) Somebody else joins the room second 3) I unpause the player (the Syncplay changes state to ready) 4) The second person in the room unpauses 5) The video starts 6) The second person pauses 7) My player continues and the pause/unpause no longer changes the Syncplay ready status.

My guess: When the Syncplay client receives the 'State' change from the server it tries to use the lua interface to change the state on the player, but it calls something that breaks the plugin (or the connection between them).

Offtopic: I saw that sometimes the madeChangeOnPlayer variable is None and tracked it to a function that returns none if it doesn't go into an if statement

diff --git a/syncplay/client.py b/syncplay/client.py
index 3d135e9..a57a4bd 100644
--- a/syncplay/client.py
+++ b/syncplay/client.py
@@ -216,6 +216,7 @@ class SyncplayClient(object):
             self._player.setPaused(paused)
             madeChangeOnPlayer = True
             return madeChangeOnPlayer
+        return False

     def _rewindPlayerDueToTimeDifference(self, position, setBy):
         madeChangeOnPlayer = False
ghost commented 8 years ago

Hi,

the problem is with the new times expressed in microseconds. When I apply the following patch the player and the Syncplay does not detach from each (I will be sure on 100% tomorrow)

diff --git a/resources/lua/intf/syncplay.lua b/resources/lua/intf/syncplay.lua
index df012d5..e13a063 100644
--- a/resources/lua/intf/syncplay.lua
+++ b/resources/lua/intf/syncplay.lua
@@ -173,8 +173,14 @@ function detectchanges()

             local titleerror
             newtitle, titleerror = get_var("title", 0)
-            if newtitle ~= oldtitle and get_var("time", 0) > 1 then
-                vlc.misc.mwait(vlc.misc.mdate() + durationdelay) -- Don't give new title with old time
+            if string.sub(vlcversion,1,2) == "3." then
+                if newtitle ~= oldtitle and math.floor(get_var("time", 0) / 1000000) > 1 then
+                    vlc.misc.mwait(vlc.misc.mdate() + durationdelay) -- Don't give new title with old time
+                end
+            else
+                if newtitle ~= oldtitle and get_var("time", 0) > 1 then
+                    vlc.misc.mwait(vlc.misc.mdate() + durationdelay) -- Don't give new title with old time
+                end
             end
             oldtitle = newtitle
             notificationbuffer = notificationbuffer .. "playstate"..msgseperator..tostring(get_play_state())..msgterminator
@@ -268,6 +274,10 @@ function get_time()
         return errormsg
     end

+    if string.sub(vlcversion,1,2) == "3." then
+        realtime = math.floor(realtime / 1000000)
+    end
+
     title = get_var("title", 0)

     if errormsg ~= nil and errormsg ~= "" then
@@ -286,9 +296,17 @@ function set_time ( timetoset)
         oldtitle = radixsafe_tonumber(get_var("title", 0))
         newtitle = (timetoset - realtime) / titlemultiplier
         if oldtitle ~= newtitle and newtitle > -1 then
-            set_var("title", radixsafe_tonumber(newtitle))
+            if string.sub(vlcversion,1,2) == "3." then
+                set_var("title", radixsafe_tonumber(newtitle * 1000000))
+            else
+                set_var("title", radixsafe_tonumber(newtitle))
+            end
+        end
+        if string.sub(vlcversion,1,2) == "3." then
+            errormsg = set_var("time", radixsafe_tonumber(math.floor(realtime * 1000000)))
+        else
+            errormsg = set_var("time", radixsafe_tonumber(realtime))
         end
-        errormsg = set_var("time", radixsafe_tonumber(realtime))
         return errormsg
     else
         return noinput
@@ -488,7 +506,7 @@ function do_command ( command, argument)
     elseif command == "get-filename"          then response           = "filename"..msgseperator..errormerge(get_filename())..msgterminator
     elseif command == "get-title"             then response           = "title"..msgseperator..errormerge(get_var("title", 0))..msgterminator
     elseif command == "set-position"          then           errormsg = set_time(radixsafe_tonumber(argument))
-    elseif command == "seek-within-title"     then           errormsg = set_var("time", radixsafe_tonumber(argument))
+    elseif command == "seek-within-title"     then           errormsg = set_var("time", radixsafe_tonumber(math.floor(argument * 1000000)))
     elseif command == "set-playstate"         then           errormsg = set_playstate(argument)
     elseif command == "set-rate"              then           errormsg = set_var("rate", radixsafe_tonumber(argument))
     elseif command == "set-title"             then           errormsg = set_var("title", radixsafe_tonumber(argument))

I know that this is not the most elegant way to fix it, so can you please provide some more proper way to fix the issue. With this fix there are a lot of jumps in the first 10-15 seconds, before the clients get in sync (probably caused by the missing accuracy).

ghost commented 8 years ago

Hi,

I've reworked my previous patch on fresh mind and produced the following elegant solution to the problem:

diff --git a/resources/lua/intf/syncplay.lua b/resources/lua/intf/syncplay.lua
index 2cbeee0..7e7a397 100644
--- a/resources/lua/intf/syncplay.lua
+++ b/resources/lua/intf/syncplay.lua
@@ -242,6 +242,10 @@ function get_var( vartoget, fallbackvar )
         errormsg = noinput
     end

+    if string.sub(vlcversion,1,2) == "3." and (vartoget == "time" or vartoget == "title") then
+        response = response / 1000000
+    end
+
     return response, errormsg
 end

@@ -252,6 +256,10 @@ function set_var(vartoset, varvalue)
     local errormsg
     local input = vlc.object.input()

+    if string.sub(vlcversion,1,2) == "3." and (vartoset == "time" or vartoset == "title") then
+        varvalue = varvalue * 1000000
+    end
+
     if input then
         vlc.var.set(input,tostring(vartoset),varvalue)
     else
Et0h commented 8 years ago

Created a VLC ticket for the initial issue with STDERR/STDOUT https://trac.videolan.org/vlc/ticket/17286

Et0h commented 8 years ago

https://trac.videolan.org/vlc/ticket/17286#comment:1 says it works when it doesn't work for us. Any idea why?

ghost commented 8 years ago

Hi,

I probably forgot to mention this (sorry). When I tried to run the VLC without the patches for the STDOUT/STDERR it works as expected. Using the 'vlc-wrapper --extraintf=luaintf --lua-intf=syncplay --no-quiet --no-input-fast-seek --play-and-pause --start-time=0 --no-one-instance --no-one-instance-when-started-from-file --lua-config=syncplay={port="33825"}' command manually it shows me '[syncplay] lua interface: Hosting Syncplay interface on port: 33825.0' in the console output.

Also when debugging the syncplay.lua for all the timing I used vlc.msg.info() and vlc.msg.err() for debugging various things and the output from vlc-wrapper was including the messages from syncplay.lua interface.

According to the VLC changelog 'Fixes for stdin/stdout and for different locale issues' it should make no difference.

Et0h commented 8 years ago

Not quite sure what you mean. If everything works fine then why did you have an issue?

ghost commented 8 years ago

The issue was caused by times being in microseconds.

Et0h commented 8 years ago

351024fcaabb2ff9399a02c5f7da235414668550 means Syncplay now fails with an appropriate error message when you try to use Syncplay with VLC 3 due to its lack of Syncplay support. Anyone wishing to fix the microsecond issue can see https://trac.videolan.org/vlc/ticket/17285 - either VLC Lua needs to support the values beyond microseconds, or needs to use the old decimal seconds system for the Lua "time" variable.

lrq3000 commented 7 years ago

I can't register on VLC's trac nor login with OpenID, it seems their trac is buggy...

/EDIT: Fixed, please see the trac for the discussion, they are discussing it currently.


I think it's a pretty serious flaw in VLC 3 if it cannot support big times, how can big videos be played? I didn't check if this impacts only LUA plugins, but even if that's the case still it's a serious limitation for LUA plugins for VLC since none can support videos bigger than about half an hour...

Why not just change the variable type (int64) to a big int? This would allow support for any amount of time while retaining the microsecond precision. Of course there would certainly be a small performance impact but depending on the bigint (or fractional) type implementation, it can be maybe be negligible for videos smaller than half an hour (like what is currently supported), and any amount of perf impact would be tolerable for longer videos since anyway currently it's totally unsupported...

Here are a few interesting links I found for the implementation of bigint in LUA:

For C/C++ (I don't know which language is used by VLC), there are lots of bigint or fractional (so-called arbitrary precision) libraries out there, so there should be no issue to define such a type in C++ (and to pass it to LUA you can use a port like this one, since it's a port it should theoretically be compatible).

lrq3000 commented 7 years ago

They are currently discussing about it on the trac ticket, you might want to give your valuable inputs.

Et0h commented 7 years ago

Thanks for the heads up. Currently on holiday so don't have Trac access at present but it would be great if they at least reverted to the old system of decimal seconds if they can't get anything better to work with long files or on the default configuration of all platforms.

Et0h commented 7 years ago

Just tested things and it looks like if you're using the 64-bit version of the LUA library (e.g. VLC 64-bit) then Syncplay works fine on VLC 3+ (so long as you are using Pilotat's syncplay.lua change to fix the order of magnitude and something along the lines of my suggested change to vlc.py to prevent Syncplay waiting for an STDOUT message which will never arrive).

As such, I should be able to make Syncplay works on VLC >3 for at least the 64-bit version even if they don't resolve the 32-bit issue, I just need to make it a bit more elegant than a sleep(3) and have a better error message to tell people to upgrade to 64-bit or move to another player.

lrq3000 commented 7 years ago

Great news Etoh! That would be great indeed!

They are working actively on the issue currently, so I guess it will be resolved for 32-bit anyway in the future :)

2016-11-26 18:12 GMT+01:00 Etoh notifications@github.com:

Just tested things and it looks like if you're using the 64-bit version of the LUA library (e.g. VLC 64-bit) then Syncplay works fine on VLC 3+ (so long as you are using Pilotat's syncplay.lua change to fix the order of magnitude and something along the lines of my suggested change to vlc.py to prevent Syncplay waiting for an STDOUT message which will never arrive).

As such, I should be able to make Syncplay works on VLC >3 for at least the 64-bit version even if they don't find the 32-bit issue, I just need to make it a bit more elegant than a sleep(3) and have a better error message.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Syncplay/syncplay/issues/108#issuecomment-263074614, or mute the thread https://github.com/notifications/unsubscribe-auth/ABES3i3-8Ehc-Cj9pJJANmMyR_qRzP5Eks5rCGh7gaJpZM4JbGi- .

Et0h commented 7 years ago

GIT Head of Syncplay should now work with latest daily of VLC 3 (32-bit and 64-bit) 401e08a