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

x264: bump to build 155 #1081

Merged
merged 8 commits into from
Jan 10, 2018
Merged

x264: bump to build 155 #1081

merged 8 commits into from
Jan 10, 2018

Conversation

jstebbins
Copy link
Contributor

Requires NASM to build
Unified 8 and 10 bit library support
AVX-512 optimizations
Various other bug fixes and improvements

@bradleysepos
Copy link
Contributor

Are we planning to merge this after release (target 1.2.0)?

@jstebbins
Copy link
Contributor Author

Whatever you guys want. I'll still be using the system version of x264 on linux. Just thought I would get this done since it required more changes than a typical bump.

@bradleysepos
Copy link
Contributor

As much as I'd like the new features, I think we should let this one soak in the nightly a bit.

@bradleysepos bradleysepos added this to the 1.2.0 milestone Dec 26, 2017
@bradleysepos
Copy link
Contributor

Well, that didn't do what I wanted at all.

@bradleysepos
Copy link
Contributor

@jstebbins I tried GitHub's resolve conflicts tool and regret it. You may want to do a hard reset to
9547979. I don't have permissions.

@bradleysepos
Copy link
Contributor

Oh hey I figured it out. Wheeeeee

@bradleysepos bradleysepos force-pushed the x264-bump branch 2 times, most recently from 4d35414 to 6cdd57f Compare December 28, 2017 16:49
@bradleysepos
Copy link
Contributor

I'm updating scripts/mac-toolchain-build to add nasm.

What packages are needed on Ubuntu/Fedora? Will need to update the documentation.

@jstebbins
Copy link
Contributor Author

A very recent version of nasm is required (2.13). Fedora 27 and Ubuntu 17.10 have it. But earlier releases do not. I think if we want to avoid pain, we should build a local copy for all platforms for the foreseeable future.

But package is called simply "nasm" in both Fedora and Ubuntu.

@bradleysepos
Copy link
Contributor

Okay. I'm not a fan of the local tools and intend to remove them, just haven't gotten around to it. But that would probably be the least pain.

On Mac, x264 isn't finding nasm despite it being on my path. What a pain.

@jstebbins
Copy link
Contributor Author

I'm ok with removing all the local tools. But it's going to increase the documentation burden. We'll have to have good detailed documentation on how to set up build environments or we'll just be repeating ourselves on the forums all the time to help people through it.

@bradleysepos
Copy link
Contributor

The documentation is complete regarding dependencies for all systems we support. A script is included to build all dependencies on Mac. Local tools can die whenever we get to it.

@bradleysepos
Copy link
Contributor

bradleysepos commented Dec 28, 2017

From x264's config.log:

checking whether nasm supports vmovdqa32 [eax]{k1}{z}, zmm0... no
Failed commandline was:
--------------------------------------------------
nasm conftest.asm  -I. -I$(SRCPATH) -DARCH_X86_64=1 -I$(SRCPATH)/common/x86/ -f macho64 -DPIC -DPREFIX  -o conftest.o
nasm: fatal: unrecognised output format `macho64' - use -hf for a list
type `nasm -h' for help
--------------------------------------------------
Failed program was:
--------------------------------------------------
vmovdqa32 [eax]{k1}{z}, zmm0
--------------------------------------------------
$ nasm -hf | grep macho64
    macho64   NeXTstep/OpenStep/Rhapsody/Darwin/MacOS X (x86_64) object files

☹️

@jstebbins
Copy link
Contributor Author

nasm --version?

@bradleysepos
Copy link
Contributor

$ nasm --version
NASM version 2.13.02 compiled on Dec 28 2017

@bradleysepos
Copy link
Contributor

I think it's ignoring PATH and picking up macOS' /usr/bin/nasm. 🙃

@bradleysepos
Copy link
Contributor

I added some supporting commits but still have no idea why x264 is not picking up nasm on my system.

@bradleysepos
Copy link
Contributor

bradleysepos commented Dec 28, 2017

Looks like it actually works when cross-compiling, though I also get Unknown option --build=x86_64-apple-darwin17.3.0, ignored. The only other things that should be different are --host and --cross-prefix. ~~~I wonder whether setting --host for a non-cross build will work.~~~ Nope.

@bradleysepos
Copy link
Contributor

The only way I can seem to make this work is to hard code AS="/usr/local/bin/nasm" (or wherever nasm >= 2.13 is installed) in configure. Something's amiss.

@bradleysepos
Copy link
Contributor

eadfc60 should fix the old nasm problem on Mac.

@kodawah
Copy link

kodawah commented Jan 3, 2018

shouldn't you use X264_BUILD 153 to check for the new bitdepth field?
never mind I missed the last patch in the series

@bradleysepos
Copy link
Contributor

I've tested with this enough that I think it's probably safe for 1.1.0 if we want to include it.

@bradleysepos bradleysepos modified the milestones: 1.2.0, 1.1.0 Jan 8, 2018
@bradleysepos
Copy link
Contributor

If nasm in distros is a sticking issue, we can push it back again.

@bradleysepos
Copy link
Contributor

Already have one report of HandBrake not compiling with newer system x264, so perhaps we should get this in. Can add nasm compilation notes to the developer docs.

jstebbins and others added 8 commits January 10, 2018 01:39
Requires NASM to build
Unified 8 and 10 bit library support
AVX-512 optimizations
Various other bug fixes and improvements
Required for recent x264. Better to fail here than part-way through a build.
Also set CONFIGURE.build to null since the configure script has no clue about --build.
Pass a couple standard variables since make sometimes makes zero sense.
Tools shipping with Xcode are still in PATH, only moved from first to last priority. Avoids accidentally using old tools where a newer version is installed. This ensures the tools identified by configure will be the exact tools used by make via Xcode.
The multi-lib change actually happend in build 153
@bradleysepos
Copy link
Contributor

Merged as we need the libhb changes to support newer distros now. Will add nasm instructions to the docs shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants