x264: new package

PATCH Superseded David du Colombier 2 participants 4 messages Oct 3, 2014 – Oct 5, 2014

lore

Share & tools

lore

David du Colombier

author

2014-10-03 08:53 UTC #0 lore raw

PATCH v3 1/2 x264: new package

This package is based on an earlier package
proposed by Ayaka in December 2013.

Signed-off-by David du Colombier <[email protected]>

---
Changes v2 -> v3:
- suggestions by Peter Korsgaard

Changes v1 -> v2:
- suggestions by Thomas Petazzoni

---
 package/Config.in      |  1 +
 package/x264/Config.in | 23 +++++++++++++++++++++
 package/x264/x264.mk   | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 package/x264/Config.in
 create mode 100644 package/x264/x264.mk

package/Config.in +1 -0

index 1021d4c..4d8a0c4 100644
@@ -755,6 +755,7 @@ menu "Multimedia"
 	source "package/libtheora/Config.in"
 	source "package/live555/Config.in"
 	source "package/mediastreamer/Config.in"
+	source "package/x264/Config.in"
 endmenu
 
 menu "Networking"

package/x264/Config.in new file +23 -0

new file mode 100644
index 0000000..93e6ed0
@@ -0,0 +1,23 @@
+config BR2_PACKAGE_X264
+	bool "x264"
+	depends on BR2_LARGEFILE
+	help
+	  x264 is a free software library and application for
+	  encoding video streams into the H.264/MPEG-4 AVC
+	  compression format, and is released under the terms
+	  of the GNU GPL.
+
+	  https://www.videolan.org/developers/x264.html
+
+if BR2_PACKAGE_X264
+
+config BR2_PACKAGE_X264_CLI
+	bool "CLI"
+	default n
+	help
+	  Command Line Interface
+
+endif
+
+comment "x264 needs a toolchain w/ largefile"
+	depends on !BR2_LARGEFILE

package/x264/x264.mk new file +55 -0

55 lines not on this page · raw message

Signature · 1 line

1.9.3

David du Colombier

author

2014-10-03 08:53 UTC #1 lore raw

PATCH v3 2/2 ffmpeg: enable x264 support

Signed-off-by David du Colombier <[email protected]>

---
 package/ffmpeg/ffmpeg.mk | 7 +++++++
 1 file changed, 7 insertions(+)

package/ffmpeg/ffmpeg.mk +7 -0

index 5b7fa20..0185749 100644
@@ -229,6 +229,13 @@ else
 FFMPEG_CONF_OPT += --disable-libvpx
 endif
 
+ifeq ($(BR2_PACKAGE_X264)$(BR2_PACKAGE_FFMPEG_GPL),yy)
+FFMPEG_CONF_OPT += --enable-libx264
+FFMPEG_DEPENDENCIES += x264
+else
+FFMPEG_CONF_OPT += --disable-libx264
+endif
+
 ifeq ($(BR2_X86_CPU_HAS_MMX),y)
 FFMPEG_CONF_OPT += --enable-yasm
 FFMPEG_DEPENDENCIES += host-yasm

Signature · 1 line

1.9.3

Thomas Petazzoni

2014-10-05 12:53 UTC #2 lore raw

PATCH v3 1/2 x264: new package

In reply to David du Colombier

Dear David du Colombier,

package/x264/Config.in line 16

Quoted 5 lines · David du Colombier

package/x264/Config.in new file +5 -0

+if BR2_PACKAGE_X264
+
+config BR2_PACKAGE_X264_CLI
+	bool "CLI"
+	default n

default n is not needed, as the default for a boolean option is to be disabled.

Review

Quoted 2 lines

(quoted diff) +2 -0
+	help
+	  Command Line Interface

Maybe more text would be needed here.

But the biggest problem is that it doesn't build:

Try this configuration:

BR2_arm=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2014.08.tar.bz2" BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_16=y
BR2_TOOLCHAIN_EXTERNAL_LARGEFILE=y
BR2_TOOLCHAIN_EXTERNAL_INET_IPV6=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
BR2_PACKAGE_X264=y
BR2_PACKAGE_X264_CLI=y

It will fail with:

/home/thomas/projets/buildroot/output/host/usr/bin/arm-linux-as -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -pipe -Os -Wall -I. -I. -std=gnu99 -mcpu=cortex-a8 -mfpu=neon -c -DSTACK_ALIGNMENT=16 -DPIC -DHIGH_BIT_DEPTH=0 -DBIT_DEPTH=8 -o common/arm/cpu-a.o common/arm/cpu-a.S /home/thomas/projets/buildroot/output/host/usr/bin/arm-linux-as: unrecognized option '-_LARGEFILE_SOURCE'

This is caused by the following code in the configure script:

if [ $asm = auto -a $ARCH = ARM ] ; then

    # set flags so neon is built by default
    echo $CFLAGS | grep -Eq '(-mcpu|-march|-mfpu)' || CFLAGS="$CFLAGS -mcpu=cortex-a8 -mfpu=neon"

if cc_check '' '' '__asm__("rev ip, ip");' ; then define HAVE_ARMV6
cc_check '' '' '__asm__("movt r0, #0");' && define HAVE_ARMV6T2
cc_check '' '' '__asm__("vadd.i16 q0, q0, q0");' && define HAVE_NEON ASFLAGS="$ASFLAGS $CFLAGS -c"
else
echo "You specified a pre-ARMv6 or Thumb-1 CPU in your CFLAGS." echo "If you really want to run on such a CPU, configure with --disable-asm." exit 1
fi
fi

When you're on ARM, CFLAGS is added to ASFLAGS. However, the CFLAGS passed by Buildroot contain options that are not suitable for AS. Basically, what x264 is doing here is completely stupid: assuming that all compiler flags can be passed as assembler flags.

Also, the "else" condition above is interesting: the assembly code is only valid for ARMv6+. So in your x264.mk file, the following snippet is wrong:

ifeq ($(BR2_arm)$(BR2_i386)$(BR2_powerpc)$(BR2_x86_64),)

        X264_CONF_OPTS += --disable-asm

endif

Because if you're on ARMv5, we will *not* pass --disable-asm, while we should. So this logic should be refined to take into account variants of the ARM architecture.

Could you look into this and send an updated version?

Thanks a lot!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

David du Colombier

author

2014-10-05 16:26 UTC #3 lore raw

PATCH v3 1/2 x264: new package

Quoted 8 lines

package/x264/Config.in line 16
Quoted 5 lines
package/x264/Config.in new file +5 -0
+if BR2_PACKAGE_X264
+
+config BR2_PACKAGE_X264_CLI
+	bool "CLI"
+	default n
default n is not needed, as the default for a boolean option is to be disabled.

Done.

Quoted 4 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+	help
+	  Command Line Interface
Maybe more text would be needed here.

Done.

Quoted 4 lines

When you're on ARM, CFLAGS is added to ASFLAGS. However, the CFLAGS passed by Buildroot contain options that are not suitable for AS. Basically, what x264 is doing here is completely stupid: assuming that all compiler flags can be passed as assembler flags.

It seems this issue has already been fixed in the x264 master branch.

http://git.videolan.org/gitweb.cgi/x264.git/?a=commit;h=716ee56d0b35e512e8e0ae1a3e71f26e65e86be3

I've imported the patch in the x264 package.

Quoted 11 lines

Also, the "else" condition above is interesting: the assembly code is only valid for ARMv6+. So in your x264.mk file, the following snippet is wrong:
ifeq ($(BR2_arm)$(BR2_i386)$(BR2_powerpc)$(BR2_x86_64),)
        X264_CONF_OPTS += --disable-asm
endif
Because if you're on ARMv5, we will *not* pass --disable-asm, while we should. So this logic should be refined to take into account variants of the ARM architecture.

It seems the ARM assembly files aren't compatible with the GNU assembler, so I finally chose to enable assembly files on x86 only.

Quoted 1 line

Could you look into this and send an updated version?

I'll send a v4 patch.

Signature · 1 line

David du Colombier