[FFmpeg-devel] [PATCH] Fix unaligned fill_rectangle in rv34.c

[Michael Niedermayer]

On Fri, Aug 21, 2009 at 02:17:08PM +0300, Kostya wrote:
> On Fri, Aug 21, 2009 at 12:54:11PM +0200, Michael Niedermayer wrote:
> > On Fri, Aug 21, 2009 at 01:28:50PM +0300, Kostya wrote:
> > > On Fri, Aug 21, 2009 at 11:57:08AM +0200, Michael Niedermayer wrote:
> > > > On Thu, Aug 20, 2009 at 10:33:19AM +0300, Kostya wrote:
> > > > > On Thu, Aug 20, 2009 at 09:14:54AM +0200, Reimar D?ffinger wrote:
> > > > > > Hello,
> > > > > > rv34.c uses fill_rectangle with only 4 bytes alignment, which causes
> > > > > > crashes on 64 bit architectures without unaligned read support like
> > > > > > Sparc/Solaris.
> > > > > > This patch hacks rectangle.h to support this kind of use - this
> > > > > > seemed the simplest, even though rather hackish, way to me.
> > > > > 
> > > > > May I add that it happens when zeroing motion vectors for some
> > > > > macroblock, i.e. where there will be non-64bit align for sure.
> > > > > 
> > > > > Probably better solution would be to disable this code automatically for
> > > > > such archs. I don't remember an outcome of my discussion with Mans
> > > > > about that though.
> > > > 
> > > > I dont know about your discussion with mans, but if i remember correctly
> > > > and dont mix this up with some other issue, then at least my conclusion
> > > > was that your code is buggy and fails to align the code correctly.
> > > > IIRC something about using a random incorrect *_stride but i might
> > > > missremember
> > > 
> > > That was another alignment issue which is fixed now.
> > > Now it happens with zeroing motion_val[] which has guaranteed alignment 32
> > > but even macroblocks will have motion_val[] with that alignment while
> > > some CPUs (Sparc) require aligment 64 to fill it as int64.
> > 
> > i see, but this looks very much like the previous issue
> > if you have an array of "structs" that have a sizeof=8,
> > aligning them by 8 seems to make sense. I cant see what one would
> > gain by not aligning them
>  
> The problem is that their sizeof = 4

let me try again
if you write a rectangle of 8xC then there exists a semantic partitioning
of sizeof=8, and things could be aligned accordingly, if not you cannot
use fill_rectangle()
you would have to replace r->avail_cache + 5 by + 6 maybe and make other
related changes, its perfectly fine as well if you dont and remove all
calls to fill_rectangle() from your code but fill_rectangle() requires
aligned memory.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090822/f202af9f/attachment.pgp>