Skip to contentSkip to conversation

vlc: New package

PATCH Superseded Ismael Luceno 3 participants 5 messages Mar 22, 2012 – Jul 16, 2012

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v5 1/1 vlc: New package

---
Changes since v4:
* Fixed VLC_CONF_OPT variable name
* Added missing ffmpeg-related disable options
* Added missing jack disable flag

Changes since v3:
* Removed scm-version bootstrapping support

Changes since v2:
* Replaced multi-line assignments to VLC_CONF_OPTS with individual ones for each line.
* Split additional commands required to build into macros and added them to the appropriate hook. Also added comments.

Changes since v1:
* Added a description for the package
* Described and signed-off the patch
* Added configuration knobs for all available dependencies and other build options.

 package/multimedia/Config.in                  |    1 +
 package/multimedia/vlc/Config.in              |  286 +++++++++++++++++++
 package/multimedia/vlc/vlc-uclibc-fixes.patch |   25 ++
 package/multimedia/vlc/vlc.mk                 |  365 +++++++++++++++++++++++++
 4 files changed, 677 insertions(+)
 create mode 100644 package/multimedia/vlc/Config.in
 create mode 100644 package/multimedia/vlc/vlc-uclibc-fixes.patch
 create mode 100644 package/multimedia/vlc/vlc.mk
package/multimedia/Config.in +1 -0
index a05d1d9..fc20d74 100644
@@ -44,6 +44,7 @@ source "package/multimedia/speex/Config.in"
 source "package/multimedia/tidsp-binaries/Config.in"
 source "package/multimedia/taglib/Config.in"
 source "package/multimedia/tremor/Config.in"
+source "package/multimedia/vlc/Config.in"
 source "package/multimedia/vorbis-tools/Config.in"
 source "package/multimedia/wavpack/Config.in"
 endmenu
package/multimedia/vlc/Config.in new file +286 -0

286 lines not on this page · raw message

