swscale: Provide the right alignment for external mmx asm

Message ID 1347144372-47145-1-git-send-email-martin@martin.st
State Committed
Commit 75c37c5ace6271dc9dc996a61b799bcd2fc1b30d
Headers show

Commit Message

Martin Storsjö Sept. 8, 2012, 10:46 p.m.
This reverts parts of e0c6cce4472. There is external mmx asm that
requires this alignment.

This fixes crashes when using swscale in builds with external mmx,
without inline assembly.
---
 libswscale/utils.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Mans Rullgard Sept. 8, 2012, 10:51 p.m. | #1
Martin Storsjö <martin@martin.st> writes:

> This reverts parts of e0c6cce4472. There is external mmx asm that
> requires this alignment.
>
> This fixes crashes when using swscale in builds with external mmx,
> without inline assembly.
> ---
>  libswscale/utils.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index 34e4744..0002e17 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -1050,7 +1050,8 @@ av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter,
>          } else
>  #endif /* HAVE_MMXEXT_INLINE */
>          {
> -            const int filterAlign = INLINE_MMX(cpu_flags)  ? 4 :
> +            const int filterAlign =
> +                (HAVE_MMX && cpu_flags & AV_CPU_FLAG_MMX) ? 4 :
>                  (HAVE_ALTIVEC && cpu_flags & AV_CPU_FLAG_ALTIVEC) ? 8 :
>                  1;
>  
> @@ -1073,7 +1074,8 @@ av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter,
>  
>      /* precalculate vertical scaler filter coefficients */
>      {
> -        const int filterAlign = INLINE_MMX(cpu_flags)  ? 2 :
> +        const int filterAlign =
> +            (HAVE_MMX && cpu_flags & AV_CPU_FLAG_MMX) ? 2 :
>              (HAVE_ALTIVEC && cpu_flags & AV_CPU_FLAG_ALTIVEC) ? 8 :
>              1;
>  
> -- 

LGTM
Ronald Bultje Sept. 9, 2012, 5:51 a.m. | #2
Hi,

On Sep 9, 2012 12:46 AM, "Martin Storsjö" <martin@martin.st> wrote:
>
> This reverts parts of e0c6cce4472. There is external mmx asm that
> requires this alignment.
>
> This fixes crashes when using swscale in builds with external mmx,
> without inline assembly.
> ---
>  libswscale/utils.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index 34e4744..0002e17 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -1050,7 +1050,8 @@ av_cold int sws_init_context(SwsContext *c,
SwsFilter *srcFilter,
>          } else
>  #endif /* HAVE_MMXEXT_INLINE */
>          {
> -            const int filterAlign = INLINE_MMX(cpu_flags)  ? 4 :
> +            const int filterAlign =
> +                (HAVE_MMX && cpu_flags & AV_CPU_FLAG_MMX) ? 4 :
>                  (HAVE_ALTIVEC && cpu_flags & AV_CPU_FLAG_ALTIVEC) ? 8 :
>                  1;
>
> @@ -1073,7 +1074,8 @@ av_cold int sws_init_context(SwsContext *c,
SwsFilter *srcFilter,
>
>      /* precalculate vertical scaler filter coefficients */
>      {
> -        const int filterAlign = INLINE_MMX(cpu_flags)  ? 2 :
> +        const int filterAlign =
> +            (HAVE_MMX && cpu_flags & AV_CPU_FLAG_MMX) ? 2 :
>              (HAVE_ALTIVEC && cpu_flags & AV_CPU_FLAG_ALTIVEC) ? 8 :
>              1;
>

OK.

Who reviewed this? This should never have happened in a cosmetic patch. If
in doubt, simply poke me.

Ronald
Diego Biurrun Sept. 9, 2012, 12:34 p.m. | #3
On Sun, Sep 09, 2012 at 07:51:24AM +0200, Ronald S. Bultje wrote:
> On Sep 9, 2012 12:46 AM, "Martin Storsjö" <martin@martin.st> wrote:
> >
> > This reverts parts of e0c6cce4472. There is external mmx asm that
> > requires this alignment.
> >
> > This fixes crashes when using swscale in builds with external mmx,
> > without inline assembly.
> > --- a/libswscale/utils.c
> > +++ b/libswscale/utils.c
> > @@ -1050,7 +1050,8 @@ av_cold int sws_init_context(SwsContext *c,
> >  #endif /* HAVE_MMXEXT_INLINE */
> >          {
> > -            const int filterAlign = INLINE_MMX(cpu_flags)  ? 4 :
> > +            const int filterAlign =
> > +                (HAVE_MMX && cpu_flags & AV_CPU_FLAG_MMX) ? 4 :
> >                  (HAVE_ALTIVEC && cpu_flags & AV_CPU_FLAG_ALTIVEC) ? 8 :
> >                  1;
> > @@ -1073,7 +1074,8 @@ av_cold int sws_init_context(SwsContext *c,
> >      /* precalculate vertical scaler filter coefficients */
> >      {
> > -        const int filterAlign = INLINE_MMX(cpu_flags)  ? 2 :
> > +        const int filterAlign =
> > +            (HAVE_MMX && cpu_flags & AV_CPU_FLAG_MMX) ? 2 :
> >              (HAVE_ALTIVEC && cpu_flags & AV_CPU_FLAG_ALTIVEC) ? 8 :
> >              1;
> 
> OK.
> 
> Who reviewed this? This should never have happened in a cosmetic patch. If
> in doubt, simply poke me.

I patched, Mans reviewed, but it was not a cosmetics patch; it separated
inline from external asm capabilities..  Notice how it fixed three FATE
instances, however, I overlooked that these hunks apply to both inline
and external asm...

Diego

Patch

diff --git a/libswscale/utils.c b/libswscale/utils.c
index 34e4744..0002e17 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -1050,7 +1050,8 @@  av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter,
         } else
 #endif /* HAVE_MMXEXT_INLINE */
         {
-            const int filterAlign = INLINE_MMX(cpu_flags)  ? 4 :
+            const int filterAlign =
+                (HAVE_MMX && cpu_flags & AV_CPU_FLAG_MMX) ? 4 :
                 (HAVE_ALTIVEC && cpu_flags & AV_CPU_FLAG_ALTIVEC) ? 8 :
                 1;
 
@@ -1073,7 +1074,8 @@  av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter,
 
     /* precalculate vertical scaler filter coefficients */
     {
-        const int filterAlign = INLINE_MMX(cpu_flags)  ? 2 :
+        const int filterAlign =
+            (HAVE_MMX && cpu_flags & AV_CPU_FLAG_MMX) ? 2 :
             (HAVE_ALTIVEC && cpu_flags & AV_CPU_FLAG_ALTIVEC) ? 8 :
             1;