All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/3] ffmpeg: disable for ARMv7-M
@ 2016-08-10 21:29 Thomas Petazzoni
  2016-08-10 21:29 ` [Buildroot] [PATCH 2/3] ffmpeg: explicitly disable NEON support Thomas Petazzoni
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Thomas Petazzoni @ 2016-08-10 21:29 UTC (permalink / raw)
  To: buildroot

In the ffmpeg code, libavutil/arm/asm.S makes some assumption about the
ARM architecture for which the code is built. Only ARMv4, ARMv5, ARMv6
and ARMv7-A is supported. Due to this, object files built out of
ARM-optimized assembly code have the wrong architecture information,
causing a failure at link time.

Adding ARMv7-M support would be possible, but it doesn't exist yet in
ffmpeg, and it's pretty unlikely that ffmpeg will ever be needed on an
ARMv7-M platform, so this commit takes the simple approach of disabling
ffmpeg for ARMv7-M.

Fixes:

  http://autobuild.buildroot.net/results/ca4c67b093afd6f14349fcdc87b02e0480172e8c/

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/ffmpeg/Config.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/package/ffmpeg/Config.in b/package/ffmpeg/Config.in
index 096ff95..3a7bfd3 100644
--- a/package/ffmpeg/Config.in
+++ b/package/ffmpeg/Config.in
@@ -2,7 +2,9 @@ config BR2_PACKAGE_FFMPEG_ARCH_SUPPORTS
 	bool
 	# fenv.h lacks FE_INVALID, FE_OVERFLOW & FE_UNDERFLOW on nios2
 	# ffmpeg's configure script only supports mips64 (r1) variant
-	default y if !BR2_nios2 && !BR2_mips_64r2 && !BR2_mips_64r6
+	# No support for ARMv7-M in the ARM assembly logic
+	default y if !BR2_nios2 && !BR2_mips_64r2 && !BR2_mips_64r6 && \
+		!BR2_ARM_CPU_ARMV7M
 
 menuconfig BR2_PACKAGE_FFMPEG
 	bool "ffmpeg"
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 2/3] ffmpeg: explicitly disable NEON support
  2016-08-10 21:29 [Buildroot] [PATCH 1/3] ffmpeg: disable for ARMv7-M Thomas Petazzoni
@ 2016-08-10 21:29 ` Thomas Petazzoni
  2016-08-12  5:30   ` Khem Raj
  2016-08-16 22:01   ` Peter Korsgaard
  2016-08-10 21:29 ` [Buildroot] [PATCH 3/3] ffmpeg: don't pass --enable-mipsfpu on non-MIPS platform Thomas Petazzoni
  2016-08-11 13:03 ` [Buildroot] [PATCH 1/3] ffmpeg: disable for ARMv7-M Thomas Petazzoni
  2 siblings, 2 replies; 7+ messages in thread
From: Thomas Petazzoni @ 2016-08-10 21:29 UTC (permalink / raw)
  To: buildroot

When BR2_ARM_CPU_HAS_NEON=y, we pass --enable-neon. However, when
BR2_ARM_CPU_HAS_NEON is disabled, we don't pass anything. This generally
works fine, but turned out to integrate NEON code in ARMv7-M
builds (since it's ARMv7, ffmpeg assumed it should enable NEON code).

Even though ffmpeg is now disabled for ARMv7-M, it still makes sense to
be explicit, and disable NEON support when the CPU doesn't have it.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/ffmpeg/ffmpeg.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/ffmpeg/ffmpeg.mk b/package/ffmpeg/ffmpeg.mk
index c42336f..2b4219a 100644
--- a/package/ffmpeg/ffmpeg.mk
+++ b/package/ffmpeg/ffmpeg.mk
@@ -437,6 +437,8 @@ FFMPEG_CONF_OPTS += --disable-vfp
 endif
 ifeq ($(BR2_ARM_CPU_HAS_NEON),y)
 FFMPEG_CONF_OPTS += --enable-neon
+else
+FFMPEG_CONF_OPTS += --disable-neon
 endif
 
 ifeq ($(BR2_MIPS_SOFT_FLOAT),y)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 3/3] ffmpeg: don't pass --enable-mipsfpu on non-MIPS platform
  2016-08-10 21:29 [Buildroot] [PATCH 1/3] ffmpeg: disable for ARMv7-M Thomas Petazzoni
  2016-08-10 21:29 ` [Buildroot] [PATCH 2/3] ffmpeg: explicitly disable NEON support Thomas Petazzoni
@ 2016-08-10 21:29 ` Thomas Petazzoni
  2016-08-16 22:02   ` Peter Korsgaard
  2016-08-11 13:03 ` [Buildroot] [PATCH 1/3] ffmpeg: disable for ARMv7-M Thomas Petazzoni
  2 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2016-08-10 21:29 UTC (permalink / raw)
  To: buildroot

