[MPlayer-dev-eng] Segfault caused by the "expand" filter

[Reimar Döffinger]

On Wed, Mar 16, 2011 at 06:19:04PM +0800, Zuxy Meng wrote:
> 2011/3/16 Reimar Döffinger <Reimar.Doeffinger at gmx.de>:
> > On 15 Mar 2011, at 03:57, Zuxy Meng <zuxy.meng at gmail.com> wrote:
> >> Hope here's still atmosphere for technical discussions:
> >>
> >> I met segfaults triggered by unaligned accesses of SSE instructions.
> >> The root cause is within the get_image() function of the "expand"
> >> filter, in lines like:
> >>
> >>    if(mpi->flags&MP_IMGFLAG_PLANAR){
> >>        mpi->planes[0]=vf->dmpi->planes[0]+
> >>        vf->priv->exp_y*vf->dmpi->stride[0]+vf->priv->exp_x;
> >>
> >> Here vf->dmpi->planes[0] is 16-byte aligned, but mpi->planes[0] may not be.
> >>
> >> I'm not sure what's the correct fix here. Would a simple forced align
> >> before the assignment work (e.g. ((vf->dmpi->planes[0] + ...)&~15))?
> >
> > To solve it properly you have to allocate a new image and memcpy instead of using EXPORT image type (or is this the DR path? Disable DR in that case).
> 
> Yes I guess it's the DR path.
> 
> > Alternatively filters requiring the alignment could do the memcpy, but I think that's more effort for little gain.
> 
> The problem happens when expand isn't the last filter, e.g. vf=expand...,pp=ac

That doesn't make sense, the line you quoted only causes the filters (and decoder)
_before_ to get unaligned pointers.
And I actually think that this code is actually correct, however it might be
necessary to change MP_IMGFLAG_ACCEPT_STRIDE to MP_IMGFLAG_ACCEPT_ALIGNED_STRIDE
in vd_ffmpeg.c.
As an optimization vf_expand could be changed to also work with MP_IMGFLAG_ACCEPT_ALIGNED_STRIDE
if vf->priv->exp_y*vf->dmpi->stride[0]+vf->priv->exp_x is a multiple of 16 for example.