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

Added ffmpeg through docker, fetchFFMPEG removed #85

Closed
wants to merge 1 commit into from

Conversation

hxrriet02
Copy link

Fixes an issue with FFMPEG not being detected when downloading. This moves FFMPEG into the Dockerfile instead of when the container starts. Docker container icon also added.

Docker container icon also added.
@Inrixia
Copy link
Owner

Inrixia commented Jul 27, 2021

This looks like it will cause the non headless version to fail if run outside of docker. Ill have to manually test off your fork before I can merge this.

@Inrixia Inrixia self-assigned this Jul 27, 2021
@Inrixia Inrixia self-requested a review July 27, 2021 21:27
@Inrixia Inrixia added this to In progress in Release 5.2.0 via automation Jul 27, 2021
@Inrixia Inrixia added this to the Next Release milestone Jul 27, 2021
Copy link
Owner

@Inrixia Inrixia left a comment

Choose a reason for hiding this comment

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

While this looks good, if not running in headless or outside of linux it will cause ffmpeg to be missing. Instead of removing fetchFFMPEG entirely it should only be disabled in a docker environment.

Note docker specifically as its possible to run headless outside of linux/docker.

@Inrixia
Copy link
Owner

Inrixia commented Nov 6, 2021

#82 Should fix any issues with this. If issues crop up again in the future can look into implementing this properly.

Thanks for the help regardless

@Inrixia Inrixia closed this Nov 6, 2021
Release 5.2.0 automation moved this from In progress to Done Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants