diff mbox

[1/2] gst1-libav: switch to unbundled

Message ID 1459366600-21925-1-git-send-email-gustavo@zacarias.com.ar
State Accepted
Headers show

Commit Message

Gustavo Zacarias March 30, 2016, 7:36 p.m. UTC
It avoids the crazy in handling all of the ffmpeg options here as well,
and potentially avoids target code duplication, hence resulting in a
size reduction.

Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 package/gstreamer1/gst1-libav/Config.in     |  2 +
 package/gstreamer1/gst1-libav/gst1-libav.mk | 79 ++---------------------------
 2 files changed, 7 insertions(+), 74 deletions(-)

Comments

Arnout Vandecappelle March 30, 2016, 8:15 p.m. UTC | #1
On 03/30/16 21:36, Gustavo Zacarias wrote:
> It avoids the crazy in handling all of the ffmpeg options here as well,
> and potentially avoids target code duplication, hence resulting in a
> size reduction.
>
> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
> ---
>   package/gstreamer1/gst1-libav/Config.in     |  2 +
>   package/gstreamer1/gst1-libav/gst1-libav.mk | 79 ++---------------------------
>   2 files changed, 7 insertions(+), 74 deletions(-)
>
> diff --git a/package/gstreamer1/gst1-libav/Config.in b/package/gstreamer1/gst1-libav/Config.in
> index 4f9c2b8..8ef3e89 100644
> --- a/package/gstreamer1/gst1-libav/Config.in
> +++ b/package/gstreamer1/gst1-libav/Config.in
> @@ -1,5 +1,7 @@
>   config BR2_PACKAGE_GST1_LIBAV
>   	bool "gst1-libav"
> +	select BR2_PACKAGE_FFMPEG
> +	select BR2_PACKAGE_FFMPEG_SWSCALE
>   	select BR2_PACKAGE_GST1_PLUGINS_BASE
>   	help
>   	  GStreamer plugin (formerly gst-ffmpeg).

  It would be good to extend the help text with something like:

	  The codecs, muxers, bitstreams and filters to build can be
	  selected in the ffmpeg package.


  Also, it's a bit unfortunate that if ffmpeg is selected automatically, 
BR2_PACKAGE_FFMPEG_FFMPEG will be enabled by default. Well, the binary is pretty 
small compared to all the codecs so it doesn't matter much.

> diff --git a/package/gstreamer1/gst1-libav/gst1-libav.mk b/package/gstreamer1/gst1-libav/gst1-libav.mk
> index 8f892d0..02ac394 100644
> --- a/package/gstreamer1/gst1-libav/gst1-libav.mk
> +++ b/package/gstreamer1/gst1-libav/gst1-libav.mk
> @@ -7,79 +7,10 @@
>   GST1_LIBAV_VERSION = 1.8.0
>   GST1_LIBAV_SOURCE = gst-libav-$(GST1_LIBAV_VERSION).tar.xz
>   GST1_LIBAV_SITE = http://gstreamer.freedesktop.org/src/gst-libav
> -GST1_LIBAV_DEPENDENCIES = host-pkgconf gstreamer1 gst1-plugins-base
> -GST1_LIBAV_CONF_EXTRA_OPTS = --cross-prefix=$(TARGET_CROSS) --target-os=linux
> -
> -ifeq ($(BR2_PACKAGE_ZLIB),y)
> -GST1_LIBAV_CONF_EXTRA_OPTS += --enable-zlib
> -GST1_LIBAV_DEPENDENCIES += zlib
> -else
> -GST1_LIBAV_CONF_EXTRA_OPTS += --disable-zlib
> -endif
> -
> -ifeq ($(BR2_PACKAGE_BZIP2),y)
> -GST1_LIBAV_CONF_EXTRA_OPTS += --enable-bzlib
> -GST1_LIBAV_DEPENDENCIES += bzip2
> -else
> -GST1_LIBAV_CONF_EXTRA_OPTS += --disable-bzlib
> -endif

  ext/libav/Makefile.am contains a condition on HAVE_BZ2, so I think this should 
be kept.

  And while you're at it: there is an optional dependency on xz (HAVE_LZMA) so 
it would be nice to add that too (separate patch).


[Huge snip, yeah!]

> +GST1_LIBAV_CONF_OPTS = --with-system-libav
> +GST1_LIBAV_DEPENDENCIES = \
> +	host-pkgconf ffmpeg gstreamer1 gst1-plugins-base \
> +	$(if $(BR2_PACKAGE_BZIP2),bzip2) \
> +	$(if $(BR2_PACKAGE_XZ),xz)

  Oops, here it is!

  Small nit: I prefer a single dependency per line, it's a bit more verbose but 
easier to read IMHO.


  Since all my comments are actually addressed already:

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

  Regards,
  Arnout

>
>   $(eval $(autotools-package))
>
Bernd Kuhls March 30, 2016, 8:31 p.m. UTC | #2
Arnout Vandecappelle <arnout@mind.be> wrote in
news:56FC33CA.1010502@mind.be: 

> Also, it's a bit unfortunate that if ffmpeg is selected automatically,
> BR2_PACKAGE_FFMPEG_FFMPEG will be enabled by default. Well, the binary
> is pretty  small compared to all the codecs so it doesn't matter much.

Hi,

what about not enabling BR2_PACKAGE_FFMPEG_FFMPEG by default?

Regards, Bernd
Thomas Petazzoni March 31, 2016, 1:45 a.m. UTC | #3
Hello,

On Wed, 30 Mar 2016 16:36:39 -0300, Gustavo Zacarias wrote:
> It avoids the crazy in handling all of the ffmpeg options here as well,
> and potentially avoids target code duplication, hence resulting in a
> size reduction.
> 
> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
> ---
>  package/gstreamer1/gst1-libav/Config.in     |  2 +
>  package/gstreamer1/gst1-libav/gst1-libav.mk | 79 ++---------------------------
>  2 files changed, 7 insertions(+), 74 deletions(-)

Applied to master, after tweaking the help text as suggested by Arnout.
Thanks!

Thomas
diff mbox

Patch

diff --git a/package/gstreamer1/gst1-libav/Config.in b/package/gstreamer1/gst1-libav/Config.in
index 4f9c2b8..8ef3e89 100644
--- a/package/gstreamer1/gst1-libav/Config.in
+++ b/package/gstreamer1/gst1-libav/Config.in
@@ -1,5 +1,7 @@ 
 config BR2_PACKAGE_GST1_LIBAV
 	bool "gst1-libav"
+	select BR2_PACKAGE_FFMPEG
+	select BR2_PACKAGE_FFMPEG_SWSCALE
 	select BR2_PACKAGE_GST1_PLUGINS_BASE
 	help
 	  GStreamer plugin (formerly gst-ffmpeg).
diff --git a/package/gstreamer1/gst1-libav/gst1-libav.mk b/package/gstreamer1/gst1-libav/gst1-libav.mk
index 8f892d0..02ac394 100644
--- a/package/gstreamer1/gst1-libav/gst1-libav.mk
+++ b/package/gstreamer1/gst1-libav/gst1-libav.mk
@@ -7,79 +7,10 @@ 
 GST1_LIBAV_VERSION = 1.8.0
 GST1_LIBAV_SOURCE = gst-libav-$(GST1_LIBAV_VERSION).tar.xz
 GST1_LIBAV_SITE = http://gstreamer.freedesktop.org/src/gst-libav
-GST1_LIBAV_DEPENDENCIES = host-pkgconf gstreamer1 gst1-plugins-base
-GST1_LIBAV_CONF_EXTRA_OPTS = --cross-prefix=$(TARGET_CROSS) --target-os=linux
-
-ifeq ($(BR2_PACKAGE_ZLIB),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --enable-zlib
-GST1_LIBAV_DEPENDENCIES += zlib
-else
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-zlib
-endif
-
-ifeq ($(BR2_PACKAGE_BZIP2),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --enable-bzlib
-GST1_LIBAV_DEPENDENCIES += bzip2
-else
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-bzlib
-endif
-
-# Generic CPU setup for bundled ffmpeg
-ifneq ($(call qstrip,$(BR2_GCC_TARGET_CPU)),)
-GST1_LIBAV_CONF_EXTRA_OPTS += --cpu=$(BR2_GCC_TARGET_CPU)
-else ifneq ($(call qstrip,$(BR2_GCC_TARGET_ARCH)),)
-GST1_LIBAV_CONF_EXTRA_OPTS += --cpu=$(BR2_GCC_TARGET_ARCH)
-endif
-
-ifeq ($(BR2_X86_CPU_HAS_MMX),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --enable-yasm
-GST1_LIBAV_DEPENDENCIES += host-yasm
-else
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-yasm
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-mmx
-endif
-
-ifeq ($(BR2_X86_CPU_HAS_SSE),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --enable-sse
-else
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-sse
-endif
-
-ifeq ($(BR2_X86_CPU_HAS_SSSE3),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --enable-ssse3
-else
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-ssse3
-endif
-
-# Explicitly disable everything that doesn't match for ARM
-# FFMPEG "autodetects" by compiling an extended instruction via AS
-# This works on compilers that aren't built for generic by default
-ifeq ($(BR2_ARM_CPU_ARMV4),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-armv5te
-endif
-ifeq ($(BR2_ARM_CPU_ARMV6)$(BR2_ARM_CPU_ARMV7A),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --enable-armv6
-else
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-armv6 --disable-armv6t2
-endif
-ifeq ($(BR2_ARM_CPU_HAS_NEON),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --enable-neon
-else
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-neon
-endif
-ifeq ($(BR2_ARM_CPU_HAS_VFPV2),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --enable-vfp
-else
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-vfp
-endif
-
-ifeq ($(BR2_POWERPC_CPU_HASH_ALTIVEC),y)
-GST1_LIBAV_CONF_EXTRA_OPTS += --enable-altivec
-else
-GST1_LIBAV_CONF_EXTRA_OPTS += --disable-altivec
-endif
-
-GST1_LIBAV_CONF_OPTS = \
-	--with-libav-extra-configure="$(GST1_LIBAV_CONF_EXTRA_OPTS)"
+GST1_LIBAV_CONF_OPTS = --with-system-libav
+GST1_LIBAV_DEPENDENCIES = \
+	host-pkgconf ffmpeg gstreamer1 gst1-plugins-base \
+	$(if $(BR2_PACKAGE_BZIP2),bzip2) \
+	$(if $(BR2_PACKAGE_XZ),xz)
 
 $(eval $(autotools-package))