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 freezes while sync play is running on Mac OS X El Captian. #83

Closed searock closed 6 years ago

searock commented 8 years ago

If Vlc is closed from the menu or the dock, VLC freezes and closes if I force quit it or close syncplay.

I'm not sure if this is just happening on my end. Can someone please confirm?

Also I'm trying to find the problem in lua script.

Etoh suggested me to have a look at this part of the script.

There's two pieces of closing code, one from Syncplay's side and one from VLC's. It is the latter which causes the problem.

The code is at https://github.com/Syncplay/syncplay/blob/master/resources/lua/intf/syncplay.lua and the logic (in Lua) is as follows:

        local quitcheckcounter = 0

        ...

        quitcheckcounter = quitcheckcounter + 1

        if quitcheckcounter > quitcheckfrequency then
            if vlc.volume.get() == -256 then
                running = false
            end
            quitcheckcounter = 0
        end

I'm looking on how I can debug the script so I can find where the problem is happening. I tried commenting the above code and it still freezes, that means the problem is somewhere else.

albertosottile commented 6 years ago

This bug has annoyed me for years. The problem is that VLC on macOS does not set the volume to -256 when VLC is closed, so the lua script keeps running endlessly. I am desperately trying to find another variable in the (awful) VLC lua documentation that changes its state after VLC has been closed, so far with no luck.

Et0h commented 6 years ago

Is there a way to detect it from Syncplay itself by running VLC in verbose debug mode, monitoring STDOUT, and looking for a telltale message like "core debug: exiting"? Unfortunately reading STDOUT from VLC3 doesn't work properly in Windows, but it might work on OS X.

albertosottile commented 6 years ago

Sorry I am posting and deleting comments, but I am figuring things out. How can I keep monitoring STDOUT and still let twisted.reactor start? Right now the monitoring is interrupted after "Hosting Syncplay" is found.

Et0h commented 6 years ago

It stops paying attention to stdout/stderr once it gets confirmation it worked because else it'd just get into an infinite loop and block other stuff. One possibly way forwards is to create a daemon thread just to check and make it run self.__playerController.drop() if it detects if '[syncplay] core interface debug: removing module "lua"' in line. However, I've not tested to see if this is possible.

Et0h commented 6 years ago

Oh, and in addition to adding --verbose=2 to the args when it is OSX you might also need to add --no-file-logging to prevent VLC settings to save debug logs to a file from being able to steal messages from stdout/stderr.

albertosottile commented 6 years ago

Understood, but I do not know how to implement all this, I do not know anything about concurrent threads in python. I will give another look on the __LIstener class tomorrow, but I do not expect to be able to have some code ready soon.

Et0h commented 6 years ago

My guess is that if it is possible then it'd look something like this:

constants.py constants.VLC_SLAVE_OSX_ARGS = VLC_SLAVE_OSX_ARGS = ['--verbose=2','--no-file-logging']

vlc.py Replace 'if not sys.platform.startswith('darwin'):' before SLAVE_ARGS.extend(constants.VLC_SLAVE_NONOSX_ARGS)' with:

    if sys.platform.startswith('darwin'):
        SLAVE_ARGS.extend(constants.VLC_SLAVE_OSX_ARGS)
    else:
        SLAVE_ARGS.extend(constants.VLC_SLAVE_NONOSX_ARGS)

Insert following below 'self._vlcVersion = None' or somewhere more sensible:

            def handle_vlcoutput(out):
                for line in iter(out.readline, b''):
                    if '[syncplay] core interface debug: removing module "lua"' in line:
                        self.__playerController.drop()
                out.close()

Replace 'self.__process.stderr = None' with:

                if not sys.platform.startswith('darwin'):
                    self.__process.stderr = None
                else:
                    vlcoutputthread = threading.Thread(target=handle_vlcoutput, args=(self.__process.stderr,))
                    vlcoutputthread.daemon = True
                    vlcoutputthread.start()
Et0h commented 6 years ago

It might also want to break after self.__playerController.drop(), I'm not sure.

albertosottile commented 6 years ago

This works perfectly. I tested it on macOS 10.11+ on VLC 2.2.6 and 3. I just had to tweak some things a little and I am not entirely sure why. The implementation of the deamon is now:

def handle_vlcoutput(self):
            out = self.__process.stderr
            for line in iter(out.readline, ''):
                  if '[syncplay] core interface debug: removing module' in line:
                        self.__playerController.drop()
                        break
            out.close()

And its initialization is: vlcoutputthread = threading.Thread(target = self.handle_vlcoutput, args=())

I am not sure why it did not work with your code. if you think those changes are fine, I can open a PR. I am currently trying to measure the CPU/battery load introduced by this code.

Et0h commented 6 years ago

The issue might be due to Unicode/ASCII/UTF-8 stuff which is platform/locale dependent. The important thing to check is that if you close via VLC or Syncplay it actually ends the process rather than just hiding VLC. If it does end the process then feel free to open the PR.

albertosottile commented 6 years ago

Solved by PR #151.