[MPlayer-dev-eng] [PATCH] SGI IRIX audio format fixes

[deganews]

Please see below...

On Fri, 2 Sep 2005, Reimar [iso-8859-1] Döffinger wrote:

> Hi,
> On Wed, Aug 31, 2005 at 11:18:29PM -0400, dega wrote:
> >  #include <stdlib.h>
> > +#include <unistd.h>
> > +#include <errno.h>
> >  #include <dmedia/audio.h>
>
> Why are these needed?

uninit() uses sginap(), which is defined in unistd.h. Without the include,
it's an implicit declaration (compiler warning).

init() uses oserror(), which is defined in errno.h. Same deal: implicit
declaration.


> > +  switch(*format & (AF_FORMAT_POINT_MASK | AF_FORMAT_SPECIAL_MASK)) {
> > +  case AF_FORMAT_I:
>
> IMHO you shouldn't use the low-level representation. If possible use the
> predefined formats like AF_FORMAT_U16_LE etc.

I was trying to get away from having to list EVERY possible combination of
size, int/float, signed/unsigned in the switch statement. If using
low-level representation is considered a no-no, then I will change this
function at the expense of an unwieldy switch statement and some code
duplication.

> > +  if(us2si_mask) {
> > +    /* Perform parallel on-the-fly unsigned-to-signed conversion */
> > +    register uint64_t *smpls = data;
> > +    const uint64_t *smple = smpls + (plen * bytes_per_frame);
> > +    const uint64_t mask = us2si_mask;
> > +    while(smpls < smple)
> > +      *smpls++ ^= mask;
> > +  }
>
> This does not belong here. Just return only the formats that are really
> supported by the hardware. You can still improve the conversion
> functions in af_format.c if that is important to you...

I don't mind pulling this out of ao_sgi, and incorporating it in libaf,
but a general SIMD implementation for all architectures won't be nearly
this clean&simple.

As a side note, I am about to submit a patch for si2us() in libaf anyway:
it is currently broken :) Hint: SCHAR_MAX + SCHAR_MAX != UCHAR_MAX.

> > +  if((ao_data.format & (AF_FORMAT_BITS_MASK | AF_FORMAT_POINT_MASK)) ==
> > +     (AF_FORMAT_32BIT | AF_FORMAT_I)) {
> > +    /* libaudio expects doubleword-aligned 24-bit values */
> > +    int32_t *smpls = data;
> > +    const int32_t *smple = smpls + (framecount * ao_data.channels);
> > +    while(smpls < smple)
> > +      *smpls++ >>= 8;
> > +  }
>
> Hmm... can this be avoided somehow? I don't like conversion code in
> aos...

I would like to avoid it as well, but I don't see how.  Mplayer insists on
keeping the 24-bit audio format in packed mode, which is totally useless
to SGI's audio library. The 32-bit format has the full 32-bit range: once
again useless to the audio library.

The only 'clean' solution would be to extend the rest of mplayer to
differentiate between a packed and a non-packed 24-bit audio format.
This sounds like a much more invasive change just to support 24-bit
audio on IRIX.

Thoughts?

Thanks,
dega