[libvpx] switch from yasm to nasm

[mcmtroffaes]

• What does your PR fix? Since upstream supports nasm, this patch switches libvpx from yasm to nasm, as nasm is better maintained. It also avoids issues with the use of the yasm helper on 64 bit systems (see for instance [libvpx[core]:x64-windows] build failure #14511). Minor patch is needed to ensure the libvpx msbuild system works with nasm. Has been tested through a custom ffmpeg build against a nasm build of libvpx, and both vp8 and vp9 work as expected, producing identical files as the yasm build, and at the same speed.

• Which triplets are supported/not supported? Have you updated the CI baseline? No changes.

• Does your PR follow the maintainer guide? To the best of my knowledge, yes.

[mcmtroffaes]

Looks like the x64_osx test is failing due to nasm not being installed on the test system. Not sure how to fix this. For the record, my own osx build with this patch is passing (with nasm installed): https://travis-ci.org/github/mcmtroffaes/ffmpeg-msvc-build/jobs/743162386

[mcmtroffaes]

I've tried to fix it in f65ad1b to no avail. Could someone with a better understanding of how azure works with vcpkg suggest how to fix the regression test so osx gets nasm installed?

[JackBoosY]

CMake Error at scripts/cmake/vcpkg_find_acquire_program.cmake:516 (message):
  Could not find nasm.  Please install it via your package manager:

      brew install nasm

[mcmtroffaes]

Thanks @JackBoosY !

[JackBoosY]

I will handle this regression on next week.

[JackBoosY]

@BillyONeal Please add nasm to the brew install list.

[mcmtroffaes]

Just resolved a conflict against master.

@longnguyen2004 Could you check if the mingw build of libvpx still works with this patch? I noticed you exported the yasm path to ENV{AS} to fix the mingw build of libvpx. I cannot easily test here locally if something similar is also necessary for nasm under mingw.

[longnguyen2004]

@mcmtroffaes Alright I'll check

[longnguyen2004]

Yes the ENV{AS} export is still required for mingw, not sure about MSVC. Can you give me write access so I can commit directly to your fork?

[mcmtroffaes]

Many thanks for looking into it, @longnguyen2004! I'm not sure how to set up write access specifically for you (it seems only official maintainers can do that), but I think it's possible to make a suggested change via the github review functionality, which I can then directly apply via the github interface. See https://haacked.com/archive/2019/06/03/suggested-changes/ Does that work?

[longnguyen2004]

Many thanks for looking into it, @longnguyen2004! I'm not sure how to set up write access specifically for you (it seems only official maintainers can do that), but I think it's possible to make a suggested change via the github review functionality, which I can then directly apply via the github interface. See https://haacked.com/archive/2019/06/03/suggested-changes/ Does that work?

Sure, although I've discovered another weird error, not sure if it's because of my toolchain or something else I forgot to clean the build tree, it's fine now (why does libvpx output files into the source folder...)

[longnguyen2004]

Also GitHub Review doesn't seem to work outside your diff. You can invite me as a collaborator to give me write access.

[mcmtroffaes]

Alas, too bad it doesn't work via github review. I think I've sent you an invite now.

[longnguyen2004]

Can someone get me the log for x64-osx? I'll see if it's the same regression as I'm getting on x64-mingw-dynamic It was a bug in the libvpx build system. The configure script assumes that we're using yasm on *-win32-* and *-win64-* triplets. I'll make a patch.

[longnguyen2004]

@mcmtroffaes I'm wondering if the lack of DWARF debug info for Windows would be a problem for mingw toolchain, since that's the standard debug format there. But since libvpx also defaults to CodeView for mingw toolchain, I'm gonna keep it that way,

[longnguyen2004]

Looking good!

[mcmtroffaes]

Excellent. Many thanks @longnguyen2004 for helping out with this! Looks like we got some new regressions but seems unrelated to this patch (hash error on vtk-m).

[longnguyen2004]

@mcmtroffaes You're welcome! Now we just need to wait for VM update on x64-osx

[JackBoosY]

LGTM. @BillyONeal Do you have any other questions?

[vicroms]

Thanks for the PR!