[FFmpeg-devel] armeb fix for mpegaudio

[Siarhei Siamashka]

On Friday 01 August 2008, M?ns Rullg?rd wrote:
> matthieu castet <castet.matthieu at free.fr> writes:
> > Hi,
> >
> > this patch fix mp2 regression test on arm big endian.
> >
> > Matthieu
> >
> > PS : as Mans said dct is broken as you can see with the video
> > regression failure.
> >
> > PS2 : test were done with qemu-armeb
> > Index: libavcodec/armv4l/mathops.h
> > ===================================================================
> > --- libavcodec/armv4l/mathops.h	(r?vision 14493)
> > +++ libavcodec/armv4l/mathops.h	(copie de travail)
> > @@ -22,6 +22,14 @@
> >  #ifndef FFMPEG_ARMV4L_MATHOPS_H
> >  #define FFMPEG_ARMV4L_MATHOPS_H
> >
> > +#ifdef WORDS_BIGENDIAN
> > +#define LOW_WORD 1
> > +#define HIGH_WORD 0
> > +#else
> > +#define LOW_WORD 0
> > +#define HIGH_WORD 1
> > +#endif
> > +
>
> I can imagine those being useful in other places.  Perhaps defining
> them in a more central location would be worthwhile.
>
> >  #ifdef FRAC_BITS
> >  #   define MULL(a, b) \
> >          ({  int lo, hi;\
> > @@ -52,7 +60,7 @@
> >  {
> >      union { uint64_t x; unsigned hl[2]; } x;
> >      asm ("smull %0, %1, %2, %3"
> > -         : "=r"(x.hl[0]), "=r"(x.hl[1]) : "r"(a), "r"(b));
> > +         : "=r"(x.hl[LOW_WORD]), "=r"(x.hl[HIGH_WORD]) : "r"(a),
> > "r"(b)); return x.x;
> >  }
> >  #define MUL64 MUL64
> > @@ -61,7 +69,7 @@
> >  {
> >      union { uint64_t x; unsigned hl[2]; } x = { d };
> >      asm ("smlal %0, %1, %2, %3"
> > -         : "+r"(x.hl[0]), "+r"(x.hl[1]) : "r"(a), "r"(b));
> > +         : "+r"(x.hl[LOW_WORD]), "+r"(x.hl[HIGH_WORD]) : "r"(a),
> > "r"(b)); return x.x;
> >  }
> >  #define MAC64(d, a, b) ((d) = MAC64(d, a, b))
>
> This looks OK.

'armv4l/mathops.h' gets included only when ARCH_ARMV4L is defined. ARCH_ARMV4L
implies little endian byte order. The problem is that apparently it also gets
defined for big endian ARM systems and it looks like a bug in configure. If
there is interest to also support big endian ARM systems and share code with
current little endian codebase, renaming ARCH_ARMV4L to something less
misleading would be very nice. As far as I remember, you were also somewhat
affected by DECLARE_ALIGNED_8 related issue some time ago?

I still would like to know if there is any practical use for big endian ARM
support or it is just academic interest? Hardware sometimes supports features
that are not much used nowadays (segmented model on x86, 16-bit real mode,
etc.). Supporting something only because it exists potentially has a negative
side of just introducing bloat. But again, I don't know anything about big
endian ARM systems and may be wrong. No harm intended, just curious.

BTW, does regression test in qemu-armeb succeed without using any assembly
optimized code for FFmpeg (just generic big endian C)?

-- 
Best regards,
Siarhei Siamashka