Skip to content

macOS: Fix yt-dlp launch through mpv #791

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.

Sign up for GitHub

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

Open
Et0h wants to merge 1 commit into
base: master
Choose a base branch
from
Open

macOS: Fix yt-dlp launch through mpv #791

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 18 additions & 24 deletions syncplay/players/mpv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import time
import subprocess
import threading
import ast

from syncplay import constants
from syncplay.messages import getMessage
Expand Down Expand Up @@ -601,30 +600,25 @@ def __init__(self, playerController, playerIPCHandler, playerPath, filePath, arg
env = os.environ.copy()
if 'TERM' in env:
del env['TERM']
# On macOS, youtube-dl requires system python to run. Set the environment
# to allow that version of python to be executed in the mpv subprocess.
if isMacOS():
try:
env['PATH'] = '/opt/homebrew/bin:/usr/local/bin:/usr/bin'
ytdl_path = subprocess.check_output(['which', 'youtube-dl'], text=True, env=env).rstrip('\n')
with open(ytdl_path, 'rb') as f:
ytdl_shebang = f.readline()
ytdl_python = ytdl_shebang.decode('utf-8').lstrip('!#').rstrip('\n')
if '/usr/bin/env' in ytdl_python:
python_name = ytdl_python.split(' ')[1]
python_executable = subprocess.check_output(['which', python_name], text=True, env=env).rstrip('\n')
else:
python_executable = ytdl_python
pythonLibs = subprocess.check_output([python_executable, '-E', '-c',
'import sys; print(sys.path)'],
text=True, env=dict())
pythonLibs = ast.literal_eval(pythonLibs)
pythonPath = ':'.join(pythonLibs[1:])
except Exception as e:
pythonPath = None
if pythonPath is not None:
env['PATH'] = python_executable + ':' + env['PATH']
env['PYTHONPATH'] = pythonPath
# Remove py2app's bundled Python environment from mpv subprocesses.
env.pop('PYTHONHOME', None)
env.pop('PYTHONPATH', None)

# Add common executable locations while preserving inherited PATH entries.
macPaths = [

@prez prez Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this necessary?

@Et0h Et0h Aug 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was trying to address the macOS yt-dlp/mpv problem discussed in #585 and #785, while making the fix work more universally rather than depending on a particular yt-dlp/Python installation.

For context, I'm doing my best to get something workable while not being a macOS user let alone developer.

@prez prez Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know what you're trying to accomplish, I authored that PR :)
my question is about the addition in this PR relative to mine (extending PATH with 'common' paths) - what makes you think PATH is not sufficiently populated therefore needs to be extended?

@Et0h Et0h Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair question. The PATH change isn't needed for the "PYTHONHOME"/"PYTHONPATH" fix itself. I added it because removing the old workaround also removes its explicit "/opt/homebrew/bin:/usr/local/bin:/usr/bin" PATH handling.

My concern is the normal DMG/Finder-launched use case: py2app documents that applications launched normally get a minimal environment and don't pick up changes from the user's shell profile. This was also discussed in #276, where it was noted that macOS GUI apps may not have "/usr/local/bin" in PATH and the suggestion was to append it rather than replace the existing PATH.

So the intention was to preserve the old code's ability to find a Homebrew-installed downloader, while avoiding overwriting any PATH that is already present. "/opt/homebrew/bin" covers current Apple Silicon Homebrew and "/usr/local/bin" covers Intel Homebrew.

On reflection, I think the patch can be narrower here: rather than adding all of the standard system paths, it could just preserve the inherited PATH and append "/opt/homebrew/bin" and "/usr/local/bin" if missing.

What's your thoughts on it being tweaked to:
if isMacOS():
env.pop('PYTHONHOME', None)
env.pop('PYTHONPATH', None)

# Add common Homebrew locations while preserving inherited PATH entries.
pathEntries = [path for path in env.get('PATH', '').split(os.pathsep) if path]
for path in ['/opt/homebrew/bin', '/usr/local/bin']:
    if path not in pathEntries:
        pathEntries.append(path)
env['PATH'] = os.pathsep.join(pathEntries)

@prez prez Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

macos applications, launched from dock, finder, or as login items generally don't pick up the PATH from the user's shell profile as you have noted.
this is intended by apple and not specific to py2app. the proper mechanism to configure this is via /etc/paths and /etc/paths.d (see man path_helper).
if homebrew users have not added homebrew to the system-wide path, then that is indicative of a misconfigured system, in my opinion. now, whether this is common or not, I wouldn't be able to tell you - I do not use homebrew (and am most knowledgeable about Linux too). and I'm not installing homebrew just to try this, but it's worth noting that some package managers (and proprietary software packages) install their paths into /etc/paths.d automatically.
should syncplay accommodate for this misconfiguration of unknown commonness? probably not. but it's not a hill I'm willing to die on. so, your call :)

'/opt/homebrew/bin',
'/usr/local/bin',
'/usr/bin',
'/bin',
'/usr/sbin',
'/sbin',
]
pathEntries = [path for path in env.get('PATH', '').split(os.pathsep) if path]
for path in macPaths:
if path not in pathEntries:
pathEntries.append(path)
env['PATH'] = os.pathsep.join(pathEntries)
try:
self.mpvpipe = self.playerIPCHandler(
loglevel="info",
Expand Down