-
Notifications
You must be signed in to change notification settings - Fork 88
Fix to detect FFmpeg from Chocolatey. #1161
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi @bcw222! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Thanks for the PR @bcw222! Let's try to implement a more general solution. Perhaps we could expose a global variable that we pass to Would that work for your workflow? |
|
Hmm, that sounds good, but it would result in a feature that should work out-of-the-box (on devices with the required FFmpeg already installed) still requiring additional manual steps. In a narrow sense, my fix already avoids hardcoding paths as much as possible by reading the Using an environment variable is certainly a good idea, but this pull request is just a quick fix I made after encountering the issue while using I would be happy to refine this further if I have the time, or if someone else is willing to help. |
|
Got it, I understand your concern that an environment variable would require an extra manual step. From our perspective, there are too many package managers to implement specific solutions for each. Our thinking is that an environment variable will provide a general solution that helps simplify your use case and others. Roughly, the implementation would be:
Would you be interested in contributing this change? |
Maybe sometimes FFmpeg's DLLs is not in
PATHso we need to add them manually.