[Ffmpeg-devel] Stack alignment warning

[Ivan Kalvachev]

2006/12/16, Michael Niedermayer <michaelni at gmx.at>:
> Hi
>
> On Fri, Dec 15, 2006 at 04:20:09PM -0700, Loren Merritt wrote:
> > On Sat, 16 Dec 2006, Uoti Urpala wrote:
> > >>2006/12/15, Ismail Donmez <ismail at pardus.org.tr>:
> > >
> > >> (support stack aligned local arrays)
> > >
> > >The warning test in ffmpeg does not use arrays (it's a single int
> > >variable), and the alignment does seem to work for arrays too in gcc
> > >4.1.
> >
> > In particular, gcc 3.4.3 completely ignores __attribute__((aligned(...)))
> > for scalar variables and 1-element arrays, but obeys it on multi-element
> > arrays (given the aligned-stack abi assumption, which was the original
> > problem). Thus it miscompiles ff_check_alignment even though the other
> > instances of alignment are ok.
>
> in dsputil_h264_template_mmx.c there is:
>     DECLARE_ALIGNED_8(uint64_t, AA);
>     DECLARE_ALIGNED_8(uint64_t, DD);
>
> also there are plenty of const static uint64_t over the code though maybe
> they are unaffected as they arent on the stack ...
>
> iam not against changing the uint64_t to somethingelse[X] and doing the
> same to ff_check_alignment() if this helps ...

Michael,

I think that the only 100% correct solution is to allocate big enough
aligned temp buffer in DSPContext that is to be used by all functions
instead of allocating it dynamically (in stack).

I guess that you won't be happy to code this solution as it requires a
lot of work just to workaround gcc bug, but theoretically it could
speed up decoding (by 0.3%;)

This solution is also thread safe as DSPContext is part of
MpegEncContext that is created per thread (for codecs with thread
support).
(If I remember right local arrays were created to workaround when lavc
is used by 2 processes at the same time causing static temp buffers to
be accessed concurrently).