[MPlayer-cvslog] r35834 - trunk/configure

[Diego Biurrun]

On Sat, Jan 26, 2013 at 03:02:46PM +0100, Alexander Strasser wrote:
> Diego Biurrun wrote:
> > On Sat, Jan 26, 2013 at 03:02:42AM +0100, Alexander Strasser wrote:
> > > 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!
> > 
> > I said no such thing.  Please read libdvdcss-devel, your comments seem
> > to imply that you miss information.
> 
>   I read the history of libdvdcss! They used HAVE_DIRECT_H in the #ifdef
> way since its introduction in 2002/2003 .
> 
>   So if some random patch on libdvdcss-devel is correct. And it accidently
> solves our configure problem your statement
> 
> > > > With the unmodified libdvdcss we are instead seeing a patch submitted
> > > > to the upstream project and the issue fixed at the root.
> 
>   is still completely wrong. It is not fixed at the root at all.
> 
>   Having that said I still don't understand your problem at all. First of all
> we, MPlayer and FFmpeg, want to use #if-way of checking availibility. So we
> want to have HAVE_DIRECT_H #define'd to 1/0 and for lavf it is already
> needed which is the cause of this fuzz here.
> 
>   Also note that the patch on libdvdcss-devel would still not fix our problem
> because there is another occurance of "#ifdef HAVE_DIRECT_H" in libdvdcss.c .

... and I just proposed removing that one as well, if you would read
the mails on libdvdcss-devel again.

>   I can only conclude that this commit is moving in the wrong direction.

Nope.  What's wrong is to deviate from upstream in order to avoid a warning
in another upstream lib that is completely harmless and absolutely nothing
we have to care about.  And as it turns out, the better solution is to
change upstream libdvdcss in a way that obviates the need for the local
workaround anyway.

So, to clarify once again:

a) Patching libdvdcss in order to silence a harmless warning in another
   upstream library copy is not worth it.
b) libdvdcss will likely be changed to make all of this moot.

Diego