NEON opts turned on by default

So upstream suggested using --disable-asm for a baseline build, and then using armv6t2 mode with NEON runtime detection, which is the default with a non-NEON fpu allowing v6t2.

NB: In Ubuntu, we probably want a baseline build without --disable-asm (with asm) since v6t2 is implemented as asm.

Do we need a baseline build with --disable-asm?

ARMv7 (the Ubuntu target) is a superset of ARMv6T2, so providing that runtime detection is used for features which are not guaranteed to be part of ARMv6T2 and asm code should work on all potential target platforms.

No we don't, and that's why the changes I did will check for v6t2 and disable an extra optimized build if present in the default toolchain; currently, this should result in only a default build, but turning off -mfpu=neon (it passes -march=armv7-a as detected from gcc -v to disable upstream's -mfpu setting code).

(On Debian, v6t2 isn't in the default toolchain and that causes a --disable-asm build by default, with an extra NEON build with the upstream default flags)

Patch to avoid building everything with -mfpu=neon, and enable HWCAP-based runtime detection of NEON.

This should give us binaries which work across our ARMv7 targets, and which doesn't use NEON code where inappropriate (Dove / Babbage2/1)

This patch applies against upstream and against lucid x264 (2:0.85.1442.1+git781d30-1).
This patch should work for debian (it should not cause NEON to be enabled where it previously was not).

Upstream might not be so happy about:

  * the --disable-assume-neon configure arg. There might be a better way to enable NEON only in the asm, but this was my best idea at the moment.

  * Disabling of the "fast NEON MRC" test. This performance counter based test may not be safe across all platforms, and won't reliably give the right answer even where it doesn't SIGILL— it also requires a patched kernel; so we probably don't want it enabled in Ubuntu (unless we can use some other decision route like identifying the CPU from the contents of /proc/cpuinfo. I haven't attempted this for now.)

See the next patch for proposed changes to debian/rules.

Patch for debian/rules to pass the new argument --disable-assume-neon to configure for armel

This should apply against Debian without problems, if the previous patch is also applied.

We should try to upstream these (or similar) patches if they look useful.

what's the status about this bug? What does upstream think about this?

I've now prepared a new upload for x264 and wanted to know if this is something that should go into this upload or can be deferred.

@Reinhard: this bug will be fixed in your upload, but we need to track another bug which is about not using SIGILL for capabilities detection. For the latter, Dave's patches would need to be adjusted slightly and discussed with upstream, but I didn't come to that. Let's land the debian/rules rework first as to not introduce too many changes in the same upload, if that's ok with you.

Allright. I'll then close this bug. The proposed patch will not be
deleted from launchpad, and we can lookup the bugnumber from the
changelog in any case.

This bug was fixed in the package x264 - 2:0.85.1448+git1a6d32-1

---------------
x264 (2:0.85.1448+git1a6d32-1) lucid; urgency=low

  [ Loic Minier ]
  * Drop --enable-pic, let's see what breaks, LP: #524859

  [ Reinhard Tartler ]
  * New upstream snapshot, no new features, LP: #526396
  * remove quilt infrastructure
  * don't set CFLAGS in debian/rules, upstream build system overrides
    this anyways
 -- Reinhard Tartler <email address hidden> Sun, 21 Feb 2010 16:57:21 +0100