[MPlayer-dev-eng] Remove libass

[Diego Biurrun]

On Wed, Jul 22, 2009 at 12:16:31AM +0200, Grigori Goronzy wrote:
> 
> I've been working on a standalone version of libass in the last few
> months.  This new standalone version removes all MPlayer-isms and
> contains a huge number of bugfixes and feature additions, basically
> bringing libass completely in line with VSFilter.  Please see the new
> project page at http://code.google.com/p/libass/
> 
> Attached is a patch that removes the internal libass (except actually
> deleting the libass directory) and adds support for using the standalone
> libass.

Hmmmm...

> There are a few possible problems.
> * It's not possible to statically link libass into the mplayer binary at
> the moment.

Why?

> Please tell me what you think about these changes.
> 
> --- a/Makefile
> +++ b/Makefile
> @@ -125,15 +125,7 @@ SRCS_COMMON-$(LIBA52_INTERNAL)       += liba52/crc.c \
>  
> +SRCS_COMMON-$(LIBASS)                += ass_mp.c \

Everything else that is subtitle-related resides in the libvo/
directory, why not this?

> --- /dev/null
> +++ b/ass_mp.c
> @@ -0,0 +1,296 @@
> + *
> + * This file is part of MPlayer.
> + *
> + * MPlayer is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * MPlayer is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with libass; if not, write to the Free Software Foundation, Inc.,

libass?

> +void process_force_style(ass_track_t *track);

Where is this declared?

> +    track->track_type = TRACK_TYPE_ASS;
> +    track->Timer = 100.;
> +    track->PlayResY = 288;
> +    track->WrapStyle = 0;

align

> +        style->PrimaryColour = c1;
> +        style->SecondaryColour = c1;
> +        style->OutlineColour = c2;
> +        style->BackColour = 0x00000000;
> +        style->BorderStyle = 1;
> +        style->Alignment = 2;
> +        style->Outline = 2;
> +        style->MarginL = 10;
> +        style->MarginR = 10;
> +        style->MarginV = 5;
> +        style->ScaleX = 1.;
> +        style->ScaleY = 1.;

ditto

more below

> --- /dev/null
> +++ b/ass_mp.h
> @@ -0,0 +1,60 @@
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with libass; if not, write to the Free Software Foundation, Inc.,

Again, libass?

> +#ifndef LIBASS_MP_H
> +#define LIBASS_MP_H

This is not the correct multiple inclusion guard.

> +typedef struct {
> +    ass_image_t *imgs;
> +    int changed;
> +} mp_eosd_images_t;

Please avoid _t typedefs, the _t namespace is reserved by POSIX.

> --- a/configure
> +++ b/configure
> @@ -6077,28 +6077,16 @@ echores "$_fontconfig"
>  
> +if test "$_ass" = auto -o "$_ass" = yes ; then
> +	if $_pkg_config libass; then
> +		_ass=yes
> +		def_ass='#define CONFIG_ASS'
> +		extra_ldflags="$extra_ldflags $($_pkg_config --libs libass)"
> +		extra_cflags="$extra_cflags $($_pkg_config --cflags libass)"
> +	else
> +		_ass=no
> +		def_ass='#undef CONFIG_ASS'
> +	fi

tabs

> --- a/libvo/vo_gl.c
> +++ b/libvo/vo_gl.c
> @@ -35,8 +35,9 @@
>  #include "fastmemcpy.h"
> -#include "libass/ass.h"
> -#include "libass/ass_mp.h"
> +#ifdef CONFIG_ASS
> +#include "ass_mp.h"
> +#endif

Is the #ifdef necessary?

Diego