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

[BUG] app crashes sometimes when another app also uses the same ipc-pipe #127

Open
Alistair1231 opened this issue Jun 26, 2021 · 17 comments
Labels
needs-more-info Awaiting more info from the user's end

Comments

@Alistair1231
Copy link

Alistair1231 commented Jun 26, 2021

Describe the bug

Sometimes when I start mpv through syncplay, trakt-scrobbler crashes, since the pipe is blocked for a moment. After restarting trakt-scrobbler everything behaves fine.

Desktop (please complete the following information):

  • OS and Version: Win 10 21H1
  • Python Version: 3.9.5
  • Player and Version: mpv 0.33.0

To Reproduce

  • Start mpv through Syncplay, and specify ipc-pipe as player argument, omitting the \pipe in Windows. e.g.:
    • trakt-scrobbler config
    mpv:
        ipc_path: \\.\pipe\mpvsocket
    
    • syncplay config (necessary even if specified in mpv.conf):
      image
  • play a file

Log file

Click to see log contents

2021-06-26 19:57:48,320 - ERROR - mpv - __init__ - Unhandled exception
Traceback (most recent call last):
  File "c:\users\alima\.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\__init__.py", line 42, in run_with_except_hook
    run_original(*args2, **kwargs2)
  File "c:\users\alima\.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\player_monitors\mpv.py", line 80, in run
    self.conn_loop()
  File "c:\users\alima\.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\player_monitors\mpv.py", line 298, in conn_loop
    self.file_handle = win32file.CreateFile(
pywintypes.error: (231, 'CreateFile', 'All pipe instances are busy.')

I think it should be sufficient to catch this exception and just recheck after a bit, since it works after restarting the program trakt-scrobbler (edit).

Even with this testing the issue from #124 didn't reappear so far.

@Alistair1231
Copy link
Author

It happened again while closing mpv.

2021-06-27 00:03:55,014 - DEBUG - mpv - monitor - {'state': 0, 'progress': 97.73, 'media_info': {'type': 'episode', 'title': "Marvel's Daredevil", 'season': 1, 'episode': 11}, 'updated_at': 1624745035.0074694}
2021-06-27 00:03:55,016 - DEBUG - scrobbler - scrobbler - Scrobbling stop at 97.73% for Marvel's Daredevil
2021-06-27 00:03:55,016 - DEBUG - mpv - mpv - Pipe closed.
2021-06-27 00:03:55,132 - ERROR - mpv - __init__ - Unhandled exception
Traceback (most recent call last):
  File "c:\users\alima\.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\__init__.py", line 42, in run_with_except_hook
    run_original(*args2, **kwargs2)
  File "c:\users\alima\.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\player_monitors\mpv.py", line 80, in run
    self.conn_loop()
  File "c:\users\alima\.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\player_monitors\mpv.py", line 298, in conn_loop
    self.file_handle = win32file.CreateFile(
pywintypes.error: (231, 'CreateFile', 'All pipe instances are busy.')
2021-06-27 00:03:55,590 - INFO - scrobbler - scrobbler - Scrobble stop successful for Marvel's Daredevil S01E11

@iamkroot
Copy link
Owner

Hmm the missing pipe in the syncplay player argument seems weird to me. I'm surprised that it is even working somehow. But i suggest following the official method for setting up syncplay to be monitored properly(new feature in the latest beta). You'll find the instructions in the Players Setup page of the wiki.

@Alistair1231
Copy link
Author

Alistair1231 commented Jun 27, 2021

This is how it behaves on my pc. With the latest (1.6.8) release of syncplay. (video has audio)

2021-06-27.17-59-36.mp4

Also, what I didn't show, with any setting other the one I normally use, trakt-scrobbler doesn't recognize mpv running.

@iamkroot
Copy link
Owner

What version of mpv are you using? How was it downloaded/installed (from the official site Windows builds page)?

@Alistair1231
Copy link
Author

Alistair1231 commented Jun 28, 2021

It's mpv 0.33.0:
image
image

installed with chocolatey (kinda like apt for windows)

They list chocolatey on their installation page. So it is supported I assume.
image

iamkroot added a commit that referenced this issue Jun 29, 2021
@iamkroot
Copy link
Owner

Huh, I've also installed the same mpv version via chocolatey, and as far as I recall, it was using the pipe path as mentioned in the official docs. I'll do some testing the next time I boot Windows. Till then, if you are fine with running some untested code (it should be ok), I've implemented your suggested solution of catching and ignoring the error. Reinstall the mpv_debug branch using pipx.

@Alistair1231
Copy link
Author

Even on the debug branch I got this error:

2021-06-29 16:52:06,451 - DEBUG - mpv - monitor - action=scrobble
2021-06-29 16:52:06,453 - DEBUG - mpv - monitor - {'state': 0, 'progress': 98.52, 'media_info': {'type': 'episode', 'title': "Marvel's Agents of S.H.I.E.L.D.", 'season': 2, 'episode': 9}, 'updated_at': 1624978326.450264}
2021-06-29 16:52:06,460 - DEBUG - mpv - mpv - Pipe closed.
2021-06-29 16:52:06,461 - DEBUG - scrobbler - scrobbler - Scrobbling stop at 98.52% for Marvel's Agents of S.H.I.E.L.D.
2021-06-29 16:52:06,569 - ERROR - mpv - __init__ - Unhandled exception
Traceback (most recent call last):
  File "c:\users\alima\.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\__init__.py", line 42, in run_with_except_hook
    run_original(*args2, **kwargs2)
  File "c:\users\alima\.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\player_monitors\mpv.py", line 80, in run
    self.conn_loop()
  File "c:\users\alima\.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\player_monitors\mpv.py", line 298, in conn_loop
    self.file_handle = win32file.CreateFile(
pywintypes.error: (231, 'CreateFile', 'All pipe instances are busy.')
2021-06-29 16:52:06,902 - INFO - scrobbler - scrobbler - Scrobble stop successful for Marvel's Agents of S.H.I.E.L.D. S02E09

The scrobble went through but I had to restart trakts for it to continue working.
This was also the first time it produced an error outside of starting up iirc.

@iamkroot
Copy link
Owner

Could you open up File "c:\users\alima.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\player_monitors\mpv.py", line 298, and check whether it is surrounded with a try:... except block?

@iamkroot
Copy link
Owner

iamkroot commented Jun 29, 2021

Also, I guess you're closing the player at the end of the file. Have you set the restart_delay for the monitor in the config? If not, I recommend setting to something like 0.5 or 1 second. Now that I recall, this was the exact reason why I'd added this config param in the first place. trakts config set players.mpv.restart_delay 1

@Alistair1231
Copy link
Author

Could you open up File "c:\users\alima.local\pipx\venvs\trakt-scrobbler\lib\site-packages\trakt_scrobbler\player_monitors[mpv.py](http://mpv.py)", line 298, and check whether it is surrounded with a try:... except block?

it wasn't there. I had to uninstall and then reinstall. Now it's there. Just install --force wasn't enough...

Also, I guess you're closing the player at the end of the file. Have you set the restart_delay for the monitor in the config? If not, I recommend setting to something like 0.5 or 1 second. Now that I recall, this was the exact reason why I'd added this config param in the first place. trakts config set players.mpv.restart_delay 1

And no, I hadn't set that. I did now. What does it do exactly?

@iamkroot
Copy link
Owner

iamkroot commented Jun 29, 2021

Just install --force wasn't enough

That's interesting. I should probably stop recommending this method in that case.

What does it do exactly?

Basically, we have a loop trying to monitor the player, which (in dormant state, when player isn't running) checks if player is alive, and if so, do the active monitoring. After the player exits, the monitor again goes into dormant state, and checks if player is alive. But if this happens too quickly, it might happen that it finds the player is alive while it is exiting, and the monitor incorrectly goes into active state, which is where we hit the error (I think). So the restart_delay does basically what it says. After the player has started shutting down, and our monitor goes into dormant mode, before the next alive check for the player, we add a slight delay to allow the player to shut down completely.

@Alistair1231
Copy link
Author

I see. Thanks for the explanation.
Will keep up to date if it keeps crashing.

@iamkroot
Copy link
Owner

iamkroot commented Jul 7, 2021

Huh, I've also installed the same mpv version via chocolatey, and as far as I recall, it was using the pipe path as mentioned in the official docs. I'll do some testing the next time I boot Windows. Till then, if you are fine with running some untested code (it should be ok), I've implemented your suggested solution of catching and ignoring the error. Reinstall the mpv_debug branch using pipx.

So, I just opened up Windows, and checked my mpv install. Same version as yours, also installed from chocalatey. My mpv.conf (under %APPDATA%\mpv) simply looks like this:

input-ipc-server=\\.\pipe\mpvsocket

And the pipe is present at the correct location. Try removing the setting from the syncplay GUI (keep it blank), since this will make MPV fall back to reading the config file. And see if that is working.

@Alistair1231
Copy link
Author

Alistair1231 commented Jul 7, 2021

Config

using input-ipc-server=\\.\pipe\mpvsocket in the config works now. But it works either way!
From here on all statements are while using this setting in my config
image

Syncplay

When I don't specify it explicitly syncplay uses a random pipe. Even with the config!
image

If I specify it like --input-ipc-server=\\.\pipe\mpvsocket it adds 'pipe'
image

If I omit the \pipe like: --input-ipc-server=\\.\mpvsocket it works with syncplay.

This does NOT happen when runnning mpv like this
image

So that might be a bug with syncplay?

@iamkroot
Copy link
Owner

iamkroot commented Jul 9, 2021

So that might be a bug with syncplay?

You're right, this is exactly the case. I went through the source code, and it mentions

*ipc_socket* is the pipe name. (Not including \\.\pipe\)

and later does

ipc_socket = "\\\\.\\pipe\\" + ipc_socket

So, when you do --input-ipc-server=\\.\pipe\mpvsocket in syncplay, it actually gets converted into \\.\pipe\.\pipe\mpvsocket, which is equivalent to \\.\pipe\pipe\mpvsocket, and hence the output you were seeing.

Instead, you should give syncplay --input-ipc-server=mpvsocket and that will do the trick.

@Alistair1231
Copy link
Author

So that might be a bug with syncplay?

You're right, this is exactly the case. I went through the source code, and it mentions

*ipc_socket* is the pipe name. (Not including \\.\pipe\)

and later does

ipc_socket = "\\\\.\\pipe\\" + ipc_socket

So, when you do --input-ipc-server=\\.\pipe\mpvsocket in syncplay, it actually gets converted into \\.\pipe\.\pipe\mpvsocket, which is equivalent to \\.\pipe\pipe\mpvsocket, and hence the output you were seeing.

Instead, you should give syncplay --input-ipc-server=mpvsocket and that will do the trick.

Ah that explains it. Weird though. Why would they do that instead of just passing it through?
--input-ipc-server=mpvsocket with this it works before with --input-ipc-server=\\.\mpvsocket it also worked but your suggestion is probably better.

@iamkroot
Copy link
Owner

Cool, btw did the original issue get cleared up after using restart_delay?

@iamkroot iamkroot added the needs-more-info Awaiting more info from the user's end label Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-more-info Awaiting more info from the user's end
Projects
None yet
Development

No branches or pull requests

2 participants