The current logic to pass the --{enable,disable}-mipsfpu option is:

ifeq ($(BR2_MIPS_SOFT_FLOAT),y)
FFMPEG_CONF_OPTS += --disable-mipsfpu
else
FFMPEG_CONF_OPTS += --enable-mipsfpu
endif

In practice, this means that on MIPS soft-float, --disable-mipsfpu is
passed, and that in *all* other cases, --enable-mipsfpu is passed,
including if we are *not* targetting the MIPS architecture.

Even though this doesn't seem to cause any problem, it is a bit weird to
see --enable-mipsfpu when you're building ffmpeg for a non-MIPS
architecture, so we better fix this by enclosing the MIPS-related
options in a MIPS condition.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/ffmpeg/ffmpeg.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/ffmpeg/ffmpeg.mk b/package/ffmpeg/ffmpeg.mk
index 2b4219a..5a19617 100644
--- a/package/ffmpeg/ffmpeg.mk
+++ b/package/ffmpeg/ffmpeg.mk
@@ -441,6 +441,7 @@ else
 FFMPEG_CONF_OPTS += --disable-neon
 endif
 
+ifeq ($(BR2_mips)$(BR2_mipsel)$(BR2_mips64)$(BR2_mips64el),y)
 ifeq ($(BR2_MIPS_SOFT_FLOAT),y)
 FFMPEG_CONF_OPTS += --disable-mipsfpu
 else
@@ -454,6 +455,7 @@ else
 FFMPEG_CONF_OPTS += \
 	--disable-mips32r2
 endif
+endif # MIPS
 
 ifeq ($(BR2_POWERPC_CPU_HAS_ALTIVEC),y)
 FFMPEG_CONF_OPTS += --enable-altivec
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 1/3] ffmpeg: disable for ARMv7-M
  2016-08-10 21:29 [Buildroot] [PATCH 1/3] ffmpeg: disable for ARMv7-M Thomas Petazzoni
  2016-08-10 21:29 ` [Buildroot] [PATCH 2/3] ffmpeg: explicitly disable NEON support Thomas Petazzoni
  2016-08-10 21:29 ` [Buildroot] [PATCH 3/3] ffmpeg: don't pass --enable-mipsfpu on non-MIPS platform Thomas Petazzoni
@ 2016-08-11 13:03 ` Thomas Petazzoni
  2 siblings, 0 replies; 7+ messages in thread
From: Thomas Petazzoni @ 2016-08-11 13:03 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 10 Aug 2016 23:29:02 +0200, Thomas Petazzoni wrote:
> In the ffmpeg code, libavutil/arm/asm.S makes some assumption about the
> ARM architecture for which the code is built. Only ARMv4, ARMv5, ARMv6
> and ARMv7-A is supported. Due to this, object files built out of
> ARM-optimized assembly code have the wrong architecture information,
> causing a failure at link time.
> 
> Adding ARMv7-M support would be possible, but it doesn't exist yet in
> ffmpeg, and it's pretty unlikely that ffmpeg will ever be needed on an
> ARMv7-M platform, so this commit takes the simple approach of disabling
> ffmpeg for ARMv7-M.
> 
> Fixes:
> 
>   http://autobuild.buildroot.net/results/ca4c67b093afd6f14349fcdc87b02e0480172e8c/
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  package/ffmpeg/Config.in | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied to master, thanks.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 2/3] ffmpeg: explicitly disable NEON support
  2016-08-10 21:29 ` [Buildroot] [PATCH 2/3] ffmpeg: explicitly disable NEON support Thomas Petazzoni
@ 2016-08-12  5:30   ` Khem Raj
  2016-08-16 22:01   ` Peter Korsgaard
  1 sibling, 0 replies; 7+ messages in thread
From: Khem Raj @ 2016-08-12  5:30 UTC (permalink / raw)
  To: buildroot


