[MPlayer-dev-eng] [PATCH] two independent patches for configure

[Alexander Strasser]

Alexander Strasser wrote:
> Xidorn Quan wrote:
> > On Sat, Oct 27, 2012 at 8:26 PM, Alexander Strasser <eclipse7 at gmx.net>wrote:
> > > Alexander Strasser wrote:
> [...]
> > > >
> > > >   I am about to apply this. But while digging up history doubts came
> > > > up this is still needed?
> > > >
> > > >   So does anyone reading this successfully compile with gcc on osx
> > > > and does it really need -falign-loops=16 -shared-libgcc ?
> > > >
> > > >   From what I read in the commit log the
> > > >
> > > >     -falign-loops=16 recommended flags on osx
> > > >     -shared-libgcc   fix missing symbol when compiling with live support
> > > on mac osx
> > > >
> > > >   If none of those are needed anymore, it might be better to just
> > > > not set these flags. If I do not hear any news on this soon I will
> > > > just go with the patch as it helps people compiling with clang as
> > > > cc. Can still be removed later though if anyone can confirm those
> > > > flags are not needed anymore.
> > >
> > >   Committed (with a remark in the commit message).
> > >
> > >   So anyone, please comment if you can clarify if those flags are still
> > > needed.
> > >
> > 
> > AFAIK, no compiler other than clang can compile mplayer properly for
> > OS X at present. GCC cannot recognize block, a nonstandard extension
> > which is widely used in system header files, and LLVM-GCC seems to
> > be buggy when compiling.
> > 
> > I'd like to try to fix the bug of LLVM-GCC myself, and try compiling
> > again. We also don't know whether GCC will add block so that it would
> > be able to compile mplayer.
> 
>   So there is no compiling possible with GCC on OS X anymore. Sorry
> I did not know that. I guess the code messing with extra_cflags is
> not really useful then.
> 
> > BTW, what about the first patch?
> 
>   I tested the first patch on sunday. I found that it was not working
> entirely as I expected it to work on one of my machines. I am currently
> trying to pinpoint the issue. As soon as I have figured out what is
> going on I will comment again. I am sorry this is taking so long.

  I got around to investigate further and found that just my expectation
was wrong. So patch looks fine to me and OK to commit!

  One little cosmetic nit: 
@@ -1976,7 +1976,9 @@
         fi
         ;;
     6) iproc=686
-        if test "$pmodel" -ge 15; then
+        if test "$pmodel" -eq 28 -o "$pmodel" -eq 38 -o "$pmodel" -eq 54; then
+          proc=atom
+        elif test "$pmodel" -ge 15; then
             proc=core2
         elif test "$pmodel" -eq 9 -o "$pmodel" -ge 13; then
             proc=pentium-m

  Here your indent is a bit short in regard to the following elif-clauses.

  So please either indent consistent with the elif-clauses or commit a
*separate* patch after this one that shortens the indentation of the
elif-clauses to match the surrounding style.

Thank you,
  Alexander