[FFmpeg-devel] [PATCH] swscale: Only copy dstW/chrDstW of the last line in the plane in planar copy

[Martin Storsjö]

On Fri, 16 Jul 2010, Michael Niedermayer wrote:

> On Fri, Jul 16, 2010 at 08:33:01PM +0300, Martin Storsj? wrote:
> > On Fri, 16 Jul 2010, Michael Niedermayer wrote:
> > 
> > > On Fri, Jul 16, 2010 at 10:25:12AM +0300, Martin Storsj? wrote:
> > > > On Fri, 16 Jul 2010, Martin Storsj? wrote:
> > > > 
> > > > > The attached patch makes planarCopyWrapper all lines but the last in one 
> > > > > block, then copies only dstW/chrDstW of the last line.
> > > > > 
> > > > > If the destination planes are offset within their buffer, copying the 
> > > > > whole plane in one memcpy may write outside of the destination buffer.
> > > > > 
> > > > > A concrete example: The source data is 144x144, stored in the upper left 
> > > > > corner of a 176x144 buffer. If this is "scaled" into 144x144 into another 
> > > > > 176x144 buffer, but centered into this buffer, the approach wich memcpying 
> > > > > the whole plane writes outside of the destination buffer.
> > > > 
> > > > Updated patch - uses the already existing length variable instead of 
> > > > recalculating it.
> > > 
> > > the caller could even expect that the non last lines width..stride is
> > > preserved as well (example picture in picture filters.
> > 
> > Yes - my code that uses swscale initially assumed that too, but I changed 
> > it to handle being overwritten.
> > 
> > > what speed effect does it have to do this with all lines?
> > > if its slower we could pass a flag to swscale.
> > 
> > Haven't benchmarked anything, but I guess it depends on the dimensions. 
> > More memcpys (height vs 1) but fewer bytes copied in total.
> > 
> > > that said your patch could be applied, it fixes a bug but i dont think
> > > this will be the final solution
> > 
> > Ok with the one updated with Loren's suggestion, too, doing it all in one 
> > memcpy?
> 
> of course

Thanks, applied.

// Martin