[MPlayer-dev-eng] [PATCH] Remove font attributes from subtitles

[The Wanderer]

Salvatore Falco wrote:

> On Mon, Sep 19, 2005 at 07:57:01PM -0400, Ergzay wrote:
> 
>> I'm new to this list so I only saw this message by this reply. I
>> would LOVE to have full subtitle rendering including support of the
>> formats in ASS/SSA subtitles.
> 
> Some time ago, around March, I coded an initial support for SSA
> styles. I also added management of multiple fonts and "advanced"
> positioning.
> 
> I am attaching the patch vs. CVS of March.
> 
> I have no time to keep on developing the code, I hope someone else
> would.

I'm not competent to evaluate the patch's functionality, at least not
without a lot more effort than I want to invest right now (although
proper advanced subtitle feature support is something I'd very much like
to see - it's the number-one most wanted missing feature I see mentioned
in the fora I frequent, mostly a few newsgroups), but a couple of points
which need to be addressed regardless:

> --- MPlayer-20050307/subreader.c	2005-02-04 19:31:03.000000000 +0100
> +++ MPlayer-20050307.patch/subreader.c	2005-03-29 00:26:42.000000000 +0200

> @@ -530,30 +557,74 @@

<about a page down>

> +		    if (!sub_styles) sub_styles = (sub_styles_t *) calloc(sizeof(sub_styles_t), 1);
> +		    sub_styles->yresolution = num;
> +printf("\nPRY %d\n", sub_styles->yresolution);

printf() should not be used, please either remove this (if it was
intended purely for debugging purposes and won't be useful later on) or
change it to mp_msg().

Also, is there any particular reason why the line is not indented at
all?

> +	text_style_t *previous_line_style = NULL;
>  
> +printf("\nPRY %d\n", sub_styles->yresolution);

Here, same as above.

> +					sub->text_style[l]->alignment = SUB_ALIGNMENT_POSITIONAL;
> +					sscanf(position, "\\pos(%d,%d", &(sub->text_style[l]->posx), &(sub->text_style[l]->posy));
> +printf("\n pre: %d (%f/%f)\n", sub->text_style[l]->posy, (float) disp_h, (float) sub_styles->yresolution);
> +					sub->text_style[l]->posy = (int) ((float)sub->text_style[l]->posy * (float) disp_h / (float) sub_styles->yresolution);
> +printf(" post: %d\n", sub->text_style[l]->posy);

And here.


-- 
       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

Secrecy is the beginning of tyranny.