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

Update mpv json ipc vendor code #400

Merged
merged 3 commits into from
Mar 19, 2021
Merged

Conversation

daniel-123
Copy link
Contributor

Update to version 1.1.13 to hopefully address #322 fully.

@FichteFoll
Copy link
Contributor

~/code/syncplay Ξ python syncplayClient.py
Attribute Qt::AA_EnableHighDpiScaling must be set before QCoreApplication is created.
MPV failed with returncode 1.
MPV start failed.
Traceback (most recent call last):
  File "/home/fichte/code/syncplay/syncplay/vendor/python_mpv_jsonipc/python_mpv_jsonipc.py", line 418, in __init__
    self.mpv_process = MPVProcess(ipc_socket, mpv_location, **kwargs)
  File "/home/fichte/code/syncplay/syncplay/vendor/python_mpv_jsonipc/python_mpv_jsonipc.py", line 242, in __init__
    raise MPVError("MPV not started.")
syncplay.vendor.python_mpv_jsonipc.python_mpv_jsonipc.MPVError: MPV not started.
MPV failed with returncode 1.
MPV start failed.
Traceback (most recent call last):
  File "/home/fichte/code/syncplay/syncplay/vendor/python_mpv_jsonipc/python_mpv_jsonipc.py", line 418, in __init__
    self.mpv_process = MPVProcess(ipc_socket, mpv_location, **kwargs)
  File "/home/fichte/code/syncplay/syncplay/vendor/python_mpv_jsonipc/python_mpv_jsonipc.py", line 242, in __init__
    raise MPVError("MPV not started.")
syncplay.vendor.python_mpv_jsonipc.python_mpv_jsonipc.MPVError: MPV not started.
MPV failed with returncode 1.
MPV start failed.
Traceback (most recent call last):
  File "/home/fichte/code/syncplay/syncplay/vendor/python_mpv_jsonipc/python_mpv_jsonipc.py", line 418, in __init__
    self.mpv_process = MPVProcess(ipc_socket, mpv_location, **kwargs)
  File "/home/fichte/code/syncplay/syncplay/vendor/python_mpv_jsonipc/python_mpv_jsonipc.py", line 242, in __init__
    raise MPVError("MPV not started.")
syncplay.vendor.python_mpv_jsonipc.python_mpv_jsonipc.MPVError: MPV not started.

@Et0h
Copy link
Contributor

Et0h commented Mar 14, 2021

I got the same error. It seems to work again if you bring back the env code by:

  • replacing def __init__(self, ipc_socket, mpv_location=None, **kwargs): with def __init__(self, ipc_socket, mpv_location=None, env=None, **kwargs): and
  • replacing self.process = subprocess.Popen(args) with self.process = subprocess.Popen(args, env=env)

Also, because ipc_iina.py relies on syncplay.vendor.python_mpv_jsonipc.python_mpv_jsonipc it is possible that @albertosottile would need to update class IINAProcess(MPVProcess): in line with any changes to the vendor code.

@albertosottile albertosottile force-pushed the update-mpv-jsonipc-vendor-code-2 branch from b8b0559 to 7301870 Compare March 18, 2021 19:00
@albertosottile
Copy link
Member

albertosottile commented Mar 18, 2021

To keep our modifications intact, I followed an inverted approach. I went to a Syncplay commit (40fef45) in which the vendor dependency had not been modified yet and compared the module in our code with version 1.1.13. Then, I went back to master and applied the same changes to our customized copy of the dependency.

In the end, I compared the diffs between the two cases confirming that the vendor dependency was patched as desired. A quick verification can be done comparing the changes in this PR with the changes upstream from here iwalton3/python-mpv-jsonipc@v1.1.11...v1.1.13.

I can confirm that I can correctly run IINA with the code in this branch. I would ask you all to confirm that you can also run mpv on your platforms.

Side note: VSCode automatically stripped some whitespace, hence why the diff shows these extra changes.

@daniel-123 daniel-123 marked this pull request as ready for review March 18, 2021 19:19
@Et0h Et0h self-requested a review March 18, 2021 22:06
Copy link
Contributor

@Et0h Et0h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and it works fine for me on Windows 10 with mpv and mpv.net.

@daniel-123 daniel-123 merged commit 77ce05d into master Mar 19, 2021
@daniel-123 daniel-123 deleted the update-mpv-jsonipc-vendor-code-2 branch March 19, 2021 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants