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

[ffmpeg] use nasm instead of yasm #14547

Merged
merged 7 commits into from Jan 28, 2021

Conversation

mcmtroffaes
Copy link
Contributor

  • What does your PR fix? Yasm is no longer being actively maintained, so it seems like a good idea to switch to nasm since upstream supports it. Patch also replaces the deprecated --enable-yasm option with --enable-x86asm.

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

  • Does your PR follow the maintainer guide? Yes, to the best of my knowledge.

@JackBoosY JackBoosY added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist depends:vm-update PR contains changes to the VM provisioning scripts labels Nov 13, 2020
@JackBoosY
Copy link
Contributor

I did the same job as a42d9e7, so when we merged one of these 2 PRs and the other should resolve the file conflict.

@mcmtroffaes
Copy link
Contributor Author

Yep, that's swell, thanks @JackBoosY!

@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

BillyONeal added a commit to BillyONeal/vcpkg that referenced this pull request Dec 24, 2020
@BillyONeal BillyONeal removed the depends:vm-update PR contains changes to the VM provisioning scripts label Dec 24, 2020
@JackBoosY
Copy link
Contributor

I will check the 86-windows manually later.

@JackBoosY
Copy link
Contributor

When building ffmpeg:x86-windows:

LD	libavcodec/avcodec-58.dll
   Creating library libavcodec/avcodec.lib and object libavcodec/avcodec.exp
LINK : fatal error LNK1000: unknown error at 00007FF6F720AC26; consult documentation for technical support options

mcmtroffaes added a commit to mcmtroffaes/ffmpeg-msvc-build that referenced this pull request Jan 21, 2021
@mcmtroffaes
Copy link
Contributor Author

I've done a fresh rebase and spent some time trying to diagnose this problem. For me, on the 32 bit build, the linker just hangs for a really long time (like 20 minutes) at the avcodec linking stage, and then causes the powershell terminal to crash, but I suppose it's the same error. Not sure where to go from here, except to fall back on yasm for x86... 😕 I will add a commit to this effect.

@mcmtroffaes
Copy link
Contributor Author

Looks like this has fixed the x86 build. 🎉

@JackBoosY
Copy link
Contributor

Need test features, should be good.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jan 27, 2021
@mcmtroffaes
Copy link
Contributor Author

Sure, FWIW here's my build matrix on appveyor (against a slightly more recent than version of ffmpeg compared to the one on vcpkg, but it also includes the nasm patch).

https://ci.appveyor.com/project/mcmtroffaes/ffmpeg-msvc-build/builds/37449560

@dan-shaw dan-shaw merged commit 3c466eb into microsoft:master Jan 28, 2021
@mcmtroffaes mcmtroffaes deleted the feature/ffmpeg-nasm branch January 28, 2021 09:05
@Baklap4
Copy link

Baklap4 commented Aug 10, 2021

@mcmtroffaes did you ever find out why it hangs on nasm in the linker phase? I was wondering if this commit was still needed in latest master. But It seems i'm running in the same problem that it just 'hangs' and doesn't do anything with nasm. I came here because of #19121 as yasm cannot be retrieved due to certificate error.

@mcmtroffaes
Copy link
Contributor Author

@Baklap4 That's an excellent question. Unfortunately, I never found out why nasm fails with x86 on ffmpeg... I agree it would be best if we could simply phase out yasm for ffmpeg completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants