[Buildroot] [PATCH 0/3] Fix libdvdread and libdvdnav support

[Buildroot] [PATCH 0/3] Fix libdvdread and libdvdnav support

[Valentine Barshak]

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.

[Buildroot] [PATCH 1/3] libdvdread: Fix dvdread-config

[Valentine Barshak]

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 <gvaxon@gmail.com>
---
 package/libdvdread/libdvdread.mk | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/package/libdvdread/libdvdread.mk b/package/libdvdread/libdvdread.mk
index 6e17731..ee616f0 100644
--- a/package/libdvdread/libdvdread.mk
+++ b/package/libdvdread/libdvdread.mk
@@ -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))
-- 
1.7.11.4

[Buildroot] [PATCH 2/3] libdvdnav: Fix dvdnav-config

[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 <gvaxon@gmail.com>
---
 package/libdvdnav/libdvdnav.mk | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/package/libdvdnav/libdvdnav.mk b/package/libdvdnav/libdvdnav.mk
index e2d42f3..2fefe43 100644
--- a/package/libdvdnav/libdvdnav.mk
+++ b/package/libdvdnav/libdvdnav.mk
@@ -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))
-- 
1.7.11.4

[Buildroot] [PATCH 2/3] libdvdnav: Fix dvdnav-config

[Thomas Petazzoni]

Dear Valentine Barshak,

On Wed, 26 Sep 2012 23:33:51 +0400, Valentine Barshak wrote:

> +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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH 2/3] libdvdnav: Fix dvdnav-config

[vaxon]

On 09/27/2012 12:46 AM, Thomas Petazzoni wrote:
> Dear Valentine Barshak,
>
> On Wed, 26 Sep 2012 23:33:51 +0400, Valentine Barshak wrote:
>
>> +ifneq ($(BR2_HAVE_DEVFILES),y)
>> +LIBDVDNAV_POST_INSTALL_TARGET_HOOKS += LIBDVDNAV_TARGET_CLEANUP
>> +endif
>
> We generally prefer positive logic:
>
> 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.

>
> Thomas
>

[Buildroot] [PATCH 3/3] mplayer: Fix external libdvdread and libdvdnav support

[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 <gvaxon@gmail.com>
---
 package/multimedia/mplayer/mplayer.mk | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/package/multimedia/mplayer/mplayer.mk b/package/multimedia/mplayer/mplayer.mk
index 05120bf..9ff4b8a 100644
--- a/package/multimedia/mplayer/mplayer.mk
+++ b/package/multimedia/mplayer/mplayer.mk
@@ -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
-- 
1.7.11.4

[Buildroot] [PATCH 3/3] mplayer: Fix external libdvdread and libdvdnav support

[Thomas Petazzoni]

Dear Valentine Barshak,

On Wed, 26 Sep 2012 23:34:51 +0400, Valentine Barshak wrote:

> +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

> +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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH 3/3] mplayer: Fix external libdvdread and libdvdnav support

[vaxon]

On 09/27/2012 12:46 AM, Thomas Petazzoni wrote:
> Dear Valentine Barshak,
>
> On Wed, 26 Sep 2012 23:34:51 +0400, Valentine Barshak wrote:
>
>> +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

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.
>
>> +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
>

[Buildroot] [PATCH 3/3] mplayer: Fix external libdvdread and libdvdnav support

[Valentine Barshak]

On 09/27/2012 12:58 AM, vaxon wrote:
> On 09/27/2012 12:46 AM, Thomas Petazzoni wrote:
>> Dear Valentine Barshak,
>>
>> On Wed, 26 Sep 2012 23:34:51 +0400, Valentine Barshak wrote:
>>
>>> +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
>
> 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.
>
>>
>>> +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.

>>
>> Thanks,
>>
>> Thomas
>>
>

[Buildroot] [PATCH 3/3] mplayer: Fix external libdvdread and libdvdnav support

[Thomas Petazzoni]

Dear Valentine Barshak,

On Thu, 27 Sep 2012 16:25:36 +0400, Valentine Barshak wrote:

> 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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH 3/3] mplayer: Fix external libdvdread and libdvdnav support

[Valentine Barshak]

On 09/27/2012 04:37 PM, Thomas Petazzoni wrote:
> Dear Valentine Barshak,
>
> On Thu, 27 Sep 2012 16:25:36 +0400, Valentine Barshak wrote:
>
>> 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.

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

Thanks,
Val.
>
> Best regards,
>
> Thomas
>

[Buildroot] [PATCH 0/3] Fix libdvdread and libdvdnav support

[Peter Korsgaard]

>>>>> "Valentine" == Valentine Barshak <gvaxon@gmail.com> 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.

-- 
Bye, Peter Korsgaard