package/multimedia/vlc/vlc-uclibc-fixes.patch new file +25 -0
new file mode 100644
index 0000000..5471b08
@@ -0,0 +1,25 @@
+Fixes compilation with uClibc, which doesn't provide gnu/libc-version.h nor
+gnu_get_libc_version.
+
+Signed-off-by: Ismael Luceno <[email protected]>
+
+diff -u a/src/posix/linux_specific.c b/src/posix/linux_specific.c
+--- a/src/posix/linux_specific.c	2012-02-08 16:43:30.000000000 -0200
++++ b/src/posix/linux_specific.c	2012-02-09 03:51:20.925504594 -0200
+@@ -74,14 +74,14 @@
+     return (path != NULL) ? path : strdup (PKGLIBDIR);
+ }
+ 
+-#ifdef __GLIBC__
++#if defined(__GLIBC__) && !defined(__UCLIBC__)
+ # include <gnu/libc-version.h>
+ # include <stdlib.h>
+ #endif
+ 
+ void system_Init (void)
+ {
+-#ifdef __GLIBC__
++#if defined(__GLIBC__) && !defined(__UCLIBC__)
+     const char *glcv = gnu_get_libc_version ();
+ 
+     /* gettext in glibc 2.5-2.7 is not thread-safe. LibVLC keeps crashing,
package/multimedia/vlc/vlc.mk new file +365 -0

365 lines not on this page · raw message

Signature · 1 line
1.7.9.4
#1 lore raw

PATCH v5 1/1 vlc: New package

Quoted 1 line · On Thursday 22 March 2012 02:54:20 Ismael Luceno wrote:

Not done yet, unfortunately... But for a big patch like this that's
to be expected.

If you're in a hurry to get this integrated, it may be worthwhile to
split the patch into a basic one that includes a minimal feature set, and an extension that adds all the config options for the different components.

You should also test it with a minimal internal toolchain, i.e.
without any largefile, wchar and gettext stuff. That will expose a great number of errors right away. For instance, the v4l2 stuff fails because of LARGEFILE.

[snip]

package/multimedia/vlc/Config.in line 39

Quoted 3 lines
package/multimedia/vlc/Config.in new file +3 -0
+config BR2_PACKAGE_VLC_PULSE
+       bool "PulseAudio support"
+       select BR2_PACKAGE_PULSEAUDIO

pulseaudio depends on BR2_USE_WCHAR, so this dependency should be
repeated here. And there should be a comment explaining that WCHAR is needed. Like this:

config BR2_PACKAGE_VLC_PULSE
bool "PulseAudio support"
select BR2_PACKAGE_PULSEAUDIO
depends on BR2_USE_WCHAR # pulseaudio

comment "PulseAudio support requires a toolchain with WCHAR support"
depends on !BR2_USE_WCHAR

You should check for such a situation for each package for which you
add a select statement. There are quite a few more like that, I've checked it for just this one. I mention a few more below, but there are probably even more.

Also, it would be nice to have a short help text for each option.
Doesn't have to be much, something like "pulseaudio is a proxy between the audio hardware and audio applications. See
http://pulseaudio.org" is sufficient. But I realize it's a lot of effort to write help texts for all these options.

[snip]

package/multimedia/vlc/Config.in line 113

Quoted 6 lines
package/multimedia/vlc/Config.in new file +6 -0
+config BR2_PACKAGE_VLC_AVCODEC
+       bool "FFmpeg (Many CODECs)"
+       select BR2_PACKAGE_FFMPEG
+       help
+	 There is an awful lot of CODECs in libavcodec. You might want to avoid
+	 duplication.

This help text isn't very clear. How about:

There is a large number of codecs in FFmpeg/libavcodec. It is advisable to avoid duplication: select either the FFmpeg or the vlc implementation for each codec, not both.

[snip]

package/multimedia/vlc/Config.in line 187

Quoted 3 lines
package/multimedia/vlc/Config.in new file +3 -0
+config BR2_PACKAGE_VLC_GVFS
+       bool "Gnome VFS support"
+       select BR2_PACKAGE_GFVS

Typo here: _GVFS. And missing depends on BR2_LARGEFILE and
BR2_USE_WCHAR.

package/multimedia/vlc/Config.in line 191

Quoted 3 lines
package/multimedia/vlc/Config.in new file +3 -0
+config BR2_PACKAGE_VLC_V4L2
+       bool "Video4Linux 2 support"
+       select BR2_PACKAGE_LIBV4L

depends on BR2_LARGEFILE

[snip]

package/multimedia/vlc/Config.in line 254

Quoted 3 lines
package/multimedia/vlc/Config.in new file +3 -0
+config BR2_PACKAGE_VLC_UDEV
+       bool "Udev support"
+       select BR2_PACKAGE_UDEV

udev is a special case, because it depends on the /dev management
option BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV. You can't use a select for that one (it would be too confusing). So just set a depends on BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV. The comment is not needed in this case, because it's quite obvious that udev support only makes sense if you have udev.

[snip]

package/multimedia/vlc/vlc-uclibc-fixes.patch line 4

Quoted 10 lines
package/multimedia/vlc/vlc-uclibc-fixes.patch new file +4 -0
new file mode 100644
index 0000000..5471b08
@@ -0,0 +1,25 @@
+Fixes compilation with uClibc, which doesn't provide gnu/libc-version.h nor
+gnu_get_libc_version.
+
+Signed-off-by: Ismael Luceno <[email protected]>

Did you send this patch upstream to vlc? If not, could you do so?

[snip]

package/multimedia/vlc/vlc-uclibc-fixes.patch line 5

Quoted 1 line
package/multimedia/vlc/vlc-uclibc-fixes.patch new file +1 -0
+VLC_CONF_OPT += $(foreach i,avcodec avformat postproc swscale,--disable-$i)

Nice!

[snip]

package/multimedia/vlc/vlc-uclibc-fixes.patch line 10

Quoted 5 lines
package/multimedia/vlc/vlc-uclibc-fixes.patch new file +5 -0
+# The configure script fails to detect vasprintf on uClibc.
+VLC_POST_CONFIGURE_HOOKS += VLC_FIX_CONF
+define VLC_FIX_CONF
+	$(SED) '1i#define HAVE_VASPRINTF 1' $(@D)/config.h
+endef

Nice! Maybe remove the existing HAVE_VASPRINTF line too:

$(SED) '/HAVE_VASPRINTF/d; 1i#define HAVE_VASPRINTF 1' $(@D)/config.h

Also, it's a bit more natural to put the
VLC_POST_CONFIGURE_HOOKS += VLC_FIX_CONF
after the definition of VLC_FIX_CONF. But it doesn't make a difference, of course.

package/multimedia/vlc/vlc-uclibc-fixes.patch line 11

Quoted 1 line
package/multimedia/vlc/vlc-uclibc-fixes.patch new file +1 -0
+# The following rmoves the relink_command. If not removed, libtool tries to
                   ^^^removes

package/multimedia/vlc/vlc-uclibc-fixes.patch line 15

Quoted 4 lines
package/multimedia/vlc/vlc-uclibc-fixes.patch new file +4 -0
+# relink against the host libraries, instead of the sysroot.
+VLC_POST_BUILD_HOOKS += VLC_FIX_LA_FILES
+define VLC_FIX_LA_FILES
+	find $(@D) -name '*.la' -exec $(SED) '/^relink_command=/d' '{}' +

For which .la's is this needed? I've compiled various configurations with
several toolchains and I don't see a relink_command in any of the .la files...

package/multimedia/vlc/vlc-uclibc-fixes.patch line 17

Quoted 2 lines
package/multimedia/vlc/vlc-uclibc-fixes.patch new file +2 -0
+	touch $(@D)/modules/access/zip/libzip_plugin.la \
+		$(@D)/modules/misc/liblogger_plugin.la

And why is this needed?

package/multimedia/vlc/vlc-uclibc-fixes.patch line 20

Quoted 3 lines
package/multimedia/vlc/vlc-uclibc-fixes.patch new file +3 -0
+endef
+
+$(eval $(call AUTOTARGETS))

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#2 lore raw

PATCH v5 1/1 vlc: New package

Quoted 5 lines · On Sun, 25 Mar 2012 21:48:57 +0200 Arnout Vandecappelle <[email protected]> wrote:
Quoted 1 line · On Thursday 22 March 2012 02:54:20 Ismael Luceno wrote:

Not done yet, unfortunately... But for a big patch like this that's
to be expected.

I know.

Quoted 4 lines

If you're in a hurry to get this integrated, it may be worthwhile to
split the patch into a basic one that includes a minimal feature set, and an extension that adds all the config options for the different components.

I would prefer to get it integrated as a whole. Sooner is better, but I would like to get it right once and forever.

<...>

Quoted 6 lines

udev is a special case, because it depends on the /dev management
option BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV. You can't use a select for that one (it would be too confusing). So just set a depends on BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV. The comment is not needed in this case, because it's quite obvious that udev support only makes sense if you have udev.

Hmmm... that sounds a bit confusing... the dependency is actually on libudev, but does that necessarily mean BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_UDEV?

Quoted 4 lines
Quoted 4 lines
(quoted diff) +0 -0

b/package/multimedia/vlc/vlc-uclibc-fixes.patch new file mode 100644 index 0000000..5471b08
--- /dev/null

Quoted 8 lines

package/multimedia/vlc/vlc-uclibc-fixes.patch line 4

Quoted 6 lines

+++ b/package/multimedia/vlc/vlc-uclibc-fixes.patch

(quoted diff) +1 -0
@@ -0,0 +1,25 @@
+Fixes compilation with uClibc, which doesn't provide

gnu/libc-version.h nor +gnu_get_libc_version.

package/multimedia/vlc/vlc-uclibc-fixes.patch new file +2 -0
+
+Signed-off-by: Ismael Luceno <[email protected]>

Did you send this patch upstream to vlc? If not, could you do so?

Yes, I sent it but was rejected, they consider it a bug in uClibc. I'll add a comment on the patch.

Quoted 12 lines
Quoted 2 lines

+# The following rmoves the relink_command. If not removed, libtool tries to

                   ^^^removes
Quoted 5 lines
package/multimedia/vlc/vlc.mk new file +4 -0
+# relink against the host libraries, instead of the sysroot.
+VLC_POST_BUILD_HOOKS += VLC_FIX_LA_FILES
+define VLC_FIX_LA_FILES
+	find $(@D) -name '*.la' -exec $(SED) '/^relink_command=/d'

'{}' +

For which .la's is this needed? I've compiled various
configurations with several toolchains and I don't see a relink_command in any of the .la files...

Well, in the scm version I was using, for all them, but perhaps that somehow changes when compiling from a tarball... strange...

Quoted 5 lines

package/multimedia/vlc/vlc.mk line 362

Quoted 3 lines
package/multimedia/vlc/vlc.mk new file +2 -0
+	touch $(@D)/modules/access/zip/libzip_plugin.la \
+		$(@D)/modules/misc/liblogger_plugin.la

And why is this needed?

Because those depend on other .la files, and otherwise get re-built, thus add the relink_command again...

Thanks for the suggestions. I'll try to address the issues and send a
new patch soon :).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20120327/6f8843ce/attachment.asc>

#3 lore raw

PATCH v5 1/1 vlc: New package

Hello,

Le Tue, 27 Mar 2012 21:36:10 -0300,
Ismael Luceno <[email protected]> a ?crit :

Quoted 2 lines

Thanks for the suggestions. I'll try to address the issues and send a new patch soon :).

In the end, do you plan to post an updated patch?

Thanks,

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#4 lore raw

PATCH v5 1/1 vlc: New package

Quoted 1 line · Thomas Petazzoni

Hello,

Hi.

Quoted 7 lines

Le Tue, 27 Mar 2012 21:36:10 -0300,
Ismael Luceno <[email protected]> a ?crit :

Quoted 2 lines

Thanks for the suggestions. I'll try to address the issues and send a new patch soon :).

In the end, do you plan to post an updated patch?

I've had no time to work on it yet, but it's on my to-do list...