[MPlayer-dev-eng] [PATCH 2/2] av_sub: support multiple rectangles.

[Nicolas George]

Le quartidi 4 floréal, an CCXX, Reimar Döffinger a écrit :
> I don't think we want to allow max values < 0?

I can imagine hypothetic situations where the subtitle decoder would output
rectangles with negative values.

The code that prepares the packet does a sanity check on the size and
position of the rectangle, so this is not a risk.

> Is num_rects == 0 handled somewhere? It seems that case would badly
> upset both code paths.

Yes, I double-checked, this branch is inside a check:
"if (got_sub && sub.num_rects > 0)".

> Why did you remove the comment? That seems to be the only thing that
> changed in that line.

That is just a diff quirk: the comment is still there, but diff does not
match the lines (because the brace is gone).

> Quite pointless to free it when it is NULL...

Fixed.

> If it's trivial like here, I prefer using the same return path, by just
> setting packet to NULL.

Ok.

> The old code used "spu" on purpose, I'd prefer if you'd not use "this"
> in any new code.

Fixed.

> I must say I am quite split.
> It is a really nice and simple change, so I well can't say no to it
> (except for those few minor issues).
> On the other hand for large resolutions it might waste a lot of memory
> and processing power,

At the very least, I made sure that my code did not add extra work for the
situations that worked (I do not consider displaying half the subtitle
"working".).

>			and I would have preferred to see an
> implementation that just uses multiple subtitle processors...
> Have you though about that way of doing it? I suspect it would be
> more complex and problematic to support an arbitrary number of subs,
> still I quite liked that idea...

I am still considering implementing colored overlays for EOSD (currently, it
is only single color + alpha mask). If this is done, sending the rectangles
to EOSD instead of OSD would be very simple, and we would gain color at the
same time.

If you are interested, I can write my thoughts about the way to do it (the
main problem is scaling) here.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20120422/287ebe8e/attachment.asc>