[MPlayer-cvslog] r35834 - trunk/configure

[Alexander Strasser]

Diego Biurrun wrote:
> On Fri, Jan 25, 2013 at 11:21:15PM +0100, Alexander Strasser wrote:
> > KO Myung-Hun wrote:
> > > Diego Biurrun wrote:
> > > > On Fri, Jan 25, 2013 at 03:34:30PM +0900, KO Myung-Hun wrote:
> > > >> Diego Biurrun wrote:
> > > >>> On Fri, Jan 25, 2013 at 12:46:45AM +0900, KO Myung-Hun wrote:
> > > >>>> diego wrote:
> > > >>>>>
> > > >>>>> Log:
> > > >>>>> configure: Unset HAVE_DIRECT_H if direct.h is not available.
> > > >>>>>
> > > >>>>> libdvdcss checks the value of that preprocessor symbol via #ifdef,
> > > >>>>> so it needs to be explicitly undefined, not set to zero.
> > > >>>>>
> > > >>>>> --- trunk/configure	Wed Jan 23 15:36:36 2013	(r35833)
> > > >>>>> +++ trunk/configure	Wed Jan 23 17:04:28 2013	(r35834)
> > > >>>>> @@ -3785,7 +3785,8 @@ header_check direct.h && _direct_h=yes
> > > >>>>>  if [ $_direct_h = yes ]; then
> > > >>>>>    def_direct_h='#define HAVE_DIRECT_H 1'
> > > >>>>>  else
> > > >>>>> -  def_direct_h='#define HAVE_DIRECT_H 0'
> > > >>>>> +  # libdvdcss checks this via #ifdef, so we need #undef here.
> > > >>>>> +  def_direct_h='#undef HAVE_DIRECT_H'
> > > >>>>>  fi
> > > >>>>>  echores "$_direct_h"
> > > >>>>
> > > >>>> libavformat/os_support.h uses #if although it is guarded by WIN32.
> > > >>>
> > > >>> That is not a problem an undefined preprocessor symbol evaluates as 0.
> > > >>
> > > >> But it generates a warning with -Wundef.
> > > > 
> > > > Having to choose between a warning and incorrect behavior, what do
> > > > you pick?
> > > 
> > > I would not do r35835.
> > > What a surprise you choose the way generating a warning.
> > 
> >   I agree it is just not worth it ATM. This is a all in all, including
> > the work to do it in the first place, a bad tradeoff for just 2 lines
> > of difference reduction to upstream.
> 
> The initial commit that introduced the difference to upstream was the
> thing not worth the trouble (and incorrect).  It entailed a configure
> change and then another.  Hacks are being stacked upon hacks here.
> 
> With the unmodified libdvdcss we are instead seeing a patch submitted
> to the upstream project and the issue fixed at the root.

  I strongly disagree that upstream is at fault here!

  We just have the problem that 2 imported libs, lavf and libdvdcss,
happen to use the same CPP symbol in different ways. So it is our fault
for not serving both correctly. So we should fix our build system to
deal with it. Until the 2 line difference to upstream should not hurt,
should it?

  The moment we have fixed our build system would be the point to revert
the first hack. Urging libdvdcss to switch their usage of preprocessor
macros is just not fair IMHO. If they want to do it or accept a patch for
it that is of course fine too.


  Alexander