| Message ID | 20120926193451.GA3983@gmail.com |
|---|---|
| State | Accepted |
| Commit | 63fbc1834f98f37cc43514f13cb75c1034dcf454 |
| Headers | show |
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
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 >
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 >> >
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
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 >
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
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(+)