Skip to content

Conversation

@cristianst85
Copy link
Contributor

@cristianst85 cristianst85 commented Oct 17, 2024

  • added PostBuild target to copy ffmpeg.exe to the build folder;
  • updated the installer file to remove ffmpeg.exe as it is picked from the build folder instead;
  • both InstallerOutput and FFmpeg folders are now created on git checkout, but their content is ignored by the source control.

This completes the work done in #266.

Now there is a single source for ffmpeg.exe, both for the installer and for when you run the app from VS. Also, you don't have to manually copy ffmpeg.exe into the build folder anymore.

@shaked6540
Copy link
Owner

Might be worth to include the ffmpeg exe itself too, so that everyone is on the same page

@cristianst85
Copy link
Contributor Author

cristianst85 commented Oct 17, 2024

To be honest I don't like the idea to include that binary in the source control. But maybe we could update the README.md file instead with instructions from where ffmpeg.exe can be obtained and which version are we currently using (both for development and the installer).

@cristianst85 cristianst85 force-pushed the ffmpeg-build-improvements branch from 1348da3 to 070ab49 Compare October 18, 2024 06:18
@shaked6540
Copy link
Owner

I guess we can include a link to the ffmpeg version we're using, the only problem with that would be that its not guaranteed to always be available, and in the future there might be breaking changes in the latest versions of ffmpeg. But since its very unlikely to happen, we can include a link to ffmpeg in the README.md file.

I usually take my builds from https://github.com/sudo-nautilus/FFmpeg-Builds-Win32/releases because its 32 bit, meaning it will work on all machines. The problem with that is that repository is mostly dead and outdated.
Switching to 64 bit will cause some problems to some users. I used to include the 64 bit version but a user came to me with a problem that was solved by switching to 32 bit. (His OS was 32 bit so naturally the 64 bit didn't work)

Lets include both the 32 bit release of ffmpeg from the repository I linked and the latest ffmpeg master build from https://www.gyan.dev/ffmpeg/builds/ffmpeg-git-full.7z (which is 64 bit)

Please update the readme.md to include the instructions on downloading/copying ffmpeg to the correct directory

@cristianst85
Copy link
Contributor Author

cristianst85 commented Oct 20, 2024

I use this one https://github.com/BtbN/FFmpeg-Builds. I see that sudo-nautilus/FFmpeg-Builds-Win32 also uses BtbN:master. Security-wise I don't trust it 100%. This is why I don't want to put it into the repository.

@cristianst85
Copy link
Contributor Author

I hadn't had a chance to work on this yet. Maybe in the following days.

@shaked6540
Copy link
Owner

shaked6540 commented Nov 1, 2024

No rush, its ok even if it takes a couple of months, The PR is not going anywhere

 - added PostBuild target to copy ffmpeg.exe to the build folder;
 - updated the installer file to remove ffmpeg.exe as it is picked from the build folder instead;
 - both InstallerOutput and FFmpeg folders are now created on git checkout, but their content is ignored by the source control.
@cristianst85 cristianst85 force-pushed the ffmpeg-build-improvements branch from 070ab49 to 990f50a Compare January 18, 2025 16:01
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.

2 participants