[FFmpeg-devel] Stack variables alignment check in libavcodec/dsputil.c

[Zuxy Meng]

Hi,

2007/8/12, Ramiro Polla <ramiro at lisha.ufsc.br>:
> Michael Niedermayer wrote:
>
> [...]
>
> > avcodec_encode/decode_* are just small wraper functions and a register less
> > doesnt matter
> > and for execute i actually would suggest to make pthread.c and co
> > do the stack alignment so that no changes to functions which are passed
> > to execute() are needed that practically means adding the attribute to
> > worker() in pthread.c
> >
> >
>
> MinGW finally released gcc 4.2, and as promised here's my patch to fix
> this alignment issue (based on Ga?l Chardon's patch).
>
> Zuxy, are you working on this too? Could you test it further too?
> Everything worked fine with all my tests (w32threads and pthreads) from
> within MSVC++.

It works fine for me with gcc.
>
> Ramiro Polla
>
> Index: configure
> ===================================================================
> --- configure   (revision 10074)
> +++ configure   (working copy)
> @@ -139,6 +139,8 @@
>   echo "  --disable-ffplay         disable ffplay build"
>   echo "  --enable-small           optimize for size instead of speed"
>   echo "  --enable-memalign-hack   emulate memalign, interferes with memory debuggers"
> +  echo "  --enable-force-align     force stack alignment for certain functions"
> +  echo "                           unneeded for sane systems. (requires gcc >= 4.2)"

Well, actually the original x86 ABI said nothing about stack alignment
more than a dword (32 bits) and it's really not a matter of being sane
or insane....

>   echo "  --disable-encoder=NAME   disables encoder NAME"
>   echo "  --enable-encoder=NAME    enables encoder NAME"
>   echo "  --disable-decoder=NAME   disables decoder NAME"
> @@ -585,6 +587,7 @@
>     gpl
>     gprof
>     gray
> +    force_align
>     ipv6
>     liba52
>     liba52bin
> Index: libavcodec/w32thread.c
> ===================================================================
> --- libavcodec/w32thread.c      (revision 10074)
> +++ libavcodec/w32thread.c      (working copy)
> @@ -36,7 +36,7 @@
>  }ThreadContext;
>
>
> -static unsigned __stdcall thread_func(void *v){
> +static unsigned __stdcall attribute_align_arg thread_func(void *v){
>     ThreadContext *c= v;
>
>     for(;;){
> Index: libavcodec/utils.c
> ===================================================================
> --- libavcodec/utils.c  (revision 10074)
> +++ libavcodec/utils.c  (working copy)
> @@ -880,7 +880,7 @@
>     return ret;
>  }
>
> -int avcodec_encode_audio(AVCodecContext *avctx, uint8_t *buf, int buf_size,
> +int attribute_align_arg avcodec_encode_audio(AVCodecContext *avctx, uint8_t *buf, int buf_size,
>                          const short *samples)
>  {
>     if(buf_size < FF_MIN_BUFFER_SIZE && 0){
> @@ -895,7 +895,7 @@
>         return 0;
>  }
>
> -int avcodec_encode_video(AVCodecContext *avctx, uint8_t *buf, int buf_size,
> +int attribute_align_arg avcodec_encode_video(AVCodecContext *avctx, uint8_t *buf, int buf_size,
>                          const AVFrame *pict)
>  {
>     if(buf_size < FF_MIN_BUFFER_SIZE){
> @@ -923,7 +923,7 @@
>     return ret;
>  }
>
> -int avcodec_decode_video(AVCodecContext *avctx, AVFrame *picture,
> +int attribute_align_arg avcodec_decode_video(AVCodecContext *avctx, AVFrame *picture,
>                          int *got_picture_ptr,
>                          uint8_t *buf, int buf_size)
>  {
> @@ -946,7 +946,7 @@
>     return ret;
>  }
>
> -int avcodec_decode_audio2(AVCodecContext *avctx, int16_t *samples,
> +int attribute_align_arg avcodec_decode_audio2(AVCodecContext *avctx, int16_t *samples,
>                          int *frame_size_ptr,
>                          uint8_t *buf, int buf_size)
>  {
> Index: libavcodec/dsputil.c
> ===================================================================
> --- libavcodec/dsputil.c        (revision 10074)
> +++ libavcodec/dsputil.c        (working copy)
> @@ -3823,7 +3823,7 @@
>     for(i=0; i<64; i++) inv_zigzag_direct16[ff_zigzag_direct[i]]= i+1;
>  }
>
> -int ff_check_alignment(void){
> +int attribute_align_arg ff_check_alignment(void){
>     static int did_fail=0;
>     DECLARE_ALIGNED_16(int, aligned);
>
> Index: libavcodec/pthread.c
> ===================================================================
> --- libavcodec/pthread.c        (revision 10074)
> +++ libavcodec/pthread.c        (working copy)
> @@ -42,7 +42,7 @@
>     int done;
>  } ThreadContext;
>
> -static void* worker(void *v)
> +static void* attribute_align_arg worker(void *v)
>  {
>     AVCodecContext *avctx = v;
>     ThreadContext *c = avctx->thread_opaque;
> Index: libavutil/internal.h
> ===================================================================
> --- libavutil/internal.h        (revision 10074)
> +++ libavutil/internal.h        (working copy)
> @@ -34,6 +34,16 @@
>  #include <stddef.h>
>  #include <assert.h>
>
> +#ifndef attribute_align_arg
> +#if defined(CONFIG_FORCE_ALIGN) && defined(__GNUC__) && \
> +    (__GNUC__ > 4 || __GNUC__ == 4 && __GNUC_MINOR__>1)
> +#    define attribute_align_arg __attribute__((force_align_arg_pointer))
> +#else
> +#    define attribute_align_arg
> +#endif
> +#endif
> +
> +
>  #ifndef attribute_used
>  #if defined(__GNUC__) && (__GNUC__ > 3 || __GNUC__ == 3 && __GNUC_MINOR__ > 0)
>  #    define attribute_used __attribute__((used))



-- 
Zuxy
Beauty is truth,
While truth is beauty.
PGP KeyID: E8555ED6