[FFmpeg-devel] [PATCH] make sure blending the subtitle does not write outside buffer

[Reimar Döffinger]

Hello,
On Wed, Aug 08, 2007 at 07:04:29PM +0200, Michael Niedermayer wrote:
> On Wed, Aug 08, 2007 at 03:51:26PM +0200, Reimar D?ffinger wrote:
> > On Wed, Aug 08, 2007 at 03:50:22PM +0200, Reimar D?ffinger wrote:
> > > On Sun, Aug 05, 2007 at 12:12:26PM +0200, Michael Niedermayer wrote:
> > > > On Sun, Aug 05, 2007 at 11:00:04AM +0200, Reimar D?ffinger wrote:
> > > > > attached patch fixes the subtitle rectangles so they do not lie outside
> > > > > the video, which can cause crashes.
> > > > 
> > > > hmm, this seems like a hack to me
> > > > shouldnt blend_subrect "skip" the parts of a rectange which are outside
> > > > of the image?
> > > > 
> > > > that said blend_subrect() needs a cleanup, theres no need to have 9 cases
> > > > handled like that, a generic and a fast one for the middle would do
> > > 
> > > Does something like this look okay? I can of course split it into two
> > > parts, on that makes sure we do not get outside the bounds of the
> > > destination image 
> 
> iam ok with the out of bounds checks if they also support x,y<0 ? well
> iam even ok if they dont, its just that x,y<0 must be fixed too :)

The version I applied does (just for the fun of it I also bounded width
and height against < 0, though it should not be necessary).
But IMO values < 0 would be a bug in the subtitle decoder.

> > > and one that "scales" the x and y positions, but then
> > > someone absolutely must suggest a better name for "scaledx/y/w/h" ;-)
> 
> the bitmap should be scaled if it is intended for a different display size
> whichever function then does the blending would only see the scaled x/y/w/h
> we of course can also support quick and dirty unscaled subs but that shouldnt
> be default

Sorry, I will not make such an effort (and it will be more than just 15
minutes of work to fix this, because the source format is paletted but
with the palette already converted to YUV, possibly problems with
scalers not handling alpha right etc.) for something that I
don't consider more than a testbed for libav*.
Well, actually if I ever get around implement it in MPlayer I won't
implement scaling the subtitles either, I don't need it, it's enough if
they show at all.

> > > Btw. the picture blend_subrect creates when x, y or x+w or y+h is odd is
> > > still completely broken, IMO we should remove the "if (rect->x & 1) {"
> > > etc. cases and just do x &= ~1; w &= ~1; etc.
> 
> x,y should have higher than 1-pixel precission, the code which scales it
> to display size should also fix the sub pixel translation
> that way the &1 code indeed is no longer needed, so yes you can remove
> it whenever you like

Well, I mostly wanted to get rid of the horrible artefacts, but since I
found and fixed the real bug for this I am no longer in a hurry.

Greetings,
Reimar D?ffinger