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

[Evgeniy Stepanov]

On Saturday 28 February 2009 06:45:45 Grigori Goronzy wrote:
> 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)...

So, together with patch 0001, this means that even -vf crop will change the 
proportions of displayed text? And the same will happen with any anamorphic 
reencoding. It seems like a very undesirable behaviour.

> >> * 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.

But it's still parsed as integer from \shad tag.

> >> * 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.

ok.