[MPlayer-dev-eng] Re: [RFC] including x264 with MPlayer tarballs

[Dominik 'Rathann' Mierzejewski]

On Tuesday, 23 August 2005 at 00:17, Alexander Izvorski wrote:
> Hello,
> 
> Here are a couple of small patches (one for mplayer, one for x264) to
> build x264 in mplayer, that I'd like you to comment on.  These are
> actually very similar to Jindrich's build script in some ways.  The
> mplayer patch adds a new --enable/disable-internal-x264 option
> (default auto), and will use the internal x264 if it is there.  The
> x264 patch makes x264 inherit compiler settings and defines from
> mplayer's config.

Looks fine to me.

> Areas that may need work:
> 
> * Assembler detection, which people have mentioned: assembler is not
> _required_ for x264 but it is certainly nice to have.  We can either
> disable x264 if there is no assembler, or just build it without the
> assembly optimizations? (and perhaps print a warning)  x264 doesn't
> test whether there is a working assembler present now, but it really
> should ;)   I think this will be fixed upstream in x264.  I'm working
> on it now, but it's not in this patch.

I think we shouldn't require the presence of external assembler, but we
should make use of it if it is available. We should print a warning, just
as we do when MPlayer is built with runtime CPU detection.

> This patch will however try to
> use the correct assembler for the platform it's on.

I don't see anything in your patch that would do that, but I assume
that's why you wrote you're working on it.

> * Creating x264/config.mak : now, I'm just running x264's configure
> script, although I also have the code (commented out) that would
> create a x264/config.mak directly from mplayer's configure.  The main
> problem with creating it directly is that the ARCH and SYS* names are
> not exactly the same, so we'd need to translate (although possibly a
> patch that makes the names the same can be applied upstream in x264).
> Also there are a few platform-specific linker flags.  I'm not sure
> whether there is any problem with running x264's configure? (although
> I noticed that other packages don't do it that way).  Thoughts?

Well, I think running x264's configure script would be acceptable if you
could guarantee it'll use the same config as MPlayer and I mean any
external libs that x264 may require that can be enabled/disabled in
MPlayer's configure.

> * Currently the internal x264 is preferred over the external by
> default if both are present.  Also the internal-x264 depends on x264
> (i.e. disable-x264 disables both internal and external).  Is that the
> correct behavior?

Yes. I see that with your patch you can disable internal x264 and use
external only. That's good for packagers.

> * CPU capabilities and autodetection: x264 always detects cpu
> abilities at runtime.  Because of that, by default it will build with
> everything on x86.  We can make it rely on the USE_* cpu flags from
> mplayer's configure instead.  Is there any reason to do that?

I think it should behave in accordance with MPlayer's
--enable-runtime-cpudetection.

> * The x264 patch would need to be applied to the version of x264 which
> is in the tarballs.  I'm not sure how to automate that as part of the
> tarball creation script?  It's a really simple patch, but in any case
> I volunteer to maintain it.

What tarballs? If we include x264 in MPlayer's sources, we'll keep it
patched and keep the diffs from official sources in CVS, too, for as long
as they're needed.

R.

-- 
MPlayer RPMs maintainer: http://rpm.greysector.net/mplayer/
"I am Grey. I stand between the candle and the star. We are Grey.
 We stand between the darkness ... and the light."
        -- Delenn in Grey Council in Babylon 5:"Babylon Squared"