Fix libdvdread and libdvdnav support

PATCH New Valentine Barshak 3 participants 12 messages Sep 26, 2012 – Oct 4, 2012 1 message not in the archive

Share & tools

Valentine Barshak

author

2012-09-26 19:31 UTC #0 lore raw

cover Fix libdvdread and libdvdnav support

The following patches fix dvdread-config and dvdnav-config scripts installed to staging directory and make mplayer use these libraries when they are enabled.

This is done to add DVD support to mplyaer.
These patches have been tested on an x86 machine.

Thanks,
Val.

Valentine Barshak

author

2012-09-26 19:33 UTC #1 lore raw

PATCH 1/3 libdvdread: Fix dvdread-config

This fixes path prefix in the staging dvdread-config and removes dvdread-config from the target filesystem if BR2_HAVE_DEVFILES is not set.

Signed-off-by Valentine Barshak <[email protected]>

---
 package/libdvdread/libdvdread.mk | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

package/libdvdread/libdvdread.mk +14 -0

index 6e17731..ee616f0 100644
@@ -11,4 +11,18 @@ LIBDVDREAD_AUTORECONF = YES
 LIBDVDREAD_LIBTOOL_PATCH = YES
 LIBDVDREAD_INSTALL_STAGING = YES
 
+define LIBDVDREAD_TARGET_CLEANUP
+	$(RM) -f $(TARGET_DIR)/usr/bin/dvdread-config
+endef
+
+ifneq ($(BR2_HAVE_DEVFILES),y)
+LIBDVDREAD_POST_INSTALL_TARGET_HOOKS += LIBDVDREAD_TARGET_CLEANUP
+endif
+
+define LIBDVDREAD_STAGING_FIXUP_DVDREAD_CONFIG
+	$(SED) "s,prefix=/usr,prefix=$(STAGING_DIR)/usr," $(STAGING_DIR)/usr/bin/dvdread-config
+endef
+
+LIBDVDREAD_POST_INSTALL_STAGING_HOOKS += LIBDVDREAD_STAGING_FIXUP_DVDREAD_CONFIG
+
 $(eval $(autotools-package))

Signature · 1 line

1.7.11.4

Valentine Barshak

author

2012-09-26 19:33 UTC #2 lore raw

PATCH 2/3 libdvdnav: Fix dvdnav-config

In reply to Valentine Barshak

This fixes path prefix in the staging dvdnav-config and removes dvdnav-config from the target filesystem if BR2_HAVE_DEVFILES is not set.

Signed-off-by Valentine Barshak <[email protected]>

---
 package/libdvdnav/libdvdnav.mk | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

package/libdvdnav/libdvdnav.mk +14 -0

index e2d42f3..2fefe43 100644
@@ -16,4 +16,18 @@ LIBDVDNAV_DEPENDENCIES = libdvdread host-pkg-config
 # of cross compilation, we prefer using pkg-config.
 LIBDVDNAV_CONF_OPT = --with-dvdread-config="$(PKG_CONFIG_HOST_BINARY) dvdread"
 
+define LIBDVDNAV_TARGET_CLEANUP
+	$(RM) -f $(TARGET_DIR)/usr/bin/dvdnav-config
+endef
+
+ifneq ($(BR2_HAVE_DEVFILES),y)
+LIBDVDNAV_POST_INSTALL_TARGET_HOOKS += LIBDVDNAV_TARGET_CLEANUP
+endif
+
+define LIBDVDNAV_STAGING_FIXUP_DVDNAV_CONFIG
+	$(SED) "s,prefix=/usr,prefix=$(STAGING_DIR)/usr," $(STAGING_DIR)/usr/bin/dvdnav-config
+endef
+
+LIBDVDNAV_POST_INSTALL_STAGING_HOOKS += LIBDVDNAV_STAGING_FIXUP_DVDNAV_CONFIG
+
 $(eval $(autotools-package))

Signature · 1 line

1.7.11.4

Thomas Petazzoni

2012-09-26 20:46 UTC #3 lore raw

PATCH 2/3 libdvdnav: Fix dvdnav-config

Dear Valentine Barshak,

package/libdvdnav/libdvdnav.mk LIBDVDNAV_DEPENDENCIES = libdvdread host-pkg-config · line 25

Quoted 3 lines · Valentine Barshak

package/libdvdnav/libdvdnav.mk +3 -0

+ifneq ($(BR2_HAVE_DEVFILES),y)
+LIBDVDNAV_POST_INSTALL_TARGET_HOOKS += LIBDVDNAV_TARGET_CLEANUP
+endif

We generally prefer positive logic:

ifeq ($(BR2_HAVE_DEVFILES),)
...
endif

Thomas

Signature · 4 lines

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

vaxon

author

2012-09-26 21:05 UTC #4 lore raw

PATCH 2/3 libdvdnav: Fix dvdnav-config

Quoted 10 lines · Thomas Petazzoni

Dear Valentine Barshak,
package/libdvdnav/libdvdnav.mk LIBDVDNAV_DEPENDENCIES = libdvdread host-pkg-config · line 25
Quoted 3 lines · Valentine Barshak
package/libdvdnav/libdvdnav.mk +3 -0
+ifneq ($(BR2_HAVE_DEVFILES),y)
+LIBDVDNAV_POST_INSTALL_TARGET_HOOKS += LIBDVDNAV_TARGET_CLEANUP
+endif
We generally prefer positive logic:

Quoted 3 lines

ifeq ($(BR2_HAVE_DEVFILES),)
...
endif

The ifneq(($BR2_HAVE_DEVFILES),y) was copy-pasted from other .mk file and it seems to be used much more often than ifeq($(BR2_HAVE_DEVFILES),).

Thanks,
Val.

Quoted 3 lines

Thomas

Valentine Barshak

author

2012-09-26 19:34 UTC #5 lore raw

PATCH 3/3 mplayer: Fix external libdvdread and libdvdnav support

In reply to Valentine Barshak

This sets paths to dvdread-config and dvdnav-config, and configuration options to enable external libdvdread and libdvdnav support.

Signed-off-by Valentine Barshak <[email protected]>

---
 package/multimedia/mplayer/mplayer.mk | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

package/multimedia/mplayer/mplayer.mk +15 -0

index 05120bf..9ff4b8a 100644
@@ -42,6 +42,21 @@ else
 MPLAYER_CONF_OPTS += --disable-freetype
 endif
 
+ifeq ($(BR2_PACKAGE_LIBDVDREAD),y)
+MPLAYER_CONF_OPTS +=  \
+	--enable-dvdread \
+	--disable-dvdread-internal \
+	--with-dvdread-config=$(STAGING_DIR)/usr/bin/dvdread-config
+MPLAYER_DEPENDENCIES += libdvdread
+endif
+
+ifeq ($(BR2_PACKAGE_LIBDVDNAV),y)
+MPLAYER_CONF_OPTS +=  \
+	--enable-dvdnav \
+	--with-dvdnav-config=$(STAGING_DIR)/usr/bin/dvdnav-config
+MPLAYER_DEPENDENCIES += libdvdnav
+endif
+
 ifeq ($(BR2_PACKAGE_MPLAYER_MPLAYER),y)
 MPLAYER_CONF_OPTS += --enable-mplayer
 else

Signature · 1 line

1.7.11.4

Thomas Petazzoni

2012-09-26 20:46 UTC #6 lore raw

PATCH 3/3 mplayer: Fix external libdvdread and libdvdnav support

Dear Valentine Barshak,

package/multimedia/mplayer/mplayer.mk else · line 51

Quoted 7 lines · Valentine Barshak

package/multimedia/mplayer/mplayer.mk +7 -0

+ifeq ($(BR2_PACKAGE_LIBDVDREAD),y)
+MPLAYER_CONF_OPTS +=  \
+	--enable-dvdread \
+	--disable-dvdread-internal \
+	--with-dvdread-config=$(STAGING_DIR)/usr/bin/dvdread-config
+MPLAYER_DEPENDENCIES += libdvdread
+endif

Don't we want a:

else
MPLAYER_CONF_OPTS += --disable-dvdread --disable-dvdread-internal endif

package/multimedia/mplayer/mplayer.mk else · line 58

Quoted 6 lines

package/multimedia/mplayer/mplayer.mk +6 -0

+ifeq ($(BR2_PACKAGE_LIBDVDNAV),y)
+MPLAYER_CONF_OPTS +=  \
+	--enable-dvdnav \
+	--with-dvdnav-config=$(STAGING_DIR)/usr/bin/dvdnav-config
+MPLAYER_DEPENDENCIES += libdvdnav
+endif

else
MPLAYER_CONF_OPTS += --disable-dvdnav
endif

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

vaxon

author

2012-09-26 20:59 UTC #7 lore raw

PATCH 3/3 mplayer: Fix external libdvdread and libdvdnav support

Quoted 14 lines · Thomas Petazzoni

Dear Valentine Barshak,
package/multimedia/mplayer/mplayer.mk else · line 51
Quoted 7 lines · Valentine Barshak
package/multimedia/mplayer/mplayer.mk +7 -0
+ifeq ($(BR2_PACKAGE_LIBDVDREAD),y)
+MPLAYER_CONF_OPTS +=  \
+	--enable-dvdread \
+	--disable-dvdread-internal \
+	--with-dvdread-config=$(STAGING_DIR)/usr/bin/dvdread-config
+MPLAYER_DEPENDENCIES += libdvdread
+endif
Don't we want a:

Quoted 3 lines

else
MPLAYER_CONF_OPTS += --disable-dvdread --disable-dvdread-internal endif

Not really, these options are set to "auto" by default and will be disabled when mplayer fails to find libcdio, which internal dvdread depends on.

I did not manually disable it. In case libcdio is added later mplayer would use it for internal dvdread when libdvdread is disabled.

Thanks,
Val.

Quoted 16 lines

