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

Locate mpv, ffmpeg binaries on macOS (resolves #29). #39

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rucker
Copy link

@rucker rucker commented Apr 4, 2023

Add manual lookup of binaries on macOS due to the fact that the user's PATH isn't exposed to GUI applications, considering the following scenarios:

  1. Homebrew: Homebrew's prefix differs between Intel Macs (/usr/local/) and Apple Silicon Macs (/opt/homebrew/)
  2. Without Homebrew: mpv.app could be installed at /Applications or ~/Applications (likely the former). The mpv binary itself lives here -- Homebrew just symlinks to it.

@SemperPeritus
Copy link

SemperPeritus commented Apr 6, 2023

I almost don't know but think something like this will be better. Not sure it works in mpv script environment.

function find_existing_path(paths)
    for _, path in ipairs(paths) do
        if file_exists(path) then
            return path
        end
    end
    return nil
end
ffmpeg_path = "ffmpeg"
if ON_MAC then
  ffmpeg_path = find_existing_path({"/opt/homebrew/bin/ffmpeg", "/usr/local/bin/ffmpeg"})

Or may be even

ffmpeg_path = find_existing_path({"ffmpeg", "/opt/homebrew/bin/ffmpeg", "/usr/local/bin/ffmpeg"})

@rucker
Copy link
Author

rucker commented Apr 22, 2023

@SemperPeritus I like the idea! I pushed a change that mostly does what you suggested but still handles Mac as a special case. It also looks for ffmpeg at HOME/bin since that's one suggested prefix in their compilation guide.

We could take that a step further and just use the ExecutableFinder on all platforms but that would require some further refactoring. Right now it looks like ExecutableFinder:get_executable_path() calls find_executable() but its return value is only used in a truthy context: i.e. if something is returned, we know we can just invoke it by name later.

@marzzzello Do I have this right? Do you have a strong opinion?

@po5
Copy link

po5 commented May 3, 2023

If your mpv is already in PATH, this is pointless. Just launch the subprocess with mpv/ffmpeg, it will resolve.

If you didn't install via Homebrew, and are using an app bundle, this won't work.
You'll need to locate the currently mpv executable, and strip -bundle to use the command line version. The app bundle can be located anywhere, it may not be in Applications.
https://github.com/po5/thumbfast/blob/ccec64d13883fdd509244bb1b19db4b5ac1dc402/thumbfast.lua#L261-L263

Note that launching cli mpv from within an app bundle results in an icon quickly appearing and disappearing in Dock.
If this is undesirable (it most certainly is since this script spawns one process per thumbnail), then you have to make a symlink to it from outside the app bundle. Then there won't be a Dock icon.
File I linked has logic for this, and for notifying the user of it.

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.

3 participants