> On Aug 10, 2016, at 2:29 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> 
> When BR2_ARM_CPU_HAS_NEON=y, we pass --enable-neon. However, when
> BR2_ARM_CPU_HAS_NEON is disabled, we don't pass anything. This generally
> works fine, but turned out to integrate NEON code in ARMv7-M
> builds (since it's ARMv7, ffmpeg assumed it should enable NEON code).
> 
> Even though ffmpeg is now disabled for ARMv7-M, it still makes sense to
> be explicit, and disable NEON support when the CPU doesn't have it.

Just an aside. You can add ?enable-thumb when the default ISA
chosen is thumb

> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> package/ffmpeg/ffmpeg.mk | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/package/ffmpeg/ffmpeg.mk b/package/ffmpeg/ffmpeg.mk
> index c42336f..2b4219a 100644
> --- a/package/ffmpeg/ffmpeg.mk
> +++ b/package/ffmpeg/ffmpeg.mk
> @@ -437,6 +437,8 @@ FFMPEG_CONF_OPTS += --disable-vfp
> endif
> ifeq ($(BR2_ARM_CPU_HAS_NEON),y)
> FFMPEG_CONF_OPTS += --enable-neon
> +else
> +FFMPEG_CONF_OPTS += --disable-neon
> endif
> 
> ifeq ($(BR2_MIPS_SOFT_FLOAT),y)
> --
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 204 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160811/e67fffad/attachment.asc>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 2/3] ffmpeg: explicitly disable NEON support
  2016-08-10 21:29 ` [Buildroot] [PATCH 2/3] ffmpeg: explicitly disable NEON support Thomas Petazzoni
  2016-08-12  5:30   ` Khem Raj
@ 2016-08-16 22:01   ` Peter Korsgaard
  1 sibling, 0 replies; 7+ messages in thread
From: Peter Korsgaard @ 2016-08-16 22:01 UTC (permalink / raw)
  To: buildroot

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > When BR2_ARM_CPU_HAS_NEON=y, we pass --enable-neon. However, when
 > BR2_ARM_CPU_HAS_NEON is disabled, we don't pass anything. This generally
 > works fine, but turned out to integrate NEON code in ARMv7-M
 > builds (since it's ARMv7, ffmpeg assumed it should enable NEON code).

 > Even though ffmpeg is now disabled for ARMv7-M, it still makes sense to
 > be explicit, and disable NEON support when the CPU doesn't have it.

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Committed, thanks.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 3/3] ffmpeg: don't pass --enable-mipsfpu on non-MIPS platform
  2016-08-10 21:29 ` [Buildroot] [PATCH 3/3] ffmpeg: don't pass --enable-mipsfpu on non-MIPS platform Thomas Petazzoni
@ 2016-08-16 22:02   ` Peter Korsgaard
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Korsgaard @ 2016-08-16 22:02 UTC (permalink / raw)
  To: buildroot

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > The current logic to pass the --{enable,disable}-mipsfpu option is:
 > ifeq ($(BR2_MIPS_SOFT_FLOAT),y)
 > FFMPEG_CONF_OPTS += --disable-mipsfpu
 > else
 > FFMPEG_CONF_OPTS += --enable-mipsfpu
 > endif

 > In practice, this means that on MIPS soft-float, --disable-mipsfpu is
 > passed, and that in *all* other cases, --enable-mipsfpu is passed,
 > including if we are *not* targetting the MIPS architecture.

 > Even though this doesn't seem to cause any problem, it is a bit weird to
 > see --enable-mipsfpu when you're building ffmpeg for a non-MIPS
 > architecture, so we better fix this by enclosing the MIPS-related
 > options in a MIPS condition.

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Committed, thanks.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-08-16 22:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-10 21:29 [Buildroot] [PATCH 1/3] ffmpeg: disable for ARMv7-M Thomas Petazzoni
2016-08-10 21:29 ` [Buildroot] [PATCH 2/3] ffmpeg: explicitly disable NEON support Thomas Petazzoni
2016-08-12  5:30   ` Khem Raj
2016-08-16 22:01   ` Peter Korsgaard
2016-08-10 21:29 ` [Buildroot] [PATCH 3/3] ffmpeg: don't pass --enable-mipsfpu on non-MIPS platform Thomas Petazzoni
2016-08-16 22:02   ` Peter Korsgaard
2016-08-11 13:03 ` [Buildroot] [PATCH 1/3] ffmpeg: disable for ARMv7-M Thomas Petazzoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.