[MPlayer-dev-eng] [PATCH] libass fixes and additions

[Grigori Goronzy]

Thanks for reviewing everything :)

Evgeniy Stepanov wrote:
> 
>> * 0005-Hack-use-a-compromise-to-force-somewhat-reasonable.patch
>> This is the "compromise" hack for the anti-aliasing "fix" I had in the
>> last patch already. This might be just a hack, "wrong" and whatever, but
>> it still makes the area where glyph and outline overlap a bit at least
>> decent looking in all cases (which I tested).
> 
> Well, the current behavoir is not much better, and is also a hack. I'll try to 
> look for a sample were you variant looks uglier then mine.
>

I tried to, but failed. I've found some cases where the new hack looks 
slightly worse than the old one, but none where it really screwed up badly.

>> * 0006-Support-one-of-these-braindead-vsfilter-special-case.patch
>> vsfilter has very stupid behaviour in case PlayResX/Y aren't specified.
>> As stupid as this may be, since vsfilter is more or less the reference
>> implementation, it makes sense to emulate it.
> 
> Are you sure? Just 1280x1024? What about 1024x768? Look very suspicious :)
>

Yes, just 1280x1024, or more exactly
a) when PlayResX is specified as 1280, and PlayResY isn't specified, it 
is assumed to be 1024
b) when PlayResY is specified as 1024, and PlayResX isn't specified, it 
is assumed to be 1280

Here's a snippet from vsfilter's STS.cpp:
schnipp---
else if(entry == L"playresx")
{
	try {ret.m_dstScreenSize.cx = GetInt(buff);}
	catch(...) {ret.m_dstScreenSize = CSize(0, 0); return(false);}

	if(ret.m_dstScreenSize.cy <= 0)
	{
		ret.m_dstScreenSize.cy = (ret.m_dstScreenSize.cx == 1280)
			? 1024
			: ret.m_dstScreenSize.cx * 3 / 4;
	}
}
else if(entry == L"playresy")
{
	try {ret.m_dstScreenSize.cy = GetInt(buff);}
	catch(...) {ret.m_dstScreenSize = CSize(0, 0); return(false);}

	if(ret.m_dstScreenSize.cx <= 0)
	{
		ret.m_dstScreenSize.cx = (ret.m_dstScreenSize.cy == 1024)
			? 1280
			: ret.m_dstScreenSize.cy * 4 / 3;
	}
}
schnapp---

There's still some difference in how vsfilter vs. libass assume a 
missing PlayResX/Y (namely, vsfilter uses 4:3 aspect ratio always, while 
libass uses the video aspect ratio)...

> 
>> * 0008-Round-shadow-displacement-to-nearest-int.patch
>> There's not much to say about it.
> 
> Why change the variable type to double, assign it an integer value, and round 
> it back later?
>

I'm not sure what you mean.
Currently, the "Shadow" property of a style is parse as double and also 
stored like that in a style. While in render_context and glyph_info it 
was only stored as integer (and because of that, rounded down always).
Now it's stored as double everywhere and rounded only when needed.

>> * 0009-Add-simple-blur-with-1-2-1-for-be.patch
>> Use a kernel blur with [[1,2,1], [1,2,1], [1,2,1]] matrix for \be. This
>> is the same filter used by vsfilter and also a bit faster than the
>> gaussian blur.
> 
> some other day )
>

Okay. The code is not very optimized anyway...

>> * 0010-Add-support-for-compositing-of-overlapping-glyph-bor.patch
>> This is very experimental, comments appreciated. :)
>> Overlapping glyphs, especially outlines, can look very ugly, in case
>> they are translucent -- the luminance just adds up!
>> This patch adds a pass after the final rendering, where the last two
>> bitmaps, if translucent and overlapping, are composited together, e.g.
>> for each pixel the maximum value of both bitmaps is used.
>> See http://greg.geekmind.org/mplayer/comp.png for a comparison.
> 
> Seems fine.
> 

Actually it wasn't that fine, the composited bitmaps weren't cached. 
Thus in some cases this was up 10% slower! (as in 10% more time spent in 
  VO in mplayer -benchmark)
I'll send a patch right away which makes it use the cache.

> Did you think of rendering the whole line of text (or just chains of 
> overlapping bitmaps) into a larger multicolor bitmap? It would solve this 
> problem, the problem patch 0005 is aimed at, and will probably improve 
> vo_gl/vo_vdpau performance (at least they won't need to join smaller bitmaps 
> into large textures). Of course, this is a lot of work.
>

Yes, and it requires changes to the EOSD API too... but of course I've 
thought about it already. :)

>> * 0011-Scale-shadow-displacement-and-blur-size-like-border.patch
>> There's not much to say about it.
> 
> Why not scale \be as well?
>

The other implementations do not seem to do it that way.

Grigori