package/multimedia/mplayer/mplayer.mk else · line 58
Quoted 7 lines
package/multimedia/mplayer/mplayer.mk +6 -0
+ifeq ($(BR2_PACKAGE_LIBDVDNAV),y)
+MPLAYER_CONF_OPTS +=  \
+	--enable-dvdnav \
+	--with-dvdnav-config=$(STAGING_DIR)/usr/bin/dvdnav-config
+MPLAYER_DEPENDENCIES += libdvdnav
+endif
else
MPLAYER_CONF_OPTS += --disable-dvdnav
endif
Thanks,
Thomas

A message that is not in the archive (sent off-list or not yet received): [email protected]

Valentine Barshak

author

2012-09-27 12:25 UTC #9 lore raw

PATCH 3/3 mplayer: Fix external libdvdread and libdvdnav support

Quoted 19 lines · vaxon

Quoted 14 lines · Thomas Petazzoni
Dear Valentine Barshak,
package/multimedia/mplayer/mplayer.mk else · line 51
Quoted 7 lines · Valentine Barshak
package/multimedia/mplayer/mplayer.mk +7 -0
+ifeq ($(BR2_PACKAGE_LIBDVDREAD),y)
+MPLAYER_CONF_OPTS +=  \
+    --enable-dvdread \
+    --disable-dvdread-internal \
+    --with-dvdread-config=$(STAGING_DIR)/usr/bin/dvdread-config
+MPLAYER_DEPENDENCIES += libdvdread
+endif
Don't we want a:
Quoted 3 lines
else
MPLAYER_CONF_OPTS += --disable-dvdread --disable-dvdread-internal endif

Quoted 20 lines

Not really, these options are set to "auto" by default and will be disabled when mplayer fails to find libcdio which internal dvdread depends on.
Thus, it builds fine with or without .
I did not manually disable it in case libcdio is added later so that mplayer would use it for internal dvdread when libdvdread is disabled.
Quoted 11 lines
package/multimedia/mplayer/mplayer.mk else · line 58
Quoted 7 lines
package/multimedia/mplayer/mplayer.mk +6 -0
+ifeq ($(BR2_PACKAGE_LIBDVDNAV),y)
+MPLAYER_CONF_OPTS +=  \
+    --enable-dvdnav \
+    --with-dvdnav-config=$(STAGING_DIR)/usr/bin/dvdnav-config
+MPLAYER_DEPENDENCIES += libdvdnav
+endif
else
MPLAYER_CONF_OPTS += --disable-dvdnav
endif

Same here. I don't think disabling dvdnav would give us anything.

Why not let mplayer try other choices (like internal dvd libs) and auto-disable dvd support if the checks fail.

Thomas, I can add this options if you like, but really don't see any gain in them.

Thanks,
Val.

Quoted 6 lines

Quoted 5 lines
Thanks,
Thomas

Thomas Petazzoni

2012-09-27 12:37 UTC #10 lore raw

PATCH 3/3 mplayer: Fix external libdvdread and libdvdnav support

Dear Valentine Barshak,

Quoted 7 lines · Valentine Barshak

Same here. I don't think disabling dvdnav would give us anything.
Why not let mplayer try other choices (like internal dvd libs) and auto-disable dvd support if the checks fail.
Thomas, I can add this options if you like, but really don't see any gain in them.

The thing is that we generally try to avoid auto-detection, because many configure script misdetect libraries installed on the build machine as being usable for the target. I.e if to check whether libdvdnav is present it looks in /usr/include/dvdnav/something.h, then it might incorrectly conclude that libdvdnav is available.

Best regards,

Thomas

Signature · 4 lines

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Valentine Barshak

author

2012-09-27 18:09 UTC #11 lore raw

PATCH 3/3 mplayer: Fix external libdvdread and libdvdnav support

Quoted 9 lines · Thomas Petazzoni

Dear Valentine Barshak,
Quoted 5 lines · Valentine Barshak
Same here. I don't think disabling dvdnav would give us anything.
Why not let mplayer try other choices (like internal dvd libs) and auto-disable dvd support if the checks fail.

Quoted 8 lines

Quoted 2 lines
Thomas, I can add this options if you like, but really don't see any gain in them.
The thing is that we generally try to avoid auto-detection, because many configure script misdetect libraries installed on the build machine as being usable for the target. I.e if to check whether libdvdnav is present it looks in /usr/include/dvdnav/something.h, then it might incorrectly conclude that libdvdnav is available.

Thanks for the explanations, Thomas.
I'll submit updated patches in a bit.

Thanks,
Val.

Quoted 5 lines

Best regards,
Thomas

Peter Korsgaard

2012-10-04 21:27 UTC #12 lore raw

cover Fix libdvdread and libdvdnav support

In reply to Valentine Barshak

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Valentine" == Valentine Barshak <[email protected]> writes:

Valentine> The following patches fix dvdread-config and dvdnav-config Valentine> scripts installed to staging directory and make mplayer Valentine> use these libraries when they are enabled.

Valentine> This is done to add DVD support to mplyaer. Valentine> These patches have been tested on an x86 machine.

Committed series, thanks.

Signature · 1 line

Bye, Peter Korsgaard