[FFmpeg-devel] [PATCH] Move DECLARE_ALIGNED_{8, 16} macros to mem.h

[Pavel Pavlov]

> Subject: Re: [FFmpeg-devel] [PATCH] Move DECLARE_ALIGNED_{8, 16} macros
> to mem.h
> 
> Pavel Pavlov <pavel at summit-tech.ca> writes:
> 
> >> > On Sat, Mar 06, 2010 at 01:27:56PM +0000, M?ns Rullg?rd wrote:
> >> >> Michael Niedermayer <michaelni at gmx.at> writes:
> >> >>
> >> >> > On Sat, Mar 06, 2010 at 02:50:30AM +0000, Mans Rullgard wrote:
> >> >> >> These macros naturally belong next to the generic
> DECLARE_ALIGNED
> >> >> >> macro.
> >> >> >> ---
> >> >> >>  libavcodec/dsputil.h |    3 ---
> >> >> >>  libavutil/mem.h      |    2 ++
> >> >> >>  2 files changed, 2 insertions(+), 3 deletions(-)
> >> >> >
> >> >> > ok if they arent redundant but they look a little redundant
> >> >> > relative to just using DECLARE_ALIGNED directly
> >> >>
> >> >> I always wondered what they were for.  OK to do a global search
> and
> >> >> replace?
> >> >
> >> > ok
> >>
> >> Done
> >
> > There is also ATTR_ALIGN left out in a couple of files
> > (dsputil_vis.c, fdct_mmx.c, idct_mmx.c). Should also be replaced
> > with DECLARE_ALIGNED
> 
> Ugh, I've missed those.  Patches welcome.
> 

By the way, I remember sending in patches for this macro like a year ago. I've been building ffmpeg with these changes for a while because my compiler needs different compiler directives.
There is a couple of tricky places that might need some code review, like these:
From
static struct
{
 const int32_t fdct_r_row_sse2[4] ATTR_ALIGN[16];
} fdct_r_row_sse2 ATTR_ALIGN[16] =

It became 
DECLARE_ALIGNED(16, static struct
{
 DECLARE_ALIGNED(16, const int32_t, fdct_r_row_sse2[4]);
}, fdct_r_row_sse2) =


But to me it seems that the original code had to be modified, since these ATTR_ALIGN are duplicates: if inner fdct_r_row_sse2 is aligned on 16, then the outer fdct_r_row_sse2 is also aligned on 16 and opposite is also true.
So, I'd rather use commented version there:

DECLARE_ALIGNED(16, static const long, fdct_r_row_sse2[]) = {RND_FRW_ROW, RND_FRW_ROW, RND_FRW_ROW, RND_FRW_ROW};

This has to work, if something like that could produce unaligned result then most of the other code in trouble as well. Intel compiler in some cases did not do proper alignment (and I had errors/crashes) for me so, I had to write some code to fix these problems. I remember that intel made an update and there was something related to stack alignment in the changelog
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DECLARE_ALIGNED.patch
Type: application/octet-stream
Size: 8581 bytes
Desc: DECLARE_ALIGNED.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100308/a8c01bb3/attachment.obj>