search

m-ab-s / media-autobuild_suite

This Windows Batchscript helps setup a Mingw-w64 compiler environment for building ffmpeg and other media tools under Windows.
GNU General Public License v3.0
1.53k stars 264 forks source link

Small fixes to shaderc and mpv (with info on luajit) #2517

Closed waldonnis closed 1 year ago

waldonnis commented 1 year ago

A couple of small fixes to help get the suite back on track. It basically just pins mpv to the 0.36 release tag, since they removed waf completely after that, thus making newer revs unable to be compiled by the suite for now. I noted that a prior PR was submitted to update mpv's build procedures, so this should help band-aid mpv until you get that worked out.

The shaderc portion removes the cloning of the dependencies into third-party and just runs the git-sync-deps script included with shaderc. This should pull the known-good commits for each dependency (as listed in the DEPS file), which not only follows their build docs, but also should cut down on the frequency that shaderc compilation breaks because they haven't accounted for upstream dependency changes yet.

Somewhat unrelated to this PR, but to explain the luajit reference in the branch name.... I also made a patch for luajit, since that broke recently as well. They went to a rolling release scheme for their install and uninstall Makefile targets, meaning they copy a - binary into bindir, then add a generically named symlink pointing to the freshly installed version. This caused issues because the suite is overriding INSTALL_TNAME at install time, but used "luajit.exe" which happened to be the same as the symlink name (INSTALL_TSYM)...meaning it tried linking luajit.exe to luajit.exe. Uninstall would also not always remove older versions since it only looks for versioned names that match what's in the Makefile (so it can only remove its own version). As a result and to cut down on old versioned binaries building up in bin-global, I thought it best to just revert the rolling release/symlink bits in the Makefile so it just installs the binary the same way the suite is used to it being done.

The alternative (which I also tried) for luajit is to change the INSTALL_TNAME override in media-suite_compile.sh to INSTALL_TSYM instead, but I ended up with three different version-named luajit binaries in my bin-global in just two days. That's going to build up for people who recompile frequently and it's likely not useful to keep those around in this context.

I can submit that luajit patch to mabs-patches and do a PR here to add it to media-suite_compile.sh, if you'd like...or I can just paste it here...or paste it into a bug report. It's a small patch and just requires a one-liner addition to media-suite_compile.sh, so it shouldn't be a big deal.

hydra3333 commented 1 year ago

The MABS author indicated he would look at PRs from time to time ... In the end, may save time for the author and thus possibly lead to a working MABS earlier than otherwise may be the case :)

waldonnis commented 1 year ago

The MABS author indicated he would look at PRs from time to time ... In the end, may save time for the author and thus possibly lead to a working MABS earlier than otherwise may be the case :)

Agreed, but it's more "where do I put this?" since the patch itself would likely belong in the mabs-patches repo and an additional PR would need to be made here for the one-liner to apply the patch. If it were me, I'd prefer to have both submissions in one place since they're really one solution, but others work differently, so I figured I'd ask. I posted a bug report with the patch already, so that can be a spot for discussion if needed, I guess.

1480c1 commented 1 year ago

I can go ahead and merge this PR if you want to do the luajit stuff in another or wait until you have it ready in this PR?

waldonnis commented 1 year ago

That makes sense. I'll fix the mpv tag for you first in a few minutes and push them to my repo to update the PR. I'm doing final testing on libdovi parts for the other PR now, so I'll take care of it after that finishes compiling.

As for luajit, I agree that it's probably better as a diff in mabs-patches for the reasons you mentioned. I'll go ahead and submit that PR against mabs-patches, then do another PR for the do_patch line change to media-suite_compile.sh (if you need me to; I don't mind either way since it's already part of my own local changes).

1480c1 commented 1 year ago

I'm fine with you doing the PR for the do_patch so it has proper authorship

waldonnis commented 1 year ago

Okay, will do. Looks like I had the tag wrong for mpv anyway. No idea why it worked before, but....

Changing the tag now and running a build against it just to make sure I didn't mess anything up since I'm juggling a few branches more than normal right now.

waldonnis commented 1 year ago

Tag's fixed, so we should be fine to merge these bits barring any other concerns. I'll get to luajit after I move the libdovi stuff from my personal branch to that PR.