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

login
register
mail settings
Submitter Valentine Barshak
Date Sept. 26, 2012, 7:34 p.m.
Message ID <20120926193451.GA3983@gmail.com>
Download mbox | patch
Permalink /patch/187185/
State Accepted
Commit 63fbc1834f98f37cc43514f13cb75c1034dcf454
Headers show

Comments

Valentine Barshak - Sept. 26, 2012, 7:34 p.m.
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(+)
Thomas Petazzoni - Sept. 26, 2012, 8:46 p.m.
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
Valentine Barshak - Sept. 26, 2012, 8:59 p.m.
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
>
Valentine Barshak - Sept. 27, 2012, 12:25 p.m.
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
>>
>
Thomas Petazzoni - Sept. 27, 2012, 12:37 p.m.
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
Valentine Barshak - Sept. 27, 2012, 6:09 p.m.
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
>

Patch

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